diff options
Diffstat (limited to 'internal/vfs/zip/vfs.go')
-rw-r--r-- | internal/vfs/zip/vfs.go | 63 |
1 files changed, 48 insertions, 15 deletions
diff --git a/internal/vfs/zip/vfs.go b/internal/vfs/zip/vfs.go index 3b69d1e9..692a4a69 100644 --- a/internal/vfs/zip/vfs.go +++ b/internal/vfs/zip/vfs.go @@ -110,6 +110,20 @@ func New(options ...Option) vfs.VFS { return zipVFS } +func (fs *zipVFS) keyFromPath(path string) (string, error) { + // We assume that our URL is https://.../artifacts.zip?content-sign=aaa + // our caching key is `https://.../artifacts.zip` + // TODO: replace caching key with file_sha256 + // https://gitlab.com/gitlab-org/gitlab-pages/-/issues/489 + key, err := url.Parse(path) + if err != nil { + return "", err + } + key.RawQuery = "" + key.Fragment = "" + return key.String(), nil +} + // Root opens an archive given a URL path and returns an instance of zipArchive // that implements the vfs.VFS interface. // To avoid using locks, the findOrOpenArchive function runs inside of a for @@ -118,14 +132,14 @@ func New(options ...Option) vfs.VFS { // to try and find the cached archive or return if there's an error, for example // if the context is canceled. func (fs *zipVFS) Root(ctx context.Context, path string) (vfs.Root, error) { - urlPath, err := url.Parse(path) + key, err := fs.keyFromPath(path) if err != nil { return nil, err } // we do it in loop to not use any additional locks for { - root, err := fs.findOrOpenArchive(ctx, urlPath.String()) + root, err := fs.findOrOpenArchive(ctx, key, path) if err == errAlreadyCached { continue } @@ -147,34 +161,53 @@ func (fs *zipVFS) Name() string { // otherwise creates the archive entry in a cache and try to save it, // if saving fails it's because the archive has already been cached // (e.g. by another concurrent request) -func (fs *zipVFS) findOrCreateArchive(ctx context.Context, path string) (*zipArchive, error) { +func (fs *zipVFS) findOrCreateArchive(ctx context.Context, key string) (*zipArchive, error) { // This needs to happen in lock to ensure that // concurrent access will not remove it // it is needed due to the bug https://github.com/patrickmn/go-cache/issues/48 fs.cacheLock.Lock() defer fs.cacheLock.Unlock() - archive, expiry, found := fs.cache.GetWithExpiration(path) + archive, expiry, found := fs.cache.GetWithExpiration(key) if found { - metrics.ZipCacheRequests.WithLabelValues("archive", "hit").Inc() + status, _ := archive.(*zipArchive).openStatus() + switch status { + case archiveOpening: + metrics.ZipCacheRequests.WithLabelValues("archive", "hit-opening").Inc() - if opened, err := archive.(*zipArchive).openStatus(); opened && err == nil { + case archiveOpenError: + // this means that archive is likely corrupted + // we keep it for duration of cache entry expiry (negative cache) + metrics.ZipCacheRequests.WithLabelValues("archive", "hit-open-error").Inc() + + case archiveOpened: if time.Until(expiry) < fs.cacheRefreshInterval { - // refresh item that has been opened successfully - fs.cache.SetDefault(path, archive) + fs.cache.SetDefault(key, archive) + metrics.ZipCacheRequests.WithLabelValues("archive", "hit-refresh").Inc() + } else { + metrics.ZipCacheRequests.WithLabelValues("archive", "hit").Inc() } + + case archiveCorrupted: + // this means that archive is likely changed + // we should invalidate it immediately + metrics.ZipCacheRequests.WithLabelValues("archive", "corrupted").Inc() + archive = nil } - } else { - archive = newArchive(fs, path, fs.openTimeout) + } + + if archive == nil { + archive = newArchive(fs, fs.openTimeout) // We call delete to ensure that expired item // is properly evicted as there's a bug in a cache library: // https://github.com/patrickmn/go-cache/issues/48 - fs.cache.Delete(path) + fs.cache.Delete(key) // 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, fs.cacheExpirationInterval) != nil { + if fs.cache.Add(key, archive, fs.cacheExpirationInterval) != nil { + metrics.ZipCacheRequests.WithLabelValues("archive", "already-cached").Inc() return nil, errAlreadyCached } @@ -186,13 +219,13 @@ func (fs *zipVFS) findOrCreateArchive(ctx context.Context, path string) (*zipArc } // findOrOpenArchive gets archive from cache and tries to open it -func (fs *zipVFS) findOrOpenArchive(ctx context.Context, path string) (*zipArchive, error) { - zipArchive, err := fs.findOrCreateArchive(ctx, path) +func (fs *zipVFS) findOrOpenArchive(ctx context.Context, key, path string) (*zipArchive, error) { + zipArchive, err := fs.findOrCreateArchive(ctx, key) if err != nil { return nil, err } - err = zipArchive.openArchive(ctx) + err = zipArchive.openArchive(ctx, path) if err != nil { return nil, err } |