Fixing flaky TestLeasingDeleteRangeContendTxn #15401
Merged
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
Fixes etcd-io#15352. Depending on the goroutine scheduling, the expected count of 8 might not have been reached yet. This ensures the routine won't stop earlier than that. Signed-off-by: Thomas Jungblut <tjungblu@redhat.com>
serathius
approved these changes
Mar 3, 2023
fuweid
added a commit
to fuweid/etcd
that referenced
this pull request
Mar 7, 2023
The TestLeasingDeleteRangeContendTxn is trying to test for RangeDelete when the target resources are being updated. When the `txnLeasing` wants a server-side transaction, it needs to ensure all the keys mod revision should be leass than what it saw. If the compare fails, it will repeat to apply the server-side transaction until it is sucessful. I believe the test-case is trying to verify how the `txnLeasing` handles the race issue. Before the patch etcd-io#15401, the resource-updating goroutine keeps updating until the RangeDelete finishes. The testcase is flaky because two goroutines are sharing one `ctx` and grpc-go client won't wait for the response if `ctx` has been canceled. For example, | DelLease Goroutine | PutLease Goroutine | ETCD Server | Key/0 Status | | -- | --- | -- | -- | | deleted | | | version = 0 | | | send update(key/0=123) req | received update(key/0=123) req | version = 0 | | cancel | | | version = 0 | | | exit because of cancel | | version = 0 | | get key/0 by putkv | | | version = 0 | | | | applied update(key/0=123) | version = 1 | | get key/0 by raw-cli | | | version = 1 | So `raw-cli` gets `[key/0=123]` while the `putkv` gets `[]`. If `putkv` applies two update reqs to ETCD server and the last one is canceled before apply, the error will be like: ``` expected [key:"key/0" version:2 value:"123" ], got [key:"key/0" version:1 value:"123" ] ``` The resource-updating goroutine should not share the ctx with RangeDelete here. And I also revert current main branch because the resource-update goroutine only updates 8 times and might exit before `RangeDelete`. In this case, the `txnLeasing` is not handling the race issue. Fixes: etcd-io#15352 Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid
added a commit
to fuweid/etcd
that referenced
this pull request
Mar 7, 2023
The TestLeasingDeleteRangeContendTxn is trying to test for RangeDelete when the target resources are being updated. When the `txnLeasing` wants a server-side transaction, it needs to ensure all the keys mod revision should be leass than what it saw. If the compare fails, it will repeat to apply the server-side transaction until it is sucessful. I believe the test-case is trying to verify how the `txnLeasing` handles the race issue. Before the patch etcd-io#15401, the resource-updating goroutine keeps updating until the RangeDelete finishes. The testcase is flaky because two goroutines are sharing one `ctx` and grpc-go client won't wait for the response if `ctx` has been canceled. For example, | DelLease Goroutine | PutLease Goroutine | ETCD Server | Key/0 Status | | -- | --- | -- | -- | | deleted | | | version = 0 | | | send update(key/0=123) req | received update(key/0=123) req | version = 0 | | cancel | | | version = 0 | | | exit because of cancel | | version = 0 | | get key/0 by putkv | | | version = 0 | | | | applied update(key/0=123) | version = 1 | | get key/0 by raw-cli | | | version = 1 | So `raw-cli` gets `[key/0=123]` while the `putkv` gets `[]`. If `putkv` applies two update reqs to ETCD server and the last one is canceled before apply, the error will be like: ``` expected [key:"key/0" version:2 value:"123" ], got [key:"key/0" version:1 value:"123" ] ``` The resource-updating goroutine should not share the ctx with RangeDelete here. And I also revert current main branch because the resource-update goroutine only updates 8 times and might exit before `RangeDelete`. In this case, the `txnLeasing` is not handling the race issue. Fixes: etcd-io#15352 Signed-off-by: Wei Fu <fuweid89@gmail.com>
pchan
pushed a commit
to pchan/etcd
that referenced
this pull request
Mar 18, 2023
The TestLeasingDeleteRangeContendTxn is trying to test for RangeDelete when the target resources are being updated. When the `txnLeasing` wants a server-side transaction, it needs to ensure all the keys mod revision should be leass than what it saw. If the compare fails, it will repeat to apply the server-side transaction until it is sucessful. I believe the test-case is trying to verify how the `txnLeasing` handles the race issue. Before the patch etcd-io#15401, the resource-updating goroutine keeps updating until the RangeDelete finishes. The testcase is flaky because two goroutines are sharing one `ctx` and grpc-go client won't wait for the response if `ctx` has been canceled. For example, | DelLease Goroutine | PutLease Goroutine | ETCD Server | Key/0 Status | | -- | --- | -- | -- | | deleted | | | version = 0 | | | send update(key/0=123) req | received update(key/0=123) req | version = 0 | | cancel | | | version = 0 | | | exit because of cancel | | version = 0 | | get key/0 by putkv | | | version = 0 | | | | applied update(key/0=123) | version = 1 | | get key/0 by raw-cli | | | version = 1 | So `raw-cli` gets `[key/0=123]` while the `putkv` gets `[]`. If `putkv` applies two update reqs to ETCD server and the last one is canceled before apply, the error will be like: ``` expected [key:"key/0" version:2 value:"123" ], got [key:"key/0" version:1 value:"123" ] ``` The resource-updating goroutine should not share the ctx with RangeDelete here. And I also revert current main branch because the resource-update goroutine only updates 8 times and might exit before `RangeDelete`. In this case, the `txnLeasing` is not handling the race issue. Fixes: etcd-io#15352 Signed-off-by: Wei Fu <fuweid89@gmail.com> Signed-off-by: Prasad Chandrasekaran <prasadc@vmware.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.
Fixes #15352.
Depending on the goroutine scheduling, the expected count of 8 might not have been reached yet. This ensures the routine won't stop earlier than that.