Skip to content

Commit 8ad4521

Browse files
committed
mirror_worker: cap buffered entries by bytes, not just package count
commit_packages bounds only the number of buffered packages, but a single package can hold 256 entries of up to 65535 bytes (~16 MiB), so a count-only cap could buffer far past the isolate's memory ceiling. Add max_chunk_bytes (default 16 MiB) and flush early once buffered entry bytes reach it, whichever cap trips first.
1 parent 51291ee commit 8ad4521

3 files changed

Lines changed: 57 additions & 4 deletions

File tree

crates/mirror_worker/config.schema.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,12 @@
3737
"default": 32,
3838
"description": "How many entry packages add-entries verifies before flushing them to storage and advancing the persisted-entry frontier. Bounds in-memory buffering and gives durable mid-request progress on large uploads. Defaults to 32 (the recommended per-request package budget) when omitted; capped at 1024 to bound worst-case buffering."
3939
},
40+
"max_chunk_bytes": {
41+
"type": "integer",
42+
"minimum": 1,
43+
"default": 16777216,
44+
"description": "Byte ceiling on the entries buffered between flushes. add-entries flushes early once buffered entry bytes reach this many, even if fewer than commit_packages packages have accumulated. Bounds peak memory independent of package sizes, since a single package can hold up to ~16 MiB. Defaults to 16777216 (16 MiB) when omitted."
45+
},
4046
"logs": {
4147
"type": "object",
4248
"description": "CAs this mirror mirrors, keyed by log_key_name: the CA cosigner's note-signature name (the CA ID) on the checkpoints it ingests. Used as a signed-note key name at runtime, so per c2sp.org/signed-note it MUST NOT contain '+', whitespace, or control characters.",

crates/mirror_worker/config/src/lib.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,16 @@ pub struct AppConfig {
6969
/// (see [`Self::commit_packages`]). Consumed by
7070
/// [`mirror_worker`](../mirror_worker/)'s `add_entries`.
7171
pub commit_packages: Option<u64>,
72+
/// Byte ceiling on the entries buffered between flushes. `add-entries`
73+
/// flushes early once the buffered entry bytes reach this many, even if
74+
/// fewer than `commit_packages` packages have accumulated. This bounds
75+
/// peak memory independent of package sizes: `commit_packages` alone
76+
/// caps only the package *count*, and a single package can hold up to
77+
/// 256 entries of 65535 bytes (~16 MiB), so a count-only bound can
78+
/// exceed the isolate's memory ceiling. `None` falls back to a default
79+
/// (see [`Self::max_chunk_bytes`]). Consumed by
80+
/// [`mirror_worker`](../mirror_worker/)'s `add_entries`.
81+
pub max_chunk_bytes: Option<u64>,
7282
/// CAs this mirror mirrors, keyed by `log_key_name`: the CA
7383
/// cosigner's note-signature name (the CA ID) carried by the
7484
/// checkpoints it ingests.
@@ -136,6 +146,18 @@ impl AppConfig {
136146
self.commit_packages.unwrap_or(32)
137147
}
138148

149+
/// Byte ceiling on buffered entries before `add-entries` flushes early,
150+
/// falling back to 16 MiB when `max_chunk_bytes` is unset. This bounds
151+
/// peak in-memory buffering regardless of how large individual packages
152+
/// are, complementing the `commit_packages` count cap. 16 MiB matches
153+
/// the worst-case size of a single spec-maximal package (256 entries *
154+
/// 65535 bytes), so the default never flushes mid-package for a
155+
/// compliant upload yet still caps a pathological one.
156+
#[must_use]
157+
pub fn max_chunk_bytes(&self) -> u64 {
158+
self.max_chunk_bytes.unwrap_or(16 * 1024 * 1024)
159+
}
160+
139161
/// Validate the configuration beyond what `serde` and the JSON schema
140162
/// can express.
141163
///
@@ -359,6 +381,7 @@ mod tests {
359381
monitoring_prefix: None,
360382
clean_interval_secs: None,
361383
commit_packages: None,
384+
max_chunk_bytes: None,
362385
logs: HashMap::from([(
363386
"example.com/log1".to_owned(),
364387
LogParams {
@@ -491,6 +514,14 @@ mod tests {
491514
assert_eq!(cfg.commit_packages(), 8);
492515
}
493516

517+
#[test]
518+
fn max_chunk_bytes_defaults_to_16_mib() {
519+
let mut cfg = good_app_config();
520+
assert_eq!(cfg.max_chunk_bytes(), 16 * 1024 * 1024);
521+
cfg.max_chunk_bytes = Some(1024);
522+
assert_eq!(cfg.max_chunk_bytes(), 1024);
523+
}
524+
494525
#[test]
495526
fn validate_rejects_inverted_window() {
496527
let cfg = with_log(|log| {
@@ -560,6 +591,7 @@ mod tests {
560591
monitoring_prefix: None,
561592
clean_interval_secs: None,
562593
commit_packages: None,
594+
max_chunk_bytes: None,
563595
logs: HashMap::from([(
564596
"a".repeat(250),
565597
LogParams {

crates/mirror_worker/src/add_entries.rs

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,11 @@ where
370370
// config.schema.json caps commit_packages (max 1024), enforced by the
371371
// build script, so this always fits usize; the fallback is unreachable.
372372
let commit_packages = usize::try_from(crate::CONFIG.commit_packages()).unwrap_or(usize::MAX);
373+
// Byte ceiling on buffered entries; flush early when reached so peak
374+
// memory is bounded regardless of package sizes. Saturating to
375+
// usize::MAX on a 32-bit target just means "never trip the byte cap",
376+
// leaving the package-count cap in force.
377+
let max_chunk_bytes = usize::try_from(crate::CONFIG.max_chunk_bytes()).unwrap_or(usize::MAX);
373378

374379
// Entries below the request-start frontier are already persisted; new
375380
// persistence begins at this fixed boundary.
@@ -384,6 +389,7 @@ where
384389
let mut chunk: Vec<Vec<u8>> = Vec::new();
385390
let mut chunk_end = frontier_size;
386391
let mut chunk_pkgs = 0usize;
392+
let mut chunk_bytes = 0usize;
387393
let mut packages_received: u64 = 0;
388394
let mut truncated = false;
389395

@@ -441,13 +447,21 @@ where
441447
if pkg_end > initial_next {
442448
let skip = usize::try_from(initial_next.saturating_sub(pkg_start))
443449
.map_err(|_| Error::from("skip count overflows usize"))?;
444-
chunk.extend(pkg.entries.into_iter().skip(skip));
450+
let tail = pkg.entries.into_iter().skip(skip);
451+
for entry in tail {
452+
chunk_bytes = chunk_bytes.saturating_add(entry.len());
453+
chunk.push(entry);
454+
}
445455
chunk_end = pkg_end;
446456
chunk_pkgs += 1;
447457

448-
// `commit_packages >= 1` (config), so a full chunk always holds
449-
// at least one package's entries: no empty-flush guard needed.
450-
if chunk_pkgs == commit_packages {
458+
// Flush when either cap trips: `commit_packages` bounds the
459+
// package count, `max_chunk_bytes` bounds peak memory when
460+
// individual packages are large. `commit_packages >= 1`
461+
// (config), so a full chunk always holds at least one package's
462+
// entries; the byte cap only fires after entries were buffered,
463+
// so neither branch can flush an empty chunk.
464+
if chunk_pkgs == commit_packages || chunk_bytes >= max_chunk_bytes {
451465
(frontier_size, frontier_hash) = flush_chunk(
452466
bucket,
453467
env,
@@ -459,6 +473,7 @@ where
459473
)
460474
.await?;
461475
chunk_pkgs = 0;
476+
chunk_bytes = 0;
462477
}
463478
}
464479
}

0 commit comments

Comments
 (0)