shounakmk219 opened a new pull request, #19278:
URL: https://github.com/apache/pinot/pull/19278

   ## Summary
   
   Minion → controller segment-replace requests (`startReplaceSegments` / 
`endReplaceSegments` / segment upload) can run for a long time. In particular, 
the controller handles `endReplaceSegments` **synchronously**, blocking on 
IdealState → ExternalView convergence, which for large tables can take many 
minutes. Several of the governing timeouts were hard-coded, so the client 
socket timeout and the controller-side work time could not be aligned per 
deployment.
   
   This change exposes those knobs as configuration, all with 
backward-compatible defaults (no behavior change unless configured).
   
   ### Controller convergence wait 
(`PinotHelixResourceManager.endReplaceSegments`)
   Previously hard-coded (`// TODO: make this configurable`). Now read from 
controller config:
   
   | Config key | Default |
   |---|---|
   | `controller.segment.replace.externalViewMaxWaitMs` | 600000 (10 min) |
   | `controller.segment.replace.externalViewCheckIntervalMs` | 1000 (1 s) |
   | `controller.segment.replace.maxRetryAttempts` | 5 |
   
   The values are resolved once from `ControllerConf` (falling back to the 
existing static constants), and the retry policy wrapping `endReplaceSegments` 
is built from the configured attempt count.
   
   ### Minion client socket timeouts
   - `pinot.minion.startReplaceSegments.timeoutMs` (new, default 10 min) — 
wired through `SegmentConversionUtils.startSegmentReplace` (new 
`socketTimeoutMs` overload) and `BaseMultipleSegmentsConversionExecutor`.
   - `segmentUploadRequestTimeoutMs` task config (new, default 10 min) — read 
inside `SegmentConversionUtils.uploadSegment`, alongside the existing retry 
knobs.
   - `endReplaceSegments` timeout was already configurable 
(`pinot.minion.endReplaceSegments.timeoutMs`).
   
   ### HttpClient
   `HttpClient.sendRequest` now sets an explicit connection-request (pool 
checkout) timeout instead of silently inheriting the Apache HttpClient default, 
so a saturated pool cannot block a replace/upload request unboundedly. The TCP 
connect timeout remains tunable via the existing 
`http.client.connectionTimeoutMs`.
   
   ## Testing
   - `ControllerConfTest` — defaults and overrides for the three new controller 
keys.
   - `MinionConfTest` — defaults/overrides for the start/end replace-segments 
timeouts.
   - `mvn spotless:apply checkstyle:check license:check` clean on all touched 
modules.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
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]

Reply via email to