From 53a5e09f08b8c81a7d1d3370a08e922a38a2ddd1 Mon Sep 17 00:00:00 2001 From: Shrey Narayan Date: Thu, 20 Aug 2026 02:21:17 -0700 Subject: [PATCH] SOLR-18298: only run ZkController reconnect recovery after ZooKeeper session expiry. Curator RECONNECTED fires on every ZK instance hop, which made rolling ZK restarts re-elect leaders and re-register cores. Restore Solr 9 behavior by treating LOST then RECONNECTED as expiration, authored by Shrey Narayan (NextBrick). Co-authored-by: Cursor --- ...SOLR-18298-zk-reconnect-session-expiry.yml | 8 +++ .../org/apache/solr/cloud/ZkController.java | 3 + .../solr/common/cloud/OnDisconnect.java | 12 +++- .../apache/solr/common/cloud/OnReconnect.java | 27 ++++++- .../cloud/TestOnReconnectSessionExpiry.java | 70 +++++++++++++++++++ 5 files changed, 117 insertions(+), 3 deletions(-) create mode 100644 changelog/unreleased/SOLR-18298-zk-reconnect-session-expiry.yml create mode 100644 solr/solrj-zookeeper/src/test/org/apache/solr/common/cloud/TestOnReconnectSessionExpiry.java diff --git a/changelog/unreleased/SOLR-18298-zk-reconnect-session-expiry.yml b/changelog/unreleased/SOLR-18298-zk-reconnect-session-expiry.yml new file mode 100644 index 000000000000..e9f6f4527c43 --- /dev/null +++ b/changelog/unreleased/SOLR-18298-zk-reconnect-session-expiry.yml @@ -0,0 +1,8 @@ +title: ZkController no longer treats every ZooKeeper reconnect as session expiration. Re-election and core re-registration now run only after Curator reports ConnectionState.LOST, restoring Solr 9 behavior during ZK rolling restarts. +type: fixed +authors: + - name: Shrey Narayan (NextBrick) + url: https://nextbrick.com +links: + - name: SOLR-18298 + url: https://issues.apache.org/jira/browse/SOLR-18298 diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkController.java b/solr/core/src/java/org/apache/solr/cloud/ZkController.java index 3ab82e68b1d6..c94218056294 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java +++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java @@ -402,6 +402,9 @@ public ZkController( } private void onDisconnect(boolean sessionExpired) { + if (!sessionExpired) { + return; + } try { overseer.close(); } catch (Exception e) { diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/OnDisconnect.java b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/OnDisconnect.java index 9535a59cef55..b178c41c9aed 100644 --- a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/OnDisconnect.java +++ b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/OnDisconnect.java @@ -20,13 +20,21 @@ import org.apache.curator.framework.state.ConnectionState; import org.apache.curator.framework.state.ConnectionStateListener; +/** + * Listener for ZooKeeper session loss. + * + *

