Fix race with table init channel #53
Merged
+14
−4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The function to mark a table initialized wrongly closed the
channel immediately. This could cause an observer to be woken
up too early (before Commit() has finished) and get an uninitialized
snapshot.
Fix the timing issue by moving the closing into Commit(), next to
the normal watch channel closing. This way the initialized table has
been committed before the init watch channel closes.
This issue was detected by `pkg/k8s/statedb_test.go` in cilium/cilium:
statedb_test.go:263:
Error Trace: /home/runner/work/cilium/cilium/pkg/k8s/statedb_test.go:263
/home/runner/work/cilium/cilium/pkg/k8s/statedb_test.go:120
Error: Not equal:
expected: "obj1"
actual : "garbage"
Diff:
--- Expected
+++ Actual
@@ -1 +1 @@
-obj1
+garbage
Test: TestStateDBReflector/default
Signed-off-by: Jussi Maki <jussi.maki@isovalent.com>
|
Verified fix for the flake in |
|
joamaki
added a commit
to joamaki/cilium
that referenced
this pull request
Sep 6, 2024
A timing bug in closing of the table init channel
is sometimes causing the test to fail:
statedb_test.go:263:
Error Trace: /.../pkg/k8s/statedb_test.go:263
/.../pkg/k8s/statedb_test.go:133
Error: Not equal:
expected: "obj1"
actual : "garbage"
Diff:
--- Expected
+++ Actual
@@ -1 +1 @@
-obj1
+garbage
Test: TestStateDBReflector/TransformMany-QueryAll
FAIL
The root cause for this is a bug in StateDB that causes
the init watch channel to be closed too early. This
is fixed in cilium/statedb#53.
As it takes a bit of time until we've bumped the StateDB
version, work around this issue by repeatedly checking
if the table is initialized, so folks aren't exposed
to the flake.
Signed-off-by: Jussi Maki <jussi@isovalent.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The function to mark a table initialized wrongly closed the channel immediately. This could cause an observer to be woken up too early (before Commit() has finished) and get an uninitialized snapshot.
Fix the timing issue by moving the closing into Commit(), next to the normal watch channel closing. This way the initialized table has been committed before the init watch channel closes.
This issue was detected by
pkg/k8s/statedb_test.goin cilium/cilium: