address review

Author: timothy-e

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

@@ -77,6 +77,9 @@ public ServerGrpcChannelBackoffResetHandler(HelixAdmin helixAdmin, String cluste
   @Override
   public synchronized void onInstanceConfigChange(List<InstanceConfig> instanceConfigs,
       NotificationContext context) {
+    // Only process INIT (listener registration) and CALLBACK (ZK data/child change).
+    // Ignore FINALIZE (listener unregistration) and other types.
+
     // INIT: first callback when the listener is registered (full cluster snapshot).
     // isChildChange: an instance ZK node was added or removed under /CONFIGS/PARTICIPANT.
     // Both require a full scan to rebuild _shuttingDownServers from the current cluster state.
@@ -87,10 +90,6 @@ public synchronized void onInstanceConfigChange(List<InstanceConfig> instanceCon
       // An existing instance's config changed (e.g. IS_SHUTDOWN_IN_PROGRESS toggled).
       // pathChanged is the ZK path of the specific instance that changed.
       String pathChanged = context.getPathChanged();
-      if (pathChanged == null) {
-        // Shouldn't happen, but be defensive.
-        return;
-      }
       String instanceName = pathChanged.substring(pathChanged.lastIndexOf('/') + 1);
       handleSingleInstanceChange(instanceName);
     }

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

@@ -230,19 +230,17 @@ private NotificationContext createCallbackContextForInstance(String instanceId)
   }
 
   @Test
-  public void testNullPathChangedIsNoOp() {
-    // Simulate a CALLBACK notification where pathChanged is null (defensive null-check).
-    // A null pathChanged is unexpected, so the handler should silently return without
-    // any interactions rather than NPE on pathChanged.substring().
-    _handler.onInstanceConfigChange(Collections.emptyList(), createCallbackContextWithNullPath());
+  public void testFinalizeNotificationIsNoOp() {
+    // Simulate a FINALIZE notification where pathChanged is null. The listener should ignore this.
+    _handler.onInstanceConfigChange(Collections.emptyList(), createFinalizeContextWithNullPath());
 
     verify(_mailboxService, never()).resetConnectBackoff(any(), anyInt());
     verify(_helixAdmin, never()).getInstancesInCluster(any());
   }
 
-  private NotificationContext createCallbackContextWithNullPath() {
+  private NotificationContext createFinalizeContextWithNullPath() {
     NotificationContext context = new NotificationContext(_helixManager);
-    context.setType(NotificationContext.Type.CALLBACK);
+    context.setType(NotificationContext.Type.FINALIZE);
     // pathChanged is not set, so getPathChanged() returns null
     return context;
   }