Skip to content

Commit

Permalink
refactor(cache): use namespace.version as part of cache key
Browse files Browse the repository at this point in the history
As namespace has a version which updates on each configuration changes
it makes sense to use it as part of the cache key. Changes focus on
improving the evaluation response time. All other API calls is not in a
critical path so there is very little benefits to cache them

Signed-off-by: Roman Dmytrenko <[email protected]>
  • Loading branch information
erka committed Dec 19, 2024
1 parent 5d85c45 commit 89ca41e
Show file tree
Hide file tree
Showing 15 changed files with 325 additions and 121 deletions.
2 changes: 1 addition & 1 deletion internal/common/store_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ func (m *StoreMock) GetEvaluationRules(ctx context.Context, flag storage.Resourc
return args.Get(0).([]*storage.EvaluationRule), args.Error(1)
}

func (m *StoreMock) GetEvaluationDistributions(ctx context.Context, rule storage.IDRequest) ([]*storage.EvaluationDistribution, error) {
func (m *StoreMock) GetEvaluationDistributions(ctx context.Context, r storage.ResourceRequest, rule storage.IDRequest) ([]*storage.EvaluationDistribution, error) {
args := m.Called(ctx, rule)
return args.Get(0).([]*storage.EvaluationDistribution), args.Error(1)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/server/evaluation/data/evaluation_store_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (e *evaluationStoreMock) GetEvaluationRules(ctx context.Context, flag stora
return args.Get(0).([]*storage.EvaluationRule), args.Error(1)
}

func (e *evaluationStoreMock) GetEvaluationDistributions(ctx context.Context, ruleID storage.IDRequest) ([]*storage.EvaluationDistribution, error) {
func (e *evaluationStoreMock) GetEvaluationDistributions(ctx context.Context, flag storage.ResourceRequest, ruleID storage.IDRequest) ([]*storage.EvaluationDistribution, error) {

Check warning on line 41 in internal/server/evaluation/data/evaluation_store_mock.go

View check run for this annotation

Codecov / codecov/patch

internal/server/evaluation/data/evaluation_store_mock.go#L41

Added line #L41 was not covered by tests
args := e.Called(ctx, ruleID)
return args.Get(0).([]*storage.EvaluationDistribution), args.Error(1)
}
Expand Down
7 changes: 3 additions & 4 deletions internal/server/evaluation/data/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ package data

import (
"context"
"crypto/sha1" //nolint:gosec

//nolint:gosec
"crypto/sha1"
"fmt"

"github.com/blang/semver/v4"
Expand Down Expand Up @@ -123,7 +123,6 @@ var (
)

func (srv *Server) EvaluationSnapshotNamespace(ctx context.Context, r *evaluation.EvaluationNamespaceSnapshotRequest) (*evaluation.EvaluationNamespaceSnapshot, error) {

var (
namespaceKey = r.Key
reference = r.Reference
Expand Down Expand Up @@ -264,7 +263,7 @@ func (srv *Server) EvaluationSnapshotNamespace(ctx context.Context, r *evaluatio
rule.Segments = append(rule.Segments, ss)
}

distributions, err := srv.store.GetEvaluationDistributions(ctx, storage.NewID(r.ID, storage.WithReference(reference)))
distributions, err := srv.store.GetEvaluationDistributions(ctx, storage.NewResource(f.NamespaceKey, f.Key), storage.NewID(r.ID, storage.WithReference(reference)))

Check warning on line 266 in internal/server/evaluation/data/server.go

View check run for this annotation

Codecov / codecov/patch

internal/server/evaluation/data/server.go#L266

Added line #L266 was not covered by tests
if err != nil {
return nil, fmt.Errorf("getting distributions for rule %q: %w", r.ID, err)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/server/evaluation/evaluation_store_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func (e *evaluationStoreMock) GetEvaluationRules(ctx context.Context, flag stora
return args.Get(0).([]*storage.EvaluationRule), args.Error(1)
}

func (e *evaluationStoreMock) GetEvaluationDistributions(ctx context.Context, ruleID storage.IDRequest) ([]*storage.EvaluationDistribution, error) {
func (e *evaluationStoreMock) GetEvaluationDistributions(ctx context.Context, r storage.ResourceRequest, ruleID storage.IDRequest) ([]*storage.EvaluationDistribution, error) {
args := e.Called(ctx, ruleID)
return args.Get(0).([]*storage.EvaluationDistribution), args.Error(1)
}
Expand Down
4 changes: 2 additions & 2 deletions internal/server/evaluation/legacy_evaluator.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ func (e *Evaluator) Evaluate(ctx context.Context, flag *flipt.Flag, r *evaluatio
resp.SegmentKeys = segmentKeys
}

distributions, err := e.store.GetEvaluationDistributions(ctx, storage.NewID(rule.ID))
distributions, err := e.store.GetEvaluationDistributions(ctx, storage.NewResource(r.NamespaceKey, r.FlagKey), storage.NewID(rule.ID))
if err != nil {
resp.Reason = flipt.EvaluationReason_ERROR_EVALUATION_REASON
return resp, err
Expand Down Expand Up @@ -287,7 +287,7 @@ func (e *Evaluator) matchConstraints(evalCtx map[string]string, constraints []st
}
}

var matched = true
matched := true

switch segmentMatchType {
case flipt.MatchType_ALL_MATCH_TYPE:
Expand Down
2 changes: 1 addition & 1 deletion internal/server/evaluation/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
type Storer interface {
GetFlag(ctx context.Context, flag storage.ResourceRequest) (*flipt.Flag, error)
GetEvaluationRules(ctx context.Context, flag storage.ResourceRequest) ([]*storage.EvaluationRule, error)
GetEvaluationDistributions(ctx context.Context, ruleID storage.IDRequest) ([]*storage.EvaluationDistribution, error)
GetEvaluationDistributions(ctx context.Context, flag storage.ResourceRequest, ruleID storage.IDRequest) ([]*storage.EvaluationDistribution, error)
GetEvaluationRollouts(ctx context.Context, flag storage.ResourceRequest) ([]*storage.EvaluationRollout, error)
}

Expand Down
Loading

0 comments on commit 89ca41e

Please sign in to comment.