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

kv: don't consider skipped snapshots to be "processed" by queues #81767

Merged

Conversation

nvanbenschoten
Copy link
Member

In a support issue, we saw a very high rate of successful raftsnapshot
queue process attempts. This was confusing, as the rate of snapshots
sent was much lower. It turns out that this was because most snapshots
were being skipped on the "replica is likely in the process of being
added" path.

To avoid this confusion in the future, we no longer return true as the
value for processed from raftSnapshotQueue.process method in these
cases.

In a support issue, we saw a very high rate of successful raftsnapshot
queue process attempts. This was confusing, as the rate of snapshots
sent was much lower. It turns out that this was because most snapshots
were being skipped on the "replica is likely in the process of being
added" path.

To avoid this confusion in the future, we no longer return `true` as the
value for `processed` from `raftSnapshotQueue.process` method in these
cases.
@nvanbenschoten nvanbenschoten requested a review from kvoli May 24, 2022 17:52
@nvanbenschoten nvanbenschoten requested a review from a team as a code owner May 24, 2022 17:52
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@nvanbenschoten
Copy link
Member Author

TFTR!

bors r+

@craig
Copy link
Contributor

craig bot commented May 25, 2022

Build succeeded:

@craig craig bot merged commit 85b2ebe into cockroachdb:master May 25, 2022
@nvanbenschoten nvanbenschoten deleted the nvanbenschoten/skippedSnapshot branch June 6, 2022 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants