Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 5xx errors on underlying error appearance with cache turned on #115

Merged
merged 1 commit into from
Aug 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/publisher/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func main() {
panic(err)
}

repo = memcache.New(cli, repo, cfg.MemcacheTTL)
repo = memcache.New(cli, repo, cfg.MemcacheTTL, "publisher")
}

awsSession, err := session.NewSession(&aws.Config{
Expand Down
85 changes: 56 additions & 29 deletions repositories/cache/metadata/memcache/memcache.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"time"

memcacheCli "github.com/bradfitz/gomemcache/memcache"
"github.com/pkg/errors"
log "github.com/sirupsen/logrus"

emodels "github.com/teran/archived/exporter/models"
Expand All @@ -19,16 +18,22 @@ import (
var _ metadata.Repository = (*memcache)(nil)

type memcache struct {
cli *memcacheCli.Client
repo metadata.Repository
ttl time.Duration
cli *memcacheCli.Client
keyPrefix string
repo metadata.Repository
ttl time.Duration
}

func New(cli *memcacheCli.Client, repo metadata.Repository, ttl time.Duration) metadata.Repository {
func New(cli *memcacheCli.Client, repo metadata.Repository, ttl time.Duration, keyPrefix string) metadata.Repository {
if keyPrefix == "" {
keyPrefix = "_"
}

return &memcache{
cli: cli,
repo: repo,
ttl: ttl,
cli: cli,
keyPrefix: keyPrefix,
repo: repo,
ttl: ttl,
}
}

Expand All @@ -37,7 +42,11 @@ func (m *memcache) CreateContainer(ctx context.Context, name string) error {
}

func (m *memcache) ListContainers(ctx context.Context) ([]string, error) {
cacheKey := "_ListContainers"
cacheKey := strings.Join([]string{
m.keyPrefix,
"ListContainers",
}, ":")

item, err := m.cli.Get(cacheKey)
if err != nil {
if err == memcacheCli.ErrCacheMiss {
Expand All @@ -47,7 +56,7 @@ func (m *memcache) ListContainers(ctx context.Context) ([]string, error) {

containers, err := m.repo.ListContainers(ctx)
if err != nil {
return nil, errors.Wrap(err, "error retrieving container list")
return nil, err
}

if err := store(m, cacheKey, containers); err != nil {
Expand All @@ -66,7 +75,7 @@ func (m *memcache) ListContainers(ctx context.Context) ([]string, error) {
var retrievedValue []string
err = json.Unmarshal(item.Value, &retrievedValue)
if err != nil {
return nil, errors.Wrap(err, "error unmarshaling cached value")
return nil, err
}

return retrievedValue, nil
Expand All @@ -81,7 +90,12 @@ func (m *memcache) CreateVersion(ctx context.Context, container string) (string,
}

func (m *memcache) GetLatestPublishedVersionByContainer(ctx context.Context, container string) (string, error) {
cacheKey := "_GetLatestPublishedVersionByContainer:" + container
cacheKey := strings.Join([]string{
m.keyPrefix,
"GetLatestPublishedVersionByContainer",
container,
}, ":")

item, err := m.cli.Get(cacheKey)
if err != nil {
if err == memcacheCli.ErrCacheMiss {
Expand All @@ -91,7 +105,7 @@ func (m *memcache) GetLatestPublishedVersionByContainer(ctx context.Context, con

version, err := m.repo.GetLatestPublishedVersionByContainer(ctx, container)
if err != nil {
return "", errors.Wrapf(err, "error retrieving latest version for container `%s`", container)
return "", err
}

if err := store(m, cacheKey, version); err != nil {
Expand All @@ -110,14 +124,19 @@ func (m *memcache) GetLatestPublishedVersionByContainer(ctx context.Context, con
var retrievedValue string
err = json.Unmarshal(item.Value, &retrievedValue)
if err != nil {
return "", errors.Wrap(err, "error unmarshaling cached value")
return "", err
}

return retrievedValue, nil
}

func (m *memcache) ListAllVersionsByContainer(ctx context.Context, container string) ([]models.Version, error) {
cacheKey := "_ListAllVersionsByContainer:" + container
cacheKey := strings.Join([]string{
m.keyPrefix,
"ListAllVersionsByContainer",
container,
}, ":")

item, err := m.cli.Get(cacheKey)
if err != nil {
if err == memcacheCli.ErrCacheMiss {
Expand All @@ -127,7 +146,7 @@ func (m *memcache) ListAllVersionsByContainer(ctx context.Context, container str

versions, err := m.repo.ListAllVersionsByContainer(ctx, container)
if err != nil {
return nil, errors.Wrapf(err, "error retrieving latest version for container `%s`", container)
return nil, err
}

if err := store(m, cacheKey, versions); err != nil {
Expand All @@ -146,14 +165,19 @@ func (m *memcache) ListAllVersionsByContainer(ctx context.Context, container str
var retrievedValue []models.Version
err = json.Unmarshal(item.Value, &retrievedValue)
if err != nil {
return nil, errors.Wrap(err, "error unmarshaling cached value")
return nil, err
}

return retrievedValue, nil
}

func (m *memcache) ListPublishedVersionsByContainer(ctx context.Context, container string) ([]models.Version, error) {
cacheKey := "_ListPublishedVersionsByContainer:" + container
cacheKey := strings.Join([]string{
m.keyPrefix,
"ListPublishedVersionsByContainer",
container,
}, ":")

item, err := m.cli.Get(cacheKey)
if err != nil {
if err == memcacheCli.ErrCacheMiss {
Expand All @@ -163,7 +187,7 @@ func (m *memcache) ListPublishedVersionsByContainer(ctx context.Context, contain

versions, err := m.repo.ListPublishedVersionsByContainer(ctx, container)
if err != nil {
return nil, errors.Wrapf(err, "error retrieving latest version for container `%s`", container)
return nil, err
}

if err := store(m, cacheKey, versions); err != nil {
Expand All @@ -182,7 +206,7 @@ func (m *memcache) ListPublishedVersionsByContainer(ctx context.Context, contain
var retrievedValue []models.Version
err = json.Unmarshal(item.Value, &retrievedValue)
if err != nil {
return nil, errors.Wrap(err, "error unmarshaling cached value")
return nil, err
}

return retrievedValue, nil
Expand All @@ -195,7 +219,8 @@ func (m *memcache) ListPublishedVersionsByContainerAndPage(ctx context.Context,
}

cacheKey := strings.Join([]string{
"_ListPublishedVersionsByContainerAndPage",
m.keyPrefix,
"ListPublishedVersionsByContainerAndPage",
container,
strconv.FormatUint(offset, 10),
strconv.FormatUint(limit, 10),
Expand All @@ -210,7 +235,7 @@ func (m *memcache) ListPublishedVersionsByContainerAndPage(ctx context.Context,

n, versions, err := m.repo.ListPublishedVersionsByContainerAndPage(ctx, container, offset, limit)
if err != nil {
return 0, nil, errors.Wrapf(err, "error retrieving published versions for container `%s` (offset=%d; limit=%d)", container, offset, limit)
return 0, nil, err
}

if err := store(m, cacheKey, proxy{Total: n, Versions: versions}); err != nil {
Expand All @@ -229,7 +254,7 @@ func (m *memcache) ListPublishedVersionsByContainerAndPage(ctx context.Context,
var retrievedValue proxy
err = json.Unmarshal(item.Value, &retrievedValue)
if err != nil {
return 0, nil, errors.Wrap(err, "error unmarshaling cached value")
return 0, nil, err
}

return retrievedValue.Total, retrievedValue.Versions, nil
Expand Down Expand Up @@ -258,7 +283,8 @@ func (m *memcache) ListObjects(ctx context.Context, container, version string, o
}

cacheKey := strings.Join([]string{
"_ListObjects",
m.keyPrefix,
"ListObjects",
container,
version,
strconv.FormatUint(offset, 10),
Expand All @@ -274,7 +300,7 @@ func (m *memcache) ListObjects(ctx context.Context, container, version string, o

n, objects, err := m.repo.ListObjects(ctx, container, version, offset, limit)
if err != nil {
return 0, nil, errors.Wrapf(err, "error retrieving objects for `%s/%s`, offset=%d; limit=%d", container, version, offset, limit)
return 0, nil, err
}

if err := store(m, cacheKey, proxy{Total: n, Objects: objects}); err != nil {
Expand All @@ -293,7 +319,7 @@ func (m *memcache) ListObjects(ctx context.Context, container, version string, o
var retrievedValue proxy
err = json.Unmarshal(item.Value, &retrievedValue)
if err != nil {
return 0, nil, errors.Wrap(err, "error unmarshaling cached value")
return 0, nil, err
}

return retrievedValue.Total, retrievedValue.Objects, nil
Expand All @@ -313,7 +339,8 @@ func (m *memcache) CreateBLOB(ctx context.Context, checksum string, size uint64,

func (m *memcache) GetBlobKeyByObject(ctx context.Context, container, version, key string) (string, error) {
cacheKey := strings.Join([]string{
"_GetBlobKeyByObject",
m.keyPrefix,
"GetBlobKeyByObject",
container,
version,
key,
Expand All @@ -328,7 +355,7 @@ func (m *memcache) GetBlobKeyByObject(ctx context.Context, container, version, k

key, err := m.repo.GetBlobKeyByObject(ctx, container, version, key)
if err != nil {
return "", errors.Wrapf(err, "error retrieving object key for `%s/%s/%s`", container, version, key)
return "", err
}

if err = store(m, cacheKey, key); err != nil {
Expand All @@ -347,7 +374,7 @@ func (m *memcache) GetBlobKeyByObject(ctx context.Context, container, version, k
var retrievedValue string
err = json.Unmarshal(item.Value, &retrievedValue)
if err != nil {
return "", errors.Wrap(err, "error unmarshaling cached value")
return "", err
}

return retrievedValue, nil
Expand Down
59 changes: 58 additions & 1 deletion repositories/cache/metadata/memcache/memcache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"time"

memcacheCli "github.com/bradfitz/gomemcache/memcache"
"github.com/pkg/errors"
log "github.com/sirupsen/logrus"
"github.com/stretchr/testify/suite"

Expand Down Expand Up @@ -33,6 +34,14 @@ func (s *memcacheTestSuite) TestListContainers() {
s.Require().Equal([]string{"container1"}, containers)
}

func (s *memcacheTestSuite) TestListContainersError() {
s.repoMock.On("ListContainers").Return([]string{}, errors.New("some error")).Once()

_, err := s.cache.ListContainers(s.ctx)
s.Require().Error(err)
s.Require().Equal("some error", err.Error())
}

func (s *memcacheTestSuite) TestGetLatestPublishedVersionByContainer() {
s.repoMock.On("GetLatestPublishedVersionByContainer", "test-container").Return("test-version", nil).Once()

Expand All @@ -45,6 +54,14 @@ func (s *memcacheTestSuite) TestGetLatestPublishedVersionByContainer() {
s.Require().Equal("test-version", version)
}

func (s *memcacheTestSuite) TestGetLatestPublishedVersionByContainerError() {
s.repoMock.On("GetLatestPublishedVersionByContainer", "test-container").Return("", errors.New("some error")).Once()

_, err := s.cache.GetLatestPublishedVersionByContainer(s.ctx, "test-container")
s.Require().Error(err)
s.Require().Equal("some error", err.Error())
}

func (s *memcacheTestSuite) TestListAllVersionsByContainer() {
s.repoMock.On("ListAllVersionsByContainer", "test-container").Return([]models.Version{{Name: "test-version"}}, nil).Once()

Expand All @@ -57,6 +74,14 @@ func (s *memcacheTestSuite) TestListAllVersionsByContainer() {
s.Require().Equal([]models.Version{{Name: "test-version"}}, versions)
}

func (s *memcacheTestSuite) TestListAllVersionsByContainerError() {
s.repoMock.On("ListAllVersionsByContainer", "test-container").Return([]models.Version{}, errors.New("some error")).Once()

_, err := s.cache.ListAllVersionsByContainer(s.ctx, "test-container")
s.Require().Error(err)
s.Require().Equal("some error", err.Error())
}

func (s *memcacheTestSuite) TestListPublishedVersionsByContainer() {
s.repoMock.On("ListPublishedVersionsByContainer", "test-container").Return([]models.Version{{Name: "test-version"}}, nil).Once()

Expand All @@ -69,6 +94,14 @@ func (s *memcacheTestSuite) TestListPublishedVersionsByContainer() {
s.Require().Equal([]models.Version{{Name: "test-version"}}, versions)
}

func (s *memcacheTestSuite) TestListPublishedVersionsByContainerError() {
s.repoMock.On("ListPublishedVersionsByContainer", "test-container").Return([]models.Version{}, errors.New("some error")).Once()

_, err := s.cache.ListPublishedVersionsByContainer(s.ctx, "test-container")
s.Require().Error(err)
s.Require().Equal("some error", err.Error())
}

func (s *memcacheTestSuite) TestListPublishedVersionsByContainerAndPage() {
s.repoMock.On("ListPublishedVersionsByContainerAndPage", "test-container", uint64(0), uint64(15)).Return(uint64(500), []models.Version{{Name: "test-version"}}, nil).Once()

Expand All @@ -83,6 +116,14 @@ func (s *memcacheTestSuite) TestListPublishedVersionsByContainerAndPage() {
s.Require().Equal(uint64(500), total)
}

func (s *memcacheTestSuite) TestListPublishedVersionsByContainerAndPageError() {
s.repoMock.On("ListPublishedVersionsByContainerAndPage", "test-container", uint64(0), uint64(15)).Return(uint64(0), []models.Version{}, errors.New("some error")).Once()

_, _, err := s.cache.ListPublishedVersionsByContainerAndPage(s.ctx, "test-container", 0, 15)
s.Require().Error(err)
s.Require().Equal("some error", err.Error())
}

func (s *memcacheTestSuite) TestListObjects() {
s.repoMock.On("ListObjects", "test-container", "test-version", uint64(0), uint64(30)).Return(uint64(500), []string{"obj1"}, nil).Once()

Expand All @@ -97,6 +138,14 @@ func (s *memcacheTestSuite) TestListObjects() {
s.Require().Equal(uint64(500), total)
}

func (s *memcacheTestSuite) TestListObjectsError() {
s.repoMock.On("ListObjects", "test-container", "test-version", uint64(0), uint64(30)).Return(uint64(0), []string{}, errors.New("some error")).Once()

_, _, err := s.cache.ListObjects(s.ctx, "test-container", "test-version", 0, 30)
s.Require().Error(err)
s.Require().Equal("some error", err.Error())
}

func (s *memcacheTestSuite) TestGetBlobKeyByObject() {
s.repoMock.On("GetBlobKeyByObject", "container", "version", "key").Return("deadbeef", nil).Once()

Expand All @@ -109,6 +158,14 @@ func (s *memcacheTestSuite) TestGetBlobKeyByObject() {
s.Require().Equal("deadbeef", casKey)
}

func (s *memcacheTestSuite) TestGetBlobKeyByObjectError() {
s.repoMock.On("GetBlobKeyByObject", "container", "version", "key").Return("", errors.New("some error")).Once()

_, err := s.cache.GetBlobKeyByObject(s.ctx, "container", "version", "key")
s.Require().Error(err)
s.Require().Equal("some error", err.Error())
}

// Non-cached methods ...
func (s *memcacheTestSuite) TestCreateContainer() {
s.repoMock.On("CreateContainer", "container1").Return(nil).Twice()
Expand Down Expand Up @@ -264,7 +321,7 @@ func (s *memcacheTestSuite) SetupTest() {
s.Require().NoError(err)

cli := memcacheCli.New(url)
s.cache = New(cli, s.repoMock, 3*time.Second)
s.cache = New(cli, s.repoMock, 3*time.Second, s.T().Name())
}

func (s *memcacheTestSuite) TearDownTest() {
Expand Down
Loading