diff options
author | Kamil Trzciński <ayufan@ayufan.eu> | 2020-10-07 16:53:50 +0300 |
---|---|---|
committer | Kamil Trzciński <ayufan@ayufan.eu> | 2020-10-13 00:13:32 +0300 |
commit | e7a571d304492cbca9f54d10f08374100f4cd66a (patch) | |
tree | 8a54ac215132d9c887821d7a69e246012d415c7d | |
parent | 61acef8cafafbab2bf1689a97fecddddbd726152 (diff) |
Update metrics
-rw-r--r-- | acceptance_test.go | 4 | ||||
-rw-r--r-- | internal/vfs/zip/archive.go | 72 | ||||
-rw-r--r-- | internal/vfs/zip/lru_cache.go | 52 | ||||
-rw-r--r-- | internal/vfs/zip/vfs.go | 31 | ||||
-rw-r--r-- | metrics/metrics.go | 41 |
5 files changed, 102 insertions, 98 deletions
diff --git a/acceptance_test.go b/acceptance_test.go index 1df6abfb..a3fedc24 100644 --- a/acceptance_test.go +++ b/acceptance_test.go @@ -497,9 +497,7 @@ func TestPrometheusMetricsCanBeScraped(t *testing.T) { // zip archives require.Contains(t, string(body), "gitlab_pages_zip_opened") require.Contains(t, string(body), "gitlab_pages_zip_cache_requests") - require.Contains(t, string(body), "gitlab_pages_zip_data_offset_cache_requests") - require.Contains(t, string(body), "gitlab_pages_zip_readlink_cache_requests") - require.Contains(t, string(body), "gitlab_pages_zip_cached_archives") + require.Contains(t, string(body), "gitlab_pages_zip_cached_entries") require.Contains(t, string(body), "gitlab_pages_zip_archive_entries_cached") require.Contains(t, string(body), "gitlab_pages_zip_opened_entries_count") } diff --git a/internal/vfs/zip/archive.go b/internal/vfs/zip/archive.go index 74a73121..630d5fd5 100644 --- a/internal/vfs/zip/archive.go +++ b/internal/vfs/zip/archive.go @@ -47,7 +47,7 @@ type zipArchive struct { done chan struct{} openTimeout time.Duration - cacheKey string + namespace string resource *httprange.Resource reader *httprange.RangedReader @@ -64,7 +64,7 @@ func newArchive(fs *zipVFS, path string, openTimeout time.Duration) *zipArchive done: make(chan struct{}), files: make(map[string]*zip.File), openTimeout: openTimeout, - cacheKey: strconv.FormatInt(atomic.AddInt64(&fs.archiveCount, 1), 10) + ":", + namespace: strconv.FormatInt(atomic.AddInt64(&fs.archiveCount, 1), 10) + ":", } } @@ -167,13 +167,15 @@ func (a *zipArchive) Open(ctx context.Context, name string) (vfs.File, error) { return nil, os.ErrNotExist } - dataOffset, err := a.getDataOffset(name, file) + dataOffset, err := a.fs.dataOffsetCache.findOrFetch(a.namespace, name, func() (interface{}, error) { + return file.DataOffset() + }) if err != nil { return nil, err } // only read from dataOffset up to the size of the compressed file - reader := a.reader.SectionReader(ctx, dataOffset, int64(file.CompressedSize64)) + reader := a.reader.SectionReader(ctx, dataOffset.(int64), int64(file.CompressedSize64)) switch file.Method { case zip.Deflate: @@ -185,29 +187,6 @@ func (a *zipArchive) Open(ctx context.Context, name string) (vfs.File, error) { } } -func (a *zipArchive) getDataOffset(name string, file *zip.File) (int64, error) { - var dataOffset int64 - - item := a.fs.dataOffsetCache.Get(a.cacheKey + name) - if item == nil || item.Expired() { - var err error - - dataOffset, err = file.DataOffset() - if err != nil { - return 0, err - } - - a.fs.dataOffsetCache.Set(a.cacheKey+name, dataOffset, DataOffsetCacheInterval) - - metrics.ZipServingArchiveDataOffsetCache.WithLabelValues("miss").Inc() - } else { - dataOffset = item.Value().(int64) - metrics.ZipServingArchiveDataOffsetCache.WithLabelValues("hit").Inc() - } - - return dataOffset, nil -} - // Lstat finds the file by name inside the zipArchive and returns its FileInfo func (a *zipArchive) Lstat(ctx context.Context, name string) (os.FileInfo, error) { file := a.findFile(name) @@ -228,27 +207,11 @@ func (a *zipArchive) Readlink(ctx context.Context, name string) (string, error) if file.FileInfo().Mode()&os.ModeSymlink != os.ModeSymlink { return "", errNotSymlink } - symlink, err := a.getSymlink(name, file) - if err != nil { - return "", err - } - // return errSymlinkSize if the number of bytes read from the link is too big - if len(symlink) > maxSymlinkSize { - return "", errSymlinkSize - } - - return symlink, nil -} - -func (a *zipArchive) getSymlink(name string, file *zip.File) (string, error) { - var symlink string - - item := a.fs.readlinkCache.Get(a.cacheKey + name) - if item == nil || item.Expired() { + symlinkValue, err := a.fs.readlinkCache.findOrFetch(a.namespace, name, func() (interface{}, error) { rc, err := file.Open() if err != nil { - return "", err + return nil, err } defer rc.Close() @@ -258,17 +221,20 @@ func (a *zipArchive) getSymlink(name string, file *zip.File) (string, error) { n, err := io.ReadFull(rc, link[:]) if err != nil && err != io.ErrUnexpectedEOF { // if err == io.ErrUnexpectedEOF the link is smaller than len(symlink) so it's OK to not return it - return "", err + return nil, err } - symlink = string(link[:n]) - // cache symlink up to desired size - a.fs.readlinkCache.Set(a.cacheKey+name, symlink, ReadLinkCacheInterval) + return string(link[:n]), nil + }) + if err != nil { + return "", err + } + + symlink := symlinkValue.(string) - metrics.ZipServingArchiveReadlinkCache.WithLabelValues("miss").Inc() - } else { - symlink = item.Value().(string) - metrics.ZipServingArchiveReadlinkCache.WithLabelValues("hit").Inc() + // return errSymlinkSize if the number of bytes read from the link is too big + if len(symlink) > maxSymlinkSize { + return "", errSymlinkSize } return symlink, nil diff --git a/internal/vfs/zip/lru_cache.go b/internal/vfs/zip/lru_cache.go new file mode 100644 index 00000000..d33c81e2 --- /dev/null +++ b/internal/vfs/zip/lru_cache.go @@ -0,0 +1,52 @@ +package zip + +import ( + "time" + + "github.com/karlseguin/ccache/v2" + "gitlab.com/gitlab-org/gitlab-pages/metrics" +) + +type lruCache struct { + op string + duration time.Duration + cache *ccache.Cache +} + +func newLruCache(op string, maxEntries uint32, duration time.Duration) *lruCache { + configuration := ccache.Configure() + configuration.MaxSize(int64(maxEntries)) + configuration.ItemsToPrune(maxEntries / 16) + configuration.GetsPerPromote(64) // if item gets requested frequently promote it + configuration.OnDelete(func(*ccache.Item) { + metrics.ZipCachedEntries.WithLabelValues(op).Dec() + }) + + c := &lruCache{ + cache: ccache.New(configuration), + duration: duration, + } + + return c +} + +func (c *lruCache) findOrFetch(namespace, key string, fetchFn func() (interface{}, error)) (interface{}, error) { + item := c.cache.Get(namespace + key) + + if item != nil && !item.Expired() { + metrics.ZipCacheRequests.WithLabelValues(c.op, "hit").Inc() + return item.Value(), nil + } + + value, err := fetchFn() + if err != nil { + metrics.ZipCacheRequests.WithLabelValues(c.op, "error").Inc() + return nil, err + } + + metrics.ZipCacheRequests.WithLabelValues(c.op, "miss").Inc() + metrics.ZipCachedEntries.WithLabelValues(c.op).Inc() + + c.cache.Set(namespace+key, value, c.duration) + return value, nil +} diff --git a/internal/vfs/zip/vfs.go b/internal/vfs/zip/vfs.go index a39a1cfe..71899e80 100644 --- a/internal/vfs/zip/vfs.go +++ b/internal/vfs/zip/vfs.go @@ -6,7 +6,6 @@ import ( "net/url" "time" - "github.com/karlseguin/ccache/v2" "github.com/patrickmn/go-cache" "gitlab.com/gitlab-org/gitlab-pages/internal/vfs" @@ -18,6 +17,16 @@ const ( defaultCacheExpirationInterval = time.Minute defaultCacheCleanupInterval = time.Minute / 2 defaultCacheRefreshInterval = time.Minute / 2 + + // we assume that each item costs around 100 bytes + // this gives around 5MB of raw memory needed without acceleration structures + defaultDataOffsetItems = 50000 + defaultDataOffsetExpirationInterval = time.Minute + + // we assume that each item costs around 200 bytes + // this gives around 2MB of raw memory needed without acceleration structures + defaultReadlinkItems = 10000 + defaultReadlinkExpirationInterval = time.Minute ) var ( @@ -26,9 +35,10 @@ var ( // zipVFS is a simple cached implementation of the vfs.VFS interface type zipVFS struct { - cache *cache.Cache - dataOffsetCache *ccache.Cache - readlinkCache *ccache.Cache + cache *cache.Cache + + dataOffsetCache *lruCache + readlinkCache *lruCache archiveCount int64 } @@ -36,14 +46,13 @@ type zipVFS struct { // New creates a zipVFS instance that can be used by a serving request func New() vfs.VFS { zipVFS := &zipVFS{ - // TODO: add cache operation callbacks https://gitlab.com/gitlab-org/gitlab-pages/-/issues/465 cache: cache.New(defaultCacheExpirationInterval, defaultCacheCleanupInterval), - dataOffsetCache: ccache.New(ccache.Configure().MaxSize(10000).ItemsToPrune(2000)), - readlinkCache: ccache.New(ccache.Configure().MaxSize(1000).ItemsToPrune(200)), + dataOffsetCache: newLruCache("data-offset", defaultDataOffsetItems, defaultDataOffsetExpirationInterval), + readlinkCache: newLruCache("readlink", defaultReadlinkItems, defaultReadlinkExpirationInterval), } zipVFS.cache.OnEvicted(func(s string, i interface{}) { - metrics.ZipCachedArchives.Dec() + metrics.ZipCachedEntries.WithLabelValues("archive").Dec() i.(*zipArchive).onEvicted() }) @@ -85,7 +94,7 @@ func (fs *zipVFS) Name() string { func (fs *zipVFS) findOrOpenArchive(ctx context.Context, path string) (*zipArchive, error) { archive, expiry, found := fs.cache.GetWithExpiration(path) if found { - metrics.ZipServingArchiveCache.WithLabelValues("hit").Inc() + 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 { @@ -100,8 +109,8 @@ func (fs *zipVFS) findOrOpenArchive(ctx context.Context, path string) (*zipArchi if fs.cache.Add(path, archive, cache.DefaultExpiration) != nil { return nil, errAlreadyCached } - metrics.ZipServingArchiveCache.WithLabelValues("miss").Inc() - metrics.ZipCachedArchives.Inc() + metrics.ZipCacheRequests.WithLabelValues("archive", "miss").Inc() + metrics.ZipCachedEntries.WithLabelValues("archive").Inc() } zipArchive := archive.(*zipArchive) diff --git a/metrics/metrics.go b/metrics/metrics.go index d2243fae..db7cae9a 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -164,41 +164,22 @@ var ( []string{"state"}, ) - // ZipServingArchiveCache is the number of zip archive cache hits/misses - ZipServingArchiveCache = prometheus.NewCounterVec( + // ZipCacheRequests is the number of cache hits/misses + ZipCacheRequests = prometheus.NewCounterVec( prometheus.CounterOpts{ Name: "gitlab_pages_zip_cache_requests", Help: "The number of zip archives cache hits/misses", }, - []string{"cache"}, + []string{"op", "cache"}, ) - // ZipServingArchiveDataOffsetCache is the number of zip archive - // data offset cache hits/misses - ZipServingArchiveDataOffsetCache = prometheus.NewCounterVec( - prometheus.CounterOpts{ - Name: "gitlab_pages_zip_data_offset_cache_requests", - Help: "The number of zip archive data offset cache hits/misses", - }, - []string{"cache"}, - ) - - // ZipServingArchiveReadlinkCache is the number of zip archive Readlink - // cache hits/misses - ZipServingArchiveReadlinkCache = prometheus.NewCounterVec( - prometheus.CounterOpts{ - Name: "gitlab_pages_zip_readlink_cache_requests", - Help: "The number of zip archives Readlink cache hits/misses", - }, - []string{"cache"}, - ) - - // ZipCachedArchives is the number of zip archives currently in the cache - ZipCachedArchives = prometheus.NewGauge( + // ZipCachedArchives is the number of entries in the cache + ZipCachedEntries = prometheus.NewGaugeVec( prometheus.GaugeOpts{ - Name: "gitlab_pages_zip_cached_archives", - Help: "The number of zip archives currently in the cache", + Name: "gitlab_pages_zip_cached_entries", + Help: "The number of entries in the cache", }, + []string{"op"}, ) // ZipArchiveEntriesCached is the number of files per zip archive currently @@ -245,10 +226,8 @@ func MustRegister() { HTTPRangeOpenRequests, ZipOpened, ZipOpenedEntriesCount, - ZipServingArchiveCache, - ZipServingArchiveDataOffsetCache, - ZipServingArchiveReadlinkCache, + ZipCacheRequests, ZipArchiveEntriesCached, - ZipCachedArchives, + ZipCachedEntries, ) } |