Skip to content

Use CRC32C for checksums instead of MD5 #6442

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.

Sign up for GitHub

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

Open
nadav-govari wants to merge 7 commits into
base: main
Choose a base branch
from
+106 −50
Open

Use CRC32C for checksums instead of MD5 #6442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
207cf76
Use CRC32C for checksums instead of MD5
nadav-govari May 18, 2026
600281b
Only apply crc32c on non-S3 flavors
nadav-govari May 19, 2026
78d6275
lints
nadav-govari May 19, 2026
f7f6a45
Make crc32 default and a turn-off button for it
nadav-govari May 19, 2026
92d1fbf
lints
nadav-govari May 19, 2026
9f2cfdf
Make it an enum
nadav-govari May 19, 2026
ed54ce6
restore the md5 computation since its a noop on the aws sdk
nadav-govari May 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions quickwit/quickwit-config/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,9 @@ pub use crate::node_config::{
};
use crate::source_config::serialize::{SourceConfigV0_7, SourceConfigV0_8, VersionedSourceConfig};
pub use crate::storage_config::{
AzureStorageConfig, FileStorageConfig, GoogleCloudStorageConfig, RamStorageConfig,
S3StorageConfig, StorageBackend, StorageBackendFlavor, StorageConfig, StorageConfigs,
AzureStorageConfig, ChecksumStrategy, FileStorageConfig, GoogleCloudStorageConfig,
RamStorageConfig, S3StorageConfig, StorageBackend, StorageBackendFlavor, StorageConfig,
StorageConfigs,
};

/// Returns true if the ingest API v2 is enabled.
Expand Down
30 changes: 22 additions & 8 deletions quickwit/quickwit-config/src/storage_config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,20 @@ pub enum StorageBackend {
S3,
}

/// Strategy used to checksum object-storage uploads.
#[derive(Debug, Clone, Copy, Default, Eq, PartialEq, Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
pub enum ChecksumStrategy {
Comment thread
nadav-govari marked this conversation as resolved.
/// CRC32C, computed and validated by the AWS SDK. Native S3 default.
#[default]
Crc32c,
/// MD5 (Content-MD5 header), computed client-side. Used by S3-compatible
/// implementations that predate the SDK's `x-amz-checksum-*` headers.
Md5,
/// No upload checksum is sent and no response checksum is validated.
Disabled,
}

#[derive(Debug, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash, Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
pub enum StorageBackendFlavor {
Expand Down Expand Up @@ -330,7 +344,7 @@ pub struct S3StorageConfig {
#[serde(default)]
pub disable_multipart_upload: bool,
#[serde(default)]
pub disable_checksums: bool,
Copy link
Copy Markdown
Collaborator

@fulmicoton-dd fulmicoton-dd May 20, 2026

Choose a reason for hiding this comment

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

that does not look backward compatible. I think noone uses it, and they will get a clear error message, so it is probably ok if it is a calculated risk.

Copy link
Copy Markdown
Member

@guilload guilload May 20, 2026

Choose a reason for hiding this comment

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

Nadav, you can make this backward compatible with some serde trick. We can use an alias and then deserialize false to disabled and true to the new default.

pub checksum_strategy: ChecksumStrategy,
Copy link
Copy Markdown
Contributor

@trinity-1686a trinity-1686a May 20, 2026

Choose a reason for hiding this comment

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

can you also update docs/configuration/storage-config.md

#[serde(default)]
pub disable_stalled_stream_protection_upload: bool,
#[serde(default)]
Expand All @@ -343,22 +357,22 @@ impl S3StorageConfig {
Some(StorageBackendFlavor::DigitalOcean) => {
self.force_path_style_access = true;
self.disable_multi_object_delete = true;
self.disable_checksums = true;
self.checksum_strategy = ChecksumStrategy::Disabled;
}
Some(StorageBackendFlavor::Garage) => {
self.region = Some("garage".to_string());
self.force_path_style_access = true;
self.disable_checksums = true;
self.checksum_strategy = ChecksumStrategy::Disabled;
}
Some(StorageBackendFlavor::Gcs) => {
self.disable_multi_object_delete = true;
self.disable_multipart_upload = true;
self.disable_checksums = true;
self.checksum_strategy = ChecksumStrategy::Disabled;
}
Some(StorageBackendFlavor::MinIO) => {
self.region = Some("minio".to_string());
self.force_path_style_access = true;
self.disable_checksums = true;
self.checksum_strategy = ChecksumStrategy::Disabled;
Comment on lines +360 to +375
Copy link
Copy Markdown
Contributor

@trinity-1686a trinity-1686a May 20, 2026

Choose a reason for hiding this comment

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

Garage and MinIO support crc32c (source: git history), Ceph documentation mention crc32c so i assume DO has it too, and guilload said Gcs supports it to, i think we can leave the checksum strategy as default for all flavours unless we end up with reports that it causes issues

Copy link
Copy Markdown
Collaborator

@fulmicoton-dd fulmicoton-dd May 20, 2026

Choose a reason for hiding this comment

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

guilload said Gcs supports it to > I am not sure this is correct, but I think we use a specific client for GS no?

Copy link
Copy Markdown
Collaborator Author

@nadav-govari nadav-govari May 20, 2026

Choose a reason for hiding this comment

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

DO support: https://docs.digitalocean.com/reference/api/spaces/ (its supported)
To use CRC32C, you have to supply a GCP-specific header, which we won't be able to do from the S3 SDK. So I'll leave it disabled for GCS and enable it for the other two.

}
_ => {}
}
Expand Down Expand Up @@ -404,7 +418,7 @@ impl fmt::Debug for S3StorageConfig {
&self.disable_multi_object_delete,
)
.field("disable_multipart_upload", &self.disable_multipart_upload)
.field("disable_checksums", &self.disable_checksums)
.field("checksum_strategy", &self.checksum_strategy)
.field(
"disable_stalled_stream_protection_upload",
&self.disable_stalled_stream_protection_upload,
Expand Down Expand Up @@ -647,7 +661,7 @@ mod tests {
force_path_style_access: true
disable_multi_object_delete_requests: true
disable_multipart_upload: true
disable_checksums: true
checksum_strategy: disabled
disable_stalled_stream_protection_upload: true
disable_stalled_stream_protection_download: true
"#;
Expand All @@ -660,7 +674,7 @@ mod tests {
force_path_style_access: true,
disable_multi_object_delete: true,
disable_multipart_upload: true,
disable_checksums: true,
checksum_strategy: ChecksumStrategy::Disabled,
disable_stalled_stream_protection_upload: true,
disable_stalled_stream_protection_download: true,
..Default::default()
Expand Down
Loading
Loading