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

raft: repurpose unused Node interface #122131

Closed
pav-kv opened this issue Apr 10, 2024 · 1 comment · Fixed by #131746
Closed

raft: repurpose unused Node interface #122131

pav-kv opened this issue Apr 10, 2024 · 1 comment · Fixed by #131746
Labels
A-kv-replication Relating to Raft, consensus, and coordination. A-testing Testing tools and infrastructure C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-kv KV Team

Comments

@pav-kv
Copy link
Collaborator

pav-kv commented Apr 10, 2024

We only use RawNode interface to raft, and don't use the Node which is a wrapper around RawNode with goroutines and channels that runs the node. In CRDB, we do all that in the raft scheduler.

We could outright remove the Node interface, but maybe we should leave if for additional testing coverage, or migrate the tests to use RawNode. The Node tests tend to be flaky (e.g. #121745) because of timing/scheduling: e.g. a test may wait for a leader and then assume that the leader is stable, but timing flakes can result in a leader change in the middle of the test.

A better approach to testing would be to have randomized, but deterministic tests based on RawNode. Then all the flakes would be preventable and/or reproducible.

We probably can repurpose Node to be a test-only type, with added benefits of randomized testing. We could improve reproducibility of these runs, by tracing all Step/Ready calls and message sends/drops done by the Nodes. The trace would be convertible to a list of datadriven commands that TestInteraction operates with, so any test run could be then reliably reproduced with RawNodes.

Jira issue: CRDB-37718

Epic CRDB-39898

@pav-kv pav-kv 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 Apr 10, 2024
Copy link

blathers-crl bot commented Apr 10, 2024

cc @cockroachdb/replication

@pav-kv pav-kv changed the title raft: remove unused Node interface raft: repurpose unused Node interface Jun 28, 2024
@pav-kv pav-kv added the A-testing Testing tools and infrastructure label Jun 28, 2024
@exalate-issue-sync exalate-issue-sync bot added T-kv KV Team and removed T-kv-replication labels Jun 28, 2024
@github-project-automation github-project-automation bot moved this to Incoming in KV Aug 28, 2024
@craig craig bot closed this as completed in 900f9fd Oct 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-kv-replication Relating to Raft, consensus, and coordination. A-testing Testing tools and infrastructure C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-kv KV Team
Projects
No open projects
Status: Incoming
Development

Successfully merging a pull request may close this issue.

1 participant