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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]