kotman12 commented on code in PR #4625: URL: https://github.com/apache/solr/pull/4625#discussion_r3547547913
########## solr/core/src/test/org/apache/solr/cloud/OverseerElectionReconnectTest.java: ########## @@ -0,0 +1,386 @@ +/* + * 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.cloud; + +import java.lang.invoke.MethodHandles; +import java.net.URI; +import java.nio.file.Path; +import java.util.concurrent.TimeUnit; +import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.common.cloud.SolrZkClient; +import org.apache.solr.common.util.TimeSource; +import org.apache.solr.core.CloudConfig; +import org.apache.solr.core.CoreContainer; +import org.apache.solr.util.SocketProxy; +import org.apache.solr.util.TimeOut; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Tests that overseer election recovers correctly after a ZooKeeper connection blip (SUSPENDED → + * RECONNECTED with same session). Reproduces the doom loop where the old ephemeral leader node + * still exists on reconnect, causing NodeExistsException → unbounded recursion → + * StackOverflowError. + * + * <p>Uses a SocketProxy between the Solr node and ZooKeeper so that pausing traffic triggers + * SUSPENDED without expiring the session (ZK stays running, session stays alive, ephemerals + * persist). + */ +public class OverseerElectionReconnectTest extends SolrTestCaseJ4 { + + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + + private static final String SOLRXML = "<solr></solr>"; + + /** + * Probabilistic reproduction of the single-node overseer livelock. Rapidly toggles connectivity to + * the ZK ensemble so that, on some cycle, {@code onReconnect} fires while the {@code + * OverseerExitThread} spawned by {@code onDisconnect} is mid-{@code runLeaderProcess} — landing a + * {@code close()} in the window between {@code makePath(leaderPath)} and the guarded {@code + * overseer.start()}. When that happens the leader znode is created with no overseer behind it (a + * "zombie"), every later election fails with NodeExists, and the overseer never recovers even after + * connectivity is stable. + * + * <p>Health/wedge is decided precisely: a healthy overseer has a live updater thread whose id equals + * the id stored in {@code /overseer_elect/leader}. A zombie has a leader znode whose id matches no + * running updater. + */ + @Test + public void testOverseerWedgesUnderRapidZkReconnects() throws Exception { + Path zkDir = createTempDir("zkData"); + Path ccDir = createTempDir("testOverseerRapidReconnect-solr"); + + ZkTestServer zkServer = new ZkTestServer(zkDir); + zkServer.setTheTickTime(1000); + try { + zkServer.run(); + + SocketProxy zkProxy = new SocketProxy(); + zkProxy.open(URI.create("http://127.0.0.1:"; + zkServer.getPort())); + String proxiedZkAddress = "127.0.0.1:" + zkProxy.getListenPort() + "/solr"; + log.info("ZK on port {}, proxy on port {}", zkServer.getPort(), zkProxy.getListenPort()); + + try { + int zkClientTimeout = 30000; // >> outage durations, so the session never expires + CloudConfig cloudConfig = + new CloudConfig.CloudConfigBuilder("127.0.0.1", 8984) + .setLeaderConflictResolveWait(180000) + .setLeaderVoteWait(180000) + .build(); + + CoreContainer cc = createCoreContainer(ccDir, SOLRXML); + try (ZkController zkController = + new ZkController(cc, proxiedZkAddress, zkClientTimeout, cloudConfig); + SolrZkClient probe = + new SolrZkClient.Builder() + .withUrl(zkServer.getZkAddress()) + .withTimeout(30000, TimeUnit.MILLISECONDS) + .build()) { + + assertNotNull("Overseer leader should be elected", waitForOverseerLeader(zkServer, 30)); + assertTrue( + "Overseer should be healthy before the storm", + waitForHealthyOverseer(zkController, probe, 30)); + log.info("Baseline healthy overseer established; starting reconnect storm"); + + // Sweep short outage durations so RECONNECTED lands at varying offsets relative to the + // OET's makePath->start window. Short outages keep the client reconnecting to the same + // session (no expiry) while giving onDisconnect time to spawn the OET. + int[] outageMs = {80, 110, 140, 170, 200, 240, 300, 130, 95, 180, 260, 150}; + boolean recoveredThisCycle = true; + for (int i = 0; i < 60 && recoveredThisCycle; i++) { + int outage = outageMs[i % outageMs.length]; + log.info( + "Reconnect storm cycle {}: cutting ZK connectivity for {}ms (leader={}, updater={})", + i, + outage, + OverseerTaskProcessor.getLeaderId(probe), + updaterId(zkController)); + zkProxy.close(); + Thread.sleep(outage); + zkProxy.reopen(); + // Give this cycle a bounded chance to recover; if it can't, stop hammering (don't risk + // eventually expiring the session, which would clear the zombie and mask the bug). + recoveredThisCycle = waitForHealthyOverseer(zkController, probe, 6); + if (!recoveredThisCycle) { + log.info("Overseer did not recover within 6s after cycle {} (outage {}ms)", i, outage); + } + } + + // Authoritative check: with connectivity now stable, a healthy cluster returns to a + // running overseer whose id matches /overseer_elect/leader. A zombie never does. + boolean healthy = waitForHealthyOverseer(zkController, probe, 45); + if (!healthy) { + log.error( + "WEDGED: /overseer_elect/leader id={} but no running updater matches it; updater={}", + OverseerTaskProcessor.getLeaderId(probe), + updaterId(zkController)); + } + assertTrue( + "Overseer wedged after rapid ZK reconnects (zombie leader / NodeExists spin) — see" + + " 'ZOMBIE LEADER' warning in the log", + healthy); + } finally { + cc.shutdown(); + } + } finally { + zkProxy.close(); + } + } finally { + zkServer.shutdown(); + } + } + + /** + * Reproduction of the <em>residual</em> overseer zombie race that survives PR #4577 (which stops + * {@code onReconnect}/{@code onDisconnect} from firing on same-session blips, so the first test's + * same-session storm can no longer trigger it). Here we drive <em>real session expiries</em> and + * try to make the expiry coincide with the reconnect. + * + * <p>Mechanics of the surviving race: on a genuine expiry the departing lineage's {@code + * OverseerExitThread} (spawned from {@code ClusterStateUpdater.run()}'s {@code finally}) still runs + * {@code checkIfIamStillLeader} → {@code getData(/overseer_elect/leader)}. If that read lands + * <em>after</em> the new session's {@code onExpiredReconnection} lineage has already recreated the + * leader znode, the OET does not take the {@code NoNode} early-out — it falls into its {@code + * finally} and calls {@code rejoinOverseerElection} unconditionally. That second lineage races the + * first on the shared {@code overseerElector}/{@code Overseer}, reopening the D1 window where a + * {@code close()} lands between {@code makePath(leaderPath)} and the guarded {@code + * overseer.start()} → a zombie leader znode with no updater behind it. + * + * <p>To hit it we need a short session timeout (so expiry is reachable in a test) and outages that + * <em>straddle</em> the session-timeout boundary so the client reconnects right as/after the server + * expires the session. ZooKeeper clamps the negotiated session timeout to [2*tickTime, 20*tickTime] + * and only detects expiry on tickTime-wide buckets, so tickTime must be lowered to make a ~1s + * session possible and to give fine timing granularity around the boundary. + */ + @Test + public void testOverseerWedgesOnExpiryRacingReconnect() throws Exception { Review Comment: I've added this check to show that this bug pre-dates the curator migration although it would be much harder to hit. Basically you'd need a strong coincidence of a reconnect at the same time as the session expiry. However, if you do a leader re-election on a small network blip that can _easily_ trigger both stop and start in a very tight time window. -- 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]
