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

clusterversion,storage: add explicit version for split user keys #84887

Merged
merged 1 commit into from
Jul 22, 2022

Conversation

nicktrav
Copy link
Collaborator

Pebble ceased splitting user keys across multiple sstables in a single
level in cockroachdb/pebble@a860bbad. A Pebble format major version was
added in 22.1 to mark existing tables with split user that formed an
"atomic compaction unit" for compaction. The compaction to recombine the
split keys happens at a low priority within Pebble, so there is no
guarantee that these tables have been recombined.

Pebble range key support depends on having split user keys recombined.
As such, the Pebble migration to upgrade to a version that support range
keys must first perform a blocking migration to compact tables with
split user keys.

Currently, the Cockroach cluster version
EnsurePebbleFormatVersionRangeKeys ratchets the Pebble version through
both the split user keys migration, up to the version that supports
range keys. Strictly speaking, the Pebble format version migration will
take care of the sequencing.

However, to provide better visibility into the various Pebble
migrations, and to cater for the fact that the split user keys migration
may take some time to recombine the necessary tables, a dedicated
cluster version for the split keys migration is added. This dedicated
cluster version makes it unambiguous what's blocking the Cockroach
version finalization.

Release note (backward-incompatible change, ops change): A cluster
version is added to allow Pebble to recombine certain SSTables
(specifically, user keys that are split across multiple files in a level
of the LSM). Recombining the split user keys is required for supporting
the range keys feature. The migration to recombine the SSTables is
expected to be short (split user keys are rare in practice), but will
block subsequent migrations until all tables have been recombined. The
storage.marked-for-compaction-files time series metric can show the
progress of the migration.

Close #84012.

Pebble ceased splitting user keys across multiple sstables in a single
level in cockroachdb/pebble@a860bbad. A Pebble format major version was
added in 22.1 to mark existing tables with split user that formed an
"atomic compaction unit" for compaction. The compaction to recombine the
split keys happens at a low priority within Pebble, so there is no
guarantee that these tables have been recombined.

Pebble range key support depends on having split user keys recombined.
As such, the Pebble migration to upgrade to a version that support range
keys must first perform a blocking migration to compact tables with
split user keys.

Currently, the Cockroach cluster version
`EnsurePebbleFormatVersionRangeKeys` ratchets the Pebble version through
both the split user keys migration, up to the version that supports
range keys. Strictly speaking, the Pebble format version migration will
take care of the sequencing.

However, to provide better visibility into the various Pebble
migrations, and to cater for the fact that the split user keys migration
may take some time to recombine the necessary tables, a dedicated
cluster version for the split keys migration is added. This dedicated
cluster version makes it unambiguous what's blocking the Cockroach
version finalization.

Release note (backward-incompatible change, ops change): A cluster
version is added to allow Pebble to recombine certain SSTables
(specifically, user keys that are split across multiple files in a level
of the LSM). Recombining the split user keys is required for supporting
the range keys feature.  The migration to recombine the SSTables is
expected to be short (split user keys are rare in practice), but will
block subsequent migrations until all tables have been recombined. The
`storage.marked-for-compaction-files` time series metric can show the
progress of the migration.

Close cockroachdb#84012.
@nicktrav nicktrav requested review from itsbilal and jbowens July 21, 2022 23:48
@nicktrav nicktrav requested a review from a team as a code owner July 21, 2022 23:48
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Version: roachpb.Version{Major: 22, Minor: 1, Internal: 6},
},
{
Key: EnablePebbleFormatVersionRangeKeys,
Key: EnsurePebbleFormatVersionRangeKeys,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I shifted everything down to slot in the PebbleFormatSplitUserKeysMarkedCompacted version before the range keys version. I think that's fine?

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:

Reviewed 2 of 5 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @itsbilal and @nicktrav)


pkg/clusterversion/cockroach_versions.go line 528 at r1 (raw file):

Previously, nicktrav (Nick Travers) wrote…

I shifted everything down to slot in the PebbleFormatSplitUserKeysMarkedCompacted version before the range keys version. I think that's fine?

I think it should be fine too

@nicktrav
Copy link
Collaborator Author

Flake is in the logictest. Ignoring.

TFTR!

bors r=jbowens

@craig
Copy link
Contributor

craig bot commented Jul 22, 2022

Build succeeded:

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.

storage: block 22.2 upgrade on removal of marked-for-compaction files
3 participants