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: include MVCC range keys in replica consistency checks #78104

Merged
merged 1 commit into from
Jun 29, 2022
Merged

kvserver: include MVCC range keys in replica consistency checks #78104

merged 1 commit into from
Jun 29, 2022

Conversation

erikgrinaker
Copy link
Contributor

@erikgrinaker erikgrinaker commented Mar 18, 2022 •
edited
Loading

This patch adds handling of MVCC range keys in replica consistency
checks. These are iterated over as part of MVCCStats calculations and
hashed similarly to point keys.

Range keys will only exist after the version gate
ExperimentalMVCCRangeTombstones has been enabled, so a separate
version gate is not necessary.

Release note: None

@erikgrinaker erikgrinaker requested review from tbg and aliher1911 March 18, 2022 21:04
@erikgrinaker erikgrinaker requested review from a team as code owners March 18, 2022 21:04
@erikgrinaker erikgrinaker self-assigned this Mar 18, 2022
@erikgrinaker erikgrinaker mentioned this pull request Mar 18, 2022
Closed
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@erikgrinaker erikgrinaker force-pushed the mvcc-range-tombstones-consistency-check branch from 0d84445 to 5e914fd Compare March 19, 2022 08:15
@erikgrinaker erikgrinaker force-pushed the mvcc-range-tombstones branch 2 times, most recently from b050b4a to bb5aa54 Compare March 24, 2022 11:39
@erikgrinaker erikgrinaker force-pushed the mvcc-range-tombstones-consistency-check branch from 5e914fd to fcd6a38 Compare March 26, 2022 19:02
@erikgrinaker erikgrinaker force-pushed the mvcc-range-tombstones branch from bb5aa54 to ba8eb78 Compare March 30, 2022 12:13
aliher1911
aliher1911 approved these changes Mar 31, 2022
View reviewed changes
Copy link
Contributor

@aliher1911 aliher1911 left a comment

Choose a reason for hiding this comment

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

Nice use of echotest.Require made tests more readable.

pkg/kv/kvserver/replica_consistency_diff.go Show resolved Hide resolved
@erikgrinaker erikgrinaker force-pushed the mvcc-range-tombstones branch from ba8eb78 to 34e6ab0 Compare April 2, 2022 17:10
@erikgrinaker erikgrinaker force-pushed the mvcc-range-tombstones-consistency-check branch from fcd6a38 to e9e5ee7 Compare April 3, 2022 17:11
@erikgrinaker erikgrinaker force-pushed the mvcc-range-tombstones branch from 34e6ab0 to 4798ae2 Compare April 4, 2022 13:43
nicktrav
nicktrav reviewed Apr 5, 2022
View reviewed changes
Copy link
Collaborator

@nicktrav nicktrav left a comment

Choose a reason for hiding this comment

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

Reviewed 8 of 8 files at r16, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @aliher1911 and @tbg)

