SOLR-16116: Use apache curator to manage the Solr Zookeeper interactions #760

HoustonPutman · 2022-03-24T17:09:55Z

https://issues.apache.org/jira/browse/SOLR-16116

This is still a WIP there are two outstanding issues:

The hadoop-auth framework requires a different version of curator
The leader election code is slightly broken, because the OnReconnect logic in ZKController expects to be called at different times, with different guarantees of watchers and ephemeral nodes than Zookeeper gives.

Might look into using curator recipes for persistent watchers and leader election, but this is a fairly complete migration to start with.

…xn is killed without an expiry

solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerRoleCmd.java

HoustonPutman · 2022-03-24T21:01:49Z

leader election code is not broken, it was that the sessionTimeout was not being populated in SolrZkClient.

risdenk · 2022-04-12T13:50:52Z

@HoustonPutman I haven't looked super closely at this, but I think that Hadoop shades curator when needed. Its possible that the Solr Hadoop classes just need to have the imports updated to use the Hadoop specific shaded curator paths. This is new with the Hadoop auth and HDFS modules in Solr when we upgraded Hadoop. I tried to switch to the Hadoop self contained shaded dependencies to avoid issues like this.

madrob · 2022-04-12T14:11:45Z

@risdenk I believe the issue currently is that for hadoop-auth we are not able to rely solely on hadoop-client-* dependencies, which are the ones that would use a relocated curator. All of the auth filter and kerberos logic and whatever else we borrow from hadoop is annotated with audience LimitedPrivate I think, so they didn't bother making a consumable client module for people to use.

HoustonPutman · 2022-04-12T14:11:54Z

Thanks for the comment @risdenk . I was talking to @madrob about it, but he hinted that there were other problems with using the shaded dependencies. However that might no longer be applicable, so I will definitely give it a shot in a week or month (things getting a bit busy haha).

dsmiley · 2023-02-19T00:05:21Z

I propose we make the loss of the reconnect boolean its own PR, plus the methods that shielded use of ZkCmdExecutor. That will make this effort here more independently reviewable and would be nice in its own right.

dsmiley · 2023-02-25T22:49:40Z

solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java

+    var clientBuilder = CuratorFrameworkFactory.builder()
+        .ensembleProvider(new FixedEnsembleProvider(zkHost))
+        .namespace(chroot)
+        .sessionTimeoutMs(zkClientTimeout)
+        .connectionTimeoutMs(clientConnectTimeout)
+        .aclProvider(this.aclProvider)
+        .authorization(zkCredentialsProvider.getCredentials())
+        .retryPolicy(retryPolicy);

+    client = clientBuilder.build();


I recently worked on an experimental hack for some days on 8x to have the Overseer use Curator for elections. One thing that I got stuck on was that Solr's tests were finding some threads lingering. Eventually I discovered that Curator has an internal executor and a way to set it on the builder runSafeService. Once I did that and tended to ensuring this executor got shot down, I didn't have that problem anymore. Just sharing this with you now in hopes it helps.

FWIW #1743 used this runSafeService stuff for the hadoop-auth changes. I think it might only come into play when there is an external zkClient provided but good to know either way.

risdenk · 2023-10-03T19:55:40Z

hadoop 3.3.6 upgraded curator to 5.2.0 - #1743. I'm working on getting that in. chatted w/ @HoustonPutman about it as well

risdenk

Overall looks great. I know it needs to be brought up to main again but 🥳

solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java

risdenk · 2023-10-04T23:31:48Z

#1743 is in main and so this PR should be ready to update :D

risdenk · 2023-10-10T13:47:24Z

I merged main (after hadoop 3.3.6/curator 5.x) and then fixed up different things after that to get all the tests passing including nightly. Would appreciate another set of eyes @HoustonPutman @dsmiley

I'll look through this again to see if things can be cleaned up more, but hopefully its progress.

risdenk · 2023-10-10T14:03:15Z

solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/BeforeReconnect.java

  public void command();
