rohityadav1993 commented on PR #19120: URL: https://github.com/apache/pinot/pull/19120#issuecomment-6249549473
@Jackie-Jiang, on review notes 1 and 2 (https://github.com/apache/pinot/pull/19120#pullrequestreview-5494854939): both are deferred to follow-up issues. **Note 1, streaming lifecycle without workers:** #19924 (item B1). - The concrete result of the mismatch (consumer-thread CPU and allocations over streamed blocks, P2e) is fixed in this PR. - A small step was considered: an interface for `start()` / `stop()`, with this combine moved onto `BaseCombineOperator`. It was not taken, because the worker phaser, futures and `getNumWorkerThreads()` stay; the coupling is only moved one level down. - The full change splits `BaseCombineOperator` into stats/termination helpers and the worker lifecycle. Every combine is touched, so a separate PR is planned. **Note 2, shared selected-row projection helper:** #19923 (item A2). - It is planned together with P2b (batch small runs, item A1), because both change the same phase-two code. - The streaming leaf will keep its own cursor progression and exhaustion, with no inheritance from `SelectionOrderByOperator`, as suggested. The other follow-ups from earlier review rounds are grouped in the same two issues, so they are tracked together under #18667. -- 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]
