This is an automated email from the ASF dual-hosted git repository. dsmiley pushed a commit to branch branch_10x in repository https://gitbox.apache.org/repos/asf/solr.git
commit 03b1a5eb0a33c7d097b6f347df1d34f6c7b0369d 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) (cherry picked from commit 16c1f81f9588c0d85b15ee60b1355ef59b882874) --- 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