@erikgrinaker erikgrinaker force-pushed the mvcc-range-tombstones branch 5 times, most recently from 2097ac5 to 3ad25e4 Compare April 11, 2022 20:36
@erikgrinaker erikgrinaker force-pushed the mvcc-range-tombstones-consistency-check branch from e9e5ee7 to 52e77e2 Compare April 12, 2022 09:48
@erikgrinaker erikgrinaker force-pushed the mvcc-range-tombstones branch from 3ad25e4 to 6a1756c Compare April 15, 2022 09:41
@erikgrinaker erikgrinaker force-pushed the mvcc-range-tombstones-consistency-check branch from 52e77e2 to cbb90f1 Compare April 15, 2022 21:58
@erikgrinaker erikgrinaker force-pushed the mvcc-range-tombstones branch from 6a1756c to 3bd8605 Compare April 22, 2022 13:06
@erikgrinaker erikgrinaker force-pushed the mvcc-range-tombstones-consistency-check branch 3 times, most recently from 603be75 to 03447f1 Compare April 24, 2022 20:42
@erikgrinaker erikgrinaker force-pushed the mvcc-range-tombstones branch from 3bd8605 to 2c06ce4 Compare May 1, 2022 10:09
@erikgrinaker erikgrinaker force-pushed the mvcc-range-tombstones branch from 2c06ce4 to cb77b2e Compare May 1, 2022 12:31
@erikgrinaker erikgrinaker force-pushed the mvcc-range-tombstones-consistency-check branch 2 times, most recently from 83a3d6b to 8dbd47a Compare May 1, 2022 15:09
@erikgrinaker erikgrinaker force-pushed the mvcc-range-tombstones branch from cb77b2e to 49aacac Compare May 9, 2022 08:36
@erikgrinaker erikgrinaker requested a review from a team as a May 9, 2022 08:36
@erikgrinaker erikgrinaker force-pushed the mvcc-range-tombstones-consistency-check branch from 8dbd47a to 220b12b Compare May 9, 2022 08:48
@erikgrinaker erikgrinaker force-pushed the mvcc-range-tombstones branch from 49aacac to e5bd560 Compare May 9, 2022 09:20
@erikgrinaker erikgrinaker force-pushed the mvcc-range-tombstones-consistency-check branch 2 times, most recently from a5c681e to b61f80d Compare May 10, 2022 17:50
@erikgrinaker erikgrinaker marked this pull request as draft May 28, 2022 16:53
@erikgrinaker erikgrinaker changed the base branch from mvcc-range-tombstones to master May 30, 2022 13:10
@erikgrinaker erikgrinaker force-pushed the mvcc-range-tombstones-consistency-check branch from b61f80d to 6401aab Compare May 30, 2022 13:51
@erikgrinaker erikgrinaker changed the title kvserver: include range keys in replica consistency checks kvserver: include MVCC range keys in replica consistency checks May 30, 2022
@erikgrinaker erikgrinaker force-pushed the mvcc-range-tombstones-consistency-check branch from 6401aab to 877af3c Compare June 4, 2022 18:16
@tbg tbg removed their request for review June 16, 2022 15:13
@erikgrinaker erikgrinaker force-pushed the mvcc-range-tombstones-consistency-check branch from 877af3c to 9c4fdf8 Compare June 28, 2022 10:30
@erikgrinaker erikgrinaker removed the request for review from a team June 28, 2022 10:31
@erikgrinaker erikgrinaker marked this pull request as ready for review June 28, 2022 10:31
@erikgrinaker
Copy link
Contributor Author

Allright, this one's ready for a final review @aliher1911. It's been 3 months since your last review, so I'll let you have another look, but shouldn't be any major changes.

@erikgrinaker
kvserver: include MVCC range keys in replica consistency checks
ee57d72 
This patch adds handling of MVCC range keys in replica consistency
checks. These are iterated over as part of `MVCCStats` calculations and
hashed similarly to point keys.

Range keys will only exist after the version gate
`ExperimentalMVCCRangeTombstones` has been enabled, so a separate
version gate is not necessary.

Release note: None
@erikgrinaker erikgrinaker force-pushed the mvcc-range-tombstones-consistency-check branch from 9c4fdf8 to ee57d72 Compare June 28, 2022 10:44
@aliher1911 aliher1911 self-requested a review June 28, 2022 11:39
aliher1911
aliher1911 reviewed Jun 28, 2022
View reviewed changes
Copy link
Contributor

@aliher1911 aliher1911 left a comment

Choose a reason for hiding this comment

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

Want to clarify thing about tombstone value in the comment below before giving a go.

@erikgrinaker
Copy link
Contributor Author

Want to clarify thing about tombstone value in the comment below before giving a go.

Forget to submit the comment?

aliher1911
aliher1911 approved these changes Jun 29, 2022
View reviewed changes
Copy link
Contributor

@aliher1911 aliher1911 left a comment

Choose a reason for hiding this comment

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

💯

@erikgrinaker
Copy link
Contributor Author

TFTR! CI failures are unrelated.

bors r=aliher1911

@craig craig bot merged commit 0917fdc into cockroachdb:master Jun 29, 2022
@craig
Copy link
Contributor

craig bot commented Jun 29, 2022

Build succeeded:

@erikgrinaker erikgrinaker deleted the mvcc-range-tombstones-consistency-check branch June 30, 2022 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicktrav nicktrav nicktrav left review comments

@aliher1911 aliher1911 aliher1911 approved these changes

Assignees

@erikgrinaker erikgrinaker

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@erikgrinaker @cockroach-teamcity @nicktrav @aliher1911