Fix race between filestore compaction and message lookups (#8623)
## Summary We found this while investigating orphaned messages accumulating in production WorkQueue streams: messages remained in storage behind the consumer delivery cursor, with no pending-ACK or redelivery tracking, even while newer messages continued to be processed. Both reproduced failure paths below can create this state. Repeated occurrences leave retained messages outside normal delivery/retry processing, consuming stream capacity. Periodic compaction and block selection can run concurrently under the file-store read lock. Compaction currently publishes the last sequence of each record as it scans, temporarily shrinking the block boundary. Concurrent readers can select the wrong block. Accumulate the last sequence and its timestamp locally, and publish the completed upper bound only after the rewritten block file is installed successfully. Preserve the existing early update of the first sequence and the existing empty-block handling. Add concurrent regressions for both initial delivery and redelivery, each covering linear and binary block selection. Four readers run alongside repeated compactions of a populated block, checking every read and consumer-state result. All coordination stays in the test code; no production test hook is needed. ## Reproduced failure paths - **Initial delivery:** a read starting at 4096 returns 4097 from the next block, skipping the still-retained last message of the first block. The consumer can advance past the unread block tail. - **Redelivery:** an exact read of a retained, pending message can falsely report not found. The consumer's real redelivery path calls `processTermLocked` with reason `Unacknowledged message was deleted`, clears pending/redelivery tracking, and can advance the ACK floor. The message remains readable after compaction. This can strand isolated messages without reaching `MaxDeliver`; it emits a termination advisory rather than a max-deliveries advisory. Both regressions are included in `server/filestore_test.go`: `TestFileStoreCompactionPreservesConcurrentBlockSelection` and `TestFileStoreCompactionPreservesRedelivery`. The latter exercises the real consumer redelivery logic with synthetic file-store data and consumer state, checking the returned sequence/delivery count, pending and retry tracking, unchanged ACK floor, and continued message retention. It is not an end-to-end live-server or restart test. ## Impact The reproduced failure is not payload corruption or physical message deletion. It creates an inconsistency between retained stream messages and consumer delivery state: initial delivery can skip messages without creating pending-ACK entries, while redelivery can clear tracking for messages that still exist. For the affected consumer, these messages can remain behind the delivery cursor and outside normal delivery/retry processing. Newer messages may continue to flow, so the consumer can appear healthy while orphans accumulate and consume stream capacity. Depending on retention and limit configuration, this can eventually restrict new publications. The retained message still provides its stream sequence, publication timestamp, headers, and payload. However, current consumer state alone may no longer establish whether it was previously delivered, whether application processing completed, or whether an ACK was received. Previous delivery counts and pending-delivery timestamps may also have been discarded. Passing the ACK floor is not sufficient evidence that an orphan was successfully processed. The redelivery failure does not cancel work already running in a client. A message absent from pending tracking may still have an active worker, so replaying it can duplicate in-flight or completed work. The fix prevents the reproduced race but does not reconstruct lost tracking or resolve existing orphans. Their disposition requires separate reconciliation with application or previously captured delivery evidence; retained-message and consumer state alone may be insufficient. ## Validation - Ran both hook-free regressions against the unchanged PR base implementation with `-count=100`, covering linear and binary block selection (400 subtest executions). - Ran `go test -race ./server -run "^TestFileStoreCompactionPreserves(ConcurrentBlockSelection|Redelivery)$" -count=100 -timeout=120s` (400 subtest executions). - Ran both hook-free regressions with `-cpu=1,2,4 -count=20 -timeout=120s` (240 subtest executions). - Ran 12 existing targeted file-store tests covering compaction, boundary restoration, tombstones, and next-message scans, including applicable compression/encryption variants. - Before this test revision, ran the broader tests serially using CI-style group selectors: stores, non-clustered JetStream, consumers, Raft, all four JetStream cluster groups, superclusters, MQTT, message tracing, JWT, non-JetStream server tests, both `TestNoRace` batches, and non-server packages. These broader runs used `-p=1 -count=1 -vet=off -timeout=30m` without `-race`.
N
Neil committed
af259718fcbb46366fda0d2e0618808e66185d15
Committed by GitHub <noreply@github.com>
on 9/23/2026, 9:47:32 AM