timothy-e opened a new pull request, #18946: URL: https://github.com/apache/pinot/pull/18946
### Summary In `BaseSingleBlockCombineOperator.getNextBlock()`, `attachExecutionStats()` was called before `stopProcess()`, meaning worker threads could still be mutating operator state (specifically `_numDocsScanned` in `SelectionOnlyOperator`, a plain non-volatile `int`) when the main thread iterated over operators to aggregate statistics. This caused `numEntriesScannedPostFilter` (derived as `_numDocsScanned * numColumnsProjected`) to diverge from `numDocsScanned` when a worker thread updated `_numDocsScanned` between the two consecutive reads of it inside `getExecutionStatistics()`. The fix restructures `getNextBlock()` so that `stopProcess()` — which already uses a `Phaser` to await all worker threads — runs in the `finally` block before `attachExecutionStats()` is called. No behavior change on the normal (non-early-termination) path. ### Test Plan Reproduced the flake by adding `@Test(invocationCount = 5000)` to `selectionOnly()` and running against the pre-fix code: failed twice with `expected [30] but found [20]` and `expected [50] but found [40]`(numEntriesScannedPostFilter ≠ numDocsScanned). The same 5000-iteration run passes cleanly with the fix applied. Safe to revert. The change only reorders when `stopProcess()` is called relative to `attachExecutionStats()` within a single query execution; it has no effect on query results or external behavior, only on the correctness of execution statistics under early termination. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
