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

storage: stream SSTs instead of KV_BATCH when sending snapshots #39716

Open
jeffrey-xiao opened this issue Aug 16, 2019 · 5 comments
Open

storage: stream SSTs instead of KV_BATCH when sending snapshots #39716

jeffrey-xiao opened this issue Aug 16, 2019 · 5 comments
Labels
A-kv-replication Relating to Raft, consensus, and coordination. C-performance Perf of queries or internals. Solution not expected to change functional behavior. T-kv-replication KV Replication Team

Comments

@jeffrey-xiao
Copy link
Contributor

jeffrey-xiao commented Aug 16, 2019

#38932 introduces the logic for incrementally building SSTs on the receiver side. This approach does not get the compression benefit from streaming SSTs when sending snapshots. I did an informal experiment to see the savings we'd get from streaming SSTs instead of KV_BATCH and the results are promising.

For an antagonist workload like kv0 where all the keys and values are random data and we see no benefits in compression, the amount of overhead that streaming SSTs instead of KV_BATCHES is fairly small. For kv0 I saw a 4% increase in the amount of data we were streaming for various range sizes.

For a more realistic workload like tpcc, I saw as much as an 80% reduction in the amount of data we were sending.

It might be worthwhile to send the user range as an SST and the replicated range-id local keys and range-local keys as KV_BATCHES.

Unlike #38932, this change would have to be gated behind a version flag since the logic on the sender side would have to change.

Jira issue: CRDB-5567

@nvanbenschoten nvanbenschoten added A-kv-replication Relating to Raft, consensus, and coordination. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) C-performance Perf of queries or internals. Solution not expected to change functional behavior. labels Aug 20, 2019
@nvanbenschoten nvanbenschoten added this to Incoming in KV via automation Aug 20, 2019
@ajwerner
Copy link
Contributor

One question I have here is about the effectiveness of transport level compression. By default outside of roachprod we use compression on our gRPC connections. My hunch is this isn't worth the complexity.

@lunevalex lunevalex moved this from Incoming to Multi-Region in KV Jul 14, 2020
@lunevalex lunevalex moved this from Distribution to Replication in KV Jul 15, 2020
@lunevalex lunevalex moved this from Replication to Distribution in KV Apr 23, 2021
@lunevalex lunevalex moved this from Distribution to Cold storage in KV Apr 23, 2021
@github-actions
Copy link

github-actions bot commented Jun 6, 2021

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
5 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!

@jlinder jlinder added the T-kv KV Team label Jun 16, 2021
KV automation moved this from On Hold to Closed Jun 28, 2021
@erikgrinaker
Copy link
Contributor

erikgrinaker commented Jun 30, 2022

Reopening this, since it'd be nice to standardize on SSTs for bulk data transport -- we have pretty mature infrastructure to handle SSTs, while the binary Pebble batch tooling in CRDB is mostly used for Raft snapshots and is much more primitive. If we could ingest the SSTs directly it'd likely net some nice performance gains too.

@blathers-crl
Copy link

blathers-crl bot commented Jun 30, 2022

cc @cockroachdb/replication

@erikgrinaker erikgrinaker removed the T-kv KV Team label Jun 30, 2022
@blathers-crl blathers-crl bot added this to Incoming in Replication Jun 30, 2022
@erikgrinaker erikgrinaker removed the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Jun 30, 2022
Copy link

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
10 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-kv-replication Relating to Raft, consensus, and coordination. C-performance Perf of queries or internals. Solution not expected to change functional behavior. T-kv-replication KV Replication Team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants