qdrant/qdrant

Verify snapshot integrity with checksum in shard snapshot transfer

Aperta

#3372 aperta il 10 gen 2024

 (6 commenti) (1 reazione) (0 assegnatari)Rust (2263 fork)batch import
enhancementgood first issue

Metriche repository

Star
 (31.321 stelle)
Metriche merge PR
 (Merge medio 2g 20h) (276 PR mergiate in 30 g)

Descrizione

Depends on https://github.com/qdrant/qdrant/issues/3371.

Is your feature request related to a problem? Please describe. In https://github.com/qdrant/qdrant/pull/2840 we've added checksums for snapshot files. The implementation is somewhat limited however, and requires further integration to make full use of it.

Since Qdrant 1.7 different shard transfer methods are supported. Snapshot transfers have been added to make these transfers more capable by utilizing snapshots.

One problem with this approach is that we have no integrity checks for the actual snapshot files. If such file would become corrupted, Qdrant will happily restore possibly resulting in a broken shard.

Describe the solution you'd like When a shard snapshot transfer happens, we should check integrity of the snapshot file by verifying the attached checksum. Since https://github.com/qdrant/qdrant/pull/2840, the checksum is attached to the SnapshotDescription object.

https://github.com/qdrant/qdrant/issues/3371 will implement a checksum field in snapshot recovery endpoints. We'll have to wait for this to be implemented so that we can utilize this in the snapshot transfer progress.

The right approach is probably to pass the checksum along in the recovery call here: https://github.com/qdrant/qdrant/blob/c6a351c82d57eb786b75dac785de7b085c6f80ff/lib/collection/src/shards/transfer/snapshot.rs#L215-L228

If checksum verification on the remote node fails, we should clean up the snapshot file and return with an error. Cleaning up on the remote is probably already handled with https://github.com/qdrant/qdrant/issues/3371.

Additional context There's other work to be done to properly integrate checksums, but that will be handled in different issues/PRs.

Guida contributor