Collector API makes read-ahead completion ordering easy to misuse

[agavra]

Problem

After the prefetch change (#383), Collector has separate fetch and ack progress. That is the right implementation for bounded read-ahead, but it makes the API easier to misuse.

Today the API still looks like this:

pub async fn next_batch(&mut self) -> Result<Option<CollectedBatch>>;
pub async fn ack(&mut self, sequence: u64) -> Result<()>;

The collector remains safe with respect to crash recovery because restart resumes from the last acked sequence, not the last fetched one. Fetched but unacked batches are replayed.

The problem is the in-process contract. Once callers can fetch ahead, completion ordering becomes an explicit part of the API, but the current interface does not help much:

• callers can fetch arbitrarily ahead unless they impose their own discipline
• callers must manage ordering themselves when work completes out of order
• ack(sequence) exposes protocol details without making the ordering rules easier to follow
• a batch-shaped ack API by itself would not really fix this unless it also addresses out-of-order completion

This is not a correctness bug in the current prefetch implementation. It is an API design issue that becomes more obvious once read-ahead exists.

Possible directions

• keep strict in-order ack() semantics and document the contract more clearly
• add collector-managed bounded read-ahead instead of leaving read-ahead behavior to the caller
• expose an API that acknowledges fetched batches rather than raw sequence numbers
• let the collector buffer out-of-order completions until earlier batches are acked
• make out-of-order completion an explicit error path with clearer ergonomics
• enforce a maximum fetch-ahead depth in the collector
• expose fetched-vs-acked depth as a metric or debug value so misuse is easier to spot

[cadonna]

@agavra Thanks for the issue!
I just wanted to add that at the moment, the cleanup of the queue (i.e. dequeuing queue entries and deleting batches from object storage) heavily depends on the in-order acks.
Also an option for exactly-once delivery using the sequence number (i.e. atomically writing the sequence number together with the ingested data) heavily depends on the in-order acks and exposing the sequence number.

Said that, I am in favor of designing a better API to avoid misuse.

[agavra]

Yeah +1 I agree we should require in order acks, it makes things much simpler to reason about. If you want concurrency then just spin up multiple queues the overhead isn't that high.

Though do note the cleanup post #382 shouldn't require in order acks because the oldest one would act as a barrier.

[cadonna]

Though do note the cleanup post #382 shouldn't require in order acks because the oldest one would act as a barrier.

Yeah, sorry! The cleanup of the batches does not need in order acks, but the dequeuing from the queue does.