cockroachdb / cockroachdb/cockroach

kvserver: decide early to apply snapshot as a write

Open
#155,377 1 comment 0 reactions 0 assignees View on GitHub
A-kv-replication branch-master C-cleanup C-enhancement C-performance T-kv
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

Snapshots can be [applied](https://github.com/cockroachdb/cockroach/blob/199a46977b974c33221c06f0fb9f29e672f2f290/pkg/kv/kvserver/replica_raftstorage.go#L641-L662) to a `Replica` storage in two ways:

1. `IngestAndExciseFiles` excises a span and applies an arbitrary number of "regular", shared and external SSTs.
2. `ConvertFilesToBatchAndCommit` is a fast-path for [small/simple](https://github.com/cockroachdb/cockroach/blob/199a46977b974c33221c06f0fb9f29e672f2f290/pkg/kv/kvserver/replica_raftstorage.go#L634-L637) snapshot ingestions (containing only "regular" SSTs and not exceeding [100 KiB](https://github.com/cockroachdb/cockroach/blob/199a46977b974c33221c06f0fb9f29e672f2f290/pkg/kv/kvserver/replica_raftstorage.go#L47)).

The decision is made shortly before applying the snapshot. The fast-path (2) needs to read back the SST files that the [MultiSSTWriter](https://github.com/cockroachdb/cockroach/blob/199a46977b974c33221c06f0fb9f29e672f2f290/pkg/kv/kvserver/kvstorage/snaprecv/multi_sst_writer.go#L24) has already written/synced to storage.

However, the information needed for this decision (the total SST size, and the number of shared/external SSTs) ~~is known way earlier at the sender~~ ([upd](https://github.com/cockroachdb/cockroach/issues/155377#issuecomment-3402938838)). If we pass this information in the snapshot header/handshake (we already have at least a proxy for that: [size](https://github.com/cockroachdb/cockroach/blob/738c039ebba5cf5bb9fc737cef43993b33a070f6/pkg/kv/kvserver/kvserverpb/raft.proto#L182-L183), [shared/external](https://github.com/cockroachdb/cockroach/blob/738c039ebba5cf5bb9fc737cef43993b33a070f6/pkg/kv/kvserver/kvserverpb/raft.proto#L197-L205)), the receiver can avoid the `MultiSSTWriter` and the conversion step (2), instead generating the batch directly. This would result in faster small snapshots.

Other benefits of this approach:

- The slightly redundant tracking of [cleared spans](https://github.com/cockroachdb/cockroach/blob/199a46977b974c33221c06f0fb9f29e672f2f290/pkg/kv/kvserver/snapshot_apply_prepare.go#L48-L50) in the snapshot preparation code can be removed.
- Eliminating `ConvertFilesToBatchAndCommit` (which is a rather ad-hoc API and can change) removes the need for this special case to leak into storage (we need to persist ingestion call parameters into WAG). This one probably can be addressed independently: convert to batch and store this batch to the WAG, instead of the call parameters.

Jira issue: CRDB-55413

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.