Expose manifest update concurrency configuration option

[aldenks]

Closes #2273

Allow users to tune the concurrency used when updating manifests.

A couple design decisions for feedback:

• Location: on RepositoryConfig next to existing max_concurrent_requests and get_partial_values_concurrency.
  • Also considered a keyword argument on each method (commit, flush, etc), but repo config seems more consistent with existing options.
• Naming: max_concurrent_manifest_updates -- no strong opinions here
• Default value: 1, matching current behavior.
  • An alternative would be min(16, available_parallelism()). Anything higher than roughly 16 doesn't help, do_flush runs all nodes on a single tokio task, so their I/O waits are overlapped but the CPU work (decode/merge/rebuild/compress) still serializes on one core.

[paraseba]

This looks good @aldenks ! Can you reformat the code so it passes our Code Quality check?

Thank you!

[aldenks]

Thanks!

@paraseba thinking a step ahead, this PR makes the i/o concurrent, but it does not touch the concurrency of the manifest decompress, update, recompress portion which is serial on a single core at the moment. That's the current bottleneck to virtual commits after this merges and will become more of one as we move to ~400 array datasets.

I mention it now because exposing cpu parallelism for compression would mean more config options, or would interact with this config option naming here. A change now might make that future update easier. Some possibilities

• Use this one option to somehow get both -- io concurrency = 3x cpu concurrency?
• Add another option -- in which case maybe we should name this max_concurrent_manifest_update_io and expect to call the other max_concurrent_manifest_update_cpu
• Maybe there's a way to make it more "smart"

Thoughts?

[paraseba]

Thanks!

@paraseba thinking a step ahead, this PR makes the i/o concurrent, but it does not touch the concurrency of the manifest decompress, update, recompress portion which is serial on a single core at the moment. That's the current bottleneck to virtual commits after this merges and will become more of one as we move to ~400 array datasets.

I don't think that's entirely true? I don't have time right now to review the code, but iirc this number will make the full processing of manifests to be concurrent, including the CPU parts. For example, by having multiple executions of flush_existing_node running concurrently.

Am I wrong? Of course, it will still be a limiting factor because there is actual work being done in these methods, so you won't get linear speed up.

[aldenks]

I might be misremembering my profiling. I'll look into it.

[aldenks]

@paraseba the decompression of manifests is parallelized (#2164), the manifest merge/rebuild/recompress is currently CPU-bound to one core.

tl;dr, if supporting 200+ array stores w/ fast commits will at some point become a priority for icechunk, some cpu concurrency on the manifest merge+recompress path will probably become needed.

I'm not advocating for implementing that now, just want to have it in mind when naming this config option. Reasonable options in my mind

• Option 1: keep the io concurrency controlled by max_concurrent_manifest_updates. Use available_parallelism() (like 2164 does) for auto setting the cpu parallelism.
• Option 2: rename current option to max_concurrent_manifest_update_io. Later add max_concurrent_manifest_update_cpu

What do you think?

---

Some profiling results I had claude do after prototyping a more cpu concurrent implementation to make sure this was real:

Setup: 64 arrays × 150k refs each (9.6M refs); commit touches 1 ref in every array → all 64 manifests rewritten. In-memory store behind LatencyStorage (read=write delay). Manifest ref-cache disabled (forces real fetch+decode per node). AMD 5600G, 6 cores/12 threads. "cores" = process CPU-time ÷ wall.

Scenario (commit wall time)	0 ms latency	30 ms latency
pre-#2274 (concurrency 1)	5.60 s · 1.0 cores	11.01 s · 0.7 cores
#2274, c=16 (decode parallel, rest inline)	3.72 s · 1.5 cores	4.05 s · 1.4 cores
#2274, c=16 + CPU offload (merge/rebuild/encode → blocking pool)	1.41 s · 9.1 cores	1.67 s · 7.4 cores

Per-step CPU breakdown for one node's manifest update (150k refs, zstd level 3):

Step	ms/node	share of inline CPU
flatbuffer rebuild (from_sorted_vec)	29.5	39%
zstd encode (level 3)	26.0	34%
merge-iterate (decode-side iter+filter+collect)	18.8	25%
sort (input is nearly-sorted in the real path)	1.2	2%
(zstd decode — already parallel via #2164)	8.4	—

---

Appendix: where current serial comes from

1. The driver: all node futures share one tokio task — do_flush polls them with buffer_unordered(max_concurrent_nodes); sub-futures are never spawned, so any CPU between .awaits blocks every other node:

• https://github.com/earth-mover/icechunk/blob/main/icechunk/src/session.rs#L2944

2. Merge-iterate (inline) — write_manifest_with_changes decodes-iterates every old ref and filters, directly in the node future:

• https://github.com/earth-mover/icechunk/blob/main/icechunk/src/session.rs#L2545-L2576 (the fold body at L2563-L2576)

3. Sort + flatbuffer rebuild (inline) — write_manifest_from_stream sorts and runs Manifest::from_sorted_vec on the polling task:

• https://github.com/earth-mover/icechunk/blob/main/icechunk/src/session.rs#L2522-L2524

4. zstd encode (inline despite looking async) — compress_with_header runs async-compression's ZstdEncoder over an in-memory &[u8]; read_to_end(...).await over memory never yields, so the whole encode is CPU on the polling task:

• https://github.com/earth-mover/icechunk/blob/main/icechunk/src/asset_manager.rs#L1368-L1380

The contrast — decode is not serial — fetch_and_decode does zstd::decode_all + deserialize on spawn_blocking, gated at available_parallelism (#2164):

• https://github.com/earth-mover/icechunk/blob/main/icechunk/src/asset_manager.rs#L1346-L1363 (gate: L1205)

[paraseba]

Thank you so much for the careful attention to detail @aldenks , and I'm sorry it took me this long to get back to this.

I think I have a made a decision about:

• what to name this
• in which config struct to put it
• a small change on how to use the config

See my inline comments for details.

Let me know what you think.

Thank you.

[paraseba]

I'm not a fan of having this low level datastructure going all the way back to the higher level config. Let's go back to creating this datastructure with a non optional parameter, and use the config at creation/edit time.

[paraseba]

I think there is a better place for this config attribute. Let's add it as:

ManifestConfig::max_concurrent_manifest_fetches_during_commit