You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by no...@apache.org on 2023/03/24 05:39:03 UTC
[solr] branch jira/refactor_securityNodeWatcher updated: Moved out more methods
This is an automated email from the ASF dual-hosted git repository.
noble pushed a commit to branch jira/refactor_securityNodeWatcher
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/jira/refactor_securityNodeWatcher by this push:
new 7d948f3529d Moved out more methods
7d948f3529d is described below
commit 7d948f3529d5d27b1041adc9a13ddf354a13e5d9
Author: Noble Paul <no...@gmail.com>
AuthorDate: Fri Mar 24 16:38:53 2023 +1100
Moved out more methods
---
.../solr/common/cloud/SecurityNodeWatcher.java | 106 ++++++++++++++++++++-
.../apache/solr/common/cloud/ZkStateReader.java | 54 ++---------
2 files changed, 111 insertions(+), 49 deletions(-)
diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SecurityNodeWatcher.java b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SecurityNodeWatcher.java
index f1b00ed2db6..be95a8802b5 100644
--- a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SecurityNodeWatcher.java
+++ b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SecurityNodeWatcher.java
@@ -16,11 +16,15 @@
*/
package org.apache.solr.common.cloud;
+import static java.util.Collections.emptyMap;
+
import java.lang.invoke.MethodHandles;
import java.nio.charset.StandardCharsets;
+import java.util.Map;
import org.apache.solr.common.Callable;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.util.Pair;
+import org.apache.solr.common.util.Utils;
import org.apache.zookeeper.KeeperException;
import org.apache.zookeeper.WatchedEvent;
import org.apache.zookeeper.Watcher;
@@ -32,12 +36,23 @@ class SecurityNodeWatcher implements Watcher {
private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
private final ZkStateReader zkStateReader;
+ private ZkStateReader.ConfigData securityData;
private final Callable<Pair<byte[], Stat>> callback;
- public SecurityNodeWatcher(ZkStateReader zkStateReader, Callable<Pair<byte[], Stat>> callback) {
+ public SecurityNodeWatcher(ZkStateReader zkStateReader, Runnable securityNodeListener) {
this.zkStateReader = zkStateReader;
- this.callback = callback;
+ callback =
+ pair -> {
+ ZkStateReader.ConfigData cd = new ZkStateReader.ConfigData();
+ cd.data =
+ pair.first() == null || pair.first().length == 0
+ ? emptyMap()
+ : Utils.getDeepCopy((Map) Utils.fromJSON(pair.first()), 4, false);
+ cd.version = pair.second() == null ? -1 : pair.second().getVersion();
+ securityData = cd;
+ if (securityNodeListener != null) securityNodeListener.run();
+ };
}
@Override
@@ -79,4 +94,91 @@ class SecurityNodeWatcher implements Watcher {
log.warn("Interrupted", e);
}
}
+
+ void register() throws InterruptedException, KeeperException {
+ addSecurityNodeWatcher();
+ securityData = getSecurityProps(true);
+ }
+
+ private void addSecurityNodeWatcher() throws KeeperException, InterruptedException {
+ zkStateReader
+ .getZkClient()
+ .exists(
+ ZkStateReader.SOLR_SECURITY_CONF_PATH,
+ new Watcher() {
+
+ @Override
+ public void process(WatchedEvent event) {
+ // session events are not change events, and do not remove the watcher
+ if (Event.EventType.None.equals(event.getType())) {
+ return;
+ }
+ try {
+ synchronized (zkStateReader) {
+ log.debug("Updating [{}] ... ", ZkStateReader.SOLR_SECURITY_CONF_PATH);
+
+ // remake watch
+ final Stat stat = new Stat();
+ byte[] data = "{}".getBytes(StandardCharsets.UTF_8);
+ if (Event.EventType.NodeDeleted.equals(event.getType())) {
+ // Node deleted, just recreate watch without attempting a read - SOLR-9679
+ zkStateReader
+ .getZkClient()
+ .exists(ZkStateReader.SOLR_SECURITY_CONF_PATH, this, true);
+ } else {
+ data =
+ zkStateReader
+ .getZkClient()
+ .getData(ZkStateReader.SOLR_SECURITY_CONF_PATH, this, stat, true);
+ }
+ try {
+ callback.call(new Pair<>(data, stat));
+ } catch (Exception e) {
+ log.error("Error running collections node listener", e);
+ }
+ }
+ } catch (KeeperException.ConnectionLossException
+ | KeeperException.SessionExpiredException e) {
+ log.warn("ZooKeeper watch triggered, but Solr cannot talk to ZK: ", e);
+ } catch (KeeperException e) {
+ log.error("A ZK error has occurred", e);
+ throw new ZooKeeperException(SolrException.ErrorCode.SERVER_ERROR, "", e);
+ } catch (InterruptedException e) {
+ // Restore the interrupted status
+ Thread.currentThread().interrupt();
+ log.warn("Interrupted", e);
+ }
+ }
+ },
+ true);
+ }
+
+ @SuppressWarnings("unchecked")
+ ZkStateReader.ConfigData getSecurityProps(boolean getFresh) {
+ if (!getFresh) {
+ if (securityData == null) return new ZkStateReader.ConfigData(emptyMap(), -1);
+ return new ZkStateReader.ConfigData(securityData.data, securityData.version);
+ }
+ try {
+ Stat stat = new Stat();
+ if (zkStateReader.getZkClient().exists(ZkStateReader.SOLR_SECURITY_CONF_PATH, true)) {
+ final byte[] data =
+ zkStateReader
+ .getZkClient()
+ .getData(ZkStateReader.SOLR_SECURITY_CONF_PATH, null, stat, true);
+ return data != null && data.length > 0
+ ? new ZkStateReader.ConfigData(
+ (Map<String, Object>) Utils.fromJSON(data), stat.getVersion())
+ : null;
+ }
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR, "Error reading security properties", e);
+ } catch (KeeperException e) {
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR, "Error reading security properties", e);
+ }
+ return null;
+ }
}
diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkStateReader.java b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkStateReader.java
index 814a61f3901..3d10656717c 100644
--- a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkStateReader.java
+++ b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkStateReader.java
@@ -49,7 +49,6 @@ import java.util.stream.Collectors;
import org.apache.solr.client.solrj.impl.CloudSolrClient;
import org.apache.solr.client.solrj.impl.ZkClientClusterStateProvider;
import org.apache.solr.common.AlreadyClosedException;
-import org.apache.solr.common.Callable;
import org.apache.solr.common.SolrCloseable;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrException.ErrorCode;
@@ -58,7 +57,6 @@ import org.apache.solr.common.params.CoreAdminParams;
import org.apache.solr.common.util.CommonTestInjection;
import org.apache.solr.common.util.ExecutorUtil;
import org.apache.solr.common.util.ObjectReleaseTracker;
-import org.apache.solr.common.util.Pair;
import org.apache.solr.common.util.SolrNamedThreadFactory;
import org.apache.solr.common.util.Utils;
import org.apache.zookeeper.KeeperException;
@@ -188,10 +186,6 @@ public class ZkStateReader implements SolrCloseable {
private volatile Map<String, Object> clusterProperties = Collections.emptyMap();
- private ConfigData securityData;
-
- private final Runnable securityNodeListener;
-
/**
* Collections with active watches. The {@link StatefulCollectionWatch} inside for each collection
* might also contain the latest DocCollection (state) observed
@@ -398,6 +392,7 @@ public class ZkStateReader implements SolrCloseable {
private volatile boolean closed = false;
private Set<CountDownLatch> waitLatches = ConcurrentHashMap.newKeySet();
+ private final SecurityNodeWatcher securityNodeWatcher;
public ZkStateReader(SolrZkClient zkClient) {
this(zkClient, null);
@@ -406,7 +401,7 @@ public class ZkStateReader implements SolrCloseable {
public ZkStateReader(SolrZkClient zkClient, Runnable securityNodeListener) {
this.zkClient = zkClient;
this.closeClient = false;
- this.securityNodeListener = securityNodeListener;
+ this.securityNodeWatcher = new SecurityNodeWatcher(this, securityNodeListener);
assert ObjectReleaseTracker.track(this);
}
@@ -434,7 +429,7 @@ public class ZkStateReader implements SolrCloseable {
})
.build();
this.closeClient = true;
- this.securityNodeListener = null;
+ this.securityNodeWatcher = null;
assert ObjectReleaseTracker.track(this);
}
@@ -570,19 +565,8 @@ public class ZkStateReader implements SolrCloseable {
refreshCollectionList(new CollectionsChildWatcher());
refreshAliases(aliasesManager);
- if (securityNodeListener != null) {
- addSecurityNodeWatcher(
- pair -> {
- ConfigData cd = new ConfigData();
- cd.data =
- pair.first() == null || pair.first().length == 0
- ? emptyMap()
- : Utils.getDeepCopy((Map) Utils.fromJSON(pair.first()), 4, false);
- cd.version = pair.second() == null ? -1 : pair.second().getVersion();
- securityData = cd;
- securityNodeListener.run();
- });
- securityData = getSecurityProps(true);
+ if (securityNodeWatcher != null) {
+ securityNodeWatcher.register();
}
collectionPropsObservers.forEach(
@@ -601,11 +585,6 @@ public class ZkStateReader implements SolrCloseable {
}
}
- private void addSecurityNodeWatcher(final Callable<Pair<byte[], Stat>> callback)
- throws KeeperException, InterruptedException {
- zkClient.exists(SOLR_SECURITY_CONF_PATH, new SecurityNodeWatcher(this, callback), true);
- }
-
/**
* Construct the total state view from all sources. Must hold {@link #getUpdateLock()} before
* calling this.
@@ -1320,28 +1299,9 @@ public class ZkStateReader implements SolrCloseable {
* Returns the content of /security.json from ZooKeeper as a Map If the files doesn't exist, it
* returns null.
*/
- @SuppressWarnings("unchecked")
public ConfigData getSecurityProps(boolean getFresh) {
- if (!getFresh) {
- if (securityData == null) return new ConfigData(emptyMap(), -1);
- return new ConfigData(securityData.data, securityData.version);
- }
- try {
- Stat stat = new Stat();
- if (getZkClient().exists(SOLR_SECURITY_CONF_PATH, true)) {
- final byte[] data =
- getZkClient().getData(ZkStateReader.SOLR_SECURITY_CONF_PATH, null, stat, true);
- return data != null && data.length > 0
- ? new ConfigData((Map<String, Object>) Utils.fromJSON(data), stat.getVersion())
- : null;
- }
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- throw new SolrException(ErrorCode.SERVER_ERROR, "Error reading security properties", e);
- } catch (KeeperException e) {
- throw new SolrException(ErrorCode.SERVER_ERROR, "Error reading security properties", e);
- }
- return null;
+ if (securityNodeWatcher == null) return new ConfigData(emptyMap(), -1);
+ return securityNodeWatcher.getSecurityProps(getFresh);
}
/**