All committers have signed the CLA.

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

TL;DR: the global-flush guarantee looks sound (the counter is published under the shard mutex before queue erasure, and the decrement in ~InsertData runs on every path including a schedule throw and pool finalize), but the table-specific flush keeps its old gap while this change slightly widens it.

SYSTEM FLUSH ASYNC INSERT QUEUE <table> (the flush(tables) overload) does not consult in_flight_flushes: a batch that a producer has removed from the queue but not yet admitted to the pool is invisible to it, so the statement can return while accepted rows for that table are still unflushed, and a subsequent SELECT misses them. The same window existed before for removed-and-scheduled-but-not-yet-run jobs (the in-code note "jobs scheduled before the call of 'flush' are not counted" covers it), but moving admission outside the shard lock lengthens the pre-admission phase of that window. Since the per-shard counter cannot distinguish tables, waiting on it there would over-wait — if that trade-off is why the table-specific path was left out, a short code comment on flush(tables) mirroring the existing note would make the asymmetry explicit for the next reader.

Thanks for the careful review, especially checking the scheduling-failure and pool-finalization paths.

You're right about the table-specific flush: it waits only for batches collected by that call, so an already-removed batch is outside its wait set. Moving pool admission outside the shard mutex extends that uncovered interval; the pre-existing limitation does not make that distinction irrelevant.

The new counter is shard-wide, so using it in the table-specific path would also wait for unrelated tables. I've added an explicit comment beside the table-specific wait in fcb6b877344ca52ca7ca43cdfc72efb64dfa457f, covering batches already scheduled and those still awaiting admission.

I ran fresh local checks with the preserved Debug builds and failpoints enabled (candidate 9cc5b1c6, base 58d0e8cb):

• All 8 existing regression cases passed, including the size/deadline pre-admission global-flush cases and parse-error cleanup.
• 4 targeted candidate diagnostics covered size/deadline removal, paused either before pool admission or inside the executor. In each case, table-specific flush returned with acknowledged data still pending and SELECT count() returning zero.
• 2 baseline diagnostics reproduced the same table-specific behavior with an already-admitted batch paused inside the executor, for both size and deadline triggers.

In all six diagnostics, global flush remained blocked during the pause; after release, it completed, all expected rows were visible, and pending/scheduled counters returned to zero. These diagnostic passes confirm the limitation described above.

The full upstream style job also passed 15/15 checks with the comment update. The follow-up commit changes only the comment; runtime tests used the existing 9cc5b1c6 binary. These are local results, not upstream CI results.

The pre-admission regression covers the global flush guarantee. My preference is to document the table-specific boundary in this PR and handle table-aware in-flight tracking separately, unless maintainers prefer that behavior change here.