diff options
author | Jaime Martinez <jmartinez@gitlab.com> | 2020-11-12 09:54:46 +0300 |
---|---|---|
committer | Jaime Martinez <jmartinez@gitlab.com> | 2020-11-30 02:15:09 +0300 |
commit | b0e17d6a69a6e81cd1ca693cba47840de1f230da (patch) | |
tree | 5a3e902a9fa427f53fd09feec4f107dbe1ac6242 /internal | |
parent | 84abda0e2cc2d47078d20abc1dad338402060b90 (diff) |
Add zip config flags
Adds a config package with specific zip configuration structure that can
be shared between packages.
Diffstat (limited to 'internal')
-rw-r--r-- | internal/config/config.go | 42 | ||||
-rw-r--r-- | internal/config/headers.go | 32 | ||||
-rw-r--r-- | internal/config/headers_test.go (renamed from internal/config/config_test.go) | 0 | ||||
-rw-r--r-- | internal/serving/disk/zip/serving.go | 7 | ||||
-rw-r--r-- | internal/vfs/zip/archive_test.go | 20 | ||||
-rw-r--r-- | internal/vfs/zip/lru_cache.go | 6 | ||||
-rw-r--r-- | internal/vfs/zip/vfs.go | 68 | ||||
-rw-r--r-- | internal/vfs/zip/vfs_test.go | 18 |
8 files changed, 93 insertions, 100 deletions
diff --git a/internal/config/config.go b/internal/config/config.go index d415b21d..ef2af686 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1,32 +1,22 @@ package config -import ( - "errors" - "net/http" - "strings" -) +import "time" -var errInvalidHeaderParameter = errors.New("invalid syntax specified as header parameter") +// TODO: refactor config flags in main.go and find a better way to handle all settings -// AddCustomHeaders adds a map of Headers to a Response -func AddCustomHeaders(w http.ResponseWriter, headers http.Header) error { - for k, v := range headers { - for _, value := range v { - w.Header().Add(k, value) - } - } - return nil -} +// ZipVFSConfig struct that can be accessed by different packages to share +// configuration parameters +var ZipVFSConfig *ZipServing -// ParseHeaderString parses a string of key values into a map -func ParseHeaderString(customHeaders []string) (http.Header, error) { - headers := http.Header{} - for _, keyValueString := range customHeaders { - keyValue := strings.SplitN(keyValueString, ":", 2) - if len(keyValue) != 2 { - return nil, errInvalidHeaderParameter - } - headers[strings.TrimSpace(keyValue[0])] = append(headers[strings.TrimSpace(keyValue[0])], strings.TrimSpace(keyValue[1])) - } - return headers, nil +// ZipServing stores all configuration values to be used by the zip VFS opening and +// caching +type ZipServing struct { + ExpirationInterval time.Duration + CleanupInterval time.Duration + RefreshInterval time.Duration + OpenTimeout time.Duration + DataOffsetItems int64 + DataOffsetExpirationInterval time.Duration + ReadlinkItems int64 + ReadlinkExpirationInterval time.Duration } diff --git a/internal/config/headers.go b/internal/config/headers.go new file mode 100644 index 00000000..d415b21d --- /dev/null +++ b/internal/config/headers.go @@ -0,0 +1,32 @@ +package config + +import ( + "errors" + "net/http" + "strings" +) + +var errInvalidHeaderParameter = errors.New("invalid syntax specified as header parameter") + +// AddCustomHeaders adds a map of Headers to a Response +func AddCustomHeaders(w http.ResponseWriter, headers http.Header) error { + for k, v := range headers { + for _, value := range v { + w.Header().Add(k, value) + } + } + return nil +} + +// ParseHeaderString parses a string of key values into a map +func ParseHeaderString(customHeaders []string) (http.Header, error) { + headers := http.Header{} + for _, keyValueString := range customHeaders { + keyValue := strings.SplitN(keyValueString, ":", 2) + if len(keyValue) != 2 { + return nil, errInvalidHeaderParameter + } + headers[strings.TrimSpace(keyValue[0])] = append(headers[strings.TrimSpace(keyValue[0])], strings.TrimSpace(keyValue[1])) + } + return headers, nil +} diff --git a/internal/config/config_test.go b/internal/config/headers_test.go index 44afd470..44afd470 100644 --- a/internal/config/config_test.go +++ b/internal/config/headers_test.go diff --git a/internal/serving/disk/zip/serving.go b/internal/serving/disk/zip/serving.go index 95894fc9..9b3a7aa7 100644 --- a/internal/serving/disk/zip/serving.go +++ b/internal/serving/disk/zip/serving.go @@ -1,16 +1,21 @@ package zip import ( + "gitlab.com/gitlab-org/gitlab-pages/internal/config" "gitlab.com/gitlab-org/gitlab-pages/internal/serving" "gitlab.com/gitlab-org/gitlab-pages/internal/serving/disk" "gitlab.com/gitlab-org/gitlab-pages/internal/vfs" "gitlab.com/gitlab-org/gitlab-pages/internal/vfs/zip" ) -var instance = disk.New(vfs.Instrumented(zip.New())) +var instance serving.Serving // Instance returns a serving instance that is capable of reading files // from a zip archives opened from a URL, most likely stored in object storage func Instance() serving.Serving { + if instance == nil { + instance = disk.New(vfs.Instrumented(zip.New(config.ZipVFSConfig))) + } + return instance } diff --git a/internal/vfs/zip/archive_test.go b/internal/vfs/zip/archive_test.go index e1b0d116..f4fd6b5a 100644 --- a/internal/vfs/zip/archive_test.go +++ b/internal/vfs/zip/archive_test.go @@ -12,12 +12,24 @@ import ( "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitlab-pages/internal/config" "gitlab.com/gitlab-org/gitlab-pages/internal/httprange" "gitlab.com/gitlab-org/gitlab-pages/internal/testhelpers" ) var chdirSet = false +var zipCfg = &config.ZipServing{ + ExpirationInterval: time.Minute, + CleanupInterval: time.Minute / 2, + RefreshInterval: time.Minute / 2, + OpenTimeout: time.Minute / 2, + DataOffsetItems: 1000, + DataOffsetExpirationInterval: time.Hour, + ReadlinkItems: 500, + ReadlinkExpirationInterval: time.Hour, +} + func TestOpen(t *testing.T) { zip, cleanup := openZipArchive(t, nil) defer cleanup() @@ -75,7 +87,7 @@ func TestOpenCached(t *testing.T) { testServerURL, cleanup := newZipFileServerURL(t, "group/zip.gitlab.io/public-without-dirs.zip", &requests) defer cleanup() - fs := New() + fs := New(zipCfg) // We use array instead of map to ensure // predictable ordering of test execution @@ -322,7 +334,7 @@ func TestArchiveCanBeReadAfterOpenCtxCanceled(t *testing.T) { testServerURL, cleanup := newZipFileServerURL(t, "group/zip.gitlab.io/public.zip", nil) defer cleanup() - fs := New().(*zipVFS) + fs := New(zipCfg).(*zipVFS) zip := newArchive(fs, time.Second) ctx, cancel := context.WithCancel(context.Background()) cancel() @@ -345,7 +357,7 @@ func TestReadArchiveFails(t *testing.T) { testServerURL, cleanup := newZipFileServerURL(t, "group/zip.gitlab.io/public.zip", nil) defer cleanup() - fs := New().(*zipVFS) + fs := New(zipCfg).(*zipVFS) zip := newArchive(fs, time.Second) err := zip.openArchive(context.Background(), testServerURL+"/unkown.html") @@ -365,7 +377,7 @@ func openZipArchive(t *testing.T, requests *int64) (*zipArchive, func()) { testServerURL, cleanup := newZipFileServerURL(t, "group/zip.gitlab.io/public-without-dirs.zip", requests) - fs := New().(*zipVFS) + fs := New(zipCfg).(*zipVFS) zip := newArchive(fs, time.Second) err := zip.openArchive(context.Background(), testServerURL+"/public.zip") diff --git a/internal/vfs/zip/lru_cache.go b/internal/vfs/zip/lru_cache.go index 36817e66..9810e245 100644 --- a/internal/vfs/zip/lru_cache.go +++ b/internal/vfs/zip/lru_cache.go @@ -24,10 +24,10 @@ type lruCache struct { cache *ccache.Cache } -func newLruCache(op string, maxEntries uint32, duration time.Duration) *lruCache { +func newLruCache(op string, maxEntries int64, duration time.Duration) *lruCache { configuration := ccache.Configure() - configuration.MaxSize(int64(maxEntries)) - configuration.ItemsToPrune(maxEntries / lruCacheItemsToPruneDiv) + configuration.MaxSize(maxEntries) + configuration.ItemsToPrune(uint32(maxEntries) / lruCacheItemsToPruneDiv) configuration.GetsPerPromote(lruCacheGetsPerPromote) // if item gets requested frequently promote it configuration.OnDelete(func(*ccache.Item) { metrics.ZipCachedEntries.WithLabelValues(op).Dec() diff --git a/internal/vfs/zip/vfs.go b/internal/vfs/zip/vfs.go index 692a4a69..feeab974 100644 --- a/internal/vfs/zip/vfs.go +++ b/internal/vfs/zip/vfs.go @@ -9,29 +9,12 @@ import ( "github.com/patrickmn/go-cache" + "gitlab.com/gitlab-org/gitlab-pages/internal/config" "gitlab.com/gitlab-org/gitlab-pages/internal/httprange" "gitlab.com/gitlab-org/gitlab-pages/internal/vfs" "gitlab.com/gitlab-org/gitlab-pages/metrics" ) -const ( - // TODO: make these configurable https://gitlab.com/gitlab-org/gitlab-pages/-/issues/464 - 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 - defaultDataOffsetItems = 50000 - defaultDataOffsetExpirationInterval = time.Hour - - // we assume that each item costs around 200 bytes - // this gives around 2MB of raw memory needed without acceleration structures - defaultReadlinkItems = 10000 - defaultReadlinkExpirationInterval = time.Hour -) - var ( errAlreadyCached = errors.New("archive already cached") ) @@ -52,48 +35,13 @@ type zipVFS struct { 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 - } -} - -// WithOpenTimeout when used it can override openTimeout -func WithOpenTimeout(interval time.Duration) Option { - return func(vfs *zipVFS) { - vfs.openTimeout = interval - } -} - // New creates a zipVFS instance that can be used by a serving request -func New(options ...Option) vfs.VFS { +func New(cfg *config.ZipServing) vfs.VFS { zipVFS := &zipVFS{ - cacheExpirationInterval: defaultCacheExpirationInterval, - cacheRefreshInterval: defaultCacheRefreshInterval, - cacheCleanupInterval: defaultCacheCleanupInterval, - openTimeout: defaultOpenTimeout, - } - - for _, option := range options { - option(zipVFS) + cacheExpirationInterval: cfg.ExpirationInterval, + cacheRefreshInterval: cfg.RefreshInterval, + cacheCleanupInterval: cfg.CleanupInterval, + openTimeout: cfg.OpenTimeout, } zipVFS.cache = cache.New(zipVFS.cacheExpirationInterval, zipVFS.cacheCleanupInterval) @@ -104,8 +52,8 @@ func New(options ...Option) vfs.VFS { }) // 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) + zipVFS.dataOffsetCache = newLruCache("data-offset", cfg.DataOffsetItems, cfg.DataOffsetExpirationInterval) + zipVFS.readlinkCache = newLruCache("readlink", cfg.ReadlinkItems, cfg.ReadlinkExpirationInterval) return zipVFS } diff --git a/internal/vfs/zip/vfs_test.go b/internal/vfs/zip/vfs_test.go index dff2ff43..aa1ed109 100644 --- a/internal/vfs/zip/vfs_test.go +++ b/internal/vfs/zip/vfs_test.go @@ -9,6 +9,7 @@ import ( "github.com/prometheus/client_golang/prometheus/testutil" "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitlab-pages/internal/config" "gitlab.com/gitlab-org/gitlab-pages/internal/httprange" "gitlab.com/gitlab-org/gitlab-pages/internal/vfs" "gitlab.com/gitlab-org/gitlab-pages/metrics" @@ -35,7 +36,7 @@ func TestVFSRoot(t *testing.T) { }, } - vfs := New() + vfs := New(zipCfg) for name, tt := range tests { t.Run(name, func(t *testing.T) { @@ -73,7 +74,11 @@ func TestVFSFindOrOpenArchiveConcurrentAccess(t *testing.T) { path := testServerURL + "/public.zip" - vfs := New().(*zipVFS) + vfs := New(config.ZipServing{ + OpenTimeout: time.Second, + DataOffsetItems: 20, + ReadlinkItems: 20, + }).(*zipVFS) root, err := vfs.Root(context.Background(), path) require.NoError(t, err) @@ -158,10 +163,11 @@ func TestVFSFindOrOpenArchiveRefresh(t *testing.T) { for name, test := range tests { t.Run(name, func(t *testing.T) { withExpectedArchiveCount(t, 1, func(t *testing.T) { - vfs := New( - WithCacheExpirationInterval(test.expirationInterval), - WithCacheRefreshInterval(test.refreshInterval), - ).(*zipVFS) + cfg := *zipCfg + cfg.ExpirationInterval = test.expirationInterval + cfg.RefreshInterval = test.refreshInterval + + vfs := New(&cfg).(*zipVFS) path := testServerURL + test.path |