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.

Sign up for GitHub

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

chore: Ignore model version when applying FDv2 updates #231

Merged
merged 1 commit into from
Dec 20, 2024
Merged

chore: Ignore model version when applying FDv2 updates #231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
39 changes: 6 additions & 33 deletions internal/memorystorev2/memory_store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,28 +55,16 @@ func (s *Store) SetBasis(allData []ldstoretypes.Collection) {
}

// ApplyDelta applies a delta update to the store. ApplyDelta should not be called until
// SetBasis has been called at least once. The return value indicates, for each DataKind
// present in the delta, whether the item in the delta was actually updated or not.
//
// An item is updated only if the version of the item in the delta is greater than the version
// in the store, or it wasn't already present.
func (s *Store) ApplyDelta(allData []ldstoretypes.Collection) map[ldstoretypes.DataKind]map[string]bool {
updatedMap := make(map[ldstoretypes.DataKind]map[string]bool)

// SetBasis has been called at least once.
func (s *Store) ApplyDelta(allData []ldstoretypes.Collection) {
s.Lock()
defer s.Unlock()

for _, coll := range allData {
for _, item := range coll.Items {
updated := s.upsert(coll.Kind, item.Key, item.Item)
if updatedMap[coll.Kind] == nil {
updatedMap[coll.Kind] = make(map[string]bool)
}
updatedMap[coll.Kind][item.Key] = updated
s.upsert(coll.Kind, item.Key, item.Item)
}
}

return updatedMap
}

// Get retrieves an item of the specified kind from the store. If the item is not found, then
Expand Down Expand Up @@ -139,27 +127,12 @@ func (s *Store) GetAllKinds() []ldstoretypes.Collection {
func (s *Store) upsert(
kind ldstoretypes.DataKind,
key string,
newItem ldstoretypes.ItemDescriptor) bool {
var coll map[string]ldstoretypes.ItemDescriptor
var ok bool
shouldUpdate := true
updated := false
if coll, ok = s.data[kind]; ok {
if item, ok := coll[key]; ok {
if item.Version >= newItem.Version {
shouldUpdate = false
}
}
newItem ldstoretypes.ItemDescriptor) {
if coll, ok := s.data[kind]; ok {
coll[key] = newItem
} else {
s.data[kind] = map[string]ldstoretypes.ItemDescriptor{key: newItem}
shouldUpdate = false // because we already initialized the map with the new item
updated = true
}
if shouldUpdate {
coll[key] = newItem
updated = true
}
return updated
}

// IsInitialized returns true if the store has ever been initialized with a basis.
Expand Down
12 changes: 6 additions & 6 deletions internal/memorystorev2/memory_store_benchmark_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ func BenchmarkInMemoryStoreUpsertExistingFlagSuccess(b *testing.B) {
benchmarkInMemoryStore(b, inMemoryStoreBenchmarkCases, nil, func(env *inMemoryStoreBenchmarkEnv, bc inMemoryStoreBenchmarkCase) {
env.targetFlagCopy.Version++
delta := makeCollections(dataKind, env.targetFlagKey, sharedtest.FlagDescriptor(*env.targetFlagCopy))
updates = env.store.ApplyDelta(delta)
env.store.ApplyDelta(delta)
})
}

Expand All @@ -212,7 +212,7 @@ func BenchmarkInMemoryStoreUpsertExistingFlagFailure(b *testing.B) {
benchmarkInMemoryStore(b, inMemoryStoreBenchmarkCases, nil, func(env *inMemoryStoreBenchmarkEnv, bc inMemoryStoreBenchmarkCase) {
env.targetFlagCopy.Version--
delta := makeCollections(dataKind, env.targetFlagKey, sharedtest.FlagDescriptor(*env.targetFlagCopy))
updates = env.store.ApplyDelta(delta)
env.store.ApplyDelta(delta)
})
}

Expand All @@ -221,7 +221,7 @@ func BenchmarkInMemoryStoreUpsertNewFlag(b *testing.B) {
benchmarkInMemoryStore(b, inMemoryStoreBenchmarkCases, nil, func(env *inMemoryStoreBenchmarkEnv, bc inMemoryStoreBenchmarkCase) {
env.targetFlagCopy.Key = env.unknownKey
delta := makeCollections(dataKind, env.unknownKey, sharedtest.FlagDescriptor(*env.targetFlagCopy))
updates = env.store.ApplyDelta(delta)
env.store.ApplyDelta(delta)
})
}

Expand All @@ -230,7 +230,7 @@ func BenchmarkInMemoryStoreUpsertExistingSegmentSuccess(b *testing.B) {
benchmarkInMemoryStore(b, inMemoryStoreBenchmarkCases, nil, func(env *inMemoryStoreBenchmarkEnv, bc inMemoryStoreBenchmarkCase) {
env.targetSegmentCopy.Version++
delta := makeCollections(dataKind, env.targetSegmentKey, sharedtest.SegmentDescriptor(*env.targetSegmentCopy))
updates = env.store.ApplyDelta(delta)
env.store.ApplyDelta(delta)
})
}

Expand All @@ -239,7 +239,7 @@ func BenchmarkInMemoryStoreUpsertExistingSegmentFailure(b *testing.B) {
benchmarkInMemoryStore(b, inMemoryStoreBenchmarkCases, nil, func(env *inMemoryStoreBenchmarkEnv, bc inMemoryStoreBenchmarkCase) {
env.targetSegmentCopy.Version--
delta := makeCollections(dataKind, env.targetSegmentKey, sharedtest.SegmentDescriptor(*env.targetSegmentCopy))
updates = env.store.ApplyDelta(delta)
env.store.ApplyDelta(delta)
})
}

Expand All @@ -248,6 +248,6 @@ func BenchmarkInMemoryStoreUpsertNewSegment(b *testing.B) {
benchmarkInMemoryStore(b, inMemoryStoreBenchmarkCases, nil, func(env *inMemoryStoreBenchmarkEnv, bc inMemoryStoreBenchmarkCase) {
env.targetSegmentCopy.Key = env.unknownKey
delta := makeCollections(dataKind, env.unknownKey, sharedtest.SegmentDescriptor(*env.targetSegmentCopy))
updates = env.store.ApplyDelta(delta)
env.store.ApplyDelta(delta)
})
}
155 changes: 53 additions & 102 deletions internal/memorystorev2/memory_store_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,116 +249,67 @@ func (k unknownDataKind) Deserialize(data []byte) (ldstoretypes.ItemDescriptor,
func testApplyDelta(t *testing.T) {
forAllDataKinds(t, func(t *testing.T, kind ldstoretypes.DataKind, makeItem collectionItemCreator, deleteItem collectionItemDeleter) {
t.Run("upserts", func(t *testing.T) {
t.Run("newer version", func(t *testing.T) {
store := makeMemoryStore()
store.SetBasis(sharedtest.NewDataSetBuilder().Build())

_, collection1 := makeItem("key", 10, false)

updates := store.ApplyDelta(collection1)
assert.True(t, updates[kind]["key"])

item1a, collection1a := makeItem("key", 11, true)

updates = store.ApplyDelta(collection1a)
assert.True(t, updates[kind]["key"])

result, err := store.Get(kind, "key")
require.NoError(t, err)
assert.Equal(t, item1a, result)

})

t.Run("older version", func(t *testing.T) {
store := makeMemoryStore()
store.SetBasis(sharedtest.NewDataSetBuilder().Build())

item1Version := 10
item1, collection1 := makeItem("key", item1Version, false)

updates := store.ApplyDelta(collection1)
assert.True(t, updates[kind]["key"])

_, collection1a := makeItem("key", item1Version-1, true)

updates = store.ApplyDelta(collection1a)
assert.False(t, updates[kind]["key"])
tests := []struct {
name string
initialVersion int
nextVersion int
}{
{"newer version", 10, 11},
{"older version", 10, 9},
{"same version", 10, 10},
}

result, err := store.Get(kind, "key")
require.NoError(t, err)
assert.Equal(t, item1, result)
})
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
store := makeMemoryStore()
store.SetBasis(sharedtest.NewDataSetBuilder().Build())

t.Run("same version", func(t *testing.T) {
store := makeMemoryStore()
store.SetBasis(sharedtest.NewDataSetBuilder().Build())
initialItem, initialCollection := makeItem("key", test.initialVersion, false)

item1Version := 10
item1, collection1 := makeItem("key", item1Version, false)
updated := store.ApplyDelta(collection1)
assert.True(t, updated[kind]["key"])
store.ApplyDelta(initialCollection)
result, err := store.Get(kind, "key")
require.NoError(t, err)
assert.Equal(t, initialItem, result)

_, collection1a := makeItem("key", item1Version, true)
updated = store.ApplyDelta(collection1a)
assert.False(t, updated[kind]["key"])
updatedItem, updatedCollection := makeItem("key", test.nextVersion, true)

result, err := store.Get(kind, "key")
require.NoError(t, err)
assert.Equal(t, item1, result)
})
store.ApplyDelta(updatedCollection)
result, err = store.Get(kind, "key")
require.NoError(t, err)
assert.Equal(t, updatedItem, result)
})
}
})

t.Run("deletes", func(t *testing.T) {
t.Run("newer version", func(t *testing.T) {
store := makeMemoryStore()
store.SetBasis(sharedtest.NewDataSetBuilder().Build())

item1, collection1 := makeItem("key", 10, false)
updated := store.ApplyDelta(collection1)
assert.True(t, updated[kind]["key"])

item1a, collection1a := deleteItem("key", item1.Version+1)
updated = store.ApplyDelta(collection1a)
assert.True(t, updated[kind]["key"])

result, err := store.Get(kind, "key")
require.NoError(t, err)
assert.Equal(t, item1a, result)
})

t.Run("older version", func(t *testing.T) {
store := makeMemoryStore()
store.SetBasis(sharedtest.NewDataSetBuilder().Build())

item1, collection1 := makeItem("key", 10, false)
updated := store.ApplyDelta(collection1)
assert.True(t, updated[kind]["key"])

_, collection1a := deleteItem("key", item1.Version-1)
updated = store.ApplyDelta(collection1a)
assert.False(t, updated[kind]["key"])

result, err := store.Get(kind, "key")
require.NoError(t, err)
assert.Equal(t, item1, result)
})

t.Run("same version", func(t *testing.T) {
store := makeMemoryStore()
store.SetBasis(sharedtest.NewDataSetBuilder().Build())

item1, collection1 := makeItem("key", 10, false)
updated := store.ApplyDelta(collection1)
assert.True(t, updated[kind]["key"])

_, collection1a := deleteItem("key", item1.Version)
updated = store.ApplyDelta(collection1a)
assert.False(t, updated[kind]["key"])

result, err := store.Get(kind, "key")
require.NoError(t, err)
assert.Equal(t, item1, result)
})
tests := []struct {
name string
initialVersion int
nextVersion int
}{
{"newer version", 10, 11},
{"older version", 10, 9},
{"same version", 10, 10},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
store := makeMemoryStore()
store.SetBasis(sharedtest.NewDataSetBuilder().Build())

initialItem, initialCollection := makeItem("key", test.initialVersion, false)
store.ApplyDelta(initialCollection)
result, err := store.Get(kind, "key")
require.NoError(t, err)
assert.Equal(t, initialItem, result)

_, updatedCollection := deleteItem("key", test.nextVersion)
store.ApplyDelta(updatedCollection)
result, err = store.Get(kind, "key")
require.NoError(t, err)
require.IsType(t, ldstoretypes.ItemDescriptor{}.NotFound(), result)
})
}
})
})
}
Expand Down
Loading