[ 
https://issues.apache.org/jira/browse/SOLR-18301?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Lucas Kot-Zaniewski updated SOLR-18301:
---------------------------------------
    Description: 
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.

  was:
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.

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.


> 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]

Reply via email to