xiangfu0 commented on PR #17707:
URL: https://github.com/apache/pinot/pull/17707#issuecomment-3915742809
Formatting correction for the previous note (shell escaped function names).
Addressed follow-up comments in `899b092f90`:
- Added tests for the missing edge cases:
- partition-id fallback path
(`testCommitSegmentMetadataFetchesIdealStateWhenPartitionIdsFallbackNeeded`)
- paused-table cleanup path
(`testCommitSegmentMetadataCleansUpMetadataWhenTablePaused`)
- non-CONSUMING committing segment cleanup/error path
(`testCommitSegmentMetadataCleansUpMetadataWhenCommittingSegmentNotConsuming`)
- Updated fake test manager `updateIdealStateOnSegmentCompletion(...)` to
enforce the CONSUMING precondition and pause-aware new-segment add behavior, so
tests now exercise the moved validation semantics more faithfully.
- Applied small nits from review:
- simplified redundant condition on segment movement
- reduced cleanup success log level to DEBUG
- added explicit INFO context when metadata is cleaned because segment was
not added to IdealState
I intentionally left the two broader design-tradeoff threads unresolved in
this patch (re-introducing pre-Step2 pause checks / early pre-Step1 CONSUMING
check), because both approaches require an additional large IdealState read in
the hot path and would partially negate the memory-footprint objective of this
PR. Happy to follow up with a dedicated tradeoff change if desired.
--
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]
