dsmiley commented on code in PR #3345: URL: https://github.com/apache/solr/pull/3345#discussion_r2083261379
########## solr/core/src/test/org/apache/solr/cloud/ConfigSetApiLockingTest.java: ########## @@ -47,7 +47,8 @@ public void monothreadedApiLockTests() throws Exception { .build()) { ConfigSetApiLockFactory apiLockFactory = new ConfigSetApiLockFactory( - new ZkDistributedConfigSetLockFactory(zkClient, "/apiLockTestRoot")); + new CuratorDistributedConfigSetLockFactory( Review Comment: IMO it was reasonable to pass SolrZkClient; consider as a convenience constructor. This relates to my view as Curator as an implementation detail. ########## solr/core/src/java/org/apache/solr/cloud/CuratorDistributedCollectionLockFactory.java: ########## @@ -18,24 +18,44 @@ package org.apache.solr.cloud; import java.util.Objects; +import org.apache.curator.framework.CuratorFramework; import org.apache.solr.cloud.api.collections.DistributedCollectionConfigSetCommandRunner; import org.apache.solr.common.SolrException; -import org.apache.solr.common.cloud.SolrZkClient; import org.apache.solr.common.params.CollectionParams; /** - * A distributed lock implementation using Zookeeper "directory" nodes created within a collection - * znode hierarchy, for use with the distributed Collection API implementation. The locks are - * implemented using ephemeral nodes placed below the "directory" nodes. + * A distributed lock factory for Solr collections, shards, and replicas, using Apache Curator's + * {@link org.apache.curator.framework.recipes.locks.InterProcessReadWriteLock} to manage Review Comment: I don't see the usage of InterProcessReadWriteLock. ########## solr/core/src/java/org/apache/solr/cloud/CuratorDistributedConfigSetLockFactory.java: ########## @@ -18,21 +18,35 @@ package org.apache.solr.cloud; import java.util.Objects; -import org.apache.solr.common.cloud.SolrZkClient; +import org.apache.curator.framework.CuratorFramework; /** - * A distributed lock implementation using Zookeeper "directory" nodes created within a collection - * znode hierarchy, for use with the distributed Collection API implementation. The locks are - * implemented using ephemeral nodes placed below the "directory" nodes. + * A distributed lock factory for Solr config sets, using Apache Curator's {@link + * org.apache.curator.framework.recipes.locks.InterProcessReadWriteLock} to manage distributed locks + * within a flat Zookeeper-backed config set znode structure. * + * <p>This factory supports locking at the config set level only. The lock path hierarchy is flat: + * + * <pre>{@code + * rootPath/ + * configSet1/ + * configSet2/ + * ... + * }</pre> + * + * <p>Each config set has its own lock directory under the root path. This is distinct from the + * collection lock factory, which supports hierarchical locking for collections, shards, and + * replicas. Review Comment: No need to say this at all; I wouldn't. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org