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

Reply via email to