yashmayya commented on code in PR #18553:
URL: https://github.com/apache/pinot/pull/18553#discussion_r3290158601
##########
pinot-broker/src/main/java/org/apache/pinot/broker/broker/helix/BaseBrokerStarter.java:
##########
@@ -305,10 +305,12 @@ protected MultiStageBrokerRequestHandler
createMultiStageBrokerRequestHandler(
QueryQuotaManager queryQuotaManager, TableCache tableCache,
MultiStageQueryThrottler multiStageQueryThrottler, FailureDetector
failureDetector,
ThreadAccountant threadAccountant, MultiClusterRoutingContext
multiClusterRoutingContext,
- WorkerManager workerManager, WorkerManager multiClusterWorkerManager) {
+ WorkerManager workerManager, WorkerManager multiClusterWorkerManager,
+ ServerRoutingStatsManager serverRoutingStatsManager) {
Review Comment:
**MAJOR — backwards-incompatible signature change to a `protected` factory
method.**
`createMultiStageBrokerRequestHandler` is `protected`, which makes it a
de-facto extension point for downstream broker starters (e.g., StarTree's
commercial fork, custom plugin broker subclasses that wire instrumentation).
Adding `ServerRoutingStatsManager` as a required parameter silently breaks any
subclass that overrode the prior 13-arg signature: after upgrade, the
subclass's override is no longer invoked at the new call site (line 531).
The constructor on `MultiStageBrokerRequestHandler` itself preserved the old
13-arg form (line 154 — delegating with `null`) for exactly this kind of
backward compatibility, but this factory method did not.
Suggest: keep the 13-arg factory method (default it to delegate to the new
14-arg one with `_serverRoutingStatsManager`), update the call site at line 531
to use the 14-arg form, and let subclassers migrate at their own pace.
--
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]
