readAheadBuffer is hard-coded to 10,000 records per partition worker. Since each aerospike.Result holds the full record bins, this can significantly increase peak memory (buffered results in pipeline + Aerospike RecordQueueSize + in-flight chunks), potentially undermining the existing chunking/memory-safety rationale (default chunkSize is 1000). Consider making the buffer size configurable and/or deriving it from s.chunkSize with a conservative cap so memory use scales predictably with settings.

Good catch — the hardcoded 10K buffer was an oversight. Now deriving it from chunkSize with a 2x multiplier capped at 10K, so memory scales predictably with the existing settings. Applied.

prunedSet.CheckAndRemove removes the parent TXID from the set on the first child/input that encounters it. A single parent transaction can have multiple outputs spent by different transactions being pruned in the same session; after the first skip, subsequent inputs referencing the same parent TXID will no longer be skipped and will still attempt the Aerospike update (likely reintroducing KEY_NOT_FOUND churn for fan-out/consolidation patterns). If the intent is to skip all updates once a parent TX is pruned, use a non-destructive lookup (or track a refcount / delayed eviction) instead of removing on first match.

Great observation — you're right that a parent TX with multiple outputs spent in the same session would only get skipped on the first input. Added a local prunedParentsSeen map that caches the result of CheckAndRemove, so all subsequent inputs referencing the same pruned parent are also skipped without hitting the shared set again.

On reflection, I've reverted this change. The CheckAndRemove behaviour is intentional — tightly chained transactions are the primary cause of the 25% wasteful Aerospike updates we observed on dev-scale, and these chains almost never have fan-out (each parent has one child spending it within the same block). The remove-on-first-check keeps the map self-managing at ~32MB peak instead of growing monotonically. For the rare fan-out case, the second child simply sends the parent update as normal — which may get KEY_NOT_FOUND, handled gracefully as before. This is a deliberate compromise that addresses 99% of what we observe in production without adding complexity for edge cases.

PrunedTxSet.Add increments count unconditionally even if the TXID is already present in the shard map. This makes Len() drift upward on duplicate adds (which can happen across retries or if the same TXID is observed twice), and CheckAndRemove will only decrement once. Update Add to only increment when inserting a new key (check _, exists := sh.m[h] under the shard lock), and add a small unit test for duplicate-add behavior to prevent regressions.

You're right — duplicate adds would cause Len() to drift upward since the count incremented unconditionally. Fixed Add to check for existence under the shard lock and only increment when inserting a new key. Also added a TestPrunedTxSet_DuplicateAdd test to prevent regressions.

prunedSet is dereferenced without a nil check (prunedSet.Add(h)). Since the signature allows prunedSet *PrunedTxSet, a nil value would panic here (while later usage in processRecordChunk is guarded). Either make prunedSet required or guard before calling Add.

Good catch -- added the nil guard around prunedSet.Add(h) to match the existing nil checks in processRecordChunk.

The reader goroutine can block forever on pipeline <- rec if the processor stops reading (e.g., ctx cancellation). Because partitionWorker waits on readerDone, this can deadlock shutdown. Make the send cancellable (e.g., select on ctx.Done() while sending) and ensure the reader can also exit promptly if the recordset stalls.

Agreed -- the bare channel send could deadlock on shutdown. Wrapped pipeline <- rec in a select with ctx.Done(), and also made the recordset.Results() recv cancellable via select so the reader can exit promptly even if the Aerospike recordset stalls.

On ctx cancellation, the processor path waits on readerDone without first ensuring the reader is unblocked from recordset.Results() or a blocked send to pipeline. This can deadlock cancellation if the Aerospike recordset is stalled or if the pipeline buffer is full. Consider creating a derived context for the worker and calling its cancel/closing the recordset before waiting, so the reader goroutine is guaranteed to exit.

Good point about the shutdown deadlock. The processor now calls recordset.Close() on ctx cancellation before waiting on readerDone, which unblocks the reader from both recordset.Results() and the pipeline send. The reader's recv is also now wrapped in a select with ctx.Done() for belt-and-suspenders safety.

Minor hot-path inefficiency: PreviousTxIDChainHash() is called twice per input (once for parentTxID, then again for CalculateKeySource). Reuse parentTxID for CalculateKeySource to avoid the extra method call/allocation in this inner loop.

