This is an automated email from the ASF dual-hosted git repository.
epugh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new 16c1f81f958 Use makePath helper consistently across codebase when
writing to Zookeeper (#4636)
16c1f81f958 is described below
commit 16c1f81f9588c0d85b15ee60b1355ef59b882874
Author: Eric Pugh <[email protected]>
AuthorDate: Sat Jul 18 07:25:23 2026 -0400
Use makePath helper consistently across codebase when writing to Zookeeper
(#4636)
---
solr/core/src/java/org/apache/solr/cli/AuthTool.java | 5 +++--
.../solr/cloud/api/collections/OverseerRoleCmd.java | 7 +------
.../handler/designer/SchemaDesignerConfigSetHelper.java | 2 ++
.../org/apache/solr/packagemanager/RepositoryManager.java | 15 ++++-----------
.../java/org/apache/solr/rest/ManagedResourceStorage.java | 9 ++-------
.../org/apache/solr/schema/ManagedIndexSchemaFactory.java | 4 +---
.../java/org/apache/solr/common/cloud/SolrZkClient.java | 13 ++++++++++++-
.../org/apache/solr/common/cloud/ZkMaintenanceUtils.java | 6 +-----
8 files changed, 26 insertions(+), 35 deletions(-)
diff --git a/solr/core/src/java/org/apache/solr/cli/AuthTool.java
b/solr/core/src/java/org/apache/solr/cli/AuthTool.java
index 1e6a2985330..df0133cfead 100644
--- a/solr/core/src/java/org/apache/solr/cli/AuthTool.java
+++ b/solr/core/src/java/org/apache/solr/cli/AuthTool.java
@@ -267,7 +267,8 @@ public class AuthTool extends ToolBase {
if (!updateIncludeFileOnly) {
echoIfVerbose("Uploading following security.json: " +
securityJson);
try (SolrZkClient zkClient = CLIUtils.getSolrZkClient(cli,
zkHost)) {
- zkClient.setData("/security.json",
securityJson.getBytes(StandardCharsets.UTF_8));
+ zkClient.makePath(
+ "/security.json",
securityJson.getBytes(StandardCharsets.UTF_8), false);
}
}
@@ -368,7 +369,7 @@ public class AuthTool extends ToolBase {
echoIfVerbose("Uploading following security.json: {}");
try (SolrZkClient zkClient = CLIUtils.getSolrZkClient(cli, zkHost)) {
- zkClient.setData("/security.json",
"{}".getBytes(StandardCharsets.UTF_8));
+ zkClient.makePath("/security.json",
"{}".getBytes(StandardCharsets.UTF_8), false);
}
}
}
diff --git
a/solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerRoleCmd.java
b/solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerRoleCmd.java
index f33e317f998..f789455c0d6 100644
---
a/solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerRoleCmd.java
+++
b/solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerRoleCmd.java
@@ -33,7 +33,6 @@ import org.apache.solr.common.util.CollectionUtil;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.common.util.Utils;
import org.apache.solr.logging.DeprecationLog;
-import org.apache.zookeeper.CreateMode;
import org.apache.zookeeper.data.Stat;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -107,11 +106,7 @@ public class OverseerRoleCmd implements
CollApiCmds.CollectionApiCommand {
nodeList.remove(node);
}
- if (nodeExists) {
- zkClient.setData(ZkStateReader.ROLES, Utils.toJSON(roles));
- } else {
- zkClient.create(ZkStateReader.ROLES, Utils.toJSON(roles),
CreateMode.PERSISTENT);
- }
+ zkClient.makePath(ZkStateReader.ROLES, Utils.toJSON(roles), false);
runPrioritizer();
}
diff --git
a/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java
b/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java
index 907e0933acf..1af273cc9d0 100644
---
a/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java
+++
b/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java
@@ -918,6 +918,8 @@ class SchemaDesignerConfigSetHelper implements
SchemaDesignerConstants {
for (String path : langFilesToRestore) {
String copyToPath = path.replace(origPathDir, replacePathDir);
try {
+ // Only restore files that are missing -- do not overwrite an
existing file with the
+ // copyFrom version.
if (!zkClient.exists(copyToPath)) {
zkClient.makePath(copyToPath, false);
zkClient.setData(copyToPath, zkClient.getData(path, null, null));
diff --git
a/solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java
b/solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java
index 940bb536f16..29a86c75e0a 100644
--- a/solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java
+++ b/solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java
@@ -57,7 +57,6 @@ import org.apache.solr.packagemanager.SolrPackage.Artifact;
import org.apache.solr.packagemanager.SolrPackage.SolrPackageRelease;
import org.apache.solr.pkg.PackageAPI;
import org.apache.solr.pkg.SolrPackageLoader;
-import org.apache.zookeeper.CreateMode;
import org.apache.zookeeper.KeeperException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -123,16 +122,10 @@ public class RepositoryManager {
@SuppressWarnings({"unchecked"})
List<PackageRepository> repos =
getMapper().readValue(existingRepositoriesJson, List.class);
repos.add(new DefaultPackageRepository(repoName, uri));
- if (packageManager.zkClient.exists(PackageUtils.REPOSITORIES_ZK_PATH) ==
false) {
- packageManager.zkClient.create(
- PackageUtils.REPOSITORIES_ZK_PATH,
-
getMapper().writeValueAsString(repos).getBytes(StandardCharsets.UTF_8),
- CreateMode.PERSISTENT);
- } else {
- packageManager.zkClient.setData(
- PackageUtils.REPOSITORIES_ZK_PATH,
-
getMapper().writeValueAsString(repos).getBytes(StandardCharsets.UTF_8));
- }
+ packageManager.zkClient.makePath(
+ PackageUtils.REPOSITORIES_ZK_PATH,
+ getMapper().writeValueAsString(repos).getBytes(StandardCharsets.UTF_8),
+ false);
try (InputStream is = new URI(uri +
"/publickey.der").toURL().openStream()) {
addKey(is.readAllBytes(), repoName + ".der");
diff --git
a/solr/core/src/java/org/apache/solr/rest/ManagedResourceStorage.java
b/solr/core/src/java/org/apache/solr/rest/ManagedResourceStorage.java
index 54e6838902b..7bcf1af467a 100644
--- a/solr/core/src/java/org/apache/solr/rest/ManagedResourceStorage.java
+++ b/solr/core/src/java/org/apache/solr/rest/ManagedResourceStorage.java
@@ -308,13 +308,8 @@ public abstract class ManagedResourceStorage {
public void close() {
byte[] znodeData = toByteArray();
try {
- if (zkClient.exists(znodePath)) {
- zkClient.setData(znodePath, znodeData);
- log.info("Wrote {} bytes to existing znode {}",
znodeData.length, znodePath);
- } else {
- zkClient.makePath(znodePath, znodeData);
- log.info("Wrote {} bytes to new znode {}", znodeData.length,
znodePath);
- }
+ zkClient.makePath(znodePath, znodeData, false);
+ log.info("Wrote {} bytes to znode {}", znodeData.length,
znodePath);
} catch (Exception e) {
// have to throw a runtimer here as we're in close,
// which doesn't throw IOException
diff --git
a/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
b/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
index 83797b7c0f8..05f78633718 100644
--- a/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
+++ b/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
@@ -30,7 +30,6 @@ import org.apache.solr.cloud.ZkSolrResourceLoader;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrException.ErrorCode;
import org.apache.solr.common.cloud.SolrZkClient;
-import org.apache.solr.common.cloud.ZkMaintenanceUtils;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.IOUtils;
import org.apache.solr.common.util.NamedList;
@@ -487,8 +486,7 @@ public class ManagedIndexSchemaFactory extends
IndexSchemaFactory implements Sol
// First, copy the non-managed schema znode content to the upgraded
schema znode
byte[] bytes =
zkController.getZkClient().getData(nonManagedSchemaPath, null, null);
final String upgradedSchemaPath = nonManagedSchemaPath +
UPGRADED_SCHEMA_EXTENSION;
- ZkMaintenanceUtils.ensureExists(upgradedSchemaPath,
zkController.getZkClient());
- zkController.getZkClient().setData(upgradedSchemaPath, bytes);
+ zkController.getZkClient().makePath(upgradedSchemaPath, bytes,
false);
// Then delete the non-managed schema znode
if (zkController.getZkClient().exists(nonManagedSchemaPath)) {
try {
diff --git
a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrZkClient.java
b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrZkClient.java
index 164af46dc05..729beb65471 100644
---
a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrZkClient.java
+++
b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrZkClient.java
@@ -493,6 +493,13 @@ public class SolrZkClient implements Closeable {
makePath(path, null, CreateMode.PERSISTENT, null, failOnExists, 0);
}
+ /**
+ * Creates the path in ZooKeeper, creating each node as necessary.
+ *
+ * @param data to set on the last zkNode
+ * @param failOnExists if {@code true}, fail if the last zkNode already
exists; if {@code false},
+ * an existing zkNode's data is overwritten instead, making this a safe
create-or-update
+ */
public void makePath(String path, byte[] data, boolean failOnExists)
throws KeeperException, InterruptedException {
makePath(path, data, CreateMode.PERSISTENT, null, failOnExists, 0);
@@ -711,7 +718,11 @@ public class SolrZkClient implements Closeable {
}
}
- /** Write data to ZooKeeper. */
+ /**
+ * Write data to ZooKeeper. The node at {@code path} must already exist;
throws {@link
+ * NoNodeException} otherwise. Use {@link #makePath(String, byte[], boolean)
makePath(path, data,
+ * false)} instead if the node may not exist yet.
+ */
public Stat setData(String path, byte[] data) throws KeeperException,
InterruptedException {
return setData(path, data, -1);
}
diff --git
a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkMaintenanceUtils.java
b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkMaintenanceUtils.java
index 4efb0827413..7e3f7f80675 100644
---
a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkMaintenanceUtils.java
+++
b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkMaintenanceUtils.java
@@ -351,11 +351,7 @@ public class ZkMaintenanceUtils {
zkClient.setData(zkNode, file);
} else if (file.equals(rootPath)) {
// We are only uploading a single file, preVisitDirectory was
never called
- if (zkClient.exists(zkPath)) {
- zkClient.setData(zkPath, file);
- } else {
- zkClient.makePath(zkPath, Files.readAllBytes(file), false);
- }
+ zkClient.makePath(zkPath, Files.readAllBytes(file), false);
} else {
// Skip path parts here because they should have been created
during
// preVisitDirectory
