akg268 commented on PR #19251: URL: https://github.com/apache/pinot/pull/19251#issuecomment-5534169166
Thanks @Jackie-Jiang. I pushed an update addressing the review: - restored the legacy `segmentConsumed(..., String)` FSM boundary and added compatibility coverage - kept legacy `reason` on the wire while `reasonCode` remains additive for new-server/old-controller compatibility - bound both `reason` and `reasonCode` in `segmentCommitStart` - split raw query-param parsing into `withReasonCodeParam(...)`, with unknown values falling back to legacy `reason` - documented the stable numeric wire contract and froze the published IDs with literal tests - moved the immediate-winner policy back into the controller FSM Local validation: - `./mvnw -pl pinot-common -am -Dtest=SegmentCompletionProtocolTest -Dsurefire.failIfNoSpecifiedTests=false -Denforcer.skip=true test` - `./mvnw -q -pl pinot-controller -DskipTests -Denforcer.skip=true -Djdk.version=21 checkstyle:check` I also attempted the focused controller tests, but this local machine has only Java 21 and the current reactor/dependencies require Java 25 classes, so I could not get that test reactor to complete locally. CI should have the right toolchain. -- 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]
