[
https://issues.apache.org/jira/browse/SOLR-18301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18094600#comment-18094600
]
Lucas Kot-Zaniewski commented on SOLR-18301:
--------------------------------------------
[~houston] I tried naively dropping the overseer close call from the
onDisconnect listener but I think the problem is that onReconnect can close the
previous Overseer as well. This kicks off the race between OverseerExitThread
and the next overseer that onReconnect wants to spawn. What do you think about
reverting to the previous behavior which is to trigger leader election on
session expiry instead of on every reconnect? There seems to be a related
[PR|https://github.com/apache/solr/pull/4577] about this. I am still not 100%
confident of that implementation yet but I think it might be the right
direction. I will raise a PR with my repro test very shortly.
All that being said I think the Overseer election code can be improved _aside_
from this. The bugs Curator migration uncovered were always there just much,
much less likely to be hit when you do this dance on session expiry (usually
larger gaps between events) instead of on reconnect (can be super quick). I
just don't know how much time to sink it to it given that there doesn't seem to
be consensus on the long-term future of Overseer in the project.
> Overseer Election May Not Converge After ZK Disconnect
> ------------------------------------------------------
>
> Key: SOLR-18301
> URL: https://issues.apache.org/jira/browse/SOLR-18301
> Project: Solr
> Issue Type: Bug
> Reporter: Lucas Kot-Zaniewski
> Priority: Blocker
> Attachments: overseer-node-election-divergence.png
>
>
> It seems the migration to curator changed when we run Overseer leader
> election. It used [to only run on session
> expiry|https://github.com/apache/solr/blob/c2091d0258400c9064b8c67fe0d974e367ecccfd/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ConnectionManager.java#L156-L200]
> (AFAICT). Now it runs on every reconnection which the current logic is not
> well-equipped to do. There are two separate issues:
> 1. OverseerElectionContext does an unsynchronized leader-node creation and
> then starts the overseer *conditionally* on it not being closed:
> {code:java}
> zkClient.makePath(leaderPath, Utils.toJSON(myProps),
> CreateMode.EPHEMERAL);
> log.info("Created overseer leader registration {} -> {}", leaderPath, id);
> /// if anything closes the overseer context while it is waiting here you
> get a zombie overseer
> synchronized (this) {
> boolean shutDown =
> overseer.getZkController().getCoreContainer().isShutDown();
> if (!this.isClosed && !shutDown) {
> overseer.start(id);
> }
> {code}
> You may wonder what would trigger this to close externally? Well there are
> actually several threads that are competing to start the overseer from a
> single node. One is the OverseerExitThread of the departing overseer but then
> there are the various callbacks registered with zookeeper that *also*
> retryElection. So when these race against one another they are liable to
> close one another and result in a stranded overseer with a leader node but no
> registered OverseerExitThread to clean it up. I have been able to recreate it
> consistently with
> [testOverseerWedgesUnderRapidZkReconnects|https://github.com/kotman12/solr/commit/9c0390bf9ac2f6370b3630a2c7c4955fc3da042c]
> There is also a tangential bug that exacerbated the first one. Overseer
> threads may actually interfere with each other even across different Solr
> nodes. The departing overseer from one node currently can delete the leader
> registration node of the next elected overseer on a different node. This can
> send the cluster into an unnecessary leader election, increasing the
> probability of hitting bug 1. Enter bug 2.
> 2. {{Overseer.ClusterStateUpdater.checkIfIamStillLeader}} version check is
> merely theatrical:
> {code:java}
> Stat stat = new Stat();
> final String path = OVERSEER_ELECT + "/leader";
> byte[] data;
> try {
> // CSU pretending to get useful stat data
> // In reality every leader node is new and
> // always has version=0
> data = zkClient.getData(path, null, stat);
> } catch (IllegalStateException | KeeperException.NoNodeException e) {
> return;
> } catch (Exception e) {
> log.warn("Error communicating with ZooKeeper", e);
> return;
> }
> try {
> Map<?, ?> m = (Map<?, ?>) Utils.fromJSON(data);
> String id = (String) m.get(ID);
> if (overseerCollectionConfigSetProcessor.getId().equals(id)) {
> try {
> overseerCollectionConfigSetProcessor.getId(),
> path,
> stat.getVersion());
> // CSU pretending to do a safe, versioned delete of the
> // overseer leader node.
> // In reality, we never call setData on this node and so
> // the version is always 0 and thus CSU is liable to delete
> // a random overseer's leader node, potentially leaving it
> // stranded as a zombie from bug 1
> zkClient.delete(path, stat.getVersion());
> {code}
> This gets triggered on every disconnection now since
> [https://github.com/apache/solr/pull/2855/changes:]
> {code:java}
> onDisconnect(SUSPENDED) → overseer.close() (ZkController:406)
> → ClusterStateUpdater.run() loop exits → finally spawns OverseerExitThread
> (Overseer:399)
> → checkIfIamStillLeader → rejoinOverseerElection →
> LeaderElector.retryElection
> → this.context.close() ← This sets OEC.isClosed=true
> (LeaderElector:377 → OverseerElectionContext:97-99)
> {code}
> I've been able to observe both of these several times in the wild already
> which sent me down this rabbit hole. I am especially confident in the
> explanation of the first defect which is the only way I can explain some of
> the behaviors I was seeing. When I saw that a particular node attached as
> overseer leader with {{n_0000000007}} but did not see {{"Overseer
> (id=...n_0000000007) starting"}} *anywhere* the only explanation is that it
> gets stuck in zombie mode on the second {{isClosed}} check. I have been able
> to verify this on multiple clouds so am confident this isn't a logging issue.
> I was initially skeptical since I did really see this happen several times
> but given the makeData call + synchronized block can potentially wait an
> "I/O-sized" amount of time I suppose it's not unlikely at all.
> Another interesting behavior is that the overseer election keeps looping,
> bumping the election nodes to sequence numbers in the *millions* while the
> actual overseer leader node is stuck on whatever generation got stuck in the
> zombie/no-man's-land state. I have attached an image showing this. The only
> thing that eventually terminates the loop of overseer election retries is a
> StackOverflowError.
> Regarding bug 2 I do wonder if we can borrow the parent-node-version-check
> pattern from ShardLeaderElectionContextBase which does this before it removes
> the shard-leader *registration* node every time cancelElection is called.
> This would appear to guarantee that we don't yank another overseer's leader
> node. The other thing I found odd is OverseerElectionContext::cancelElection
> doesn't delete the overseer's leader node even thoughit is seemingly *very*
> similar to the shard leader registration node concept (in that it is a
> single-node materialization of the election result) and that flow *does*
> delete its registration node on election cancel. I haven't figured out why
> this is.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
