Skip to content

Commit 8c90035

Browse files
committed
Optionally prepending s3 compatible storage's key with a hash of the key.
The point is to workaround S3 rate limiting. Since it is based on keys, our ULID naming scheme can lead to hotspot in the keyspace. This solution has a downside. External scripts listing files will have a their job multiplied. For this reason, the prefix cardinality is configurable. Closes #4824
1 parent e08eb9f commit 8c90035

File tree

6 files changed

+114
-19
lines changed

6 files changed

+114
-19
lines changed

quickwit/Cargo.lock

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

quickwit/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ matches = "0.1.9"
152152
md5 = "0.7"
153153
mime_guess = "2.0.4"
154154
mockall = "0.11"
155+
murmurhash32 = "0.3"
155156
mrecordlog = { git = "https://github.com/quickwit-oss/mrecordlog", rev = "306c0a7" }
156157
new_string_template = "1.5.1"
157158
nom = "7.1.3"

quickwit/quickwit-config/src/storage_config.rs

+36-1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ use itertools::Itertools;
2626
use quickwit_common::get_bool_from_env;
2727
use serde::{Deserialize, Serialize};
2828
use serde_with::{serde_as, EnumMap};
29+
use tracing::warn;
2930

3031
/// Lists the storage backends supported by Quickwit.
3132
#[derive(Debug, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash, Serialize, Deserialize)]
@@ -93,6 +94,9 @@ impl StorageConfigs {
9394
}
9495

9596
pub fn validate(&self) -> anyhow::Result<()> {
97+
for storage_config in self.0.iter() {
98+
storage_config.validate()?;
99+
}
96100
let backends: Vec<StorageBackend> = self
97101
.0
98102
.iter()
@@ -216,6 +220,14 @@ impl StorageConfig {
216220
_ => None,
217221
}
218222
}
223+
224+
pub fn validate(&self) -> anyhow::Result<()> {
225+
if let StorageConfig::S3(config) = self {
226+
config.validate()
227+
} else {
228+
Ok(())
229+
}
230+
}
219231
}
220232

221233
impl From<AzureStorageConfig> for StorageConfig {
@@ -313,6 +325,8 @@ impl fmt::Debug for AzureStorageConfig {
313325
}
314326
}
315327

