capistrant commented on code in PR #19271: URL: https://github.com/apache/druid/pull/19271#discussion_r3623582826
########## docs/configuration/index.md: ########## @@ -739,7 +739,7 @@ These Coordinator static configurations can be defined in the `coordinator/runti |`druid.coordinator.kill.bufferPeriod`|The amount of time that a segment must be unused before it is able to be permanently removed from metadata and deep storage. This can serve as a buffer period to prevent data loss if data ends up being needed after being marked unused.|`P30D`| |`druid.coordinator.kill.maxSegments`|The number of unused segments to kill per kill task. This number must be greater than 0. This only applies when `druid.coordinator.kill.on=true`.|100| |`druid.coordinator.kill.maxInterval`|The largest interval, as an [ISO 8601 duration](https://en.wikipedia.org/wiki/ISO_8601#Durations), of segments to delete per kill task. Set to zero, e.g. `PT0S`, for unlimited. This only applies when `druid.coordinator.kill.on=true`.|`P30D`| - +|`druid.coordinator.server.maxConcurrentRequests`| Maximum number of concurrent requests to coordinator API endpoints (`/druid/coordinator/v1/*`, `/druid-internal/*`) that the Coordinator will process simultaneously. This prevents thread exhaustion while preserving access to health check endpoints. Set to any value <=0 like `-1` to disable quality of service filtering entirely. If not specified, defaults to `max(1, max(serverHttpNumThreads - 4, serverHttpNumThreads * 0.8))`.|`max(1, max(serverHttpNumThreads - 4, serverHttpNumThreads * 0.8))`| Review Comment: suggested re-word to more explicitly state config purpose and discourage operators from lowering value below default (this would be an exceptional circumstance IMO... more harm can be done than good in almost all cases if the value is lowered below default) ```suggestion |`druid.coordinator.server.maxConcurrentRequests`|Maximum number of requests to non-exempt Coordinator API paths processed concurrently. Requests beyond this are queued briefly and rejected with HTTP 503 if no slot frees. Leadership endpoints (`/leader`, `/isLeader`) and non-matching paths such as `/status/*` are never throttled. This bounds Jetty thread-pool exhaustion from heavy or excessive calls (for example large `/loadstatus`, `/metadata/segments`, or `/intervals` scans). Note: this path also carries recurring internal control-plane traffic (segment handoff checks, historical bootstrap, lookup and rule sync), so setting this too low can throttle that traffic and degrade the cluster; lowering the default is discouraged. Set to any value `<= 0` (for example `-1`) to disable QoS filtering entirely.|`max(1, max(serverHttpNumThreads - 4, serverHttpNumThreads * 0.8))`| ``` ########## indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisor.java: ########## @@ -1456,12 +1456,12 @@ public int registerNewVersionOfPendingSegment( // from actively reading to pending completion, etc. // This is a potential silent-loss window where data will not be queryable until handoff. log.info( - "Could not find any task matching taskAllocatorId[%s] in supervisor[%s] for upgraded pending segment[%s]" - + " (upgradedFrom[%s]); it will not be re-announced until handoff.", - taskAllocatorId, - supervisorId, - pendingSegmentRecord.getId(), - pendingSegmentRecord.getUpgradedFromSegmentId() + "Could not find any task matching taskAllocatorId[%s] in supervisor[%s] for upgraded pending segment[%s]" + + " (upgradedFrom[%s]); it will not be re-announced until handoff.", + taskAllocatorId, + supervisorId, + pendingSegmentRecord.getId(), + pendingSegmentRecord.getUpgradedFromSegmentId() Review Comment: unrelated reformatting. if it is fixing legit formatting issue I guess ok to leave, but it is unclear why this file would be being touched for this PR -- 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]
