Adapt stream decrypt batch sizing to fetch concurrency and memory#90
Conversation
grumbach
left a comment
There was a problem hiding this comment.
LGTM. Trace through the math:
adaptive_stream_decrypt_batch_sizereturnsclamp(min(memory_cap, max(floor, fetch_cap × 4)), 1..=total_chunks)when sysinfo can read RAM, else falls back to the configured floor. The× 4multiplier is exactly the headroom needed to keepbuffer_unordered(cap)in the per-batch closure from stalling at batch boundaries —cap = min(fetch_limiter.current(), batch_owned.len()), and withbatch_owned.len() = batch × 4the fetch side never gets clamped by the batch.usable_memory_bytes()correctly takes the min of available/free/unused/cgroup_free — the cgroup branch is the one that protects nodes-in-containers from over-budgeting. Conservative÷ 4of usable is sane.- The 6 unit tests cover every interesting branch: u64::MAX (fetch-bound), small total (total-bound),
Nonememory (no-expand), tight memory (clamp to 1), explicit memory cap. Nice. block_in_placeis safe: ant-cli runs#[tokio::main(flavor = "multi_thread", worker_threads = 4)].- Wire format unchanged — between self_encryption 0.35.0 and 0.36 the only relevant commits are
feat: configurable stream decrypt batch size(this API),fix: range-read batch sizing, and the test. No changes todata_map/encryptormodules. So the bump is API-only, not on-the-wire.
A couple of non-blocking notes:
-
Dual self_encryption versions when
--features devnetis on. ant-core gets 0.36; the optionalant-nodedev-dep still pins 0.35.0. The Cargo.toml comment ("don't add deps here or the version can skew between ant-client and ant-node") is now skewed by this PR. Default builds aren't affected — devnet feature is opt-in — but full-feature CI compiles both. Probably worth a follow-up PR to bump self_encryption in ant-node too, just to keep one copy in the tree. -
usable_memoryis sampled once at download start. If the host comes under memory pressure mid-download (think a 100GB file behind a desktop user opening Chrome), batch size won't shrink. Matches the existing AIMD design (fetch fan-out adapts inside batches viacontroller.fetch.current()), so probably fine, but if you ever see OOM reports on long downloads this is the first place to look. -
The
info!line that reports the chosen batch size is a great observability hook — please leave it at INFO, not DEBUG. Future-me debugging slow downloads will appreciate it.
Summary
Testing