When registered as a Curator {@link ConnectionStateListener}, {@link #onDisconnect(boolean)} + * runs only after session expiration ({@link ConnectionState#LOST}). A transient {@link + * ConnectionState#SUSPENDED} keeps the session and must not tear down SolrCloud leadership. See + * SOLR-18298. + */ public interface OnDisconnect extends ConnectionStateListener { void onDisconnect(boolean sessionExpired); @Override default void stateChanged(CuratorFramework client, ConnectionState newState) { - if (newState == ConnectionState.LOST || newState == ConnectionState.SUSPENDED) { - onDisconnect(newState == ConnectionState.LOST); + if (newState == ConnectionState.LOST) { + onDisconnect(true); } } } diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/OnReconnect.java b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/OnReconnect.java index 8d54312d3e0f..88f95efffea7 100644 --- a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/OnReconnect.java +++ b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/OnReconnect.java @@ -16,6 +16,9 @@ */ package org.apache.solr.common.cloud; +import java.util.Collections; +import java.util.Set; +import java.util.WeakHashMap; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.state.ConnectionState; import org.apache.curator.framework.state.ConnectionStateListener; @@ -26,14 +29,36 @@ * implementation should call * org.apache.solr.cloud.ZkController#removeOnReconnectListener(OnReconnect) when it no longer needs * to be notified of ZK reconnection events. + * + *

When registered as a Curator {@link ConnectionStateListener}, {@link #onReconnect()} runs only + * after a session expiration ({@link ConnectionState#LOST} then {@link + * ConnectionState#RECONNECTED}). A reconnect after {@link ConnectionState#SUSPENDED} keeps the + * ZooKeeper session and must not trigger SolrCloud recovery. See SOLR-18298. */ public interface OnReconnect extends ConnectionStateListener { void onReconnect(); @Override default void stateChanged(CuratorFramework client, ConnectionState newState) { - if (ConnectionState.RECONNECTED.equals(newState)) { + if (newState == ConnectionState.LOST) { + LostSessions.mark(this); + } else if (newState == ConnectionState.RECONNECTED && LostSessions.consume(this)) { onReconnect(); } } + + /** Tracks listeners that have observed {@link ConnectionState#LOST} and still need reconnect. */ + final class LostSessions { + private static final Set lost = Collections.newSetFromMap(new WeakHashMap<>()); + + private LostSessions() {} + + static synchronized void mark(OnReconnect listener) { + lost.add(listener); + } + + static synchronized boolean consume(OnReconnect listener) { + return lost.remove(listener); + } + } } diff --git a/solr/solrj-zookeeper/src/test/org/apache/solr/common/cloud/TestOnReconnectSessionExpiry.java b/solr/solrj-zookeeper/src/test/org/apache/solr/common/cloud/TestOnReconnectSessionExpiry.java new file mode 100644 index 000000000000..9d6c83fa65af --- /dev/null +++ b/solr/solrj-zookeeper/src/test/org/apache/solr/common/cloud/TestOnReconnectSessionExpiry.java @@ -0,0 +1,70 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.common.cloud; + +import java.util.concurrent.atomic.AtomicInteger; +import org.apache.curator.framework.state.ConnectionState; +import org.apache.solr.SolrTestCase; +import org.junit.Test; + +/** SOLR-18298: Curator RECONNECTED is not the same as Solr 9 session-expiration reconnect. */ +public class TestOnReconnectSessionExpiry extends SolrTestCase { + + @Test + public void testReconnectAfterSuspendDoesNotFire() { + AtomicInteger reconnects = new AtomicInteger(); + OnReconnect listener = () -> reconnects.incrementAndGet(); + + listener.stateChanged(null, ConnectionState.SUSPENDED); + listener.stateChanged(null, ConnectionState.RECONNECTED); + + assertEquals(0, reconnects.get()); + } + + @Test + public void testReconnectAfterLostDoesFire() { + AtomicInteger reconnects = new AtomicInteger(); + OnReconnect listener = () -> reconnects.incrementAndGet(); + + listener.stateChanged(null, ConnectionState.LOST); + listener.stateChanged(null, ConnectionState.RECONNECTED); + + assertEquals(1, reconnects.get()); + } + + @Test + public void testReconnectWithoutPriorLostDoesNotFire() { + AtomicInteger reconnects = new AtomicInteger(); + OnReconnect listener = () -> reconnects.incrementAndGet(); + + listener.stateChanged(null, ConnectionState.RECONNECTED); + + assertEquals(0, reconnects.get()); + } + + @Test + public void testDisconnectFiresOnlyOnLost() { + AtomicInteger disconnects = new AtomicInteger(); + OnDisconnect listener = sessionExpired -> disconnects.incrementAndGet(); + + listener.stateChanged(null, ConnectionState.SUSPENDED); + assertEquals(0, disconnects.get()); + + listener.stateChanged(null, ConnectionState.LOST); + assertEquals(1, disconnects.get()); + } +}