xiangfu0 commented on code in PR #18784:
URL: https://github.com/apache/pinot/pull/18784#discussion_r3442392161
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/InstanceDataManager.java:
##########
@@ -52,7 +53,8 @@ public interface InstanceDataManager {
*/
void init(PinotConfiguration config, HelixManager helixManager,
ServerMetrics serverMetrics,
@Nullable SegmentOperationsThrottlerSet segmentOperationsThrottlerSet,
- ServerReloadJobStatusCache reloadJobStatusCache)
+ ServerReloadJobStatusCache reloadJobStatusCache,
+ ServerIngestionOomProtectionManager.ServerThrottleState
serverIngestionOomProtectionThrottleState)
Review Comment:
This widens an existing extension-point signature used by
`pinot.server.instance.data.manager.class`. Any custom `InstanceDataManager`
compiled against the current release will still load, but the first `init(...)`
call on an upgraded server will hit `AbstractMethodError` because the old 5-arg
method no longer satisfies the interface. Please preserve the old signature and
thread the throttle state through a backward-compatible overload or setter
instead of rewriting the contract in place.
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/provider/TableDataManagerProvider.java:
##########
@@ -56,6 +58,7 @@ TableDataManager getTableDataManager(TableConfig tableConfig,
@Nullable Cache<Pair<String, String>, SegmentErrorInfo> errorCache,
BooleanSupplier isServerReadyToConsumeData,
BooleanSupplier isServerReadyToServeQueries,
+ ServerIngestionOomProtectionManager.ServerThrottleState
serverIngestionOomProtectionThrottleState,
Review Comment:
Same compatibility issue here for
`pinot.server.instance.table.data.manager.provider.class`: adding the new
parameter to the interface method breaks existing custom providers at runtime.
Pinot already supports external provider implementations via config, so this
needs a compatibility shim rather than a signature rewrite.
--
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]
