Files
ssshr-66andChris Lu 361fd6b263 [Filer] Parallelize Chunk Manifest Resolution to Reduce Large File Read Latency (#11215)
* fix issue-11214

* fix(filer): cancel sibling manifest reads on failure

* fix(filer): scope manifest cancellation to read batch and propagate context to encrypted reads

Address PR review comments on #11215:

- Scope cancellation to each parallel read batch instead of the resolver-wide
  context, so a later manifest failure does not cancel recursive work for an
  earlier successful manifest (CodeRabbit #3952446554).
- Propagate the resolver context through GetAuthenticatedWithContext so
  encrypted sibling reads observe cancellation and stop promptly when another
  manifest fails (Greptile #3952422343).
- Use net.ListenConfig.Listen with an explicit context in the test fixture to
  satisfy the noctx linter (CodeRabbit #3952111696).
- Add regression tests for encrypted sibling cancellation and for preserving
  earlier manifest children on later failure.

* fix(filer): return known manifest errors without blocking on earlier children

Address Greptile review comment on #11215:

After all parallel reads complete, pre-scan slots for the first real
(non-internal-cancel) error before recursing into earlier manifests'
children. If a later manifest already failed, return its error promptly
with data chunks already in hand, instead of blocking on recursive
network reads of earlier manifests' children.

Updated the regression test to verify the error returns within 1 second
when an earlier manifest's child has a 2-second delay, and that the
child is never loaded.

* fix(filer): filter partial child chunks by requested range on error path

Address CodeRabbit review comment on #11215:

The pre-scan error path appended non-manifest child chunks from earlier
manifests without applying the [startOffset, stopOffset) overlap check
used for top-level chunks. A child outside the requested range could be
returned in dataChunks alongside the later manifest's error.

Apply the same range predicate before appending. Add regression test
with an out-of-range child chunk.

* fix(filer): buffer job channel and abort submission on batch cancellation

Address Greptile review comment on #11215:

- Use a buffered job channel (capacity 128) so submission does not block
  when all workers are busy. This ensures a promptly-failing manifest is
  always queued and can cancel stalled sibling reads once a worker picks
  it up, instead of blocking the caller on the unbuffered channel send.
- Add batchCtx.Done() to the submit select so submission aborts promptly
  when the batch is already cancelled by a sibling failure.
- Add regression test with 5 manifests (4 stalled + 1 failing) verifying
  the failing job is queued and picked up after a stalled worker is freed.

* fix(filer): avoid double WaitGroup decrement on batch cancellation in submit

Address Devin review comment on #11215:

When batchCtx.Done() fired in submit, it called job.done.Done() and
returned false. The caller in resolve also called reads.Done() on the
same WaitGroup, causing a double decrement that would panic with a
negative counter.

Fix: submit sets the result error but does not decrement the WaitGroup.
The caller always owns the decrement and skips overwriting the result
when submit already set it.

* fix(filer): overflow execution when job queue buffer is full

Address Greptile follow-up review comment on #11215:

With a 128-entry buffer, if more than 132 in-range manifests (4 workers +
128 buffer) stall at one level, a promptly-failing manifest beyond the
buffer cannot be submitted and cannot cancel the stalled reads.

Fix: when the buffer is full, run the job directly in a goroutine instead
of blocking on the channel send. This only triggers for >132 manifests at
one level (exceedingly rare), so the bounded concurrency guarantee (4
workers) holds for all normal workloads. Extracted executeJob method
shared by both workers and overflow goroutines.

* fix(filer): bound overflow execution with a semaphore

Address Devin review comment on #11215:

The unbounded overflow goroutines could create thousands of concurrent
reads for large files, defeating the four-worker resource bound.

Fix: add a semaphore (capacity = maxChunkManifestResolveWorkers) that
overflow goroutines must acquire before doing the read. While waiting for
the semaphore, they also watch batchCtx and r.ctx so they exit promptly
on cancellation. Total concurrency is now bounded to 2 * workers (4
workers + 4 overflow) in the degenerate case.

* refactor(http): add ctx to GetAuthenticated signature instead of new function

Reuse the existing GetAuthenticated name by adding ctx as the first
parameter, matching the pattern of ReadUrl, ReadUrlAsStream, and
RetriedFetchChunkData. Removes the GetAuthenticatedWithContext wrapper.

---------

Co-authored-by: Chris Lu <chris.lu@gmail.com>
2026-09-07 18:44:47 -07:00
..