diff options
author | Kamil Trzciński <ayufan@ayufan.eu> | 2020-10-27 13:24:47 +0300 |
---|---|---|
committer | Kamil Trzciński <ayufan@ayufan.eu> | 2020-10-27 13:24:47 +0300 |
commit | 0db58b5ce378aad7106ea1c0d04fa50e3b5ad54e (patch) | |
tree | b7f76c1b8fb5853e2136152a8a863ade0ee0ba88 | |
parent | 9cf62c0fc1f31a6e175bb3e8b2321ca19584dee3 (diff) |
Make timeouts for ZIP configurable
-rw-r--r-- | internal/vfs/zip/archive.go | 3 | ||||
-rw-r--r-- | internal/vfs/zip/vfs.go | 61 | ||||
-rw-r--r-- | internal/vfs/zip/vfs_test.go | 14 |
3 files changed, 61 insertions, 17 deletions
diff --git a/internal/vfs/zip/archive.go b/internal/vfs/zip/archive.go index ba15af20..47295dc0 100644 --- a/internal/vfs/zip/archive.go +++ b/internal/vfs/zip/archive.go @@ -24,9 +24,6 @@ import ( const ( dirPrefix = "public/" maxSymlinkSize = 256 - - // DefaultOpenTimeout to request an archive and read its contents the first time - DefaultOpenTimeout = 30 * time.Second ) var ( diff --git a/internal/vfs/zip/vfs.go b/internal/vfs/zip/vfs.go index 78a77e1c..0685c5d9 100644 --- a/internal/vfs/zip/vfs.go +++ b/internal/vfs/zip/vfs.go @@ -18,6 +18,7 @@ const ( defaultCacheExpirationInterval = time.Minute defaultCacheCleanupInterval = time.Minute / 2 defaultCacheRefreshInterval = time.Minute / 2 + defaultOpenTimeout = time.Minute / 2 // we assume that each item costs around 100 bytes // this gives around 5MB of raw memory needed without acceleration structures @@ -39,26 +40,72 @@ type zipVFS struct { cache *cache.Cache cacheLock sync.Mutex + defaultOpenTimeout time.Duration + cacheExpirationInterval time.Duration + cacheRefreshInterval time.Duration + cacheCleanupInterval time.Duration + dataOffsetCache *lruCache readlinkCache *lruCache archiveCount int64 } +// Option function allows to override default values +type Option func(*zipVFS) + +// WithCacheRefreshInterval when used it can override defaultCacheRefreshInterval +func WithCacheRefreshInterval(interval time.Duration) Option { + return func(vfs *zipVFS) { + vfs.cacheRefreshInterval = interval + } +} + +// WithCacheExpirationInterval when used it can override defaultCacheExpirationInterval +func WithCacheExpirationInterval(interval time.Duration) Option { + return func(vfs *zipVFS) { + vfs.cacheExpirationInterval = interval + } +} + +// WithCacheCleanupInterval when used it can override defaultCacheCleanupInterval +func WithCacheCleanupInterval(interval time.Duration) Option { + return func(vfs *zipVFS) { + vfs.cacheCleanupInterval = interval + } +} + +// WithDefaultOpenTimeout when used it can override defaultOpenTimeout +func WithDefaultOpenTimeout(interval time.Duration) Option { + return func(vfs *zipVFS) { + vfs.defaultOpenTimeout = interval + } +} + // New creates a zipVFS instance that can be used by a serving request -func New() vfs.VFS { +func New(options ...Option) vfs.VFS { zipVFS := &zipVFS{ - cache: cache.New(defaultCacheExpirationInterval, defaultCacheCleanupInterval), - dataOffsetCache: newLruCache("data-offset", defaultDataOffsetItems, defaultDataOffsetExpirationInterval), - readlinkCache: newLruCache("readlink", defaultReadlinkItems, defaultReadlinkExpirationInterval), + cacheExpirationInterval: defaultCacheExpirationInterval, + cacheRefreshInterval: defaultCacheRefreshInterval, + cacheCleanupInterval: defaultCacheCleanupInterval, + defaultOpenTimeout: defaultOpenTimeout, + } + + for _, option := range options { + option(zipVFS) } + zipVFS.cache = cache.New(zipVFS.cacheExpirationInterval, zipVFS.cacheCleanupInterval) zipVFS.cache.OnEvicted(func(s string, i interface{}) { metrics.ZipCachedEntries.WithLabelValues("archive").Dec() i.(*zipArchive).onEvicted() }) + // TODO: To be removed with https://gitlab.com/gitlab-org/gitlab-pages/-/issues/480 + zipVFS.dataOffsetCache = newLruCache("data-offset", defaultDataOffsetItems, defaultDataOffsetExpirationInterval) + zipVFS.readlinkCache = newLruCache("readlink", defaultReadlinkItems, defaultReadlinkExpirationInterval) + return zipVFS } @@ -106,12 +153,12 @@ func (fs *zipVFS) findOrCreateArchive(ctx context.Context, path string) (*zipArc metrics.ZipCacheRequests.WithLabelValues("archive", "hit").Inc() // TODO: do not refreshed errored archives https://gitlab.com/gitlab-org/gitlab-pages/-/merge_requests/351 - if time.Until(expiry) < defaultCacheRefreshInterval { + if time.Until(expiry) < fs.cacheRefreshInterval { // refresh item fs.cache.SetDefault(path, archive) } } else { - archive = newArchive(fs, path, DefaultOpenTimeout) + archive = newArchive(fs, path, fs.defaultOpenTimeout) // We call delete to ensure that expired item // is properly evicted as there's a bug in a cache library: @@ -120,7 +167,7 @@ func (fs *zipVFS) findOrCreateArchive(ctx context.Context, path string) (*zipArc // if adding the archive to the cache fails it means it's already been added before // this is done to find concurrent additions. - if fs.cache.Add(path, archive, cache.DefaultExpiration) != nil { + if fs.cache.Add(path, archive, fs.cacheExpirationInterval) != nil { return nil, errAlreadyCached } diff --git a/internal/vfs/zip/vfs_test.go b/internal/vfs/zip/vfs_test.go index c12e49cd..a9737a43 100644 --- a/internal/vfs/zip/vfs_test.go +++ b/internal/vfs/zip/vfs_test.go @@ -99,29 +99,29 @@ func TestVFSFindOrOpenArchiveConcurrentAccess(t *testing.T) { }, time.Second, time.Nanosecond) } -func TestVFSFindOrCreateArchiveCacheEvict(t *testing.T) { +func TestVFSArchiveCacheEvict(t *testing.T) { testServerURL, cleanup := newZipFileServerURL(t, "group/zip.gitlab.io/public.zip", nil) defer cleanup() path := testServerURL + "/public.zip" - vfs := New().(*zipVFS) + vfs := New( + WithCacheExpirationInterval(time.Nanosecond), + ).(*zipVFS) archivesMetric := metrics.ZipCachedEntries.WithLabelValues("archive") archivesCount := testutil.ToFloat64(archivesMetric) // create a new archive and increase counters - archive, err := vfs.findOrOpenArchive(context.Background(), path) + archive, err := vfs.Root(context.Background(), path) require.NoError(t, err) require.NotNil(t, archive) - // inject into cache to be "expired" - // (we could as well wait `defaultCacheExpirationInterval`) - vfs.cache.Set(path, archive, time.Nanosecond) + // wait for archive to expire time.Sleep(time.Nanosecond) // a new object is created - archive2, err := vfs.findOrOpenArchive(context.Background(), path) + archive2, err := vfs.Root(context.Background(), path) require.NoError(t, err) require.NotNil(t, archive2) require.NotEqual(t, archive, archive2, "a different archive is returned") |