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.

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

tests/integration: Update TestLeasingDeleteRangeContendTxn #15425

Merged
merged 1 commit into from
Mar 8, 2023

Conversation

fuweid
Copy link
Member

@fuweid fuweid commented 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 #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: #15352

Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.


cc @ahrtr @chaochn47 @serathius @tjungblu

@tjungblu
Copy link
Contributor

tjungblu commented Mar 7, 2023

ahh it's the context, not the counter. Great find @fuweid!

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 fuweid force-pushed the update-TestLeasingDeleteRangeContendTxn branch from 26366f0 to 3419230 Compare March 7, 2023 15:20
Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thank you @fuweid

I really like such fix for flaky test.

@ahrtr ahrtr merged commit aaa8bfe into etcd-io:main Mar 8, 2023
@fuweid fuweid deleted the update-TestLeasingDeleteRangeContendTxn branch March 8, 2023 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Flacky test TestLeasingDeleteRangeContendTxn
3 participants