From 4253b9ff9a475c5fb94f5016246e03beb7161e44 Mon Sep 17 00:00:00 2001 From: Martin Redolatti Date: Wed, 12 Jul 2023 17:23:17 -0300 Subject: [PATCH] attempt to fix delete-after-update possible issue --- splitio/commitversion.go | 2 +- .../proxy/storage/optimized/changesummary.go | 73 +++++++++++++++++-- 2 files changed, 67 insertions(+), 8 deletions(-) diff --git a/splitio/commitversion.go b/splitio/commitversion.go index 7893db39..4518302e 100644 --- a/splitio/commitversion.go +++ b/splitio/commitversion.go @@ -5,4 +5,4 @@ This file is created automatically, please do not edit */ // CommitVersion is the version of the last commit previous to release -const CommitVersion = "ac31078" +const CommitVersion = "199471a" diff --git a/splitio/proxy/storage/optimized/changesummary.go b/splitio/proxy/storage/optimized/changesummary.go index 0f5221e2..c0aa9f87 100644 --- a/splitio/proxy/storage/optimized/changesummary.go +++ b/splitio/proxy/storage/optimized/changesummary.go @@ -20,11 +20,16 @@ type SplitMinimalView struct { // ChangeSummary represents a set of changes from/in a specific point in time type ChangeSummary struct { Updated map[string]string // feature flag name -> trafficType - Removed map[string]string // featyre flag name -> trafficType + Removed map[string]string // feature flag name -> trafficType + Current splitSet // list of splits originally available at this point in time } -func newEmptyChangeSummary() ChangeSummary { - return ChangeSummary{Updated: map[string]string{}, Removed: map[string]string{}} +func newEmptyChangeSummary(ss *splitSet) ChangeSummary { + ns := newSplitSet() + if ss != nil { + ns = *ss + } + return ChangeSummary{Updated: map[string]string{}, Removed: map[string]string{}, Current: ns} } func (c *ChangeSummary) applyChange(toAdd []SplitMinimalView, toRemove []SplitMinimalView) { @@ -36,7 +41,9 @@ func (c *ChangeSummary) applyChange(toAdd []SplitMinimalView, toRemove []SplitMi for _, split := range toRemove { if _, ok := c.Updated[split.Name]; ok { delete(c.Updated, split.Name) - } else { + } + + if c.Current.contains(split.Name) { c.Removed[split.Name] = split.TrafficType } } @@ -56,7 +63,7 @@ func NewSplitChangesSummaries(maxRecipes int) *SplitChangesSummaries { return &SplitChangesSummaries{ maxRecipes: maxRecipes + 1, // we keep an extra slot for -1 which is fixed currentCN: -1, - changes: map[int64]ChangeSummary{-1: newEmptyChangeSummary()}, + changes: map[int64]ChangeSummary{-1: newEmptyChangeSummary(nil)}, } } @@ -67,6 +74,17 @@ func (s *SplitChangesSummaries) AddChanges(added []dtos.SplitDTO, removed []dtos addedViews := toSplitMinimalViews(added) removedViews := toSplitMinimalViews(removed) + + if cn == -1 { + // During the first hit (cn=-1) we want to capture ALL split names, to form an initial snapshot of what the user will get + // and nothing else. + ss := newSplitSet() + ss.update(addedViews, nil) + cs := s.changes[0] + cs.Current = ss + s.changes[0] = cs + } + if cn <= s.currentCN { return } @@ -75,13 +93,23 @@ func (s *SplitChangesSummaries) AddChanges(added []dtos.SplitDTO, removed []dtos s.removeOldestRecipe() } + var lastCheckpoint int64 = -1 + var lastSplitSet splitSet for key, summary := range s.changes { + if key > lastCheckpoint { + lastCheckpoint = key + lastSplitSet = summary.Current + } + summary.applyChange(addedViews, removedViews) s.changes[key] = summary } s.currentCN = cn - s.changes[cn] = newEmptyChangeSummary() + + newSS := lastSplitSet.clone() + newSS.update(addedViews, removedViews) + s.changes[cn] = newEmptyChangeSummary(&newSS) } // AddOlderChange is used to add a change older than the oldest one currently stored (when the sync started) @@ -99,7 +127,7 @@ func (s *SplitChangesSummaries) AddOlderChange(added []dtos.SplitDTO, removed [] s.removeOldestRecipe() } - summary := newEmptyChangeSummary() + summary := newEmptyChangeSummary(nil) // TODO(mredolatti): see if we can do better than this for _, split := range added { summary.Updated[split.Name] = split.TrafficTypeName } @@ -169,3 +197,34 @@ func toSplitMinimalViews(items []dtos.SplitDTO) []SplitMinimalView { } return views } + +type splitSet struct { + data map[string]struct{} +} + +func newSplitSet() splitSet { + return splitSet{data: make(map[string]struct{})} +} + +func (s *splitSet) clone() splitSet { + x := newSplitSet() + for key := range s.data { + x.data[key] = struct{}{} + } + return x +} + +func (s *splitSet) update(toAdd []SplitMinimalView, toRemove []SplitMinimalView) { + for idx := range toAdd { + s.data[toAdd[idx].Name] = struct{}{} + } + + for idx := range toRemove { + delete(s.data, toAdd[idx].Name) + } +} + +func (s *splitSet) contains(name string) bool { + _, ok := s.data[name] + return ok +}