Repository: qdrant/qdrant
File: lib/collection/src/collection/snapshots.rs (Rust)

Code before:
                    ))
                })?;
            let shards_holder = self.shards_holder.read().await;

            let mut futures = Vec::new();
            // Create snapshot of each shard
            for (shard_id, replica_set) in shards_holder.get_shards() {
                let shard_snapshot_path = shard_path(Path::new(""), shard_id);

                // If node is listener, we can save whatever currently is in the storage
                let save_wal = self.shared_storage_config.node_type != NodeType::Listener;
                let future = replica_set
                    .create_snapshot(
                        snapshot_temp_temp_dir.path(),
                        tar.descend(&shard_snapshot_path)?,
                        SnapshotFormat::Regular,
                        None,
                        save_wal,
                    )
                    .await?;
                futures.push(future);
            }

            // Do not lock shards_holder for the whole duration of snapshot creation,
            // because it may take a long time, and we don't want to block other operations on the collection.
            drop(shards_holder);

            for future in futures {
                future.await.map_err(|err| {
                    CollectionError::service_error(format!("failed to create snapshot: {err}"))
                })?;
            }
        }

        // Save collection config and version
        tar.append_data(
            CollectionVersion::current_raw().as_bytes().to_vec(),
            Path::new(io::storage_version::VERSION_FILE),
        )
        .await?;

        tar.append_data(
            self.collection_config.read().await.to_bytes()?,
            Path::new(COLLECTION_CONFIG_FILE),
        )
        .await?;

        self.shards_holder
            .read()
            .await
            .save_key_mapping_to_tar(&tar)

Review comment (bug):
I'd restructure to use `{}` block instead of explicit `drop`. We had bugs due to explicit drops before.

```rust
let mut futures = Vec::new();

{
  let shards_holder = self.shards_holder.read().await;
  // ...
}
```

Code after:
                    ))
                })?;

            let mut futures = Vec::new();
            {
                let shards_holder = self.shards_holder.read().await;

                // Create snapshot of each shard
                for (shard_id, replica_set) in shards_holder.get_shards() {
                    let shard_snapshot_path = shard_path(Path::new(""), shard_id);

                    // If node is listener, we can save whatever currently is in the storage
                    let save_wal = self.shared_storage_config.node_type != NodeType::Listener;
                    let future = replica_set
                        .create_snapshot(
                            snapshot_temp_temp_dir.path(),
                            tar.descend(&shard_snapshot_path)?,
                            SnapshotFormat::Regular,
                            None,
                            save_wal,
                        )
                        .await?;
                    futures.push(future);
                }
            }

            for future in futures {
                future.await.map_err(|err| {
                    CollectionError::service_error(format!("failed to create snapshot: {err}"))
                })?;
            }
        }

        // Save collection config and version
        tar.append_data(
            CollectionVersion::current_raw().as_bytes().to_vec(),
            Path::new(io::storage_version::VERSION_FILE),
        )
        .await?;

        tar.append_data(
            self.collection_config.read().await.to_bytes()?,
            Path::new(COLLECTION_CONFIG_FILE),
        )
        .await?;

        self.shards_holder
            .read()
            .await
            .save_key_mapping_to_tar(&tar)
            .await?;