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

Jump to bottom

kvserver: add test to verify tryReproposeWithNewLeaseIndex behaviour #106504

Closed
aliher1911 opened this issue Jul 10, 2023 · 4 comments · Fixed by #113658
Closed

kvserver: add test to verify tryReproposeWithNewLeaseIndex behaviour #106504

aliher1911 opened this issue Jul 10, 2023 · 4 comments · Fixed by #113658
Assignees
@pav-kv
Labels
A-kv-replication Relating to Raft, consensus, and coordination. branch-master Failures and bugs on the master branch. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@aliher1911
Copy link
Contributor

aliher1911 commented Jul 10, 2023 •
edited by cockroach-jira-scripts
Loading

This PR #106502 removes a flaky integration test that verified reproposals with new lease index doesn't break pipelined writes (errors of this type are correctly handled below request evaluation).
Removed integration test was too high level to pinpoint this use case and precisely filter all other associated failure modes.
Test was originally introduced in #35261 and PR has context on what test verifies and motivation for it.

This reproposal functionality is not covered by any tests as of now.

Jira issue: CRDB-29593
@aliher1911 aliher1911 added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-kv-replication Relating to Raft, consensus, and coordination. labels Jul 10, 2023
@blathers-crl
Copy link

blathers-crl bot commented Jul 10, 2023

cc @cockroachdb/replication

@aliher1911 aliher1911 mentioned this issue Jul 10, 2023
Merged
tbg added a commit to tbg/cockroach that referenced this issue Jul 13, 2023
@tbg
kvserver: delete TestReplicaRefreshMultiple and TestReplicaReproposal…
5fe9415 
…WithNewLeaseIndexError

These tests don't work with v2 and aren't maintainable or super useful anyway.

We have good randomized coverage of these code paths now via kvnemesis and
TestProposalsWithInjectedLeaseIndexAndReproposalError, and adding better unit
test coverage is tracked in
cockroachdb#106504. In addition to this, it
would be desirable to extend this unit test coverage upwards through the stack
via cockroachdb#105177.
@erikgrinaker erikgrinaker added branch-master Failures and bugs on the master branch. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. labels Sep 11, 2023
@erikgrinaker
Copy link
Contributor

Conservatively marking as blocker, to improve reproposal confidence before the release.

@erikgrinaker erikgrinaker added GA-blocker and removed release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. labels Sep 13, 2023
@erikgrinaker
Copy link
Contributor

We won't have time to get to this, dropping GA-blocker.

@pav-kv pav-kv mentioned this issue Nov 2, 2023
Merged
@pav-kv pav-kv self-assigned this Nov 2, 2023
@pav-kv
Copy link
Collaborator

pav-kv commented Nov 2, 2023 •
edited
Loading

#113658 essentially fixes this because we have a test that verifies LAI reproposals under pipelined writes.

@craig craig bot closed this as completed in 08a253e Nov 3, 2023
@blathers-crl blathers-crl bot mentioned this issue Nov 3, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pav-kv pav-kv

Labels
A-kv-replication Relating to Raft, consensus, and coordination. branch-master Failures and bugs on the master branch. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

kvserver: fix reproposals test with pipelined writes pav-kv/cockroach
4 participants
@erikgrinaker @pav-kv @tbg @aliher1911