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

release-23.1.0: go.mod: bump Pebble to 0a342eccdea0 #102070

Conversation

RaduBerinde
Copy link
Member

0a342ecc crl-release-23.1: db: fix bug with restricted checkpoints
fb56da99 sstable: include SINGLEDELs within NumDeletions table property
8a10d6a0 sstable: reuse block property filterers in all cases

Informs #100935.

Release note: None
Epic: none

Release justification: one commit fixes a bug where in some cases store checkpoints (generated when replica mismatches are detected) cannot be opened. This can be a big problem when debugging production issues. One commit fixes store statistics that are used to make compaction decisions. One commit is a performance improvement that offsets a performance regression.


crl-release-23.1: db: fix bug with restricted checkpoints

Backport of cockroachdb/pebble#2460 for 23.1.x

When a checkpoint is restricted to a set of spans, we append a record
to the checkpoint's manifest removing all the excluded ssts.

We append the record after copying the raw data from the existing
manifest. This is not quite ok: the record library works by breaking
up records in chunks and packing chunks into 32KB blocks. Chunks
cannot straddle a 32KB boundary, but this invariant is violated by our
append method. In practice, this only happens if the record we are
appending is big and/or we are very unlucky and the existing manifest
is close to a 32KB boundary.

To fix this: instead of doing a raw data copy of the existing
manifest, we copy at the record level (using a record reader and a
record writer). Then we can add a new record using the same writer.

sstable: include SINGLEDELs within NumDeletions table property

Previously, the sstable property recording the number of deletion tombstones
within a file included DELs and RANGEDELs but not SINGLEDELs. This caused
SINGLEDELs to be excluded from point tombstone compaction heuristics that use
these properties to estimate the amount of disk space that may be reclaimed by
compacting a file.

sstable: reuse block property filterers in all cases

Previously, if block-property filters were in use and a sstable was found to
NOT intersect the filters, the allocated BlockPropertiesFilterer was dropped
instead of being returned to the pool.

This commit refactors the sstable package's interface to hide the
IntersectsUserPropsAndFinishInit function's subtle semantics behind a new
IntersectsTable function that returns a new filterer if the table intersects
the provided filters. If the table does not intersect the provided filters, the
obtained filterer is automatically returned to the sync pool.

0a342ecc crl-release-23.1: db: fix bug with restricted checkpoints
fb56da99 sstable: include SINGLEDELs within NumDeletions table property
8a10d6a0 sstable: reuse block property filterers in all cases

Informs cockroachdb#100935.

Release note: None
Epic: none
@RaduBerinde RaduBerinde requested review from jbowens and a team April 22, 2023 16:21
@RaduBerinde RaduBerinde requested a review from a team as a code owner April 22, 2023 16:21
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@RaduBerinde RaduBerinde changed the title go.mod: bump Pebble to 0a342eccdea0 release-23.1.0: go.mod: bump Pebble to 0a342eccdea0 Apr 22, 2023
Copy link
Collaborator

@jbowens jbowens left a comment

Choose a reason for hiding this comment

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

lgtm

@RaduBerinde
Copy link
Member Author

TFTRs!

@RaduBerinde RaduBerinde merged commit 0ff83a5 into cockroachdb:release-23.1.0 Apr 24, 2023
@RaduBerinde RaduBerinde deleted the bump-pebbble-23.1.0-0a342eccdea0 branch April 24, 2023 15:29
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.

4 participants