You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/02/25 16:18:57 UTC

[GitHub] [solr] dsmiley commented on a change in pull request #708: SOLR-15342: Separate out a SolrJ-Zookeeper module

dsmiley commented on a change in pull request #708:
URL: https://github.com/apache/solr/pull/708#discussion_r814891991



##########
File path: solr/core/src/test/org/apache/solr/handler/TestBlobHandler.java
##########
@@ -65,7 +65,7 @@ public void doBlobHandlerTest() throws Exception {
       response1 = createCollectionRequest.process(client);
       assertEquals(0, response1.getStatus());
       assertTrue(response1.isSuccess());
-      DocCollection sysColl = ZkStateReader.from(client).getClusterState().getCollection(".system");
+      DocCollection sysColl = ZkStateReader.from(cloudClient).getClusterState().getCollection(".system");

Review comment:
       ZkStateReader isn't needed

##########
File path: solr/test-framework/src/java/org/apache/solr/cloud/CloudSolrClientUtils.java
##########
@@ -0,0 +1,134 @@
+package org.apache.solr.cloud;
+
+import org.apache.solr.client.solrj.impl.BaseCloudSolrClient;
+import org.apache.solr.client.solrj.impl.ZkClientClusterStateProvider;
+import org.apache.solr.common.cloud.*;
+
+import java.io.Serializable;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.function.Predicate;
+
+public class CloudSolrClientUtils implements Serializable {
+    /**
+     * @return the zkHost value used to connect to zookeeper.
+     */
+    public static String getZkHost(BaseCloudSolrClient client) {
+        return assertZKStateProvider(client).zkHost;
+    }
+
+    /**
+     * Set the connect timeout to the zookeeper ensemble in ms
+     */
+    public static void setZkConnectTimeout(BaseCloudSolrClient client, int zkConnectTimeout) {
+        assertZKStateProvider(client).zkConnectTimeout = zkConnectTimeout;
+    }
+
+    /**
+     * Set the timeout to the zookeeper ensemble in ms
+     */
+    public static void setZkClientTimeout(BaseCloudSolrClient client, int zkClientTimeout) {
+        assertZKStateProvider(client).zkClientTimeout = zkClientTimeout;
+    }
+
+    static ZkClientClusterStateProvider assertZKStateProvider(BaseCloudSolrClient client) {
+        if (client.getClusterStateProvider() instanceof ZkClientClusterStateProvider) {
+            return (ZkClientClusterStateProvider) client.getClusterStateProvider();
+        }
+        throw new IllegalArgumentException("This client does not use ZK");
+
+    }
+
+    /**
+     * Block until a CollectionStatePredicate returns true, or the wait times out
+     *
+     * <p>
+     * Note that the predicate may be called again even after it has returned true, so
+     * implementors should avoid changing state within the predicate call itself.
+     * </p>
+     *
+     * <p>
+     * This implementation utilizes {@link CollectionStateWatcher} internally.
+     * Callers that don't care about liveNodes are encouraged to use a {@link DocCollection} {@link Predicate}
+     * instead
+     * </p>
+     *
+     * @param collection the collection to watch
+     * @param wait       how long to wait
+     * @param unit       the units of the wait parameter
+     * @param predicate  a {@link CollectionStatePredicate} to check the collection state
+     * @throws InterruptedException on interrupt
+     * @throws TimeoutException     on timeout
+     * @see #waitForState(BaseCloudSolrClient, String, long, TimeUnit, Predicate)
+     * @see #registerCollectionStateWatcher
+     */
+    public static void waitForState(BaseCloudSolrClient client, String collection, long wait, TimeUnit unit, CollectionStatePredicate predicate)
+            throws InterruptedException, TimeoutException {
+        client.getClusterStateProvider().connect();
+        assertZKStateProvider(client).getZkStateReader().waitForState(collection, wait, unit, predicate);
+    }
+
+    /**
+     * Block until a Predicate returns true, or the wait times out
+     *
+     * <p>
+     * Note that the predicate may be called again even after it has returned true, so
+     * implementors should avoid changing state within the predicate call itself.
+     * </p>
+     *
+     * @param collection the collection to watch
+     * @param wait       how long to wait
+     * @param unit       the units of the wait parameter
+     * @param predicate  a {@link Predicate} to test against the {@link DocCollection}
+     * @throws InterruptedException on interrupt
+     * @throws TimeoutException     on timeout
+     * @see #registerDocCollectionWatcher
+     */
+    public static void waitForState(BaseCloudSolrClient client, String collection, long wait, TimeUnit unit, Predicate<DocCollection> predicate)

Review comment:
       same as I said above

##########
File path: solr/test-framework/src/java/org/apache/solr/cloud/CloudSolrClientUtils.java
##########
@@ -0,0 +1,134 @@
+package org.apache.solr.cloud;
+
+import org.apache.solr.client.solrj.impl.BaseCloudSolrClient;
+import org.apache.solr.client.solrj.impl.ZkClientClusterStateProvider;
+import org.apache.solr.common.cloud.*;
+
+import java.io.Serializable;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.function.Predicate;
+
+public class CloudSolrClientUtils implements Serializable {
+    /**
+     * @return the zkHost value used to connect to zookeeper.
+     */
+    public static String getZkHost(BaseCloudSolrClient client) {
+        return assertZKStateProvider(client).zkHost;
+    }
+
+    /**
+     * Set the connect timeout to the zookeeper ensemble in ms
+     */
+    public static void setZkConnectTimeout(BaseCloudSolrClient client, int zkConnectTimeout) {
+        assertZKStateProvider(client).zkConnectTimeout = zkConnectTimeout;
+    }
+
+    /**
+     * Set the timeout to the zookeeper ensemble in ms
+     */
+    public static void setZkClientTimeout(BaseCloudSolrClient client, int zkClientTimeout) {
+        assertZKStateProvider(client).zkClientTimeout = zkClientTimeout;
+    }
+
+    static ZkClientClusterStateProvider assertZKStateProvider(BaseCloudSolrClient client) {
+        if (client.getClusterStateProvider() instanceof ZkClientClusterStateProvider) {
+            return (ZkClientClusterStateProvider) client.getClusterStateProvider();
+        }
+        throw new IllegalArgumentException("This client does not use ZK");
+
+    }
+
+    /**
+     * Block until a CollectionStatePredicate returns true, or the wait times out
+     *
+     * <p>
+     * Note that the predicate may be called again even after it has returned true, so
+     * implementors should avoid changing state within the predicate call itself.
+     * </p>
+     *
+     * <p>
+     * This implementation utilizes {@link CollectionStateWatcher} internally.
+     * Callers that don't care about liveNodes are encouraged to use a {@link DocCollection} {@link Predicate}
+     * instead
+     * </p>
+     *
+     * @param collection the collection to watch
+     * @param wait       how long to wait
+     * @param unit       the units of the wait parameter
+     * @param predicate  a {@link CollectionStatePredicate} to check the collection state
+     * @throws InterruptedException on interrupt
+     * @throws TimeoutException     on timeout
+     * @see #waitForState(BaseCloudSolrClient, String, long, TimeUnit, Predicate)
+     * @see #registerCollectionStateWatcher
+     */
+    public static void waitForState(BaseCloudSolrClient client, String collection, long wait, TimeUnit unit, CollectionStatePredicate predicate)

Review comment:
       Instead, I think we should make the caller call ZkStateReader.from(client).waitForState 
   If you change the method implementation here to do that, you can then tell IntelliJ to *inline* this method, and it'll do all the work for you.
   Ensure waitForState over there has this same great javadocs.

##########
File path: solr/test-framework/src/java/org/apache/solr/cloud/CloudSolrClientUtils.java
##########
@@ -0,0 +1,134 @@
+package org.apache.solr.cloud;
+
+import org.apache.solr.client.solrj.impl.BaseCloudSolrClient;
+import org.apache.solr.client.solrj.impl.ZkClientClusterStateProvider;
+import org.apache.solr.common.cloud.*;
+
+import java.io.Serializable;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.function.Predicate;
+
+public class CloudSolrClientUtils implements Serializable {
+    /**
+     * @return the zkHost value used to connect to zookeeper.
+     */
+    public static String getZkHost(BaseCloudSolrClient client) {
+        return assertZKStateProvider(client).zkHost;
+    }
+
+    /**
+     * Set the connect timeout to the zookeeper ensemble in ms
+     */
+    public static void setZkConnectTimeout(BaseCloudSolrClient client, int zkConnectTimeout) {
+        assertZKStateProvider(client).zkConnectTimeout = zkConnectTimeout;
+    }
+
+    /**
+     * Set the timeout to the zookeeper ensemble in ms
+     */
+    public static void setZkClientTimeout(BaseCloudSolrClient client, int zkClientTimeout) {
+        assertZKStateProvider(client).zkClientTimeout = zkClientTimeout;
+    }
+
+    static ZkClientClusterStateProvider assertZKStateProvider(BaseCloudSolrClient client) {
+        if (client.getClusterStateProvider() instanceof ZkClientClusterStateProvider) {
+            return (ZkClientClusterStateProvider) client.getClusterStateProvider();
+        }
+        throw new IllegalArgumentException("This client does not use ZK");
+
+    }
+
+    /**
+     * Block until a CollectionStatePredicate returns true, or the wait times out
+     *
+     * <p>
+     * Note that the predicate may be called again even after it has returned true, so
+     * implementors should avoid changing state within the predicate call itself.
+     * </p>
+     *
+     * <p>
+     * This implementation utilizes {@link CollectionStateWatcher} internally.
+     * Callers that don't care about liveNodes are encouraged to use a {@link DocCollection} {@link Predicate}
+     * instead
+     * </p>
+     *
+     * @param collection the collection to watch
+     * @param wait       how long to wait
+     * @param unit       the units of the wait parameter
+     * @param predicate  a {@link CollectionStatePredicate} to check the collection state
+     * @throws InterruptedException on interrupt
+     * @throws TimeoutException     on timeout
+     * @see #waitForState(BaseCloudSolrClient, String, long, TimeUnit, Predicate)
+     * @see #registerCollectionStateWatcher
+     */
+    public static void waitForState(BaseCloudSolrClient client, String collection, long wait, TimeUnit unit, CollectionStatePredicate predicate)
+            throws InterruptedException, TimeoutException {
+        client.getClusterStateProvider().connect();
+        assertZKStateProvider(client).getZkStateReader().waitForState(collection, wait, unit, predicate);
+    }
+
+    /**
+     * Block until a Predicate returns true, or the wait times out
+     *
+     * <p>
+     * Note that the predicate may be called again even after it has returned true, so
+     * implementors should avoid changing state within the predicate call itself.
+     * </p>
+     *
+     * @param collection the collection to watch
+     * @param wait       how long to wait
+     * @param unit       the units of the wait parameter
+     * @param predicate  a {@link Predicate} to test against the {@link DocCollection}
+     * @throws InterruptedException on interrupt
+     * @throws TimeoutException     on timeout
+     * @see #registerDocCollectionWatcher
+     */
+    public static void waitForState(BaseCloudSolrClient client, String collection, long wait, TimeUnit unit, Predicate<DocCollection> predicate)
+            throws InterruptedException, TimeoutException {
+        client.getClusterStateProvider().connect();
+        assertZKStateProvider(client).getZkStateReader().waitForState(collection, wait, unit, predicate);
+    }
+
+    /**
+     * Register a CollectionStateWatcher to be called when the cluster state for a collection changes
+     * <em>or</em> the set of live nodes changes.
+     *
+     * <p>
+     * The Watcher will automatically be removed when it's
+     * <code>onStateChanged</code> returns <code>true</code>
+     * </p>
+     *
+     * <p>
+     * This implementation utilizes {@link ZkStateReader#registerCollectionStateWatcher} internally.
+     * Callers that don't care about liveNodes are encouraged to use a {@link DocCollectionWatcher}
+     * instead
+     * </p>
+     *
+     * @param collection the collection to watch
+     * @param watcher    a watcher that will be called when the state changes
+     * @see #registerDocCollectionWatcher(BaseCloudSolrClient, String, DocCollectionWatcher)
+     * @see ZkStateReader#registerCollectionStateWatcher
+     */
+    public static void registerCollectionStateWatcher(BaseCloudSolrClient client, String collection, CollectionStateWatcher watcher) {
+        client.getClusterStateProvider().connect();
+        assertZKStateProvider(client).getZkStateReader().registerCollectionStateWatcher(collection, watcher);
+    }
+
+    /**
+     * Register a DocCollectionWatcher to be called when the cluster state for a collection changes.
+     *
+     * <p>
+     * The Watcher will automatically be removed when it's
+     * <code>onStateChanged</code> returns <code>true</code>
+     * </p>
+     *
+     * @param collection the collection to watch
+     * @param watcher    a watcher that will be called when the state changes
+     * @see ZkStateReader#registerDocCollectionWatcher
+     */
+    public static void registerDocCollectionWatcher(BaseCloudSolrClient client, String collection, DocCollectionWatcher watcher) {

Review comment:
       same as I said above

##########
File path: solr/test-framework/src/java/org/apache/solr/cloud/CloudSolrClientUtils.java
##########
@@ -0,0 +1,134 @@
+package org.apache.solr.cloud;
+
+import org.apache.solr.client.solrj.impl.BaseCloudSolrClient;
+import org.apache.solr.client.solrj.impl.ZkClientClusterStateProvider;
+import org.apache.solr.common.cloud.*;
+
+import java.io.Serializable;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.function.Predicate;
+
+public class CloudSolrClientUtils implements Serializable {
+    /**
+     * @return the zkHost value used to connect to zookeeper.
+     */
+    public static String getZkHost(BaseCloudSolrClient client) {
+        return assertZKStateProvider(client).zkHost;
+    }
+
+    /**
+     * Set the connect timeout to the zookeeper ensemble in ms
+     */
+    public static void setZkConnectTimeout(BaseCloudSolrClient client, int zkConnectTimeout) {
+        assertZKStateProvider(client).zkConnectTimeout = zkConnectTimeout;
+    }
+
+    /**
+     * Set the timeout to the zookeeper ensemble in ms
+     */
+    public static void setZkClientTimeout(BaseCloudSolrClient client, int zkClientTimeout) {
+        assertZKStateProvider(client).zkClientTimeout = zkClientTimeout;
+    }
+
+    static ZkClientClusterStateProvider assertZKStateProvider(BaseCloudSolrClient client) {
+        if (client.getClusterStateProvider() instanceof ZkClientClusterStateProvider) {
+            return (ZkClientClusterStateProvider) client.getClusterStateProvider();
+        }
+        throw new IllegalArgumentException("This client does not use ZK");
+
+    }
+
+    /**
+     * Block until a CollectionStatePredicate returns true, or the wait times out
+     *
+     * <p>
+     * Note that the predicate may be called again even after it has returned true, so
+     * implementors should avoid changing state within the predicate call itself.
+     * </p>
+     *
+     * <p>
+     * This implementation utilizes {@link CollectionStateWatcher} internally.
+     * Callers that don't care about liveNodes are encouraged to use a {@link DocCollection} {@link Predicate}
+     * instead
+     * </p>
+     *
+     * @param collection the collection to watch
+     * @param wait       how long to wait
+     * @param unit       the units of the wait parameter
+     * @param predicate  a {@link CollectionStatePredicate} to check the collection state
+     * @throws InterruptedException on interrupt
+     * @throws TimeoutException     on timeout
+     * @see #waitForState(BaseCloudSolrClient, String, long, TimeUnit, Predicate)
+     * @see #registerCollectionStateWatcher
+     */
+    public static void waitForState(BaseCloudSolrClient client, String collection, long wait, TimeUnit unit, CollectionStatePredicate predicate)
+            throws InterruptedException, TimeoutException {
+        client.getClusterStateProvider().connect();
+        assertZKStateProvider(client).getZkStateReader().waitForState(collection, wait, unit, predicate);
+    }
+
+    /**
+     * Block until a Predicate returns true, or the wait times out
+     *
+     * <p>
+     * Note that the predicate may be called again even after it has returned true, so
+     * implementors should avoid changing state within the predicate call itself.
+     * </p>
+     *
+     * @param collection the collection to watch
+     * @param wait       how long to wait
+     * @param unit       the units of the wait parameter
+     * @param predicate  a {@link Predicate} to test against the {@link DocCollection}
+     * @throws InterruptedException on interrupt
+     * @throws TimeoutException     on timeout
+     * @see #registerDocCollectionWatcher
+     */
+    public static void waitForState(BaseCloudSolrClient client, String collection, long wait, TimeUnit unit, Predicate<DocCollection> predicate)
+            throws InterruptedException, TimeoutException {
+        client.getClusterStateProvider().connect();
+        assertZKStateProvider(client).getZkStateReader().waitForState(collection, wait, unit, predicate);
+    }
+
+    /**
+     * Register a CollectionStateWatcher to be called when the cluster state for a collection changes
+     * <em>or</em> the set of live nodes changes.
+     *
+     * <p>
+     * The Watcher will automatically be removed when it's
+     * <code>onStateChanged</code> returns <code>true</code>
+     * </p>
+     *
+     * <p>
+     * This implementation utilizes {@link ZkStateReader#registerCollectionStateWatcher} internally.
+     * Callers that don't care about liveNodes are encouraged to use a {@link DocCollectionWatcher}
+     * instead
+     * </p>
+     *
+     * @param collection the collection to watch
+     * @param watcher    a watcher that will be called when the state changes
+     * @see #registerDocCollectionWatcher(BaseCloudSolrClient, String, DocCollectionWatcher)
+     * @see ZkStateReader#registerCollectionStateWatcher
+     */
+    public static void registerCollectionStateWatcher(BaseCloudSolrClient client, String collection, CollectionStateWatcher watcher) {

Review comment:
       same as I said above




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org