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/07/27 23:25:28 UTC

[lucene-solr] branch reference_impl updated: @409 Tweaking on close.

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

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


The following commit(s) were added to refs/heads/reference_impl by this push:
     new 1931123  @409 Tweaking on close.
1931123 is described below

commit 1931123978b8b70d3d628e8ab8df7f796bdbb4d8
Author: markrmiller@gmail.com <ma...@gmail.com>
AuthorDate: Mon Jul 27 18:24:29 2020 -0500

    @409 Tweaking on close.
---
 .../solr/client/solrj/impl/CloudHttp2SolrClient.java   |  4 ++--
 .../apache/solr/client/solrj/impl/CloudSolrClient.java |  3 ++-
 .../solrj/impl/ZkClientClusterStateProvider.java       | 13 +++++--------
 .../apache/solr/common/cloud/ConnectionManager.java    |  4 ++--
 .../org/apache/solr/common/cloud/SolrZkClient.java     | 18 +++++++++++++++++-
 .../org/apache/solr/common/cloud/SolrZooKeeper.java    |  2 +-
 .../org/apache/solr/common/cloud/ZkStateReader.java    |  2 +-
 .../src/java/org/apache/solr/SolrTestCase.java         |  2 +-
 8 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java
index 1617b80..7fd1bbf 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java
@@ -71,7 +71,7 @@ public class CloudHttp2SolrClient  extends BaseCloudSolrClient {
       if (builder.zkHosts != null) {
         this.zkStateReader = new ZkStateReader(ZkClientClusterStateProvider.buildZkHostString(builder.zkHosts, builder.zkChroot), 40000, 15000);
         this.zkStateReader.createClusterStateWatchersAndUpdate();
-        this.stateProvider = new ZkClientClusterStateProvider(zkStateReader, true);
+        this.stateProvider = new ZkClientClusterStateProvider(zkStateReader, false);
       } else if (builder.solrUrls != null && !builder.solrUrls.isEmpty()) {
         try {
           this.stateProvider = new Http2ClusterStateProvider(builder.solrUrls, builder.httpClient);
@@ -94,7 +94,7 @@ public class CloudHttp2SolrClient  extends BaseCloudSolrClient {
   @Override
   public void close() throws IOException {
     try (ParWork closer = new ParWork(this, true)) {
-      closer.add("CloudHttp2SolrClient#close", stateProvider, lbClient);
+      closer.add("CloudHttp2SolrClient#close", stateProvider, zkStateReader, lbClient);
       if (clientIsInternal && myClient!=null) {
         closer.add("http2Client", myClient);
       }
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java
index 76b81c0..1b62a96 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java
@@ -84,7 +84,7 @@ public class CloudSolrClient extends BaseCloudSolrClient {
       if (builder.zkHosts != null && builder.zkHosts.size() > 0) {
         this.zkStateReader = new ZkStateReader(ZkClientClusterStateProvider.buildZkHostString(builder.zkHosts, builder.zkChroot), 40000, 15000);
         this.zkStateReader.createClusterStateWatchersAndUpdate();
-        this.stateProvider = new ZkClientClusterStateProvider(zkStateReader, true);
+        this.stateProvider = new ZkClientClusterStateProvider(zkStateReader);
       } else if (builder.solrUrls != null && !builder.solrUrls.isEmpty()) {
         try {
           this.stateProvider = new HttpClusterStateProvider(builder.solrUrls, builder.httpClient);
@@ -177,6 +177,7 @@ public class CloudSolrClient extends BaseCloudSolrClient {
       if (clientIsInternal) {
         closer.collect(myClient);
       }
+      closer.collect(zkStateReader);
       closer.addCollect("cloudclient");
     }
     super.close();
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java
index 4b8c3fc..f5fe632 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java
@@ -165,14 +165,11 @@ public class ZkClientClusterStateProvider implements ClusterStateProvider {
   @Override
   public void close() throws IOException {
     synchronized (this) {
-      if (false == isClosed && zkStateReader != null) {
-        isClosed = true;
-        
-        // force zkStateReader to null first so that any parallel calls drop into the synch block 
-        // getZkStateReader() as soon as possible.
-        final ZkStateReader zkToClose = zkStateReader;
-        if (closeZkStateReader) {
-          zkToClose.close();
+      isClosed = true;
+      final ZkStateReader zkToClose = zkStateReader;
+      if (false == isClosed && zkToClose != null) {
+        if (closeZkStateReader && zkStateReader != null) {
+          ParWork.close(zkToClose);
         }
       }
     }
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ConnectionManager.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ConnectionManager.java
index 17ce443..b3f6f3e 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/ConnectionManager.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ConnectionManager.java
@@ -180,7 +180,7 @@ public class ConnectionManager implements Watcher, Closeable {
 
   private synchronized void updatezk() throws IOException {
     if (keeper != null) {
-      keeper.close();
+      ParWork.close(keeper);
     }
     SolrZooKeeper zk = createSolrZooKeeper(zkServerAddress, zkTimeout, this);
     keeper = zk;
@@ -232,7 +232,7 @@ public class ConnectionManager implements Watcher, Closeable {
           // or if we cannot run our reconnect command, close the keeper
           // our retry loop will try to create one again
           try {
-            keeper.close();
+            ParWork.close(keeper);
             keeper = null;
           } catch (Exception e) {
             ParWork.propegateInterrupt("Exception closing keeper after hitting exception", e);
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java b/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java
index 918d22d..8338cb0 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java
@@ -178,11 +178,26 @@ public class SolrZkClient implements Closeable {
       connManager.start();
       connManager.waitForConnected(this.zkClientConnectTimeout);
     } catch (TimeoutException e) {
+      try (ParWork worker = new ParWork(this, true)) {
+        worker.add("zkCallbackExecutor", zkCallbackExecutor);
+        worker.add("connectionManager", connManager);
+        worker.add("zkCallbackExecutor", zkConnManagerCallbackExecutor);
+      }
       throw new SolrException(SolrException.ErrorCode.SERVICE_UNAVAILABLE, e);
     } catch (InterruptedException e) {
       ParWork.propegateInterrupt(e);
+      try (ParWork worker = new ParWork(this, true)) {
+        worker.add("zkCallbackExecutor", zkCallbackExecutor);
+        worker.add("connectionManager", connManager);
+        worker.add("zkCallbackExecutor", zkConnManagerCallbackExecutor);
+      }
       throw new SolrException(SolrException.ErrorCode.SERVICE_UNAVAILABLE, e);
     } catch (IOException e) {
+      try (ParWork worker = new ParWork(this, true)) {
+        worker.add("zkCallbackExecutor", zkCallbackExecutor);
+        worker.add("connectionManager", connManager);
+        worker.add("zkCallbackExecutor", zkConnManagerCallbackExecutor);
+      }
       throw new SolrException(SolrException.ErrorCode.SERVICE_UNAVAILABLE, e);
     }
     return this;
@@ -850,8 +865,9 @@ public class SolrZkClient implements Closeable {
     isClosed = true;
 
     try (ParWork worker = new ParWork(this, true)) {
-      worker.add("zkExecutors", zkCallbackExecutor, zkConnManagerCallbackExecutor);
+      worker.add("zkCallbackExecutor", zkCallbackExecutor);
       worker.add("connectionManager", connManager);
+      worker.add("zkCallbackExecutor", zkConnManagerCallbackExecutor);
     }
     assert ObjectReleaseTracker.release(this);
   }
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZooKeeper.java b/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZooKeeper.java
index 5ac7f68..ae086ab 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZooKeeper.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZooKeeper.java
@@ -120,7 +120,7 @@ public class SolrZooKeeper extends ZooKeeper {
     } catch (InterruptedException e) {
       ParWork.propegateInterrupt(e);
     }
-             ZooKeeperExposed exposed = new ZooKeeperExposed(this, cnxn);
+     ZooKeeperExposed exposed = new ZooKeeperExposed(this, cnxn);
      //exposed.intteruptSendThread();
   //  exposed.interruptEventThread();
    // exposed.interruptSendThread();
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
index 8a7f5de..52ad1b5 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
@@ -904,7 +904,7 @@ public class ZkStateReader implements SolrCloseable {
     closeTracker.close();
     this.closed = true;
 
-    try (ParWork closer = new ParWork(this)) {
+    try (ParWork closer = new ParWork(this, true)) {
       notifications.shutdown();
       collectionPropsNotifications.shutdown();
 
diff --git a/solr/test-framework/src/java/org/apache/solr/SolrTestCase.java b/solr/test-framework/src/java/org/apache/solr/SolrTestCase.java
index 3b9041a..4a4fe57 100644
--- a/solr/test-framework/src/java/org/apache/solr/SolrTestCase.java
+++ b/solr/test-framework/src/java/org/apache/solr/SolrTestCase.java
@@ -394,7 +394,7 @@ public class SolrTestCase extends LuceneTestCase {
 
       SysStats.getSysStats().stopMonitor();
 
-      if (!failed) {
+      if (!failed && suiteFailureMarker.wasSuccessful() ) {
         // if the tests passed, make sure everything was closed / released
         String orr = ObjectReleaseTracker.checkEmpty();
         ObjectReleaseTracker.clear();