Good spot — was calling PreviousTxIDChainHash() twice in the hot path unnecessarily. Now reusing the parentTxID variable for CalculateKeySource.

prunedSet is populated in the reader for every scanned record before the processor determines whether that record will actually be deleted (e.g., defensive mode can continue without deleting when a child is unstable). In defensive mode this can put TXIDs into prunedSet for records that remain in Aerospike, causing CheckAndRemove to incorrectly skip required parent updates later in the same run. Consider disabling this optimization when s.defensiveEnabled is true, or only registering TXIDs after the record has been confirmed for deletion (so the set represents “will be pruned”, not merely “was scanned”).

Really good catch — this is a genuine correctness issue. In defensive mode, the reader registers TXIDs before the processor decides whether to actually delete the record (child stability check can skip it). A skipped record stays in Aerospike but its TXID would be in the set, incorrectly suppressing a future parent update. Fixed by disabling the PrunedTxSet entirely when defensive mode is enabled. Defensive mode is about safety during reorgs, not throughput, so this is the right tradeoff.

prunedSet is allocated unconditionally, but it’s only used inside the !s.skipParentUpdates path. When SkipParentUpdates is enabled, this adds avoidable per-record overhead (hash copy + sharded map insert) and retains memory with no effect. Consider only creating/passing a PrunedTxSet when parent updates are enabled (otherwise pass nil).

Agreed — when skipParentUpdates is enabled, the set is never consulted so the per-record Add() overhead is pure waste. Now only allocating the PrunedTxSet when parent updates are enabled (and defensive mode is off). Passes nil otherwise, which the reader and processor already guard against.

Good catch — confirmed dead code. skipParentUpdates was never initialized in NewService and no setting feeds it. Removed the field and simplified the conditional to if !s.defensiveEnabled { prunedSet = NewPrunedTxSet(256) }. The comment was misleading too, so it's gone with the field.

Removed the dead skipParentUpdates field in b72cff7 — the gate at the only usage site is now simply !s.defensiveEnabled, matching the actual semantics ("only allocate the dedup set when defensive mode is off"). The field had no setter, no settings binding, and the surviving boolean expression contributed no information.

Added TestProcessRecordChunk_SkipsParentsInPrunedSet and TestProcessRecordChunk_EmptyPrunedSetIsNoOp in pruned_set_skip_test.go. The first verifies that when both parents of a child's inputs are in PrunedTxSet, processRecordChunk accumulates zero parent updates and utxo_pruner_parents_skipped_pruned_total increments once per input. The second verifies an empty set is a no-op. Both run as pure unit tests (no testcontainers) because flushCleanupBatches short-circuits on empty parentUpdates and SkipDeletions gates the deletion path.

Added TestProcessRecordChunk_SkipsParentsInPrunedSet and TestProcessRecordChunk_EmptyPrunedSetIsNoOp in stores/utxo/aerospike/pruner/pruned_set_skip_test.go (b72cff7). The first test pre-populates the shared PrunedTxSet with both parent TXIDs of a synthetic child record, drives processRecordChunk directly, and asserts that utxo_pruner_parents_skipped_pruned_total increments by exactly 2 — once per input — while the parent-update map stays empty so flushCleanupBatches never touches Aerospike. The second test confirms no metric movement when the set is empty (coinbase-style input list). Both pass under go test -race.

[Minor] Potential correctness issue with prunedSet reuse across retries

The prunedSet is allocated once and shared across all retry attempts. On retry after a timeout, the same prunedSet instance is passed to workers, but it may still contain TXIDs from successfully completed partitions or partially processed records before the timeout.

This could lead to false skips if a parent TXID remains in the set but the parent was not actually deleted due to timeout. The retry will incorrectly skip updating that parent.

Suggested fix: Create a fresh prunedSet for each retry attempt inside the retry loop, or document why reuse is safe if there is a guarantee.

Good question — worth documenting. Reuse across retries is intentional, and the correctness argument hinges on the fact that prunedSet is only ever non-nil when defensive mode is OFF. In that mode the deletedChildren bin update being skipped has no behavioural consequence (it's only consulted by the defensive safety check). Add is idempotent so re-scanning a timed-out partition is safe, and CheckAndRemove being destructive is the documented bound on the set's memory footprint. Reallocating per attempt would break the cross-partition dedup that's the whole point of the optimisation. I've extended the comment block at the allocation site to spell this out.