Permit intra-L0 on ingested files in consistency check#67
Conversation
Previously, the consistency check was verifying files were ordered by smallest seqnum after sorting them by largest seqnum. There was an exception for ingested files since they could be assigned a seqnum in the memtable's seqnum range if the memtable didn't contain overlapping keys. However, this exception didn't account for ingested files undergoing intra-L0 compaction, after which they can no longer be identified as ingested. Now that we can no longer tell which files should be excepted from this check, I just removed the whole thing.
petermattis
left a comment
There was a problem hiding this comment.
Nice that you were able to add a test for this condition. Does this need to be backported to 19.1 and earlier? I recall we backported enabling consistency checks.
Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on @itsbilal and @sumeerbhola)
sumeerbhola
left a comment
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @itsbilal)
itsbilal
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! all files reviewed, all discussions resolved (waiting on @itsbilal)
ajkr
left a comment
There was a problem hiding this comment.
Nice that you were able to add a test for this condition. Does this need to be backported to 19.1 and earlier? I recall we backported enabling consistency checks.
It should be backported to 19.2 as it supports running with ENABLE_ROCKSDB_ASSERTIONS=1, which enables these checks. For release builds we have not turned on these checks yet either on 19.2 or master -- that will require setting force_consistency_checks=true.
Reviewable status:
complete! all files reviewed, all discussions resolved
Picks up cockroachdb/rocksdb#67. Release note: None
41728: c-deps: bump rocksdb for L0 file invariant fix r=ajkr a=ajkr Picks up cockroachdb/rocksdb#67. Release note: None Co-authored-by: Andrew Kryczka <andrew.kryczka2@gmail.com>
Picks up cockroachdb/rocksdb#67. Release note: None
Picks up cockroachdb/rocksdb#67. Release note: None
Previously, the consistency check was verifying files were ordered by
smallest seqnum after sorting them by largest seqnum. There was an
exception for ingested files since they could be assigned a seqnum in
the memtable's seqnum range if the memtable didn't contain overlapping
keys. However, this exception didn't account for ingested files
undergoing intra-L0 compaction, after which they can no longer be
identified as ingested. Now that we can no longer tell which files
should be excepted from this check, I just removed the whole thing.
This change is