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

kvserver: opt-in "fast" external file snapshotting #120030

Merged
merged 2 commits into from
Mar 20, 2024

Conversation

stevendanna
Copy link
Collaborator

Like SharedSSTs, we may want to send a pebble.ExternalFile's metadata
rather than its content during a snapshot.

This is opt-in via a cluster setting and is only attempted when the
store appears to actually have external files.

Epic: none
Release note: None

@stevendanna stevendanna requested review from a team as code owners March 7, 2024 03:18
@stevendanna stevendanna requested a review from a team March 7, 2024 03:18
@stevendanna stevendanna requested a review from a team as a code owner March 7, 2024 03:18
@stevendanna stevendanna requested a review from RaduBerinde March 7, 2024 03:18
Copy link

blathers-crl bot commented Mar 7, 2024

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@stevendanna
Copy link
Collaborator Author

Requires cockroachdb/pebble#3384

@stevendanna stevendanna force-pushed the external-file-snapshotting branch from a48ecca to 396c31a Compare March 11, 2024 11:27
@stevendanna stevendanna requested a review from itsbilal March 12, 2024 19:21
@stevendanna
Copy link
Collaborator Author

@itsbilal I believe we have some work to do on the pebble side re exclusive keys to do. But this should otherwise be more or less ready to review if you want to take a look.

Copy link
Member

@itsbilal itsbilal left a comment

Choose a reason for hiding this comment

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

LGTM save for some minor comments (mostly about the end key inclusive work)

pkg/kv/kvserver/replica_command.go Outdated Show resolved Hide resolved
pkg/kv/kvserver/store_snapshot.go Outdated Show resolved Hide resolved
pkg/kv/kvserver/kvserverpb/raft.proto Show resolved Hide resolved
@stevendanna stevendanna force-pushed the external-file-snapshotting branch from 396c31a to 37113e7 Compare March 20, 2024 10:09
Like SharedSSTs, we may want to send a pebble.ExternalFile's metadata
rather than its content during a snapshot.

This is opt-in via a cluster setting and is only attempted when the
store appears to actually have external files.

Epic: none
Release note: None
@stevendanna stevendanna force-pushed the external-file-snapshotting branch from 37113e7 to 85386fd Compare March 20, 2024 10:13
@stevendanna
Copy link
Collaborator Author

bors r=itsbilal

@craig
Copy link
Contributor

craig bot commented Mar 20, 2024

@craig craig bot merged commit ecedafa into cockroachdb:master Mar 20, 2024
21 of 22 checks passed
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.

3 participants