You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ma...@apache.org on 2020/08/11 03:07:57 UTC

[lucene-solr] branch reference_impl_dev updated (3947349 -> 42d72fa)

This is an automated email from the ASF dual-hosted git repository.

markrmiller pushed a change to branch reference_impl_dev
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git.


    from 3947349  @490 Work on missing imports / forbidden apis.
     new 5a4ddc7  @491 Tweaks around jetty stop in tests.
     new 42d72fa  @492 ZkShardTerms improvement.

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../client/solrj/embedded/JettySolrRunner.java     | 15 +-----
 .../java/org/apache/solr/cloud/ZkShardTerms.java   | 63 +++++++---------------
 .../apache/solr/handler/admin/PrepRecoveryOp.java  |  2 +-
 .../solr/cloud/DeleteInactiveReplicaTest.java      |  5 --
 .../solr/common/util/SolrQueuedThreadPool.java     |  7 +--
 .../org/apache/solr/cloud/SolrCloudTestCase.java   |  2 +-
 6 files changed, 25 insertions(+), 69 deletions(-)


[lucene-solr] 01/02: @491 Tweaks around jetty stop in tests.

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markrmiller pushed a commit to branch reference_impl_dev
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit 5a4ddc736cbbddcc3846241f195a60190a44606c
Author: markrmiller@gmail.com <ma...@gmail.com>
AuthorDate: Mon Aug 10 22:07:03 2020 -0500

    @491 Tweaks around jetty stop in tests.
---
 .../solr/client/solrj/embedded/JettySolrRunner.java       | 15 +--------------
 .../org/apache/solr/cloud/DeleteInactiveReplicaTest.java  |  5 -----
 .../org/apache/solr/common/util/SolrQueuedThreadPool.java |  7 ++++---
 .../src/java/org/apache/solr/cloud/SolrCloudTestCase.java |  2 +-
 4 files changed, 6 insertions(+), 23 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java b/solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java
index 38c12ef..ad89877 100644
--- a/solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java
+++ b/solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java
@@ -758,19 +758,13 @@ public class JettySolrRunner implements Closeable {
         closer.addCollect("stopServer");
       }
 
-
       try {
-
         server.join();
       } catch (InterruptedException e) {
         SolrZkClient.checkInterrupted(e);
         throw new RuntimeException(e);
       }
 
-      if (config.qtp == null) {
-      //  qtp.stop();
-      }
-
     } catch (Exception e) {
       SolrZkClient.checkInterrupted(e);
       log.error("", e);
@@ -802,14 +796,7 @@ public class JettySolrRunner implements Closeable {
           throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
         }
       }
