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

Deflake: TestFirstCommitNotification #12889

Merged

Conversation

ptabor
Copy link
Contributor

@ptabor ptabor commented Apr 23, 2021

Infrequently the test flaked. Reproducable with:

  go test go.etcd.io/etcd/tests/v3/integration --run TestFirstCommitNotification --count=500

The moveLeader finishes when configchange is commited by quorum.
It doesn't guarantee that the 'empty' record was committed by the new leader.
From time to time happened that appliedLeaderIndex was returning 9
(without empty entry) and the test flaked. In healthy case the
appliedIndex returned 10.

Fixed by putting kv pair after leader change. The pair is guaranteed
to be stored on index when put finishes (so the empty entry as well).

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

@ptabor
Copy link
Contributor Author

ptabor commented Apr 23, 2021

@wpedrak - please take a look.

@ptabor ptabor force-pushed the 20210423-deflake-TestFirstCommitNotification branch from 5dd8344 to 75079a0 Compare April 23, 2021 09:57
Copy link
Contributor

@wpedrak wpedrak left a comment

Choose a reason for hiding this comment

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

Thanks a lot for fixing (and explaining) this, LGTM

Infrequently the test flaked. Reproducable with:

```
  go test go.etcd.io/etcd/tests/v3/integration --run TestFirstCommitNotification --count=500
```

The moveLeader finishes when configchange is commited by quorum.
It doesn't guarantee that the 'empty' record was committed by the new leader.
From time to time happened that appliedLeaderIndex was returning 9
(without empty entry) and the test flaked. In healthy case the
appliedIndex returned 10.

Fixed by putting kv pair after leader change. The pair is guaranteed
to be stored on index when put finishes (so the empty entry as well).
@ptabor ptabor force-pushed the 20210423-deflake-TestFirstCommitNotification branch from 75079a0 to 9f55977 Compare April 23, 2021 11:22
@ptabor ptabor merged commit 9a3aff6 into etcd-io:master Apr 23, 2021
@ptabor ptabor deleted the 20210423-deflake-TestFirstCommitNotification branch April 23, 2021 12:55
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.

None yet

2 participants