sollhui commented on PR #65584: URL: https://github.com/apache/doris/pull/65584#issuecomment-5305602115
@liaoxin01 Thanks for the detailed review. I agree with points 2 and 3. `num_incremental_streams > 0` means that the sender has incremental destinations and is passed to both the non-incremental and incremental stream groups, so `is_incremental` and the related comments are misleading. The PR description also should not claim a separate close-before-EOS race: master already registers each stream only after `_report_result()` returns, and this patch does not establish a stronger EOS ordering. For point 1, I don't think the proposed late-`add_source()` interleaving is reachable in the current protocol. Incremental streams are opened during the write phase. `VTabletWriterV2` then sends and waits for non-incremental CLOSE_LOADs before sending any incremental CLOSE_LOAD. The fixed-total normal-stream stage acts as the cross-source barrier, so the incremental receiver's source set is stable once its CLOSE_LOAD phase starts (the empty auto-partition case also has the initial dummy-tablet stream barrier). Also, removing the latch would not fully fix the hypothetical sequence. Source A would already be drained when `cnt == total == 1`, before B and C open; supporting late sources would require an explicit sealed/final source-set state rather than only rechecking the mutable total. The latch is redundant under the current invariant and can be simplified, but it is not the cause of a reachable fencing bug. I also agree with the private/naming/typo/RAII cleanup suggestions. The cloud-mode duplication, timeout increase, and extra drain logging look optional rather than correctness blockers. -- 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]
