[GitHub] lucene-solr issue #294: ZkStateReader: cache LazyCollectionRef (SOLR-8327)
Github user dragonsinth commented on the issue: https://github.com/apache/lucene-solr/pull/294 This approach seems fine to me. Remind me why we use nanoTime vs. normal clock? I'm sure you're right I just want to refresh my brain. --- - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #257: SOLR-11423: Overseer queue needs a hard cap (...
GitHub user dragonsinth opened a pull request: https://github.com/apache/lucene-solr/pull/257 SOLR-11423: Overseer queue needs a hard cap (maximum size) that clients respect You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/lucene-solr jira/SOLR-11423 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/lucene-solr/pull/257.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #257 commit ef8e0934fb27530f0c9450b58872b2b11028f50a Author: Scott Blum <dragonsi...@apache.org> Date: 2017-10-02T20:50:57Z SOLR-11423: Overseer queue needs a hard cap (maximum size) that clients respect --- - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #256: SOLR-11423: Overseer queue needs a hard cap (...
GitHub user dragonsinth opened a pull request: https://github.com/apache/lucene-solr/pull/256 SOLR-11423: Overseer queue needs a hard cap (maximum size) that clients respect You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/lucene-solr SOLR-11423 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/lucene-solr/pull/256.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #256 commit ef8e0934fb27530f0c9450b58872b2b11028f50a Author: Scott Blum <dragonsi...@apache.org> Date: 2017-10-02T20:50:57Z SOLR-11423: Overseer queue needs a hard cap (maximum size) that clients respect --- - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #42: SOLR-8744 blockedTasks
Github user dragonsinth closed the pull request at: https://github.com/apache/lucene-solr/pull/42 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr issue #42: SOLR-8744 blockedTasks
Github user dragonsinth commented on the issue: https://github.com/apache/lucene-solr/pull/42 FYI: both of these SHAs passed the test suite --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #42: SOLR-8744 blockedTasks
GitHub user dragonsinth opened a pull request: https://github.com/apache/lucene-solr/pull/42 SOLR-8744 blockedTasks @noblepaul You can merge this pull request into a Git repository by running: $ git pull https://github.com/fullstorydev/lucene-solr SOLR-8744 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/lucene-solr/pull/42.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #42 commit 668d426633fe5551b24ab38036e14c15e7ed4cdf Author: Scott Blum <dragonsi...@apache.org> Date: 2016-06-09T21:28:07Z WIP: SOLR-8744 blockedTasks commit 4b2512abcc5e55c653c923eba9762988cf1faae8 Author: Scott Blum <dragonsi...@apache.org> Date: 2016-06-09T22:11:26Z Simplifications --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr issue #41: SOLR-9191: OverseerTaskQueue.peekTopN() fatally flawe...
Github user dragonsinth commented on the issue: https://github.com/apache/lucene-solr/pull/41 This passes `ant test`. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #41: SOLR-9191: OverseerTaskQueue.peekTopN() fatall...
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/41#discussion_r66172870
--- Diff:
solr/core/src/test/org/apache/solr/cloud/DistributedQueueTest.java ---
@@ -137,6 +136,49 @@ public void testDistributedQueueBlocking() throws
Exception {
assertNull(dq.poll());
}
+ @Test
+ public void testPeekElements() throws Exception {
+String dqZNode = "/distqueue/test";
+byte[] data = "hello world".getBytes(UTF8);
+
+DistributedQueue dq = makeDistributedQueue(dqZNode);
+
+// Populate with data.
+dq.offer(data);
+dq.offer(data);
+dq.offer(data);
+
+// Should be able to get 0, 1, 2, or 3 instantly
+for (int i = 0; i <= 3; ++i) {
+ assertEquals(i, dq.peekElements(i, 0, child -> true).size());
+}
+
+// Asking for more should return only 3.
+assertEquals(3, dq.peekElements(4, 0, child -> true).size());
+
+// If we filter everything out, we should block for the full time.
+long start = System.nanoTime();
+assertEquals(0, dq.peekElements(4, 1000, child -> false).size());
+assertTrue(System.nanoTime() - start >=
TimeUnit.MILLISECONDS.toNanos(500));
+
+// If someone adds a new matching element while we're waiting, we
should return immediately.
+executor.submit(() -> {
+ try {
+Thread.sleep(500);
+dq.offer(data);
+ } catch (Exception e) {
+// ignore
+ }
+});
+start = System.nanoTime();
+assertEquals(1, dq.peekElements(4, 2000, child -> {
+ // The 4th element in the queue will end with a "3".
+ return child.endsWith("3");
+}).size());
+assertTrue(System.nanoTime() - start <
TimeUnit.MILLISECONDS.toNanos(1000));
+assertTrue(System.nanoTime() - start >=
TimeUnit.MILLISECONDS.toNanos(250));
+ }
+
--- End diff --
@markrmiller the new test you suggested
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #41: SOLR-9191: OverseerTaskQueue.peekTopN() fatall...
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/41#discussion_r66172805
--- Diff:
solr/core/src/java/org/apache/solr/cloud/OverseerTaskProcessor.java ---
@@ -466,6 +466,8 @@ private void markTaskComplete(String id, String asyncId)
log.warn("Could not find and remove async call [" + asyncId + "]
from the running map.");
}
}
+
+ workQueue.remove(head);
--- End diff --
@markrmiller can you think of any reason not to do this? I don't
understand why currently getting things out of the queue takes an extra
iteration. I think my fix unmasked a latent problem exposed by
DeleteStatusTest; to get that test to pass I have to eagerly remove completed
items from the work queue, which seems correct to me. Not sure why we'd want
to wait for a loop-around to `cleanUpWorkQueue()`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #41: SOLR-9191: OverseerTaskQueue.peekTopN() fatall...
GitHub user dragonsinth opened a pull request: https://github.com/apache/lucene-solr/pull/41 SOLR-9191: OverseerTaskQueue.peekTopN() fatally flawed @noblepaul CC: @shalinmangar You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/lucene-solr SOLR-9191 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/lucene-solr/pull/41.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #41 commit 34e30860488ebf08bdf0e3cb36fddcb67991b800 Author: Scott Blum <dragonsi...@gmail.com> Date: 2016-06-07T05:52:16Z SOLR-9191: OverseerTaskQueue.peekTopN() fatally flawed --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/32#discussion_r62900842
--- Diff:
solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java ---
@@ -1069,32 +1100,190 @@ public static String getCollectionPath(String
coll) {
return COLLECTIONS_ZKNODE+"/"+coll + "/state.json";
}
- public void addCollectionWatch(String coll) {
-if (interestingCollections.add(coll)) {
- LOG.info("addZkWatch [{}]", coll);
- new StateWatcher(coll).refreshAndWatch(false);
+ /**
+ * Notify this reader that a local Core is a member of a collection, and
so that collection
+ * state should be watched.
+ *
+ * Not a public API. This method should only be called from
ZkController.
+ *
+ * The number of cores per-collection is tracked, and adding multiple
cores from the same
+ * collection does not increase the number of watches.
+ *
+ * @param collection the collection that the core is a member of
+ *
+ * @see ZkStateReader#unregisterCore(String)
+ */
+ public void registerCore(String collection) {
+AtomicBoolean reconstructState = new AtomicBoolean(false);
+collectionWatches.compute(collection, (k, v) -> {
+ if (v == null) {
+reconstructState.set(true);
+v = new CollectionWatch();
+ }
+ v.coreRefCount++;
+ return v;
+});
+if (reconstructState.get()) {
+ new StateWatcher(collection).refreshAndWatch();
+ synchronized (getUpdateLock()) {
+constructState();
+ }
+}
+ }
+
+ /**
+ * Notify this reader that a local core that is a member of a collection
has been closed.
+ *
+ * Not a public API. This method should only be called from
ZkController.
+ *
+ * If no cores are registered for a collection, and there are no {@link
CollectionStateWatcher}s
+ * for that collection either, the collection watch will be removed.
+ *
+ * @param collection the collection that the core belongs to
+ */
+ public void unregisterCore(String collection) {
+AtomicBoolean reconstructState = new AtomicBoolean(false);
+collectionWatches.compute(collection, (k, v) -> {
+ if (v == null)
+return null;
+ if (v.coreRefCount > 0)
+v.coreRefCount--;
+ if (v.canBeRemoved()) {
+watchedCollectionStates.remove(collection);
+lazyCollectionStates.put(collection, new
LazyCollectionRef(collection));
+reconstructState.set(true);
+return null;
+ }
+ return v;
+});
+if (reconstructState.get()) {
+ synchronized (getUpdateLock()) {
+constructState();
+ }
+}
+ }
+
+ /**
+ * Register a CollectionStateWatcher to be called when the state of a
collection changes
+ *
+ * A given CollectionStateWatcher will be only called once. If you want
to have a persistent watcher,
+ * it should register itself again in its {@link
CollectionStateWatcher#onStateChanged(Set, DocCollection)}
+ * method.
+ */
+ public void registerCollectionStateWatcher(String collection,
CollectionStateWatcher stateWatcher) {
+AtomicBoolean watchSet = new AtomicBoolean(false);
+collectionWatches.compute(collection, (k, v) -> {
+ if (v == null) {
+v = new CollectionWatch();
+watchSet.set(true);
+ }
+ v.stateWatchers.add(stateWatcher);
+ return v;
+});
+if (watchSet.get()) {
+ new StateWatcher(collection).refreshAndWatch();
--- End diff --
ditto, ignore this
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/32#discussion_r62900820
--- Diff:
solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java ---
@@ -1069,32 +1100,190 @@ public static String getCollectionPath(String
coll) {
return COLLECTIONS_ZKNODE+"/"+coll + "/state.json";
}
- public void addCollectionWatch(String coll) {
-if (interestingCollections.add(coll)) {
- LOG.info("addZkWatch [{}]", coll);
- new StateWatcher(coll).refreshAndWatch(false);
+ /**
+ * Notify this reader that a local Core is a member of a collection, and
so that collection
+ * state should be watched.
+ *
+ * Not a public API. This method should only be called from
ZkController.
+ *
+ * The number of cores per-collection is tracked, and adding multiple
cores from the same
+ * collection does not increase the number of watches.
+ *
+ * @param collection the collection that the core is a member of
+ *
+ * @see ZkStateReader#unregisterCore(String)
+ */
+ public void registerCore(String collection) {
+AtomicBoolean reconstructState = new AtomicBoolean(false);
+collectionWatches.compute(collection, (k, v) -> {
+ if (v == null) {
+reconstructState.set(true);
+v = new CollectionWatch();
+ }
+ v.coreRefCount++;
+ return v;
+});
+if (reconstructState.get()) {
+ new StateWatcher(collection).refreshAndWatch();
--- End diff --
Ignore this, I'm dumb. You want a state watcher either way (the old code
did this).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on the pull request: https://github.com/apache/lucene-solr/pull/32#issuecomment-218321437 Almost LGTM. There's a few nits, but the only real issue is potentially setting up a StateWatcher on legacy. Nice work, I think we're almost done!! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/32#discussion_r62771180
--- Diff:
solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java ---
@@ -1069,32 +1100,190 @@ public static String getCollectionPath(String
coll) {
return COLLECTIONS_ZKNODE+"/"+coll + "/state.json";
}
- public void addCollectionWatch(String coll) {
-if (interestingCollections.add(coll)) {
- LOG.info("addZkWatch [{}]", coll);
- new StateWatcher(coll).refreshAndWatch(false);
+ /**
+ * Notify this reader that a local Core is a member of a collection, and
so that collection
+ * state should be watched.
+ *
+ * Not a public API. This method should only be called from
ZkController.
+ *
+ * The number of cores per-collection is tracked, and adding multiple
cores from the same
+ * collection does not increase the number of watches.
+ *
+ * @param collection the collection that the core is a member of
+ *
+ * @see ZkStateReader#unregisterCore(String)
+ */
+ public void registerCore(String collection) {
+AtomicBoolean reconstructState = new AtomicBoolean(false);
+collectionWatches.compute(collection, (k, v) -> {
+ if (v == null) {
+reconstructState.set(true);
+v = new CollectionWatch();
+ }
+ v.coreRefCount++;
+ return v;
+});
+if (reconstructState.get()) {
+ new StateWatcher(collection).refreshAndWatch();
--- End diff --
(same) I feel like this needs to check whether or not the collection exists
/ is a legacy collection. If it's a legacy collection, you don't want to try
to create a StateWatcher.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/32#discussion_r62771165
--- Diff:
solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java ---
@@ -1069,32 +1100,190 @@ public static String getCollectionPath(String
coll) {
return COLLECTIONS_ZKNODE+"/"+coll + "/state.json";
}
- public void addCollectionWatch(String coll) {
-if (interestingCollections.add(coll)) {
- LOG.info("addZkWatch [{}]", coll);
- new StateWatcher(coll).refreshAndWatch(false);
+ /**
+ * Notify this reader that a local Core is a member of a collection, and
so that collection
+ * state should be watched.
+ *
+ * Not a public API. This method should only be called from
ZkController.
+ *
+ * The number of cores per-collection is tracked, and adding multiple
cores from the same
+ * collection does not increase the number of watches.
+ *
+ * @param collection the collection that the core is a member of
+ *
+ * @see ZkStateReader#unregisterCore(String)
+ */
+ public void registerCore(String collection) {
+AtomicBoolean reconstructState = new AtomicBoolean(false);
+collectionWatches.compute(collection, (k, v) -> {
+ if (v == null) {
+reconstructState.set(true);
+v = new CollectionWatch();
+ }
+ v.coreRefCount++;
+ return v;
+});
+if (reconstructState.get()) {
+ new StateWatcher(collection).refreshAndWatch();
+ synchronized (getUpdateLock()) {
+constructState();
+ }
+}
+ }
+
+ /**
+ * Notify this reader that a local core that is a member of a collection
has been closed.
+ *
+ * Not a public API. This method should only be called from
ZkController.
+ *
+ * If no cores are registered for a collection, and there are no {@link
CollectionStateWatcher}s
+ * for that collection either, the collection watch will be removed.
+ *
+ * @param collection the collection that the core belongs to
+ */
+ public void unregisterCore(String collection) {
+AtomicBoolean reconstructState = new AtomicBoolean(false);
+collectionWatches.compute(collection, (k, v) -> {
+ if (v == null)
+return null;
+ if (v.coreRefCount > 0)
+v.coreRefCount--;
+ if (v.canBeRemoved()) {
+watchedCollectionStates.remove(collection);
+lazyCollectionStates.put(collection, new
LazyCollectionRef(collection));
+reconstructState.set(true);
+return null;
+ }
+ return v;
+});
+if (reconstructState.get()) {
+ synchronized (getUpdateLock()) {
+constructState();
+ }
+}
+ }
+
+ /**
+ * Register a CollectionStateWatcher to be called when the state of a
collection changes
+ *
+ * A given CollectionStateWatcher will be only called once. If you want
to have a persistent watcher,
+ * it should register itself again in its {@link
CollectionStateWatcher#onStateChanged(Set, DocCollection)}
+ * method.
+ */
+ public void registerCollectionStateWatcher(String collection,
CollectionStateWatcher stateWatcher) {
+AtomicBoolean watchSet = new AtomicBoolean(false);
+collectionWatches.compute(collection, (k, v) -> {
+ if (v == null) {
+v = new CollectionWatch();
+watchSet.set(true);
+ }
+ v.stateWatchers.add(stateWatcher);
+ return v;
+});
+if (watchSet.get()) {
+ new StateWatcher(collection).refreshAndWatch();
--- End diff --
I feel like this needs to check whether or not the collection exists / is a
legacy collection. If it's a legacy collection, you don't want to try to
create a StateWatcher.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/32#discussion_r62771056
--- Diff:
solr/solrj/src/test/org/apache/solr/common/cloud/TestCollectionStateWatchers.java
---
@@ -0,0 +1,235 @@
+package org.apache.solr.common.cloud;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import java.util.HashMap;
+import java.util.Set;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.apache.solr.client.solrj.embedded.JettySolrRunner;
+import org.apache.solr.client.solrj.impl.CloudSolrClient;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.cloud.SolrCloudTestCase;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.SolrjNamedThreadFactory;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import static org.hamcrest.core.Is.is;
+
+public class TestCollectionStateWatchers extends SolrCloudTestCase {
+
+ private static final int CLUSTER_SIZE = 4;
+
+ private static final ExecutorService executor =
ExecutorUtil.newMDCAwareCachedThreadPool(
+ new SolrjNamedThreadFactory("backgroundWatchers")
+ );
+
+ private static final int MAX_WAIT_TIMEOUT = 30;
+
+ @BeforeClass
+ public static void startCluster() throws Exception {
+configureCluster(CLUSTER_SIZE)
+.addConfig("config",
getFile("solrj/solr/collection1/conf").toPath())
+.configure();
+ }
+
+ @AfterClass
+ public static void shutdownBackgroundExecutors() {
+executor.shutdown();
+ }
+
+ @Before
+ public void prepareCluster() throws Exception {
+int missingServers = CLUSTER_SIZE -
cluster.getJettySolrRunners().size();
+for (int i = 0; i < missingServers; i++) {
+ cluster.startJettySolrRunner();
+}
+cluster.waitForAllNodes(30);
+ }
+
+ private static Future waitInBackground(String collection, long
timeout, TimeUnit unit,
+ CollectionStatePredicate
predicate) {
+return executor.submit(() -> {
+ try {
+cluster.getSolrClient().waitForState(collection, timeout, unit,
predicate);
+ } catch (InterruptedException | TimeoutException e) {
+return Boolean.FALSE;
+ }
+ return Boolean.TRUE;
+});
+ }
+
+
+ @Test
+ public void testSimpleCollectionWatch() throws Exception {
+
+CloudSolrClient client = cluster.getSolrClient();
+cluster.createCollection("testcollection", CLUSTER_SIZE, 1, "config",
new HashMap<>());
+
+client.waitForState("testcollection", MAX_WAIT_TIMEOUT,
TimeUnit.SECONDS, DocCollection::isFullyActive);
+
+// shutdown a node and check that we get notified about the change
+final AtomicInteger nodeCount = new AtomicInteger(0);
+final CountDownLatch latch = new CountDownLatch(1);
+client.registerCollectionStateWatcher("testcollection", (liveNodes,
collectionState) -> {
+ // we can't just count liveNodes here, because that's updated by a
separate watcher,
+ // and it may be the case that we're triggered by a node setting
itself to DOWN before
+ // the liveNodes watcher is called
+ for (Slice slice : collectionState) {
+for (Replica replica : slice) {
+ if (replica.isActive(liveNodes))
+nodeCount.incrementAndGet();
+}
+ }
+ latch.countDown();
+});
+
+
cluster.stopJettySolrRu
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/32#discussion_r62770640
--- Diff: solr/solrj/src/java/org/apache/solr/common/util/ExecutorUtil.java
---
@@ -154,6 +147,20 @@ public static ExecutorService
newMDCAwareSingleThreadExecutor(ThreadFactory thre
}
/**
+ * Create a single thread executor using a named thread factory
+ */
+ public static ExecutorService newMDCAwareSingleThreadExecutor(String
name) {
+return newMDCAwareSingleThreadExecutor(new
SolrjNamedThreadFactory(name));
+ }
--- End diff --
not used
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/32#discussion_r62770569
--- Diff:
solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java ---
@@ -1069,32 +1100,190 @@ public static String getCollectionPath(String
coll) {
return COLLECTIONS_ZKNODE+"/"+coll + "/state.json";
}
- public void addCollectionWatch(String coll) {
-if (interestingCollections.add(coll)) {
- LOG.info("addZkWatch [{}]", coll);
- new StateWatcher(coll).refreshAndWatch(false);
+ /**
+ * Notify this reader that a local Core is a member of a collection, and
so that collection
+ * state should be watched.
+ *
+ * Not a public API. This method should only be called from
ZkController.
+ *
+ * The number of cores per-collection is tracked, and adding multiple
cores from the same
+ * collection does not increase the number of watches.
+ *
+ * @param collection the collection that the core is a member of
+ *
+ * @see ZkStateReader#unregisterCore(String)
+ */
+ public void registerCore(String collection) {
+AtomicBoolean reconstructState = new AtomicBoolean(false);
+collectionWatches.compute(collection, (k, v) -> {
+ if (v == null) {
+reconstructState.set(true);
+v = new CollectionWatch();
+ }
+ v.coreRefCount++;
+ return v;
+});
+if (reconstructState.get()) {
+ new StateWatcher(collection).refreshAndWatch();
+ synchronized (getUpdateLock()) {
+constructState();
+ }
+}
+ }
+
+ /**
+ * Notify this reader that a local core that is a member of a collection
has been closed.
+ *
+ * Not a public API. This method should only be called from
ZkController.
+ *
+ * If no cores are registered for a collection, and there are no {@link
CollectionStateWatcher}s
+ * for that collection either, the collection watch will be removed.
+ *
+ * @param collection the collection that the core belongs to
+ */
+ public void unregisterCore(String collection) {
+AtomicBoolean reconstructState = new AtomicBoolean(false);
+collectionWatches.compute(collection, (k, v) -> {
+ if (v == null)
+return null;
+ if (v.coreRefCount > 0)
+v.coreRefCount--;
+ if (v.canBeRemoved()) {
+watchedCollectionStates.remove(collection);
+lazyCollectionStates.put(collection, new
LazyCollectionRef(collection));
+reconstructState.set(true);
+return null;
+ }
+ return v;
+});
+if (reconstructState.get()) {
+ synchronized (getUpdateLock()) {
+constructState();
+ }
+}
+ }
+
+ /**
+ * Register a CollectionStateWatcher to be called when the state of a
collection changes
+ *
+ * A given CollectionStateWatcher will be only called once. If you want
to have a persistent watcher,
+ * it should register itself again in its {@link
CollectionStateWatcher#onStateChanged(Set, DocCollection)}
+ * method.
+ */
+ public void registerCollectionStateWatcher(String collection,
CollectionStateWatcher stateWatcher) {
+AtomicBoolean watchSet = new AtomicBoolean(false);
+collectionWatches.compute(collection, (k, v) -> {
+ if (v == null) {
+v = new CollectionWatch();
+watchSet.set(true);
+ }
+ v.stateWatchers.add(stateWatcher);
+ return v;
+});
+if (watchSet.get()) {
+ new StateWatcher(collection).refreshAndWatch();
synchronized (getUpdateLock()) {
constructState();
}
}
}
+ /**
+ * Block until a CollectionStatePredicate returns true, or the wait
times out
+ *
+ * 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.
+ *
+ * @param collection the collection to watch
+ * @param wait how long to wait
+ * @param unit the units of the wait parameter
+ * @param predicate the predicate to call on state changes
+ * @throws InterruptedException on interrupt
+ * @throws TimeoutException on timeout
+ */
+ public void waitForState(final String collection, long wait, TimeUnit
unit, CollectionStatePredicate predicate)
+ throws InterruptedException, TimeoutException {
+
+final CountDownLatch latch = new CountDownLatch(1);
+
+CollectionStateWatcher watcher = new CollectionStateWatcher() {
+ @Override
+
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/32#discussion_r62770248
--- Diff:
solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java ---
@@ -635,6 +669,8 @@ public Object getUpdateLock() {
public void close() {
this.closed = true;
+notifications.shutdownNow(); // interrupt
--- End diff --
@markrmiller and I went on a hunt a while ago to try to remove most thread
interruptions from Solr due to certain Lucene NIO operations getting
permanently wedged due to interrupts. Is this necessary?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/32#discussion_r62770279
--- Diff:
solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java ---
@@ -635,6 +669,8 @@ public Object getUpdateLock() {
public void close() {
this.closed = true;
+notifications.shutdownNow(); // interrupt
+ExecutorUtil.shutdownAndAwaitTermination(notifications);
--- End diff --
I think I would just shutdown and not wait.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/32#discussion_r62770082
--- Diff:
solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java ---
@@ -485,6 +506,20 @@ private void refreshLegacyClusterState(Watcher watcher)
// Nothing to do, someone else updated same or newer.
return;
}
+Set liveNodes = this.liveNodes; // volatile read
+for (Map.Entry<String, CollectionWatch> watchEntry :
this.collectionWatches.entrySet()) {
+ String coll = watchEntry.getKey();
+ CollectionWatch collWatch = watchEntry.getValue();
+ ClusterState.CollectionRef ref =
this.legacyCollectionStates.get(coll);
+ if (ref == null)
+continue;
--- End diff --
Q: what happens if you try to set a watcher on a collection that doesn't
exist yet? Mostly curious.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/32#discussion_r62769742
--- Diff:
solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java ---
@@ -485,6 +506,20 @@ private void refreshLegacyClusterState(Watcher watcher)
// Nothing to do, someone else updated same or newer.
return;
}
+Set liveNodes = this.liveNodes; // volatile read
+for (Map.Entry<String, CollectionWatch> watchEntry :
this.collectionWatches.entrySet()) {
+ String coll = watchEntry.getKey();
+ CollectionWatch collWatch = watchEntry.getValue();
+ ClusterState.CollectionRef ref =
this.legacyCollectionStates.get(coll);
+ if (ref == null)
+continue;
+ // watched collection, so this will always be local
--- End diff --
nit `legacy collections are always in-memory`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on the pull request: https://github.com/apache/lucene-solr/pull/32#issuecomment-218226081 Hmm, isn't an executor a fancier way of doing a Queue + Thread(s)? :) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on the pull request: https://github.com/apache/lucene-solr/pull/32#issuecomment-217984743 I did like the idea of a dedicated executor for collection events, just to ensure clean separation. But I'll take a look in its current form. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on the pull request: https://github.com/apache/lucene-solr/pull/32#issuecomment-216931783 Correct, the queuing implementation where the waiting thread loops only helps waitForState(). Maybe we should just go with that for now and consider making CSW public as a follow up? If we do make it public, I think we'd still want a separate executor, you don't want to end up blocking ZKSR's internal operations. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on the pull request:
https://github.com/apache/lucene-solr/pull/32#issuecomment-216351404
BTW, here's an implementation of waitForState() that does the work on the
calling thread. This passes your tests:
```
public void waitForState(final String collection, long wait, TimeUnit
unit, CollectionStatePredicate predicate)
throws InterruptedException, TimeoutException {
long stop = System.nanoTime() + unit.toNanos(wait);
if (predicate.matches(this.liveNodes,
clusterState.getCollectionOrNull(collection))) {
return;
}
LinkedBlockingQueue<Pair<Set, DocCollection>> queue = new
LinkedBlockingQueue<>();
CollectionStateWatcher watcher = new CollectionStateWatcher() {
@Override
public void onStateChanged(Set liveNodes, DocCollection
collectionState) {
queue.add(new Pair<>(liveNodes, collectionState));
registerCollectionStateWatcher(collection, this);
}
};
registerCollectionStateWatcher(collection, watcher);
try {
while (true) {
Pair<Set, DocCollection> pair = queue.poll(stop -
System.nanoTime(), TimeUnit.NANOSECONDS);
if (pair == null) {
throw new TimeoutException();
}
if (predicate.matches(pair.getKey(), pair.getValue())) {
return;
}
}
} finally {
removeCollectionStateWatcher(collection, watcher);
}
}
```
One thing I noticed in writing this is that it's uncertain whether you'll
miss any states or not. I kind of like the idea that you could have your
watcher return true or false to decide whether to keep watching, as it would
ensure we could get all updates without missing any.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on the pull request: https://github.com/apache/lucene-solr/pull/32#issuecomment-216333963 @romseygeek nice job on the changes so far, and sorry to have so much feedback and so many asks. This is a pretty complicated change so I feel like it merits the attention to detail. I feel like we're at a fork in the road with this patch at the moment though, and we need to get more people involved to proceed. Let me explain. Even having fixed the "calling watchers while holding locks issue", the one thing that makes me most nervous about the current state is that we're still potentially executing user-provided predicates on threads that belong to a variety of other people-- e.g. the caller of forceUpdateCollection() or even the Zk event callback thread. We could make a tactical fix to the implementation of waitForState() by turning that method into a loop and running the predicate on the actual thread that called waitForState(), such that the onStateChanged() handler doesn't dip into client code. But honestly, I feel like having privatized CollectionStateWatcher and the ability to register / unregister is a missed opportunity. I can think of uses for the feature, like in some cases Overseer operations could watch a collection for the duration of an operation to prevent having to re-query ZK. To make that solid, we'd need to either introduce an Executor in ZkStateReader for publishing events, or else require the watch registration to provide an executor, the way Guava's ListenableFuture does. Thoughts? I'd also like to hear from others. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/32#discussion_r61644721
--- Diff:
solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java ---
@@ -491,19 +493,28 @@ private void refreshLegacyClusterState(Watcher
watcher)
final Stat stat = new Stat();
final byte[] data = zkClient.getData(CLUSTER_STATE, watcher, stat,
true);
final ClusterState loadedData = ClusterState.load(stat.getVersion(),
data, emptySet(), CLUSTER_STATE);
+ final Set liveNodes = new HashSet<>(this.liveNodes);
synchronized (getUpdateLock()) {
if (this.legacyClusterStateVersion >= stat.getVersion()) {
// Nothing to do, someone else updated same or newer.
return;
}
-this.legacyCollectionStates = loadedData.getCollectionStates();
-this.legacyClusterStateVersion = stat.getVersion();
-for (Map.Entry<String, ClusterState.CollectionRef> entry :
this.legacyCollectionStates.entrySet()) {
- if (entry.getValue().isLazilyLoaded() == false) {
-// a watched collection - trigger notifications
-notifyStateWatchers(entry.getKey(), entry.getValue().get());
+LOG.info("Updating legacy cluster state - {} entries in
legacyCollectionStates", legacyCollectionStates.size());
+for (Map.Entry<String, CollectionWatch> watchEntry :
this.collectionWatches.entrySet()) {
+ String coll = watchEntry.getKey();
+ CollectionWatch collWatch = watchEntry.getValue();
+ ClusterState.CollectionRef ref =
this.legacyCollectionStates.get(coll);
+ if (ref == null)
+continue;
+ // watched collection, so this will always be local
+ DocCollection newState = ref.get();
+ if (!collWatch.stateWatchers.isEmpty()
+ &&
!Objects.equals(loadedData.getCollectionStates().get(coll).get(), newState)) {
+notifyStateWatchers(liveNodes, coll, newState);
--- End diff --
I just realized you don't want to call user code while holding the update
lock. I think you're going to need to move this out of the synchronized block.
In fact this is really nasty now that I think about it. In general,
you're going to want to defer calling any user code until the current
constuctState() operation finishes. Otherwise, the user code is potentially
going to see a stale copy of the state that you haven't finished updating yet.
I think we're going to have to build a queue of outstanding state watchers
to notify and always call them later, probably in an executor. I know that
sounds like a bit of work, but I'm not sure I can see how it would be safe
otherwise.
@markrmiller any thoughts?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/32#discussion_r61643877
--- Diff:
solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java ---
@@ -256,9 +257,10 @@ public void updateClusterState() throws
KeeperException, InterruptedException {
refreshLegacyClusterState(null);
// Need a copy so we don't delete from what we're iterating over.
Collection safeCopy = new
ArrayList<>(watchedCollectionStates.keySet());
+ Set liveNodes = new HashSet<>(this.liveNodes);
--- End diff --
You don't actually need a copy here, since `liveNodes` is an immutable set.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request: https://github.com/apache/lucene-solr/pull/32#discussion_r61643749 --- Diff: solr/solrj/src/java/org/apache/solr/common/cloud/CollectionStatePredicate.java --- @@ -30,8 +30,9 @@ /** * Check the collection state matches a required state * - * The collectionState parameter may be null if the collection does not exist - * or has been deleted + * @param liveNodes the current set of live nodes + * @param collectionState the latest collection state, or null if the collection + *does not exist --- End diff -- I think this needs to be below the "Note" lines to get formatted right. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/32#discussion_r61643539
--- Diff:
solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java ---
@@ -1066,32 +1079,201 @@ public static String getCollectionPath(String
coll) {
return COLLECTIONS_ZKNODE+"/"+coll + "/state.json";
}
- public void addCollectionWatch(String coll) {
-if (interestingCollections.add(coll)) {
- LOG.info("addZkWatch [{}]", coll);
- new StateWatcher(coll).refreshAndWatch(false);
+ /**
+ * Notify this reader that a local Core is a member of a collection, and
so that collection
+ * state should be watched.
+ *
+ * Not a public API. This method should only be called from
ZkController.
+ *
+ * The number of cores per-collection is tracked, and adding multiple
cores from the same
+ * collection does not increase the number of watches.
+ *
+ * @param collection the collection that the core is a member of
+ *
+ * @see ZkStateReader#unregisterCore(String)
+ */
+ public void registerCore(String collection) {
+AtomicBoolean reconstructState = new AtomicBoolean(false);
+collectionWatches.compute(collection, (k, v) -> {
+ if (v == null) {
+reconstructState.set(true);
+v = new CollectionWatch();
+ }
+ v.coreRefCount++;
+ return v;
+});
+if (reconstructState.get()) {
+ new StateWatcher(collection).refreshAndWatch();
+ synchronized (getUpdateLock()) {
+constructState();
+ }
+}
+ }
+
+ /**
+ * Notify this reader that a local core that is a member of a collection
has been closed.
+ *
+ * Not a public API. This method should only be called from
ZkController.
+ *
+ * If no cores are registered for a collection, and there are no {@link
CollectionStateWatcher}s
+ * for that collection either, the collection watch will be removed.
+ *
+ * @param collection the collection that the core belongs to
+ */
+ public void unregisterCore(String collection) {
+AtomicBoolean reconstructState = new AtomicBoolean(false);
+collectionWatches.compute(collection, (k, v) -> {
+ if (v == null)
+return null;
+ if (v.coreRefCount > 0)
+v.coreRefCount--;
+ if (v.canBeRemoved()) {
+watchedCollectionStates.remove(collection);
+lazyCollectionStates.put(collection, new
LazyCollectionRef(collection));
+reconstructState.set(true);
+return null;
+ }
+ return v;
+});
+if (reconstructState.get()) {
+ synchronized (getUpdateLock()) {
+constructState();
+ }
+}
+ }
+
+ /**
+ * Register a CollectionStateWatcher to be called when the state of a
collection changes
+ *
+ * A given CollectionStateWatcher will be only called once. If you want
to have a persistent watcher,
+ * it should register itself again in its {@link
CollectionStateWatcher#onStateChanged(Set, DocCollection)}
+ * method.
+ */
+ public void registerCollectionStateWatcher(String collection,
CollectionStateWatcher stateWatcher) {
+AtomicBoolean watchSet = new AtomicBoolean(false);
+collectionWatches.compute(collection, (k, v) -> {
+ if (v == null) {
+v = new CollectionWatch();
+watchSet.set(true);
+ }
+ v.stateWatchers.add(stateWatcher);
+ return v;
+});
+if (watchSet.get()) {
+ new StateWatcher(collection).refreshAndWatch();
synchronized (getUpdateLock()) {
constructState();
}
}
}
+ /**
+ * Block until a CollectionStatePredicate returns true, or the wait
times out
+ *
+ * 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.
--- End diff --
It seems like it would be nice to shield callers from doing any kind of
similar mutexing. If you don't want to bother right now, I can come back and
see if I can do something not yucky looking here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/32#discussion_r61622572
--- Diff:
solr/solrj/src/java/org/apache/solr/common/cloud/CollectionStateWatcher.java ---
@@ -0,0 +1,42 @@
+package org.apache.solr.common.cloud;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+*/
+
+import java.util.Set;
+
+/**
+ * Callback registered with {@link
ZkStateReader#registerCollectionStateWatcher(String, CollectionStateWatcher)}
+ * and called whenever the collection state changes.
+ */
--- End diff --
Not sure! ¯\_(ã)_/¯
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on the pull request: https://github.com/apache/lucene-solr/pull/32#issuecomment-215576790 Looking good, a little more high-level feedback. @shalinmangar I think you should take a look also. Have you run the tests extensively? The first time I ran I got a failure, but after that it's been fairly reliable, but I haven't beasted. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/32#discussion_r61510208
--- Diff:
solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java ---
@@ -348,7 +358,13 @@ public JettySolrRunner stopJettySolrRunner(int index)
throws Exception {
return jetty;
}
- protected JettySolrRunner startJettySolrRunner(JettySolrRunner jetty)
throws Exception {
+ /**
+ * Add a previously stopped node back to the cluster
+ * @param jetty a {@link JettySolrRunner} previously returned by {@link
#stopJettySolrRunner(int)}
+ * @return the started node
+ * @throws Exception on error
+ */
+ public JettySolrRunner startJettySolrRunner(JettySolrRunner jetty)
throws Exception {
--- End diff --
Are the changes in this file related to this PR?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/32#discussion_r61510100
--- Diff:
solr/solrj/src/java/org/apache/solr/common/cloud/CollectionStateWatcher.java ---
@@ -0,0 +1,42 @@
+package org.apache.solr.common.cloud;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+*/
+
+import java.util.Set;
+
+/**
+ * Callback registered with {@link
ZkStateReader#registerCollectionStateWatcher(String, CollectionStateWatcher)}
+ * and called whenever the collection state changes.
+ */
--- End diff --
If we're not going to be firing events on all watchers whenever live_nodes
changes, we should be very clear about this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/32#discussion_r61509937
--- Diff:
solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java ---
@@ -1066,32 +1079,201 @@ public static String getCollectionPath(String
coll) {
return COLLECTIONS_ZKNODE+"/"+coll + "/state.json";
}
- public void addCollectionWatch(String coll) {
-if (interestingCollections.add(coll)) {
- LOG.info("addZkWatch [{}]", coll);
- new StateWatcher(coll).refreshAndWatch(false);
+ /**
+ * Notify this reader that a local Core is a member of a collection, and
so that collection
+ * state should be watched.
+ *
+ * Not a public API. This method should only be called from
ZkController.
+ *
+ * The number of cores per-collection is tracked, and adding multiple
cores from the same
+ * collection does not increase the number of watches.
+ *
+ * @param collection the collection that the core is a member of
+ *
+ * @see ZkStateReader#unregisterCore(String)
+ */
+ public void registerCore(String collection) {
+AtomicBoolean reconstructState = new AtomicBoolean(false);
+collectionWatches.compute(collection, (k, v) -> {
+ if (v == null) {
+reconstructState.set(true);
+v = new CollectionWatch();
+ }
+ v.coreRefCount++;
+ return v;
+});
+if (reconstructState.get()) {
+ new StateWatcher(collection).refreshAndWatch();
+ synchronized (getUpdateLock()) {
+constructState();
+ }
+}
+ }
+
+ /**
+ * Notify this reader that a local core that is a member of a collection
has been closed.
+ *
+ * Not a public API. This method should only be called from
ZkController.
+ *
+ * If no cores are registered for a collection, and there are no {@link
CollectionStateWatcher}s
+ * for that collection either, the collection watch will be removed.
+ *
+ * @param collection the collection that the core belongs to
+ */
+ public void unregisterCore(String collection) {
+AtomicBoolean reconstructState = new AtomicBoolean(false);
+collectionWatches.compute(collection, (k, v) -> {
+ if (v == null)
+return null;
+ if (v.coreRefCount > 0)
+v.coreRefCount--;
+ if (v.canBeRemoved()) {
+watchedCollectionStates.remove(collection);
+lazyCollectionStates.put(collection, new
LazyCollectionRef(collection));
+reconstructState.set(true);
+return null;
+ }
+ return v;
+});
+if (reconstructState.get()) {
+ synchronized (getUpdateLock()) {
+constructState();
+ }
+}
+ }
+
+ /**
+ * Register a CollectionStateWatcher to be called when the state of a
collection changes
+ *
+ * A given CollectionStateWatcher will be only called once. If you want
to have a persistent watcher,
+ * it should register itself again in its {@link
CollectionStateWatcher#onStateChanged(Set, DocCollection)}
+ * method.
+ */
+ public void registerCollectionStateWatcher(String collection,
CollectionStateWatcher stateWatcher) {
+AtomicBoolean watchSet = new AtomicBoolean(false);
+collectionWatches.compute(collection, (k, v) -> {
+ if (v == null) {
+v = new CollectionWatch();
+watchSet.set(true);
+ }
+ v.stateWatchers.add(stateWatcher);
+ return v;
+});
+if (watchSet.get()) {
+ new StateWatcher(collection).refreshAndWatch();
synchronized (getUpdateLock()) {
constructState();
}
}
}
+ /**
+ * Block until a CollectionStatePredicate returns true, or the wait
times out
+ *
+ * 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.
+ *
+ * @param collection the collection to watch
+ * @param wait how long to wait
+ * @param unit the units of the wait parameter
+ * @param predicate the predicate to call on state changes
+ * @throws InterruptedException on interrupt
+ * @throws TimeoutException on timeout
+ */
+ public void waitForState(final String collection, long wait, TimeUnit
unit, CollectionStatePredicate predicate)
+ throws InterruptedException, TimeoutException {
+
+final CountDownLatch latch = new CountDownLatch(1);
+
+CollectionStateWatcher watcher = new CollectionStateWatcher() {
+ @Override
+
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/32#discussion_r61509699
--- Diff:
solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java ---
@@ -1066,32 +1079,201 @@ public static String getCollectionPath(String
coll) {
return COLLECTIONS_ZKNODE+"/"+coll + "/state.json";
}
- public void addCollectionWatch(String coll) {
-if (interestingCollections.add(coll)) {
- LOG.info("addZkWatch [{}]", coll);
- new StateWatcher(coll).refreshAndWatch(false);
+ /**
+ * Notify this reader that a local Core is a member of a collection, and
so that collection
+ * state should be watched.
+ *
+ * Not a public API. This method should only be called from
ZkController.
+ *
+ * The number of cores per-collection is tracked, and adding multiple
cores from the same
+ * collection does not increase the number of watches.
+ *
+ * @param collection the collection that the core is a member of
+ *
+ * @see ZkStateReader#unregisterCore(String)
+ */
+ public void registerCore(String collection) {
+AtomicBoolean reconstructState = new AtomicBoolean(false);
+collectionWatches.compute(collection, (k, v) -> {
+ if (v == null) {
+reconstructState.set(true);
+v = new CollectionWatch();
+ }
+ v.coreRefCount++;
+ return v;
+});
+if (reconstructState.get()) {
+ new StateWatcher(collection).refreshAndWatch();
+ synchronized (getUpdateLock()) {
+constructState();
+ }
+}
+ }
+
+ /**
+ * Notify this reader that a local core that is a member of a collection
has been closed.
+ *
+ * Not a public API. This method should only be called from
ZkController.
+ *
+ * If no cores are registered for a collection, and there are no {@link
CollectionStateWatcher}s
+ * for that collection either, the collection watch will be removed.
+ *
+ * @param collection the collection that the core belongs to
+ */
+ public void unregisterCore(String collection) {
+AtomicBoolean reconstructState = new AtomicBoolean(false);
+collectionWatches.compute(collection, (k, v) -> {
+ if (v == null)
+return null;
+ if (v.coreRefCount > 0)
+v.coreRefCount--;
+ if (v.canBeRemoved()) {
+watchedCollectionStates.remove(collection);
+lazyCollectionStates.put(collection, new
LazyCollectionRef(collection));
+reconstructState.set(true);
+return null;
+ }
+ return v;
+});
+if (reconstructState.get()) {
+ synchronized (getUpdateLock()) {
+constructState();
+ }
+}
+ }
+
+ /**
+ * Register a CollectionStateWatcher to be called when the state of a
collection changes
+ *
+ * A given CollectionStateWatcher will be only called once. If you want
to have a persistent watcher,
+ * it should register itself again in its {@link
CollectionStateWatcher#onStateChanged(Set, DocCollection)}
+ * method.
+ */
+ public void registerCollectionStateWatcher(String collection,
CollectionStateWatcher stateWatcher) {
+AtomicBoolean watchSet = new AtomicBoolean(false);
+collectionWatches.compute(collection, (k, v) -> {
+ if (v == null) {
+v = new CollectionWatch();
+watchSet.set(true);
+ }
+ v.stateWatchers.add(stateWatcher);
+ return v;
+});
+if (watchSet.get()) {
+ new StateWatcher(collection).refreshAndWatch();
synchronized (getUpdateLock()) {
constructState();
}
}
}
+ /**
+ * Block until a CollectionStatePredicate returns true, or the wait
times out
+ *
+ * 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.
--- End diff --
I think we could tighten this code up to ensure that predicate never gets
call concurrently from two different threads at the same time, this would
simplify things for clients and handle the case of calling it twice when it
succeeds immediately.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-un
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/32#discussion_r61508998
--- Diff:
solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java ---
@@ -1066,32 +1079,201 @@ public static String getCollectionPath(String
coll) {
return COLLECTIONS_ZKNODE+"/"+coll + "/state.json";
}
- public void addCollectionWatch(String coll) {
-if (interestingCollections.add(coll)) {
- LOG.info("addZkWatch [{}]", coll);
- new StateWatcher(coll).refreshAndWatch(false);
+ /**
+ * Notify this reader that a local Core is a member of a collection, and
so that collection
+ * state should be watched.
+ *
+ * Not a public API. This method should only be called from
ZkController.
+ *
+ * The number of cores per-collection is tracked, and adding multiple
cores from the same
+ * collection does not increase the number of watches.
+ *
+ * @param collection the collection that the core is a member of
+ *
+ * @see ZkStateReader#unregisterCore(String)
+ */
+ public void registerCore(String collection) {
+AtomicBoolean reconstructState = new AtomicBoolean(false);
+collectionWatches.compute(collection, (k, v) -> {
+ if (v == null) {
+reconstructState.set(true);
+v = new CollectionWatch();
+ }
+ v.coreRefCount++;
+ return v;
+});
+if (reconstructState.get()) {
+ new StateWatcher(collection).refreshAndWatch();
+ synchronized (getUpdateLock()) {
+constructState();
+ }
+}
+ }
+
+ /**
+ * Notify this reader that a local core that is a member of a collection
has been closed.
+ *
+ * Not a public API. This method should only be called from
ZkController.
+ *
+ * If no cores are registered for a collection, and there are no {@link
CollectionStateWatcher}s
+ * for that collection either, the collection watch will be removed.
+ *
+ * @param collection the collection that the core belongs to
+ */
+ public void unregisterCore(String collection) {
+AtomicBoolean reconstructState = new AtomicBoolean(false);
+collectionWatches.compute(collection, (k, v) -> {
+ if (v == null)
+return null;
+ if (v.coreRefCount > 0)
+v.coreRefCount--;
+ if (v.canBeRemoved()) {
+watchedCollectionStates.remove(collection);
+lazyCollectionStates.put(collection, new
LazyCollectionRef(collection));
+reconstructState.set(true);
+return null;
+ }
+ return v;
+});
+if (reconstructState.get()) {
+ synchronized (getUpdateLock()) {
+constructState();
+ }
+}
+ }
+
+ /**
+ * Register a CollectionStateWatcher to be called when the state of a
collection changes
+ *
+ * A given CollectionStateWatcher will be only called once. If you want
to have a persistent watcher,
+ * it should register itself again in its {@link
CollectionStateWatcher#onStateChanged(Set, DocCollection)}
+ * method.
+ */
+ public void registerCollectionStateWatcher(String collection,
CollectionStateWatcher stateWatcher) {
+AtomicBoolean watchSet = new AtomicBoolean(false);
+collectionWatches.compute(collection, (k, v) -> {
+ if (v == null) {
+v = new CollectionWatch();
+watchSet.set(true);
+ }
+ v.stateWatchers.add(stateWatcher);
+ return v;
+});
+if (watchSet.get()) {
+ new StateWatcher(collection).refreshAndWatch();
synchronized (getUpdateLock()) {
constructState();
}
}
}
+ /**
+ * Block until a CollectionStatePredicate returns true, or the wait
times out
+ *
+ * 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.
+ *
+ * @param collection the collection to watch
+ * @param wait how long to wait
+ * @param unit the units of the wait parameter
+ * @param predicate the predicate to call on state changes
+ * @throws InterruptedException on interrupt
+ * @throws TimeoutException on timeout
+ */
+ public void waitForState(final String collection, long wait, TimeUnit
unit, CollectionStatePredicate predicate)
--- End diff --
@shalinmangar this is what I was referring to, I know you're working on
getting Overseer to not peg ZK polling for state changes on unwatched
collections, this PR provides an easy mechanism to temporarily watch
coll
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request: https://github.com/apache/lucene-solr/pull/32#discussion_r61507961 --- Diff: solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java --- @@ -131,6 +132,19 @@ private final Runnable securityNodeListener; + private Map<String, CollectionWatch> collectionWatches = new ConcurrentHashMap<>(); --- End diff -- The reason I made a comment about using the concrete type here is that it makes it much easier to work with as a developer. When you can see that the static type of this variable is ConcurrentHashMap, that helps you evaluate the code that touches it. For example, when you use IDE features to 'click through' a method call or view the javadoc on a called method, you get the ConcurrentHashMap version of the javadoc/method instead of the Map version, which helps you more easily evaluate the correctness. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/32#discussion_r61507382
--- Diff:
solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java ---
@@ -484,6 +498,12 @@ private void refreshLegacyClusterState(Watcher watcher)
}
this.legacyCollectionStates = loadedData.getCollectionStates();
this.legacyClusterStateVersion = stat.getVersion();
+for (Map.Entry<String, ClusterState.CollectionRef> entry :
this.legacyCollectionStates.entrySet()) {
+ if (entry.getValue().isLazilyLoaded() == false) {
+// a watched collection - trigger notifications
+notifyStateWatchers(entry.getKey(), entry.getValue().get());
+ }
+}
--- End diff --
I think it would add a lot of value here to actually check differences.
There really wouldn't be much computational work since you could restrict it to
watched collections. Something like:
```
for (Map.Entry<String, CollectionWatch> watchEntry :
this.collectionWatches.entrySet()) {
String coll = watchEntry.getKey();
CollectionWatch collWatch = watchEntry.getValue();
DocCollection newState =
this.legacyCollectionStates.get(coll).get();
if (!collWatch.stateWatchers.isEmpty()
&& !Objects.equals(oldCollectionStates.get(coll).get(),
newState)) {
notifyStateWatchers(coll, newState);
}
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/32#discussion_r61504824
--- Diff:
solr/solrj/src/java/org/apache/solr/common/cloud/DocCollection.java ---
@@ -210,6 +213,38 @@ public Replica getReplica(String coreNodeName) {
return null;
}
+ /**
+ * Check that all replicas in a collection are live
+ *
+ * @see CollectionStatePredicate
+ */
+ public static boolean isFullyActive(Set liveNodes, DocCollection
collectionState) {
+Objects.requireNonNull(liveNodes);
+if (collectionState == null)
+ return false;
+for (Slice slice : collectionState) {
+ for (Replica replica : slice) {
+if (replica.isActive(liveNodes) == false)
+ return false;
+ }
+}
+return true;
+ }
+
+ /**
+ * Returns true if the passed in DocCollection is null
+ *
+ * @see CollectionStatePredicate
+ */
+ public static boolean isDeleted(Set liveNodes, DocCollection
collectionState) {
+return collectionState == null;
+ }
--- End diff --
maybe `exists`? isDeleted implies that it used to exist, but it may have
never been created
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/32#discussion_r61504670
--- Diff:
solr/solrj/src/java/org/apache/solr/common/cloud/CollectionStateWatcher.java ---
@@ -0,0 +1,42 @@
+package org.apache.solr.common.cloud;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+*/
+
+import java.util.Set;
+
+/**
+ * Callback registered with {@link
ZkStateReader#registerCollectionStateWatcher(String, CollectionStateWatcher)}
+ * and called whenever the collection state changes.
+ */
+public interface CollectionStateWatcher {
+
+ /**
+ * Called when the collection we are registered against has a change of
state
+ *
+ * Note that, due to the way Zookeeper watchers are implemented, a
single call may be
+ * the result of several state changes
+ *
+ * A watcher is unregistered after it has been called once. To make a
watcher persistent,
+ * implementors should re-register during this call.
+ *
+ * @param liveNodes the set of live nodes
+ * @param collectionState the new collection state
+ */
+ void onStateChanged(Set liveNodes, DocCollection
collectionState);
+
+}
--- End diff --
I just want to toss out an idea here after looking at this some more. I
notice that CollectionStateWatcher and CollectionStatePredicate are nearly
identical. What would you think about combining the two into a single
interface?
The signature could be e.g.:
bool stateChanged(liveNodes, collectionState)
In a watch context, the return value means "keep watching?". So return
true to reset the watcher and continue getting updates, or return false to stop
watching.
In a predicate context, the return value means "keep waiting?". So return
true to keep waiting, or return false if you've finally seen what you were
waiting for.
They'll both have the same semantic meaning either way.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/32#discussion_r61504017
--- Diff:
solr/solrj/src/java/org/apache/solr/common/cloud/CollectionStatePredicate.java
---
@@ -0,0 +1,41 @@
+package org.apache.solr.common.cloud;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Interface to determine if a collection state matches a required state
+ *
+ * @see ZkStateReader#waitForState(String, long, TimeUnit,
CollectionStatePredicate)
+ */
+public interface CollectionStatePredicate {
+
+ /**
+ * Check the collection state matches a required state
+ *
+ * The collectionState parameter may be null if the collection does not
exist
+ * or has been deleted
--- End diff --
This wants to be `@param collectionState the current collection state, or
null if the collection doesn't exist or has been deleted`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request: SOLR-8323
Github user dragonsinth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/32#discussion_r61503724
--- Diff:
solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java ---
@@ -572,6 +574,40 @@ public void downloadConfig(String configName, Path
downloadPath) throws IOExcept
zkStateReader.getConfigManager().downloadConfigDir(configName,
downloadPath);
}
+ /**
+ * Block until a collection state matches a predicate, or a timeout
+ *
+ * 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.
+ *
+ * @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
+ */
+ public void waitForState(String collection, long wait, TimeUnit unit,
CollectionStatePredicate predicate)
+ throws InterruptedException, TimeoutException {
+connect();
+zkStateReader.waitForState(collection, wait, unit, predicate);
+ }
+
+ /**
+ * Register a CollectionStateWatcher to be called when the cluster state
for a collection changes
+ *
+ * Note that the watcher is unregistered after it has been called once.
To make a watcher persistent,
+ * it should re-register itself in its {@link
CollectionStateWatcher#onStateChanged(Set, DocCollection)}
+ * call
+ *
+ * @param collection the collection to watch
+ * @param watchera watcher that will be called when the state changes
+ */
+ public void registerCollectionStateWatcher(String collection,
CollectionStateWatcher watcher) {
+connect();
+zkStateReader.registerCollectionStateWatcher(collection, watcher);
+ }
+
--- End diff --
I would note that getZkStateReader() is a public method, is there value in
adding these forwarding methods?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