328+
const MAX_S3_HASH_PREFIX_CARDINALITY: usize = 16usize.pow(3);
329+
316330
#[derive(Clone, Default, Eq, PartialEq, Serialize, Deserialize)]
317331
#[serde(deny_unknown_fields)]
318332
pub struct S3StorageConfig {
@@ -334,9 +348,30 @@ pub struct S3StorageConfig {
334348
pub disable_multi_object_delete: bool,
335349
#[serde(default)]
336350
pub disable_multipart_upload: bool,
351+
#[serde(default)]
352+
#[serde(skip_serializing_if = "lower_than_2")]
353+
pub hash_prefix_cardinality: usize,
354+
}
355+
356+
fn lower_than_2(n: &usize) -> bool {
357+
*n < 2
337358
}
338359

339360
impl S3StorageConfig {
361+
fn validate(&self) -> anyhow::Result<()> {
362+
if self.hash_prefix_cardinality == 1 {
363+
warn!("A hash prefix of 1 will be ignored");
364+
}
365+
if self.hash_prefix_cardinality > MAX_S3_HASH_PREFIX_CARDINALITY {
366+
anyhow::bail!(
367+
"hash_prefix_cardinality can take values of at most \
368+
{MAX_S3_HASH_PREFIX_CARDINALITY}, currently set to {}",
369+
self.hash_prefix_cardinality
370+
);
371+
}
372+
Ok(())
373+
}
374+
340375
fn apply_flavor(&mut self) {
341376
match self.flavor {
342377
Some(StorageBackendFlavor::DigitalOcean) => {
@@ -383,7 +418,7 @@ impl S3StorageConfig {
383418
}
384419

385420
impl fmt::Debug for S3StorageConfig {
386-
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
421+
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
387422
f.debug_struct("S3StorageConfig")
388423
.field("access_key_id", &self.access_key_id)
389424
.field(

quickwit/quickwit-storage/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ once_cell = { workspace = true }
2626
pin-project = { workspace = true }
2727
rand = { workspace = true }
2828
regex = { workspace = true }
29+
murmurhash32 = { workspace = true }
2930
serde = { workspace = true }
3031
serde_json = { workspace = true }
3132
tantivy = { workspace = true }

quickwit/quickwit-storage/src/object_storage/s3_compatible_storage.rs

+74-17
Original file line numberDiff line numberDiff line change
@@ -86,11 +86,13 @@ pub struct S3CompatibleObjectStorage {
8686
s3_client: S3Client,
8787
uri: Uri,
8888
bucket: String,
89-
prefix: PathBuf,
89+
prefix: String,
9090
multipart_policy: MultiPartPolicy,
9191
retry_params: RetryParams,
9292
disable_multi_object_delete: bool,
9393
disable_multipart_upload: bool,
94+
// If 0, we don't have any prefix
95+
hash_prefix_cardinality: usize,
9496
}
9597

9698
impl fmt::Debug for S3CompatibleObjectStorage {
@@ -99,6 +101,7 @@ impl fmt::Debug for S3CompatibleObjectStorage {
99101
.debug_struct("S3CompatibleObjectStorage")
100102
.field("bucket", &self.bucket)
101103
.field("prefix", &self.prefix)
104+
.field("hash_prefix_cardinality", &self.hash_prefix_cardinality)
102105
.finish()
103106
}
104107
}
@@ -181,19 +184,20 @@ impl S3CompatibleObjectStorage {
181184
s3_client,
182185
uri: uri.clone(),
183186
bucket,
184-
prefix,
187+
prefix: prefix.to_string_lossy().to_string(),
185188
multipart_policy: MultiPartPolicy::default(),
186189
retry_params,
187190
disable_multi_object_delete,
188191
disable_multipart_upload,
192+
hash_prefix_cardinality: s3_storage_config.hash_prefix_cardinality,
189193
})
190194
}
191195

192196
/// Sets a specific for all buckets.
193197
///
194198
/// This method overrides any existing prefix. (It does NOT
195199
/// append the argument to any existing prefix.)
196-
pub fn with_prefix(self, prefix: PathBuf) -> Self {
200+
pub fn with_prefix(self, prefix: String) -> Self {
197201
Self {
198202
s3_client: self.s3_client,
199203
uri: self.uri,
@@ -203,6 +207,7 @@ impl S3CompatibleObjectStorage {
203207
retry_params: self.retry_params,
204208
disable_multi_object_delete: self.disable_multi_object_delete,
205209
disable_multipart_upload: self.disable_multipart_upload,
210+
hash_prefix_cardinality: self.hash_prefix_cardinality,
206211
}
207212
}
208213

@@ -262,12 +267,49 @@ async fn compute_md5<T: AsyncRead + std::marker::Unpin>(mut read: T) -> io::Resu
262267
}
263268
}
264269
}
270+
const HEX_ALPHABET: [u8; 16] = *b"0123456789abcdef";
271+
const UNINITIALIZED_HASH_PREFIX: &str = "00000000";
272+
273+
fn build_key(prefix: &str, relative_path: &str, hash_prefix_cardinality: usize) -> String {
274+
let mut key = String::with_capacity(
275+
UNINITIALIZED_HASH_PREFIX.len() + 1 + prefix.len() + 1 + relative_path.len(),
276+
);
277+
if hash_prefix_cardinality > 1 {
278+
key.push_str(UNINITIALIZED_HASH_PREFIX);
279+
key.push('/');
280+
}
281+
key.push_str(prefix);
282+
if key.as_bytes().last().copied() != Some(b'/') {
283+
key.push('/');
284+
}
285+
key.push_str(relative_path);
286+
// We then set up the prefix.
287+
if hash_prefix_cardinality > 1 {
288+
let key_without_prefix = &key.as_bytes()[UNINITIALIZED_HASH_PREFIX.len() + 1..];
289+
let mut prefix_hash: usize =
290+
murmurhash32::murmurhash3(key_without_prefix) as usize % hash_prefix_cardinality;
291+
unsafe {
292+
let prefix_buf: &mut [u8] = &mut key.as_bytes_mut()[..UNINITIALIZED_HASH_PREFIX.len()];
293+
for prefix_byte in prefix_buf {
294+
let hex: u8 = HEX_ALPHABET[(prefix_hash % 16) as usize];
295+
*prefix_byte = hex;
296+
if prefix_hash < 16 {
297+
break;
298+
}
299+
prefix_hash /= 16;
300+
}
301+
}
302+
}
303+
key
304+
}
265305

266306
impl S3CompatibleObjectStorage {
267307
fn key(&self, relative_path: &Path) -> String {
268-
// FIXME: This may not work on Windows.
269-
let key_path = self.prefix.join(relative_path);
270-
key_path.to_string_lossy().to_string()
308+
build_key(
309+
&self.prefix,
310+
relative_path.to_string_lossy().as_ref(),
311+
self.hash_prefix_cardinality,
312+
)
271313
}
272314

273315
fn relative_path(&self, key: &str) -> PathBuf {
@@ -945,13 +987,13 @@ mod tests {
945987
let s3_client = S3Client::new(&sdk_config);
946988
let uri = Uri::for_test("s3://bucket/indexes");
947989
let bucket = "bucket".to_string();
948-
let prefix = PathBuf::new();
949990

950991
let mut s3_storage = S3CompatibleObjectStorage {
951992
s3_client,
952993
uri,
953994
bucket,
954-
prefix,
995+
prefix: String::new(),
996+
hash_prefix_cardinality: 0,
955997
multipart_policy: MultiPartPolicy::default(),
956998
retry_params: RetryParams::for_test(),
957999
disable_multi_object_delete: false,
@@ -962,7 +1004,7 @@ mod tests {
9621004
PathBuf::from("indexes/foo")
9631005
);
9641006

965-
s3_storage.prefix = PathBuf::from("indexes");
1007+
s3_storage.prefix = "indexes".to_string();
9661008

9671009
assert_eq!(
9681010
s3_storage.relative_path("indexes/foo"),
@@ -1000,13 +1042,13 @@ mod tests {
10001042
let s3_client = S3Client::from_conf(config);
10011043
let uri = Uri::for_test("s3://bucket/indexes");
10021044
let bucket = "bucket".to_string();
1003-
let prefix = PathBuf::new();
10041045

10051046
let s3_storage = S3CompatibleObjectStorage {
10061047
s3_client,
10071048
uri,
10081049
bucket,
1009-
prefix,
1050+
prefix: String::new(),
1051+
hash_prefix_cardinality: 0,
10101052
multipart_policy: MultiPartPolicy::default(),
10111053
retry_params: RetryParams::for_test(),
10121054
disable_multi_object_delete: true,
@@ -1041,13 +1083,13 @@ mod tests {
10411083
let s3_client = S3Client::from_conf(config);
10421084
let uri = Uri::for_test("s3://bucket/indexes");
10431085
let bucket = "bucket".to_string();
1044-
let prefix = PathBuf::new();
10451086

10461087
let s3_storage = S3CompatibleObjectStorage {
10471088
s3_client,
10481089
uri,
10491090
bucket,
1050-
prefix,
1091+
prefix: String::new(),
1092+
hash_prefix_cardinality: 0,
10511093
multipart_policy: MultiPartPolicy::default(),
10521094
retry_params: RetryParams::for_test(),
10531095
disable_multi_object_delete: false,
@@ -1123,13 +1165,13 @@ mod tests {
11231165
let s3_client = S3Client::from_conf(config);
11241166
let uri = Uri::for_test("s3://bucket/indexes");
11251167
let bucket = "bucket".to_string();
1126-
let prefix = PathBuf::new();
11271168

11281169
let s3_storage = S3CompatibleObjectStorage {
11291170
s3_client,
11301171
uri,
11311172
bucket,
1132-
prefix,
1173+
prefix: String::new(),
1174+
hash_prefix_cardinality: 0,
11331175
multipart_policy: MultiPartPolicy::default(),
11341176
retry_params: RetryParams::for_test(),
11351177
disable_multi_object_delete: false,
@@ -1216,13 +1258,13 @@ mod tests {
12161258
let s3_client = S3Client::from_conf(config);
12171259
let uri = Uri::for_test("s3://bucket/indexes");
12181260
let bucket = "bucket".to_string();
1219-
let prefix = PathBuf::new();
12201261

12211262
let s3_storage = S3CompatibleObjectStorage {
12221263
s3_client,
12231264
uri,
12241265
bucket,
1225-
prefix,
1266+
prefix: String::new(),
1267+
hash_prefix_cardinality: 0,
12261268
multipart_policy: MultiPartPolicy::default(),
12271269
retry_params: RetryParams::for_test(),
12281270
disable_multi_object_delete: false,
@@ -1233,4 +1275,19 @@ mod tests {
12331275
.await
12341276
.unwrap();
12351277
}
1278+
1279+
#[test]
1280+
fn test_build_key() {
1281+
assert_eq!(build_key("hello", "coucou", 0), "hello/coucou");
1282+
assert_eq!(build_key("hello/", "coucou", 0), "hello/coucou");
1283+
assert_eq!(build_key("hello/", "coucou", 1), "hello/coucou");
1284+
assert_eq!(build_key("hello", "coucou", 1), "hello/coucou");
1285+
assert_eq!(build_key("hello/", "coucou", 2), "10000000/hello/coucou");
1286+
assert_eq!(build_key("hello", "coucou", 2), "10000000/hello/coucou");
1287+
assert_eq!(build_key("hello/", "coucou", 16), "d0000000/hello/coucou");
1288+
assert_eq!(build_key("hello", "coucou", 16), "d0000000/hello/coucou");
1289+
assert_eq!(build_key("hello/", "coucou", 17), "50000000/hello/coucou");
1290+
assert_eq!(build_key("hello", "coucou", 17), "50000000/hello/coucou");
1291+
assert_eq!(build_key("hello/", "coucou", 70), "f0000000/hello/coucou");
1292+
}
12361293
}

quickwit/quickwit-storage/tests/s3_storage.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ pub mod s3_storage_test_suite {
6969
S3CompatibleObjectStorage::from_uri(&s3_storage_config, &storage_uri)
7070
.await
7171
.unwrap()
72-
.with_prefix(PathBuf::from("test-s3-compatible-storage"));
72+
.with_prefix("test-s3-compatible-storage".to_string());
7373

7474
quickwit_storage::storage_test_single_part_upload(&mut object_storage)
7575
.await

0 commit comments

Comments
 (0)