Re: [PR] Introduce reason codes for segment completion requests [pinot]

["akg268 (via GitHub)" <gi...@apache.org>]

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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org