+
+  @Override
+  default void stateChanged(CuratorFramework client, ConnectionState newState) {


I really don't know if this is right. Its just like OnDisconnect which seems wrong but tests pass :/

I'm not really sure about any of these classes. It seems like OnDisconnect should do !newState.isConnected() instead of newState == ConnectionState.LOST... I have a slight memory about doing this stuff and there being reasons, but I can't remember why I did this... It definitely warrants a much more thorough look. I'm not even sure having a BeforeReconnect is a good idea. The curator APIs don't really give us a good way of implementing it.

dsmiley · 2024-03-11T12:59:19Z

AFAIK, ever since #1743 landed, we're no longer blocked on Hadoop-Auth

HoustonPutman · 2024-03-11T17:22:02Z

Yeah let's get this in main. Probably best to not introduce it in a late 9x version

risdenk · 2024-03-11T19:16:38Z

@HoustonPutman @dsmiley I'm happy to merge main into this again and resolve any conflicts. can you do another review to make sure all is good before merging to main?

dsmiley

Wow this is a doozy

dsmiley · 2024-03-11T19:57:45Z

solr/core/build.gradle

@@ -121,6 +121,16 @@ dependencies {
  implementation 'org.eclipse.jetty:jetty-io'
  implementation 'org.eclipse.jetty.toolchain:jetty-servlet-api'

+  implementation('org.apache.curator:curator-framework', {


Rather minor but I think moving this below so that we have a "ZooKeeper / Curator" section is more organized.

dsmiley · 2024-03-11T20:06:43Z

solr/core/src/java/org/apache/solr/cloud/ZkController.java

@@ -366,7 +361,6 @@ public ZkController(
            .withUrl(zkServerAddress)
            .withTimeout(clientTimeout, TimeUnit.MILLISECONDS)
            .withConnTimeOut(zkClientConnectTimeout, TimeUnit.MILLISECONDS)
-            .withConnStrategy(strat)


What happened with this?

dsmiley · 2024-03-11T20:12:05Z

solr/core/src/java/org/apache/solr/cloud/ZkController.java

-        Op.create(
-            nodePath,
-            null,
-            zkClient.getZkACLProvider().getACLsToAdd(nodePath),


I'm wondering where ACLs are handled

dsmiley · 2024-03-11T20:19:14Z

solr/core/src/java/org/apache/solr/cloud/ZkDistributedQueue.java

-              zookeeper.delete(ops.get(j).getPath(), -1, true);
-            } catch (KeeperException.NoNodeException e2) {
-              if (log.isDebugEnabled()) {
-                log.debug("Can not remove node which is not exist : {}", ops.get(j).getPath());


Did you remove because you saw too much of these?

dsmiley · 2024-03-11T20:48:54Z

solr/core/src/java/org/apache/solr/cloud/ZkShardTerms.java

@@ -398,16 +397,6 @@ private void retryRegisterWatcher() {
        return;
      } catch (KeeperException e) {
        log.warn("Failed watching shard term for collection: {}, retrying!", collection, e);
-        try {


why remove?

dsmiley · 2024-03-11T21:29:37Z

solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrZkClient.java

+  }
+
+  protected <T> T runWithCorrectThrows(String action, SupplierWithException<T> func)
+      throws KeeperException, InterruptedException {


but we catch InterruptedException now

dsmiley · 2024-03-11T21:30:21Z

solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrZkClient.java

@@ -1048,6 +1125,11 @@ public void downloadFromZK(String zkPath, Path dir) throws IOException {
    ZkMaintenanceUtils.downloadFromZK(this, zkPath, dir);
  }

+  @FunctionalInterface
+  public interface IsClosed {


Does this new interface need to be public?

dsmiley · 2024-03-11T21:34:18Z

solr/test-framework/build.gradle

@@ -43,6 +43,17 @@ dependencies {
  var zkExcludes = {
    exclude group: "org.apache.yetus", module: "audience-annotations"
  }
+  api('org.apache.curator:curator-client', {


Debatable if "api" is appropriate here

dsmiley · 2024-03-11T21:36:17Z

solr/test-framework/build.gradle

+  api('org.apache.curator:curator-framework', {
+    exclude group: 'org.apache.zookeeper', module: 'zookeeper'
+  })
+  api('org.apache.curator:curator-test') {


Thus all solr-test-framework users (plugin users outside of the project) will need curator-test. Ah; I see this is because ChaosMonkey is here and uses the KillSession thing. Okay; a little comment might be nice.

dsmiley · 2024-03-11T21:41:25Z

solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java

+      long sessionId = zkClient.getZkSessionId();
+      zkServer.expire(sessionId);
+      try {
+        KillSession.kill(zkClient.getCuratorFramework().getZookeeperClient().getZooKeeper());


didn't you add a convenience method for that? If I'm wrong; never mind.

dsmiley · 2024-04-25T18:27:18Z

This is an important initiative. If you've given up on it @risdenk, let us know so someone can take over.

risdenk · 2024-04-30T13:36:13Z

This is an important initiative. If you've given up on it @risdenk, let us know so someone can take over.

I'm not actively working on it. @HoustonPutman started this and I just tried to push it a bit further. I might get back to it but nothing immediately. Someone else can definitely push it further.

HoustonPutman · 2024-04-30T14:34:29Z

Thanks for putting in the work @risdenk ! I will push to add this for 10x only this summer.

github-actions · 2024-06-30T00:00:30Z

This PR had no visible activity in the past 60 days, labeling it as stale. Any new activity will remove the stale label. To attract more reviewers, please tag someone or notify the dev@solr.apache.org mailing list. Thank you for your contribution!

github-actions · 2024-10-05T00:00:44Z

This PR is now closed due to 60 days of inactivity after being marked as stale. Re-opening this PR is still possible, in which case it will be marked as active again.

epugh · 2024-10-05T10:48:32Z

Thanks for reopening this @dsmiley and would be great to get it in now that summer is over ;-)

HoustonPutman added 7 commits March 18, 2022 16:04

Use Curator - Step 1

b4fbc8c

It compiles

e31dea0

SolrJ Tests all pass

df93241

Fix a lot of errors

3923bf6

Everything works. Just some leader election issues when the client cn…

fbcce63

…xn is killed without an expiry

Small changes.

d3949b1

Add-back-in path start index for leader election

bb7d04a

sonatype-lift bot reviewed Mar 24, 2022

View reviewed changes

solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerRoleCmd.java Outdated Show resolved Hide resolved

Change test to do stuff correctly. Add in sessionTimeout

5137cb8

Fix unrelated issue found in PR

fc773af

dsmiley reviewed Feb 25, 2023

View reviewed changes

risdenk reviewed Oct 3, 2023

View reviewed changes

solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java Outdated Show resolved Hide resolved

risdenk added 2 commits October 7, 2023 20:53

Merge branch 'main' into curator

9cea7af

Fixes after merging main

539ca79

risdenk reviewed Oct 10, 2023

View reviewed changes

risdenk requested review from dsmiley, epugh, noblepaul, stillalex, janhoy, risdenk and magibney October 10, 2023 19:12

risdenk added 3 commits March 11, 2024 15:17

Merge branch 'main' into curator

b71f0c5

handle SOLR-16699

d3bbefa

updateLicenses

eed8450

github-actions bot added dependencies Dependency upgrades test-framework client:solrj module:hadoop-auth tool:build tests cat:search cat:cloud cat:index cat:security labels Mar 11, 2024

dsmiley reviewed Mar 11, 2024

View reviewed changes

github-actions bot removed the stale PR not updated in 60 days label Mar 16, 2024

github-actions bot added the stale PR not updated in 60 days label Jun 30, 2024

github-actions bot added the closed-stale Closed after being stale for 60 days label Oct 5, 2024

github-actions bot closed this Oct 5, 2024

dsmiley reopened this Oct 5, 2024

github-actions bot removed closed-stale Closed after being stale for 60 days stale PR not updated in 60 days labels Oct 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SOLR-16116: Use apache curator to manage the Solr Zookeeper interactions #760

SOLR-16116: Use apache curator to manage the Solr Zookeeper interactions #760

HoustonPutman commented Mar 24, 2022 •

edited by risdenk

Loading

HoustonPutman commented Mar 24, 2022

risdenk commented Apr 12, 2022

madrob commented Apr 12, 2022

HoustonPutman commented Apr 12, 2022

dsmiley commented Feb 19, 2023

dsmiley Feb 25, 2023

risdenk Oct 4, 2023

risdenk commented Oct 3, 2023

risdenk left a comment

risdenk commented Oct 4, 2023

risdenk commented Oct 10, 2023

risdenk Oct 10, 2023

HoustonPutman Oct 12, 2023

dsmiley commented Mar 11, 2024

HoustonPutman commented Mar 11, 2024

risdenk commented Mar 11, 2024

dsmiley left a comment

dsmiley Mar 11, 2024

dsmiley Mar 11, 2024

dsmiley Mar 11, 2024

dsmiley Mar 11, 2024

dsmiley Mar 11, 2024

dsmiley Mar 11, 2024

dsmiley Mar 11, 2024

dsmiley Mar 11, 2024

dsmiley Mar 11, 2024

dsmiley Mar 11, 2024

dsmiley commented Apr 25, 2024

risdenk commented Apr 30, 2024

HoustonPutman commented Apr 30, 2024

github-actions bot commented Jun 30, 2024

github-actions bot commented Oct 5, 2024

epugh commented Oct 5, 2024

SOLR-16116: Use apache curator to manage the Solr Zookeeper interactions #760

Are you sure you want to change the base?

SOLR-16116: Use apache curator to manage the Solr Zookeeper interactions #760

Conversation

HoustonPutman commented Mar 24, 2022 • edited by risdenk Loading

HoustonPutman commented Mar 24, 2022

risdenk commented Apr 12, 2022

madrob commented Apr 12, 2022

HoustonPutman commented Apr 12, 2022

dsmiley commented Feb 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

risdenk commented Oct 3, 2023

risdenk left a comment

Choose a reason for hiding this comment

risdenk commented Oct 4, 2023

risdenk commented Oct 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dsmiley commented Mar 11, 2024

HoustonPutman commented Mar 11, 2024

risdenk commented Mar 11, 2024

dsmiley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dsmiley commented Apr 25, 2024

risdenk commented Apr 30, 2024

HoustonPutman commented Apr 30, 2024

github-actions bot commented Jun 30, 2024

github-actions bot commented Oct 5, 2024

epugh commented Oct 5, 2024

HoustonPutman commented Mar 24, 2022 •

edited by risdenk

Loading