From 4b6f13b36ca7d7c433a6cd3c0b7863fe9cd3431a Mon Sep 17 00:00:00 2001 From: James Ranson Date: Wed, 1 Apr 2020 06:39:06 -0600 Subject: [PATCH 1/4] Update roadmap.md --- docs/roadmap.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/roadmap.md b/docs/roadmap.md index 51b4ca722..ed2174b27 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -11,10 +11,11 @@ The roadmap for Trickster in 2020 focuses on delivering incremental enhancements - [x] Submit Helm charts to Helm Hub - [ ] Submit Trickster for CNCF Sandbox Consideration - [ ] Trickster v1.1 Release + - [x] Relocate project to `tricksterproxy` organization - [x] Release Binaries for Windows - [x] Change default frontend listen port to 8480 - [x] Frontend HTTP 2.0 Support - - [ ] Rules-based Request Routing and Rewriting + - [x] Rules-based Request Routing and Rewriting - [ ] Use RWMutex for cache synchronization - [ ] Reload configuration without process restart - [x] Add implementation-specific Tracing options in config From 41fbf794f4457d1d6f3453ab480612d5eb10ae92 Mon Sep 17 00:00:00 2001 From: James Ranson Date: Thu, 16 Apr 2020 08:32:46 -0600 Subject: [PATCH 2/4] V1.0.3 bugfix (#410) * remove errant locks release * make atime asynchronous * use new context for all upstream requests * named locks optimizations * cache index bulk remove optimizations * bump version to 1.0.3 --- cmd/trickster/main.go | 2 +- internal/cache/badger/badger.go | 2 +- internal/cache/badger/badger_test.go | 4 +- internal/cache/bbolt/bbolt.go | 12 +++-- internal/cache/bbolt/bbolt_test.go | 4 +- internal/cache/cache.go | 4 +- internal/cache/filesystem/filesystem.go | 12 ++--- internal/cache/filesystem/filesystem_test.go | 4 +- internal/cache/index/index.go | 40 +++++++++----- internal/cache/index/index_test.go | 7 +-- internal/cache/memory/memory.go | 18 +++++-- internal/cache/memory/memory_test.go | 4 +- internal/cache/redis/redis.go | 2 +- internal/cache/redis/redis_test.go | 4 +- internal/proxy/engines/cache_test.go | 10 ++-- internal/proxy/engines/client_test.go | 3 -- internal/proxy/engines/deltaproxycache.go | 5 +- internal/proxy/engines/proxy_request.go | 17 +++--- pkg/locks/locks.go | 31 ++++++----- pkg/locks/locks_test.go | 56 +++++++++++++++++--- 20 files changed, 152 insertions(+), 89 deletions(-) diff --git a/cmd/trickster/main.go b/cmd/trickster/main.go index f96988832..0a21182cd 100644 --- a/cmd/trickster/main.go +++ b/cmd/trickster/main.go @@ -44,7 +44,7 @@ var ( const ( applicationName = "trickster" - applicationVersion = "1.0.2" + applicationVersion = "1.0.3" ) // Package main is the main package for the Trickster application diff --git a/internal/cache/badger/badger.go b/internal/cache/badger/badger.go index 52c8fea4e..0c7570326 100644 --- a/internal/cache/badger/badger.go +++ b/internal/cache/badger/badger.go @@ -102,7 +102,7 @@ func (c *Cache) Remove(cacheKey string) { } // BulkRemove removes a list of objects from the cache. noLock is not used for Badger -func (c *Cache) BulkRemove(cacheKeys []string, noLock bool) { +func (c *Cache) BulkRemove(cacheKeys []string) { log.Debug("badger cache bulk remove", log.Pairs{}) c.dbh.Update(func(txn *badger.Txn) error { diff --git a/internal/cache/badger/badger_test.go b/internal/cache/badger/badger_test.go index a3b574644..9ba93a18f 100644 --- a/internal/cache/badger/badger_test.go +++ b/internal/cache/badger/badger_test.go @@ -161,8 +161,8 @@ func TestBadgerCache_BulkRemove(t *testing.T) { t.Errorf("expected %s got %s", status.LookupStatusHit, ls) } - bc.BulkRemove([]string{""}, true) - bc.BulkRemove([]string{cacheKey}, true) + bc.BulkRemove([]string{""}) + bc.BulkRemove([]string{cacheKey}) // it should be a cache miss _, ls, err = bc.Retrieve(cacheKey, false) diff --git a/internal/cache/bbolt/bbolt.go b/internal/cache/bbolt/bbolt.go index a0b49c950..0aad7e6fc 100644 --- a/internal/cache/bbolt/bbolt.go +++ b/internal/cache/bbolt/bbolt.go @@ -159,7 +159,7 @@ func (c *Cache) retrieve(cacheKey string, allowExpired bool, atime bool) ([]byte if allowExpired || o.Expiration.IsZero() || o.Expiration.After(time.Now()) { log.Debug("bbolt cache retrieve", log.Pairs{"cacheKey": cacheKey}) if atime { - c.Index.UpdateObjectAccessTime(cacheKey) + go c.Index.UpdateObjectAccessTime(cacheKey) } cache.ObserveCacheOperation(c.Name, c.Config.CacheType, "get", "hit", float64(len(data))) locks.Release(lockPrefix + cacheKey) @@ -187,7 +187,7 @@ func (c *Cache) Remove(cacheKey string) { locks.Release(lockPrefix + cacheKey) } -func (c *Cache) remove(cacheKey string, noLock bool) error { +func (c *Cache) remove(cacheKey string, isBulk bool) error { err := c.dbh.Update(func(tx *bbolt.Tx) error { b := tx.Bucket([]byte(c.Config.BBolt.Bucket)) @@ -197,16 +197,18 @@ func (c *Cache) remove(cacheKey string, noLock bool) error { log.Error("bbolt cache key delete failure", log.Pairs{"cacheKey": cacheKey, "reason": err.Error()}) return err } - c.Index.RemoveObject(cacheKey, noLock) + if !isBulk { + c.Index.RemoveObject(cacheKey) + } cache.ObserveCacheDel(c.Name, c.Config.CacheType, 0) log.Debug("bbolt cache key delete", log.Pairs{"key": cacheKey}) return nil } // BulkRemove removes a list of objects from the cache -func (c *Cache) BulkRemove(cacheKeys []string, noLock bool) { +func (c *Cache) BulkRemove(cacheKeys []string) { for _, cacheKey := range cacheKeys { - c.remove(cacheKey, noLock) + c.remove(cacheKey, true) } } diff --git a/internal/cache/bbolt/bbolt_test.go b/internal/cache/bbolt/bbolt_test.go index 6ccc28eeb..2d55bb938 100644 --- a/internal/cache/bbolt/bbolt_test.go +++ b/internal/cache/bbolt/bbolt_test.go @@ -414,7 +414,7 @@ func TestBboltCache_BulkRemove(t *testing.T) { if ls != status.LookupStatusHit { t.Errorf("expected %s got %s", status.LookupStatusHit, ls) } - bc.BulkRemove([]string{cacheKey}, true) + bc.BulkRemove([]string{cacheKey}) // it should be a cache miss _, ls, err = bc.Retrieve(cacheKey, false) @@ -436,7 +436,7 @@ func BenchmarkCache_BulkRemove(b *testing.B) { keyArray = append(keyArray, cacheKey+strconv.Itoa(n)) } - bc.BulkRemove(keyArray, true) + bc.BulkRemove(keyArray) // it should be a cache miss for n := 0; n < b.N; n++ { diff --git a/internal/cache/cache.go b/internal/cache/cache.go index 556997326..1f74eaaf5 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -36,7 +36,7 @@ type Cache interface { Retrieve(cacheKey string, allowExpired bool) ([]byte, status.LookupStatus, error) SetTTL(cacheKey string, ttl time.Duration) Remove(cacheKey string) - BulkRemove(cacheKeys []string, noLock bool) + BulkRemove(cacheKeys []string) Close() error Configuration() *config.CachingConfig } @@ -49,7 +49,7 @@ type MemoryCache interface { Retrieve(cacheKey string, allowExpired bool) ([]byte, status.LookupStatus, error) SetTTL(cacheKey string, ttl time.Duration) Remove(cacheKey string) - BulkRemove(cacheKeys []string, noLock bool) + BulkRemove(cacheKeys []string) Close() error Configuration() *config.CachingConfig StoreReference(cacheKey string, data ReferenceObject, ttl time.Duration) error diff --git a/internal/cache/filesystem/filesystem.go b/internal/cache/filesystem/filesystem.go index e43b4efc1..bef3e4348 100644 --- a/internal/cache/filesystem/filesystem.go +++ b/internal/cache/filesystem/filesystem.go @@ -138,7 +138,7 @@ func (c *Cache) retrieve(cacheKey string, allowExpired bool, atime bool) ([]byte if allowExpired || o.Expiration.IsZero() || o.Expiration.After(time.Now()) { log.Debug("filesystem cache retrieve", log.Pairs{"key": cacheKey, "dataFile": dataFile}) if atime { - c.Index.UpdateObjectAccessTime(cacheKey) + go c.Index.UpdateObjectAccessTime(cacheKey) } cache.ObserveCacheOperation(c.Name, c.Config.CacheType, "get", "hit", float64(len(data))) locks.Release(lockPrefix + cacheKey) @@ -163,18 +163,18 @@ func (c *Cache) Remove(cacheKey string) { locks.Release(lockPrefix + cacheKey) } -func (c *Cache) remove(cacheKey string, noLock bool) { +func (c *Cache) remove(cacheKey string, isBulk bool) { - if err := os.Remove(c.getFileName(cacheKey)); err == nil { - c.Index.RemoveObject(cacheKey, noLock) + if err := os.Remove(c.getFileName(cacheKey)); err == nil && !isBulk { + c.Index.RemoveObject(cacheKey) } cache.ObserveCacheDel(c.Name, c.Config.CacheType, 0) } // BulkRemove removes a list of objects from the cache -func (c *Cache) BulkRemove(cacheKeys []string, noLock bool) { +func (c *Cache) BulkRemove(cacheKeys []string) { for _, cacheKey := range cacheKeys { - c.remove(cacheKey, noLock) + c.remove(cacheKey, true) } } diff --git a/internal/cache/filesystem/filesystem_test.go b/internal/cache/filesystem/filesystem_test.go index fd38749ac..f3663d2c4 100644 --- a/internal/cache/filesystem/filesystem_test.go +++ b/internal/cache/filesystem/filesystem_test.go @@ -570,7 +570,7 @@ func TestFilesystemCache_BulkRemove(t *testing.T) { t.Errorf("expected %s got %s", status.LookupStatusHit, ls) } - fc.BulkRemove([]string{cacheKey}, true) + fc.BulkRemove([]string{cacheKey}) // it should be a cache miss _, ls, err = fc.Retrieve(cacheKey, false) @@ -591,7 +591,7 @@ func BenchmarkCache_BulkRemove(b *testing.B) { keyArray = append(keyArray, cacheKey+strconv.Itoa(n)) } - fc.BulkRemove(keyArray, true) + fc.BulkRemove(keyArray) // it should be a cache miss for n := 0; n < b.N; n++ { diff --git a/internal/cache/index/index.go b/internal/cache/index/index.go index f11cc39b0..d855e2833 100644 --- a/internal/cache/index/index.go +++ b/internal/cache/index/index.go @@ -45,7 +45,7 @@ type Index struct { name string `msg:"-"` cacheType string `msg:"-"` config config.CacheIndexConfig `msg:"-"` - bulkRemoveFunc func([]string, bool) `msg:"-"` + bulkRemoveFunc func([]string) `msg:"-"` reapInterval time.Duration `msg:"-"` flushInterval time.Duration `msg:"-"` flushFunc func(cacheKey string, data []byte) `msg:"-"` @@ -92,7 +92,7 @@ func ObjectFromBytes(data []byte) (*Object, error) { } // NewIndex returns a new Index based on the provided inputs -func NewIndex(cacheName, cacheType string, indexData []byte, cfg config.CacheIndexConfig, bulkRemoveFunc func([]string, bool), flushFunc func(cacheKey string, data []byte)) *Index { +func NewIndex(cacheName, cacheType string, indexData []byte, cfg config.CacheIndexConfig, bulkRemoveFunc func([]string), flushFunc func(cacheKey string, data []byte)) *Index { i := &Index{} if len(indexData) > 0 { @@ -183,25 +183,37 @@ func (idx *Index) UpdateObject(obj *Object) { } // RemoveObject removes an Object's Metadata from the Index -func (idx *Index) RemoveObject(key string, noLock bool) { - - if !noLock { - indexLock.Lock() - idx.lastWrite = time.Now() - } +func (idx *Index) RemoveObject(key string) { + indexLock.Lock() + idx.lastWrite = time.Now() if o, ok := idx.Objects[key]; ok { idx.CacheSize -= o.Size idx.ObjectCount-- - cache.ObserveCacheOperation(idx.name, idx.cacheType, "del", "none", float64(o.Size)) - delete(idx.Objects, key) cache.ObserveCacheSizeChange(idx.name, idx.cacheType, idx.CacheSize, idx.ObjectCount) } + indexLock.Unlock() +} + +// RemoveObjects removes a list of Objects' Metadata from the Index +func (idx *Index) RemoveObjects(keys []string, noLock bool) { + if !noLock { + indexLock.Lock() + } + for _, key := range keys { + if o, ok := idx.Objects[key]; ok { + idx.CacheSize -= o.Size + idx.ObjectCount-- + cache.ObserveCacheOperation(idx.name, idx.cacheType, "del", "none", float64(o.Size)) + delete(idx.Objects, key) + cache.ObserveCacheSizeChange(idx.name, idx.cacheType, idx.CacheSize, idx.ObjectCount) + } + } + idx.lastWrite = time.Now() if !noLock { indexLock.Unlock() } - } // GetExpiration returns the cache index's expiration for the object of the given key @@ -276,7 +288,8 @@ func (idx *Index) reap() { if len(removals) > 0 { cache.ObserveCacheEvent(idx.name, idx.cacheType, "eviction", "ttl") - idx.bulkRemoveFunc(removals, true) + go idx.bulkRemoveFunc(removals) + idx.RemoveObjects(removals, true) cacheChanged = true } @@ -332,7 +345,8 @@ func (idx *Index) reap() { if len(removals) > 0 { cache.ObserveCacheEvent(idx.name, idx.cacheType, "eviction", evictionType) - idx.bulkRemoveFunc(removals, true) + go idx.bulkRemoveFunc(removals) + idx.RemoveObjects(removals, true) cacheChanged = true } diff --git a/internal/cache/index/index_test.go b/internal/cache/index/index_test.go index ea0f451fc..98bf07b89 100644 --- a/internal/cache/index/index_test.go +++ b/internal/cache/index/index_test.go @@ -28,10 +28,7 @@ func init() { var testBulkIndex *Index -func testBulkRemoveFunc(cacheKeys []string, noLock bool) { - for _, cacheKey := range cacheKeys { - testBulkIndex.RemoveObject(cacheKey, noLock) - } +func testBulkRemoveFunc(cacheKeys []string) { } func fakeFlusherFunc(string, []byte) {} @@ -219,7 +216,7 @@ func TestRemoveObject(t *testing.T) { t.Errorf("test object missing from index") } - idx.RemoveObject("test", false) + idx.RemoveObject("test") if _, ok := idx.Objects["test"]; ok { t.Errorf("test object should be missing from index") } diff --git a/internal/cache/memory/memory.go b/internal/cache/memory/memory.go index bebd90427..d2069ebc8 100644 --- a/internal/cache/memory/memory.go +++ b/internal/cache/memory/memory.go @@ -129,7 +129,7 @@ func (c *Cache) retrieve(cacheKey string, allowExpired bool, atime bool) (*index if allowExpired || o.Expiration.IsZero() || o.Expiration.After(time.Now()) { log.Debug("memory cache retrieve", log.Pairs{"cacheKey": cacheKey}) if atime { - c.Index.UpdateObjectAccessTime(cacheKey) + go c.Index.UpdateObjectAccessTime(cacheKey) } cache.ObserveCacheOperation(c.Name, c.Config.CacheType, "get", "hit", float64(len(o.Value))) locks.Release(lockPrefix + cacheKey) @@ -154,19 +154,27 @@ func (c *Cache) Remove(cacheKey string) { c.remove(cacheKey, false) } -func (c *Cache) remove(cacheKey string, noLock bool) { +func (c *Cache) remove(cacheKey string, isBulk bool) { locks.Acquire(lockPrefix + cacheKey) c.client.Delete(cacheKey) - c.Index.RemoveObject(cacheKey, noLock) + if !isBulk { + c.Index.RemoveObject(cacheKey) + } cache.ObserveCacheDel(c.Name, c.Config.CacheType, 0) locks.Release(lockPrefix + cacheKey) } // BulkRemove removes a list of objects from the cache -func (c *Cache) BulkRemove(cacheKeys []string, noLock bool) { +func (c *Cache) BulkRemove(cacheKeys []string) { + wg := &sync.WaitGroup{} for _, cacheKey := range cacheKeys { - c.remove(cacheKey, noLock) + wg.Add(1) + go func(key string) { + c.remove(key, true) + wg.Done() + }(cacheKey) } + wg.Wait() } // Close is not used for Cache, and is here to fully prototype the Cache Interface diff --git a/internal/cache/memory/memory_test.go b/internal/cache/memory/memory_test.go index 391bea357..ea725dbb4 100644 --- a/internal/cache/memory/memory_test.go +++ b/internal/cache/memory/memory_test.go @@ -348,7 +348,7 @@ func TestCache_BulkRemove(t *testing.T) { t.Errorf("expected %s got %s", status.LookupStatusHit, ls) } - mc.BulkRemove([]string{cacheKey}, true) + mc.BulkRemove([]string{cacheKey}) // it should be a cache miss _, ls, err = mc.Retrieve(cacheKey, false) @@ -369,7 +369,7 @@ func BenchmarkCache_BulkRemove(b *testing.B) { mc := storeBenchmark(b) - mc.BulkRemove(keyArray, true) + mc.BulkRemove(keyArray) // it should be a cache miss for n := 0; n < b.N; n++ { diff --git a/internal/cache/redis/redis.go b/internal/cache/redis/redis.go index 3270348a1..1ace2bd60 100644 --- a/internal/cache/redis/redis.go +++ b/internal/cache/redis/redis.go @@ -119,7 +119,7 @@ func (c *Cache) SetTTL(cacheKey string, ttl time.Duration) { } // BulkRemove removes a list of objects from the cache. noLock is not used for Redis -func (c *Cache) BulkRemove(cacheKeys []string, noLock bool) { +func (c *Cache) BulkRemove(cacheKeys []string) { log.Debug("redis cache bulk remove", log.Pairs{}) c.client.Del(cacheKeys...) cache.ObserveCacheDel(c.Name, c.Config.CacheType, float64(len(cacheKeys))) diff --git a/internal/cache/redis/redis_test.go b/internal/cache/redis/redis_test.go index 66632993e..e9476820f 100644 --- a/internal/cache/redis/redis_test.go +++ b/internal/cache/redis/redis_test.go @@ -483,7 +483,7 @@ func TestCache_BulkRemove(t *testing.T) { t.Errorf("expected %s got %s", status.LookupStatusHit, ls) } - rc.BulkRemove([]string{cacheKey}, true) + rc.BulkRemove([]string{cacheKey}) // it should be a cache miss _, ls, err = rc.Retrieve(cacheKey, false) @@ -504,7 +504,7 @@ func BenchmarkCache_BulkRemove(b *testing.B) { keyArray = append(keyArray, cacheKey+strconv.Itoa(n)) } - rc.BulkRemove(keyArray, true) + rc.BulkRemove(keyArray) // it should be a cache miss for n := 0; n < b.N; n++ { diff --git a/internal/proxy/engines/cache_test.go b/internal/proxy/engines/cache_test.go index cb6a19b28..15a6bc89e 100644 --- a/internal/proxy/engines/cache_test.go +++ b/internal/proxy/engines/cache_test.go @@ -451,8 +451,8 @@ func (tc *testCache) Retrieve(cacheKey string, allowExpired bool) ([]byte, statu return nil, status.LookupStatusError, errTest } -func (tc *testCache) SetTTL(cacheKey string, ttl time.Duration) {} -func (tc *testCache) Remove(cacheKey string) {} -func (tc *testCache) BulkRemove(cacheKeys []string, noLock bool) {} -func (tc *testCache) Close() error { return errTest } -func (tc *testCache) Configuration() *config.CachingConfig { return tc.configuration } +func (tc *testCache) SetTTL(cacheKey string, ttl time.Duration) {} +func (tc *testCache) Remove(cacheKey string) {} +func (tc *testCache) BulkRemove(cacheKeys []string) {} +func (tc *testCache) Close() error { return errTest } +func (tc *testCache) Configuration() *config.CachingConfig { return tc.configuration } diff --git a/internal/proxy/engines/client_test.go b/internal/proxy/engines/client_test.go index 7abdc9f88..e137313f2 100644 --- a/internal/proxy/engines/client_test.go +++ b/internal/proxy/engines/client_test.go @@ -756,9 +756,6 @@ func (c *TestClient) HealthHandler(w http.ResponseWriter, r *http.Request) { } func (c *TestClient) QueryRangeHandler(w http.ResponseWriter, r *http.Request) { - - //rsc := request.NewResources(c.config, c.path - r.URL = c.BuildUpstreamURL(r) DeltaProxyCacheRequest(w, r) } diff --git a/internal/proxy/engines/deltaproxycache.go b/internal/proxy/engines/deltaproxycache.go index 6ceb5820a..86be638e6 100644 --- a/internal/proxy/engines/deltaproxycache.go +++ b/internal/proxy/engines/deltaproxycache.go @@ -91,7 +91,7 @@ func DeltaProxyCacheRequest(w http.ResponseWriter, r *http.Request) { } } - client.SetExtent(r, trq, &trq.Extent) + client.SetExtent(pr.upstreamRequest, trq, &trq.Extent) key := oc.CacheKeyPrefix + "." + pr.DeriveCacheKey(trq.TemplateURL, "") locks.Acquire(key) @@ -229,7 +229,7 @@ func DeltaProxyCacheRequest(w http.ResponseWriter, r *http.Request) { go func(e *timeseries.Extent, rq *proxyRequest) { defer wg.Done() rq.Request = rq.WithContext(tctx.WithResources(r.Context(), request.NewResources(oc, pc, cc, cache, client))) - client.SetExtent(rq.Request, trq, e) + client.SetExtent(rq.upstreamRequest, trq, e) body, resp, _ := rq.Fetch() if resp.StatusCode == http.StatusOK && len(body) > 0 { nts, err := client.UnmarshalTimeseries(body) @@ -343,7 +343,6 @@ func DeltaProxyCacheRequest(w http.ResponseWriter, r *http.Request) { } else { cdata, err := client.MarshalTimeseries(cts) if err != nil { - locks.Release(key) return } doc.Body = cdata diff --git a/internal/proxy/engines/proxy_request.go b/internal/proxy/engines/proxy_request.go index 70d23562f..48b824658 100644 --- a/internal/proxy/engines/proxy_request.go +++ b/internal/proxy/engines/proxy_request.go @@ -76,24 +76,23 @@ type proxyRequest struct { // newProxyRequest accepts the original inbound HTTP Request and Response // and returns a proxyRequest object func newProxyRequest(r *http.Request, w io.Writer) *proxyRequest { - + rsc := request.GetResources(r) pr := &proxyRequest{ Request: r, - upstreamRequest: r.Clone(context.Background()), + upstreamRequest: r.Clone(tctx.WithResources(context.Background(), rsc)), contentLength: -1, responseWriter: w, started: time.Now(), } - - rsc := request.GetResources(r) - pr.upstreamRequest = pr.upstreamRequest.WithContext(tctx.WithResources(pr.upstreamRequest.Context(), rsc)) - return pr } func (pr *proxyRequest) Clone() *proxyRequest { + rsc := request.GetResources(pr.Request) return &proxyRequest{ - Request: pr.Request.Clone(context.Background()), + Request: pr.Request.Clone(context.Background()), + upstreamRequest: pr.upstreamRequest. + Clone(tctx.WithResources(context.Background(), rsc)), cacheDocument: pr.cacheDocument, key: pr.key, cacheStatus: pr.cacheStatus, @@ -114,7 +113,7 @@ func (pr *proxyRequest) Clone() *proxyRequest { // response and elapsed time to the caller. func (pr *proxyRequest) Fetch() ([]byte, *http.Response, time.Duration) { - rsc := request.GetResources(pr.Request) + rsc := request.GetResources(pr.upstreamRequest) oc := rsc.OriginConfig pc := rsc.PathConfig @@ -124,7 +123,7 @@ func (pr *proxyRequest) Fetch() ([]byte, *http.Response, time.Duration) { } start := time.Now() - reader, resp, _ := PrepareFetchReader(pr.Request) + reader, resp, _ := PrepareFetchReader(pr.upstreamRequest) var body []byte var err error diff --git a/pkg/locks/locks.go b/pkg/locks/locks.go index dcb6dc9b7..0e638f85f 100644 --- a/pkg/locks/locks.go +++ b/pkg/locks/locks.go @@ -16,6 +16,7 @@ package locks import ( + "fmt" "sync" ) @@ -23,44 +24,43 @@ var locks = make(map[string]*namedLock) var mapLock = sync.Mutex{} type namedLock struct { + *sync.Mutex name string - mtx *sync.Mutex queueSize int } func newNamedLock(name string) *namedLock { return &namedLock{ - name: name, - mtx: &sync.Mutex{}, + name: name, + Mutex: &sync.Mutex{}, } } // Acquire returns a named lock, and blocks until it is acquired -func Acquire(lockName string) *sync.Mutex { - - var nl *namedLock - var ok bool +func Acquire(lockName string) error { if lockName == "" { - return nil + return fmt.Errorf("invalid lock name: %s", lockName) } mapLock.Lock() - if nl, ok = locks[lockName]; !ok { + nl, ok := locks[lockName] + if !ok { nl = newNamedLock(lockName) locks[lockName] = nl } nl.queueSize++ mapLock.Unlock() - nl.mtx.Lock() - return nl.mtx + + nl.Lock() + return nil } // Release unlocks and releases a named lock -func Release(lockName string) { +func Release(lockName string) error { if lockName == "" { - return + return fmt.Errorf("invalid lock name: %s", lockName) } mapLock.Lock() @@ -69,7 +69,10 @@ func Release(lockName string) { if nl.queueSize == 0 { delete(locks, lockName) } - nl.mtx.Unlock() + mapLock.Unlock() + nl.Unlock() + return nil } mapLock.Unlock() + return fmt.Errorf("no such lock name: %s", lockName) } diff --git a/pkg/locks/locks_test.go b/pkg/locks/locks_test.go index 196648f09..2f0d57a2b 100644 --- a/pkg/locks/locks_test.go +++ b/pkg/locks/locks_test.go @@ -14,11 +14,14 @@ package locks import ( + "math/rand" "sync" "testing" "time" ) +const testKey = "testKey" + func TestLocks(t *testing.T) { var testVal = 0 @@ -44,12 +47,53 @@ func TestLocks(t *testing.T) { t.Errorf("expected 11 got %d", testVal) } - // Cover Empty String Cases - mtx := Acquire("") - if mtx != nil { - t.Errorf("expected nil got %v", mtx) + expected := "invalid lock name: " + err := Acquire("") + if err.Error() != expected { + t.Errorf("got %s expected %s", err.Error(), expected) + } + + err = Release("") + if err.Error() != expected { + t.Errorf("got %s expected %s", err.Error(), expected) + } + + expected = "no such lock name: invalid" + err = Release("invalid") + if err.Error() != expected { + t.Errorf("got %s expected %s", err.Error(), expected) + } + +} + +func TestLocksConcurrent(t *testing.T) { + + const size = 10000000 + + wg := &sync.WaitGroup{} + errs := make([]error, 0, size) + + rand.Seed(time.Now().UnixNano()) + + for i := 0; i < size; i++ { + wg.Add(1) + go func() { + err := Acquire(testKey) + if err != nil { + errs = append(errs, err) + } + err = Release(testKey) + if err != nil { + errs = append(errs, err) + } + wg.Done() + }() + } + + wg.Wait() + + for _, err := range errs { + t.Error(err) } - // Shouldn't matter but covers the code - Release("") } From 8b22016adec87db654ad383c8a396c92db43f9bd Mon Sep 17 00:00:00 2001 From: James Ranson Date: Thu, 23 Apr 2020 17:08:50 -0600 Subject: [PATCH 3/4] Create publish-release.yml --- .github/workflows/publish-release.yml | 103 ++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 .github/workflows/publish-release.yml diff --git a/.github/workflows/publish-release.yml b/.github/workflows/publish-release.yml new file mode 100644 index 000000000..5e83c9967 --- /dev/null +++ b/.github/workflows/publish-release.yml @@ -0,0 +1,103 @@ +on: + push: + tags: + - 'v[0-9]+\.[0-9]+\.[0-9]+' + +name: Publish Trickster Release to GitHub and Docker Hub + +jobs: + build: + name: Publish Release + runs-on: ubuntu-latest + steps: + - name: Get current date + id: date + run: echo "::set-output name=date::$(date +'%Y-%m-%d')" + - name: Get bare tag + id: baretag + run: echo "::set-output name=baretag::$(echo ${{ github.ref }} | cut -b 12-)" + - name: Checkout code + uses: actions/checkout@v2 + - name: Build project + run: | + TAGVER=${{ steps.baretag.outputs.baretag }} make release + - name: Create Release + id: create_release + uses: actions/create-release@v1 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + tag_name: ${{ github.ref }} + release_name: Release v${{ steps.baretag.outputs.baretag }} / ${{ steps.date.outputs.date }} + draft: false + prerelease: false + - name: Upload Release Asset (linux-amd64) + id: upload-release-asset-linux-amd64 + uses: actions/upload-release-asset@v1 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + upload_url: ${{ steps.create_release.outputs.upload_url }} + asset_path: ./OPATH/trickster-${{ steps.baretag.outputs.baretag }}.linux-amd64.tar.gz + asset_name: trickster-${{ steps.baretag.outputs.baretag }}.linux-amd64.tar.gz + asset_content_type: application/gzip + - name: Upload Release Asset (linux-arm64) + id: upload-release-asset-linux-arm64 + uses: actions/upload-release-asset@v1 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + upload_url: ${{ steps.create_release.outputs.upload_url }} + asset_path: ./OPATH/trickster-${{ steps.baretag.outputs.baretag }}.linux-arm64.tar.gz + asset_name: trickster-${{ steps.baretag.outputs.baretag }}.linux-arm64.tar.gz + asset_content_type: application/gzip + - name: Upload Release Asset (darwin-amd64) + id: upload-release-asset-darwin-amd64 + uses: actions/upload-release-asset@v1 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + upload_url: ${{ steps.create_release.outputs.upload_url }} + asset_path: ./OPATH/trickster-${{ steps.baretag.outputs.baretag }}.darwin-amd64.tar.gz + asset_name: trickster-${{ steps.baretag.outputs.baretag }}.darwin-amd64.tar.gz + asset_content_type: application/gzip + - name: Upload Release Asset (windows-amd64) + id: upload-release-asset-windows-amd64 + uses: actions/upload-release-asset@v1 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + upload_url: ${{ steps.create_release.outputs.upload_url }} + asset_path: ./OPATH/trickster-${{ steps.baretag.outputs.baretag }}.windows-amd64.tar.gz + asset_name: trickster-${{ steps.baretag.outputs.baretag }}.windows-amd64.tar.gz + asset_content_type: application/gzip + - name: build-push-tricksterio-amd + uses: docker/build-push-action@v1 + with: + username: ${{ secrets.DOCKER_USERNAME }} + password: ${{ secrets.DOCKER_PASSWORD }} + repository: tricksterio/trickster + tags: ${{ steps.baretag.outputs.baretag }} + - name: build-push-tricksterproxy-amd + uses: docker/build-push-action@v1 + with: + username: ${{ secrets.DOCKER_USERNAME }} + password: ${{ secrets.DOCKER_PASSWORD }} + repository: tricksterproxy/trickster + tags: ${{ steps.baretag.outputs.baretag }} + - name: build-push-tricksterio-arm + uses: docker/build-push-action@v1 + with: + username: ${{ secrets.DOCKER_USERNAME }} + password: ${{ secrets.DOCKER_PASSWORD }} + repository: tricksterio/trickster + build_args: IMAGE_ARCH=arm64v8,GOARCH=arm64 + tags: arm64v8-${{ steps.baretag.outputs.baretag }} + - name: build-push-tricksterproxy-arm + uses: docker/build-push-action@v1 + with: + username: ${{ secrets.DOCKER_USERNAME }} + password: ${{ secrets.DOCKER_PASSWORD }} + repository: tricksterproxy/trickster + build_args: IMAGE_ARCH=arm64v8,GOARCH=arm64 + tags: arm64v8-${{ steps.baretag.outputs.baretag }} From 2093f389ad83bbfeffacd338a167c58b0176afbc Mon Sep 17 00:00:00 2001 From: James Ranson Date: Sun, 3 May 2020 16:42:35 -0600 Subject: [PATCH 4/4] Create publish-release-candidate.yaml --- .../workflows/publish-release-candidate.yaml | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 .github/workflows/publish-release-candidate.yaml diff --git a/.github/workflows/publish-release-candidate.yaml b/.github/workflows/publish-release-candidate.yaml new file mode 100644 index 000000000..b07150a99 --- /dev/null +++ b/.github/workflows/publish-release-candidate.yaml @@ -0,0 +1,103 @@ +on: + push: + tags: + - 'v[0-9]+\.[0-9]+\.[0-9]+-rc[0-9]+' + +name: Publish Trickster Release Candidate to GitHub and Docker Hub + +jobs: + build: + name: Publish Release Candidate + runs-on: ubuntu-latest + steps: + - name: Get current date + id: date + run: echo "::set-output name=date::$(date +'%Y-%m-%d')" + - name: Get bare tag + id: baretag + run: echo "::set-output name=baretag::$(echo ${{ github.ref }} | cut -b 12-)" + - name: Checkout code + uses: actions/checkout@v2 + - name: Build project + run: | + TAGVER=${{ steps.baretag.outputs.baretag }} make release + - name: Create Release + id: create_release + uses: actions/create-release@v1 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + tag_name: ${{ github.ref }} + release_name: Release v${{ steps.baretag.outputs.baretag }} / ${{ steps.date.outputs.date }} + draft: false + prerelease: true + - name: Upload Release Asset (linux-amd64) + id: upload-release-asset-linux-amd64 + uses: actions/upload-release-asset@v1 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + upload_url: ${{ steps.create_release.outputs.upload_url }} + asset_path: ./OPATH/trickster-${{ steps.baretag.outputs.baretag }}.linux-amd64.tar.gz + asset_name: trickster-${{ steps.baretag.outputs.baretag }}.linux-amd64.tar.gz + asset_content_type: application/gzip + - name: Upload Release Asset (linux-arm64) + id: upload-release-asset-linux-arm64 + uses: actions/upload-release-asset@v1 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + upload_url: ${{ steps.create_release.outputs.upload_url }} + asset_path: ./OPATH/trickster-${{ steps.baretag.outputs.baretag }}.linux-arm64.tar.gz + asset_name: trickster-${{ steps.baretag.outputs.baretag }}.linux-arm64.tar.gz + asset_content_type: application/gzip + - name: Upload Release Asset (darwin-amd64) + id: upload-release-asset-darwin-amd64 + uses: actions/upload-release-asset@v1 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + upload_url: ${{ steps.create_release.outputs.upload_url }} + asset_path: ./OPATH/trickster-${{ steps.baretag.outputs.baretag }}.darwin-amd64.tar.gz + asset_name: trickster-${{ steps.baretag.outputs.baretag }}.darwin-amd64.tar.gz + asset_content_type: application/gzip + - name: Upload Release Asset (windows-amd64) + id: upload-release-asset-windows-amd64 + uses: actions/upload-release-asset@v1 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + upload_url: ${{ steps.create_release.outputs.upload_url }} + asset_path: ./OPATH/trickster-${{ steps.baretag.outputs.baretag }}.windows-amd64.tar.gz + asset_name: trickster-${{ steps.baretag.outputs.baretag }}.windows-amd64.tar.gz + asset_content_type: application/gzip + - name: build-push-tricksterio-amd + uses: docker/build-push-action@v1 + with: + username: ${{ secrets.DOCKER_USERNAME }} + password: ${{ secrets.DOCKER_PASSWORD }} + repository: tricksterio/trickster + tags: ${{ steps.baretag.outputs.baretag }} + - name: build-push-tricksterproxy-amd + uses: docker/build-push-action@v1 + with: + username: ${{ secrets.DOCKER_USERNAME }} + password: ${{ secrets.DOCKER_PASSWORD }} + repository: tricksterproxy/trickster + tags: ${{ steps.baretag.outputs.baretag }} + - name: build-push-tricksterio-arm + uses: docker/build-push-action@v1 + with: + username: ${{ secrets.DOCKER_USERNAME }} + password: ${{ secrets.DOCKER_PASSWORD }} + repository: tricksterio/trickster + build_args: IMAGE_ARCH=arm64v8,GOARCH=arm64 + tags: arm64v8-${{ steps.baretag.outputs.baretag }} + - name: build-push-tricksterproxy-arm + uses: docker/build-push-action@v1 + with: + username: ${{ secrets.DOCKER_USERNAME }} + password: ${{ secrets.DOCKER_PASSWORD }} + repository: tricksterproxy/trickster + build_args: IMAGE_ARCH=arm64v8,GOARCH=arm64 + tags: arm64v8-${{ steps.baretag.outputs.baretag }}