Skip to content

Commit 9ff14da

Browse files
authored
Fix NPE in ServerGrpcChannelBackoffResetHandler (#18139)
1 parent c9027e9 commit 9ff14da

2 files changed

Lines changed: 20 additions & 1 deletion

File tree

pinot-server/src/main/java/org/apache/pinot/server/starter/helix/ServerGrpcChannelBackoffResetHandler.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,16 @@ public ServerGrpcChannelBackoffResetHandler(HelixAdmin helixAdmin, String cluste
7777
@Override
7878
public synchronized void onInstanceConfigChange(List<InstanceConfig> instanceConfigs,
7979
NotificationContext context) {
80+
// Only process INIT (listener registration) and CALLBACK (ZK data/child change).
81+
// Ignore FINALIZE (listener unregistration) and other types.
82+
8083
// INIT: first callback when the listener is registered (full cluster snapshot).
8184
// isChildChange: an instance ZK node was added or removed under /CONFIGS/PARTICIPANT.
8285
// Both require a full scan to rebuild _shuttingDownServers from the current cluster state.
8386
NotificationContext.Type type = context.getType();
8487
if (type == NotificationContext.Type.INIT || context.getIsChildChange()) {
8588
handleFullScan();
86-
} else {
89+
} else if (type == NotificationContext.Type.CALLBACK) {
8790
// An existing instance's config changed (e.g. IS_SHUTDOWN_IN_PROGRESS toggled).
8891
// pathChanged is the ZK path of the specific instance that changed.
8992
String pathChanged = context.getPathChanged();

pinot-server/src/test/java/org/apache/pinot/server/starter/helix/ServerGrpcChannelBackoffResetHandlerTest.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,22 @@ private NotificationContext createCallbackContextForInstance(String instanceId)
229229
return context;
230230
}
231231

232+
@Test
233+
public void testFinalizeNotificationIsNoOp() {
234+
// Simulate a FINALIZE notification where pathChanged is null. The listener should ignore this.
235+
_handler.onInstanceConfigChange(Collections.emptyList(), createFinalizeContextWithNullPath());
236+
237+
verify(_mailboxService, never()).resetConnectBackoff(any(), anyInt());
238+
verify(_helixAdmin, never()).getInstancesInCluster(any());
239+
}
240+
241+
private NotificationContext createFinalizeContextWithNullPath() {
242+
NotificationContext context = new NotificationContext(_helixManager);
243+
context.setType(NotificationContext.Type.FINALIZE);
244+
// pathChanged is not set, so getPathChanged() returns null
245+
return context;
246+
}
247+
232248
private NotificationContext createInitContext() {
233249
NotificationContext context = new NotificationContext(_helixManager);
234250
context.setType(NotificationContext.Type.INIT);

0 commit comments

Comments
 (0)