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

storage: table/block property collectors ignore Pebble range tombstones #83376

Closed
erikgrinaker opened this issue Jun 25, 2022 · 0 comments · Fixed by #84603
Closed

storage: table/block property collectors ignore Pebble range tombstones #83376

erikgrinaker opened this issue Jun 25, 2022 · 0 comments · Fixed by #84603
Assignees
@jbowens
Labels
A-storage Relating to our storage engine (Pebble) on-disk storage. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-storage Storage Team

Comments

@erikgrinaker
Copy link
Contributor

erikgrinaker commented Jun 25, 2022 •
edited
Loading

As seen in cockroachdb/pebble#1786, it appears that table and block property collectors do not record Pebble range tombstones. Since these typically have bounds without timestamps they are ignored by the collectors, but conceptually they write at all timestamps. This can cause the TBI filtering to omit SSTs that contain these range tombstones, thus surfacing keys that should have been removed.

This isn't currently believed to be a correctness problem, because all TBI callers only use them as a performance optimization (to possibly skip irrelevant keys) and fall back to regular non-TBI iterators for processing. However, future callers may begin relying on them.

Jira issue: CRDB-17032

Epic CRDB-2624
@erikgrinaker erikgrinaker added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-storage Relating to our storage engine (Pebble) on-disk storage. T-storage Storage Team labels Jun 25, 2022
@erikgrinaker erikgrinaker mentioned this issue Jun 25, 2022
Closed
@erikgrinaker erikgrinaker changed the title storage: table/block property collectors do not handle range tombstones storage: table/block property collectors ignore Pebble range tombstones Jun 25, 2022
@jbowens jbowens mentioned this issue Jul 18, 2022
Merged
@craig craig bot closed this as completed in 6e9f5a9 Jul 19, 2022
nicktrav added a commit to nicktrav/cockroach that referenced this issue Sep 15, 2022
@nicktrav
pkg/storage: unskip TestPebbleMVCCTimeIntervalWithRangeClears
efa6126 
Unskips a test that was skipped due to cockroachdb#83376, which has since been
fixed.

Release note: None.
@nicktrav nicktrav mentioned this issue Sep 15, 2022
Merged
craig bot pushed a commit that referenced this issue Sep 15, 2022
@nicktrav
Merge #87916 #88020
9a05046 
87916: ui: fix sql activity % of all runtime when filtered r=j82w a=j82w

Before this shows the % value was 2.5%, but after a filter applied it was changed to 100%.
https://loom.com/share/5b627ca2c0524653ae2ad6840a32e0a3

After this shows the % value stays the same after the filter was applied.
https://loom.com/share/338fde861349447f8725f991d01fd231

closes #87415

Release justification: Category 2: Bug fixes and
low-risk updates to new functionality

Release note: (ui change): Fixed the % of all
runtime when filtered. It previously used only
the filtered data to calculate the value instead
of all the data in that time period.

88020: pkg/storage: unskip TestPebbleMVCCTimeIntervalWithRangeClears r=jbowens,erikgrinaker a=nicktrav

Unskips a test that was skipped due to #83376, which has since been fixed.

Release note: None.

Co-authored-by: j82w <[email protected]>
Co-authored-by: Nick Travers <[email protected]>
@blathers-crl blathers-crl bot mentioned this issue Sep 16, 2022
Merged
blathers-crl bot pushed a commit that referenced this issue Sep 16, 2022
@nicktrav
pkg/storage: unskip TestPebbleMVCCTimeIntervalWithRangeClears
873102b 
Unskips a test that was skipped due to #83376, which has since been
fixed.

Release note: None.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jbowens jbowens

Labels
A-storage Relating to our storage engine (Pebble) on-disk storage. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-storage Storage Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

vendor: bump Pebble to 6b9be031039a jbowens/cockroach
2 participants
@erikgrinaker @jbowens