-//      if (server.getState().equals(Server.FAILED)) {
-//        if (filter != null) filter.destroy();
-//        if (extraFilters != null) {
-//          for (FilterHolder f : extraFilters) {
-//            f.getFilter().destroy();
-//          }
-//        }
-//      }
+
       assert ObjectReleaseTracker.release(this);
       if (prevContext != null) {
         MDC.setContextMap(prevContext);
diff --git a/solr/core/src/test/org/apache/solr/cloud/DeleteInactiveReplicaTest.java b/solr/core/src/test/org/apache/solr/cloud/DeleteInactiveReplicaTest.java
index 5fe963d..9ddd965 100644
--- a/solr/core/src/test/org/apache/solr/cloud/DeleteInactiveReplicaTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/DeleteInactiveReplicaTest.java
@@ -72,11 +72,6 @@ public class DeleteInactiveReplicaTest extends SolrCloudTestCase {
 
     cluster.stopJettySolrRunner(jetty);
 
-    waitForState("Expected replica " + replica.getName() + " on down node to be removed from cluster state", collectionName, (n, c) -> {
-      Replica r = c.getReplica(replica.getCoreName());
-      return r == null || r.getState() != Replica.State.ACTIVE;
-    });
-
     cluster.getSolrClient().getZkStateReader().waitForState(collectionName, 10, TimeUnit.SECONDS, (n,c)->{
       if (c == null) return false;
       Replica rep = c.getReplica(replica.getName());
diff --git a/solr/solrj/src/java/org/apache/solr/common/util/SolrQueuedThreadPool.java b/solr/solrj/src/java/org/apache/solr/common/util/SolrQueuedThreadPool.java
index f21aa13..4a661c7 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/SolrQueuedThreadPool.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/SolrQueuedThreadPool.java
@@ -1015,8 +1015,8 @@ public class SolrQueuedThreadPool extends ContainerLifeCycle implements ThreadFa
             jobs.offer(NOOP);
         }
 
-        // try to let jobs complete naturally for half our stop time
-        joinThreads( TimeUnit.MILLISECONDS.toNanos(10000));
+        // try to let jobs complete naturally our stop time
+        joinThreads( TimeUnit.MILLISECONDS.toNanos(getStopTimeout()));
 
     }
 
@@ -1027,10 +1027,11 @@ public class SolrQueuedThreadPool extends ContainerLifeCycle implements ThreadFa
                 stop();
             }
             while (isStopping()) {
-                Thread.sleep(1);
+                Thread.sleep(10);
             }
         } catch (Exception e) {
             ParWork.propegateInterrupt("Exception closing", e);
+            throw new RuntimeException(e);
         }
 
         assert ObjectReleaseTracker.release(this);
diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java b/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java
index e6f05b6..a6b35f0 100644
--- a/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java
+++ b/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java
@@ -312,7 +312,7 @@ public class SolrCloudTestCase extends SolrTestCaseJ4 {
           try {
             qtp.stop();
           } catch (Exception e) {
-            log.error("Error stopping qtp", e);
+            ParWork.propegateInterrupt(e);
           }
         });
         closer.collect(() -> {


[lucene-solr] 02/02: @492 ZkShardTerms improvement.

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markrmiller pushed a commit to branch reference_impl_dev
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit 42d72fa44e5d144f6064362c999f1045cfaaa406
Author: markrmiller@gmail.com <ma...@gmail.com>
AuthorDate: Mon Aug 10 22:07:29 2020 -0500

    @492 ZkShardTerms improvement.
---
 .../java/org/apache/solr/cloud/ZkShardTerms.java   | 63 +++++++---------------
 .../apache/solr/handler/admin/PrepRecoveryOp.java  |  2 +-
 2 files changed, 19 insertions(+), 46 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkShardTerms.java b/solr/core/src/java/org/apache/solr/cloud/ZkShardTerms.java
index ef90260..4dafb63 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ZkShardTerms.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ZkShardTerms.java
@@ -26,12 +26,14 @@ import org.apache.solr.common.util.ObjectReleaseTracker;
 import org.apache.solr.common.util.Utils;
 import org.apache.solr.core.CoreDescriptor;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
 import org.apache.zookeeper.data.Stat;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.lang.invoke.MethodHandles;
+import java.sql.Connection;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
@@ -73,7 +75,7 @@ public class ZkShardTerms implements AutoCloseable{
   private final Set<CoreTermWatcher> listeners = ConcurrentHashMap.newKeySet();
   private final AtomicBoolean isClosed = new AtomicBoolean(false);
 
-  private AtomicReference<ShardTerms> terms = new AtomicReference<>();
+  private final AtomicReference<ShardTerms> terms = new AtomicReference<>();
 
   /**
    * Listener of a core for shard's term change events
@@ -99,8 +101,7 @@ public class ZkShardTerms implements AutoCloseable{
     this.collection = collection;
     this.shard = shard;
     this.zkClient = zkClient;
-    refreshTerms();
-    retryRegisterWatcher();
+    registerWatcher();
     ObjectReleaseTracker.track(this);
   }
 
@@ -308,7 +309,7 @@ public class ZkShardTerms implements AutoCloseable{
       return true;
     } catch (KeeperException.BadVersionException e) {
       log.info("Failed to save terms, version is not a match, retrying");
-      refreshTerms();
+      // TODO: wait till next version shows up
     } catch (KeeperException.NoNodeException e) {
       throw e;
     } catch (Exception e) {
@@ -321,14 +322,13 @@ public class ZkShardTerms implements AutoCloseable{
   /**
    * Fetch latest terms from ZK
    */
-  public void refreshTerms() {
+  public void refreshTerms(Watcher watcher) {
     ShardTerms newTerms;
     try {
       Stat stat = new Stat();
-      byte[] data = zkClient.getData(znodePath, null, stat);
+      byte[] data = zkClient.getData(znodePath, watcher, stat);
       newTerms = new ShardTerms((Map<String, Long>) Utils.fromJSON(data), stat.getVersion());
     } catch (KeeperException e) {
-      Thread.interrupted();
       throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Error updating shard term for collection: " + collection, e);
     } catch (InterruptedException e) {
       ParWork.propegateInterrupt(e);
@@ -339,49 +339,22 @@ public class ZkShardTerms implements AutoCloseable{
   }
 
   /**
-   * Retry register a watcher to the correspond ZK term node
+   * Register a watcher to the correspond ZK term node
    */
-  private void retryRegisterWatcher() {
-    while (!isClosed.get()) {
-      try {
-        registerWatcher();
-        return;
-      } catch (KeeperException.SessionExpiredException | KeeperException.AuthFailedException e) {
-        isClosed.set(true);
-        log.error("Failed watching shard term for collection: {} due to unrecoverable exception", collection, e);
-        return;
-      } catch (KeeperException e) {
-        log.warn("Failed watching shard term for collection: {}, retrying!", collection, e);
-        try {
-          zkClient.getConnectionManager().waitForConnected(zkClient.getZkClientTimeout());
-        } catch (TimeoutException | InterruptedException te) {
-          ParWork.propegateInterrupt(te);
-          throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Error watching shard term for collection: " + collection, te);
+  private void registerWatcher() {
+    Watcher watcher = new Watcher() {
+      @Override
+      public void process(WatchedEvent event) {
+        // session events are not change events, and do not remove the watcher
+        if (Watcher.Event.EventType.None == event.getType()) {
+          return;
         }
-      }
-    }
-  }
 
-  /**
-   * Register a watcher to the correspond ZK term node
-   */
-  private void registerWatcher() throws KeeperException {
-    Watcher watcher = event -> {
-      // session events are not change events, and do not remove the watcher
-      if (Watcher.Event.EventType.None == event.getType()) {
-        return;
+        // Some events may be missed during register a watcher, so it is safer to refresh terms after registering watcher
+        refreshTerms(this);
       }
-      retryRegisterWatcher();
-      // Some events may be missed during register a watcher, so it is safer to refresh terms after registering watcher
-      refreshTerms();
     };
-    try {
-      // exists operation is faster than getData operation
-      zkClient.exists(znodePath, watcher);
-    } catch (InterruptedException e) {
-      ParWork.propegateInterrupt(e);
-      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Error watching shard term for collection: " + collection, e);
-    }
+    refreshTerms(watcher);
   }
 
 
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/PrepRecoveryOp.java b/solr/core/src/java/org/apache/solr/handler/admin/PrepRecoveryOp.java
index b784a03..342e371 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/PrepRecoveryOp.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/PrepRecoveryOp.java
@@ -126,7 +126,7 @@ class PrepRecoveryOp implements CoreAdminHandler.CoreAdminOp {
               // The replica changed it term, then published itself as RECOVERING.
               // This core already see replica as RECOVERING
               // so it is guarantees that a live-fetch will be enough for this core to see max term published
-              shardTerms.refreshTerms();
+              shardTerms.refreshTerms(null);
             }
 
             boolean onlyIfActiveCheckResult = onlyIfLeaderActive != null && onlyIfLeaderActive