SOLR-5371: Solr should consistently call SolrServer#shutdown

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1534002 13f79535-47bb-0310-9956-ffa450edef68

solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/SolrEntityProcessor.java

@@ -94,13 +94,16 @@ public class SolrEntityProcessor extends EntityProcessorBase {
             "SolrEntityProcessor: parameter 'url' is required");
       }
 
+      // TODO: we should close this client!
       HttpClient client = getHttpClient();
       URL url = new URL(serverPath);
       // (wt="javabin|xml") default is javabin
       if ("xml".equals(context.getResolvedEntityAttribute(CommonParams.WT))) {
+        // TODO: it doesn't matter for this impl when passing a client currently, but we should shutdown this!
         solrServer = new HttpSolrServer(url.toExternalForm(), client, new XMLResponseParser());
         LOG.info("using XMLResponseParser");
       } else {
+        // TODO: it doesn't matter for this impl when passing a client currently, but we should shutdown this!
         solrServer = new HttpSolrServer(url.toExternalForm(), client);
         LOG.info("using BinaryResponseParser");
       }

solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestSolrEntityProcessorEndToEnd.java

@@ -282,10 +282,14 @@ public class TestSolrEntityProcessorEndToEnd extends AbstractDataImportHandlerTe
     }
     
     HttpSolrServer solrServer = new HttpSolrServer(getSourceUrl());
+    try {
       solrServer.setConnectionTimeout(15000);
       solrServer.setSoTimeout(30000);
       solrServer.add(sidl);
       solrServer.commit(true, true);
+    } finally {
+      solrServer.shutdown();
+    }
   }
   
   private static class SolrInstance {

solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java

@@ -168,8 +168,10 @@ public class RecoveryStrategy extends Thread implements ClosableThread {
     
   }
 
-  private void commitOnLeader(String leaderUrl) throws SolrServerException, IOException {
+  private void commitOnLeader(String leaderUrl) throws SolrServerException,
+      IOException {
     HttpSolrServer server = new HttpSolrServer(leaderUrl);
+    try {
       server.setConnectionTimeout(30000);
       server.setSoTimeout(60000);
       UpdateRequest ureq = new UpdateRequest();
@@ -178,13 +180,15 @@ public class RecoveryStrategy extends Thread implements ClosableThread {
       ureq.getParams().set(UpdateParams.OPEN_SEARCHER, false);
       ureq.setAction(AbstractUpdateRequest.ACTION.COMMIT, false, true).process(
           server);
+    } finally {
       server.shutdown();
     }
+  }
 
-  private void sendPrepRecoveryCmd(String leaderBaseUrl,
+  private void sendPrepRecoveryCmd(String leaderBaseUrl, String leaderCoreName)
-      String leaderCoreName) throws SolrServerException,
+      throws SolrServerException, IOException {
-      IOException {
     HttpSolrServer server = new HttpSolrServer(leaderBaseUrl);
+    try {
       server.setConnectionTimeout(45000);
       server.setSoTimeout(120000);
       WaitForState prepCmd = new WaitForState();
@@ -196,8 +200,10 @@ public class RecoveryStrategy extends Thread implements ClosableThread {
       prepCmd.setOnlyIfLeader(true);
       
       server.request(prepCmd);
+    } finally {
       server.shutdown();
     }
+  }
 
   @Override
   public void run() {

solr/core/src/java/org/apache/solr/cloud/SyncStrategy.java

@@ -285,12 +285,14 @@ public class SyncStrategy {
         recoverRequestCmd.setCoreName(coreName);
         
         HttpSolrServer server = new HttpSolrServer(baseUrl, client);
+        try {
           server.setConnectionTimeout(15000);
           server.setSoTimeout(60000);
-        try {
           server.request(recoverRequestCmd);
         } catch (Throwable t) {
           SolrException.log(log, ZkCoreNodeProps.getCoreUrl(leaderProps) + ": Could not tell a replica to recover", t);
+        } finally {
+          server.shutdown();
         }
       }
     };

solr/core/src/java/org/apache/solr/handler/SnapPuller.java

@@ -75,12 +75,15 @@ import org.apache.lucene.index.IndexWriter;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.IndexInput;
 import org.apache.lucene.store.IndexOutput;
+
 import static org.apache.lucene.util.IOUtils.CHARSET_UTF_8;
+
 import org.apache.solr.client.solrj.SolrServerException;
 import org.apache.solr.client.solrj.impl.HttpClientUtil;
 import org.apache.solr.client.solrj.impl.HttpSolrServer;
 import org.apache.solr.client.solrj.request.QueryRequest;
 import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrException.ErrorCode;
 import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.params.SolrParams;
@@ -247,13 +250,18 @@ public class SnapPuller {
     params.set(CommonParams.QT, "/replication");
     QueryRequest req = new QueryRequest(params);
     HttpSolrServer server = new HttpSolrServer(masterUrl, myHttpClient); //XXX modify to use shardhandler
+    NamedList rsp;
+    try {
       server.setSoTimeout(60000);
       server.setConnectionTimeout(15000);
-    try {
+      
-      return server.request(req);
+      rsp = server.request(req);
     } catch (SolrServerException e) {
-      throw new IOException(e);
+      throw new SolrException(ErrorCode.SERVER_ERROR, e.getMessage());
+    } finally {
+      server.shutdown();
     }
+    return rsp;
   }
 
   /**
@@ -267,10 +275,9 @@ public class SnapPuller {
     params.set(CommonParams.QT, "/replication");
     QueryRequest req = new QueryRequest(params);
     HttpSolrServer server = new HttpSolrServer(masterUrl, myHttpClient);  //XXX modify to use shardhandler
+    try {
       server.setSoTimeout(60000);
       server.setConnectionTimeout(15000);
-
-    try {
       NamedList response = server.request(req);
 
       List<Map<String, Object>> files = (List<Map<String,Object>>) response.get(CMD_GET_FILE_LIST);
@@ -287,6 +294,8 @@ public class SnapPuller {
 
     } catch (SolrServerException e) {
       throw new IOException(e);
+    } finally {
+      server.shutdown();
     }
   }
 
@@ -1273,9 +1282,7 @@ public class SnapPuller {
      * Open a new stream using HttpClient
      */
     FastInputStream getStream() throws IOException {
-      HttpSolrServer s = new HttpSolrServer(masterUrl, myHttpClient, null);  //XXX use shardhandler
+
-      s.setSoTimeout(60000);
-      s.setConnectionTimeout(15000);
       ModifiableSolrParams params = new ModifiableSolrParams();
 
 //    //the method is command=filecontent
@@ -1307,7 +1314,11 @@ public class SnapPuller {
 
       NamedList response;
       InputStream is = null;
+      
+      HttpSolrServer s = new HttpSolrServer(masterUrl, myHttpClient, null);  //XXX use shardhandler
       try {
+        s.setSoTimeout(60000);
+        s.setConnectionTimeout(15000);
         QueryRequest req = new QueryRequest(params);
         response = s.request(req);
         is = (InputStream) response.get("stream");
@@ -1319,6 +1330,8 @@ public class SnapPuller {
         //close stream on error
         IOUtils.closeQuietly(is);
         throw new IOException("Could not download file '" + fileName + "'", t);
+      } finally {
+        s.shutdown();
       }
     }
   }
@@ -1534,9 +1547,7 @@ public class SnapPuller {
      * Open a new stream using HttpClient
      */
     FastInputStream getStream() throws IOException {
-      HttpSolrServer s = new HttpSolrServer(masterUrl, myHttpClient, null);  //XXX use shardhandler
+
-      s.setSoTimeout(60000);
-      s.setConnectionTimeout(15000);
       ModifiableSolrParams params = new ModifiableSolrParams();
 
 //    //the method is command=filecontent
@@ -1568,7 +1579,10 @@ public class SnapPuller {
 
       NamedList response;
       InputStream is = null;
+      HttpSolrServer s = new HttpSolrServer(masterUrl, myHttpClient, null);  //XXX use shardhandler
       try {
+        s.setSoTimeout(60000);
+        s.setConnectionTimeout(15000);
         QueryRequest req = new QueryRequest(params);
         response = s.request(req);
         is = (InputStream) response.get("stream");
@@ -1580,6 +1594,8 @@ public class SnapPuller {
         //close stream on error
         IOUtils.closeQuietly(is);
         throw new IOException("Could not download file '" + fileName + "'", t);
+      } finally {
+        s.shutdown();
       }
     }
   }
@@ -1590,10 +1606,16 @@ public class SnapPuller {
     params.set("slave", false);
     params.set(CommonParams.QT, "/replication");
     HttpSolrServer server = new HttpSolrServer(masterUrl, myHttpClient); //XXX use shardhandler
+    NamedList rsp;
+    try {
       server.setSoTimeout(60000);
       server.setConnectionTimeout(15000);
       QueryRequest request = new QueryRequest(params);
-    return server.request(request);
+      rsp = server.request(request);
+    } finally {
+      server.shutdown();
+    }
+    return rsp;
   }
 
   static Integer readInterval(String interval) {

solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java

@@ -232,6 +232,7 @@ public class CollectionsHandler extends RequestHandlerBase {
     ZkCoreNodeProps nodeProps = new ZkCoreNodeProps(leaderProps);
     
     HttpSolrServer server = new HttpSolrServer(nodeProps.getBaseUrl());
+    try {
       server.setConnectionTimeout(15000);
       server.setSoTimeout(60000);
       RequestSyncShard reqSyncShard = new CoreAdminRequest.RequestSyncShard();
@@ -239,6 +240,9 @@ public class CollectionsHandler extends RequestHandlerBase {
       reqSyncShard.setShard(shard);
       reqSyncShard.setCoreName(nodeProps.getCoreName());
       server.request(reqSyncShard);
+    } finally {
+      server.shutdown();
+    }
   }
   
   private void handleCreateAliasAction(SolrQueryRequest req,

solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java

@@ -152,7 +152,11 @@ public class HttpShardHandler extends ShardHandler {
             String url = urls.get(0);
             srsp.setShardAddress(url);
             SolrServer server = new HttpSolrServer(url, httpClient);
+            try {
               ssr.nl = server.request(req);
+            } finally {
+              server.shutdown();
+            }
           } else {
             LBHttpSolrServer.Rsp rsp = httpShardHandlerFactory.makeLoadBalancedRequest(req, urls);
             ssr.nl = rsp.getResponse();

solr/core/src/java/org/apache/solr/update/SolrCmdDistributor.java

@@ -56,11 +56,13 @@ public class SolrCmdDistributor {
   }
   
   public void finish() {
+    try {
       servers.blockUntilFinished();
       doRetriesIfNeeded();
-  
+    } finally {
       servers.shutdown();
     }
+  }
 
   private void doRetriesIfNeeded() {
     // NOTE: retries will be forwards to a single url
@@ -189,12 +191,14 @@ public class SolrCmdDistributor {
       
       HttpSolrServer server = new HttpSolrServer(req.node.getUrl(),
           servers.getHttpClient());
-
       try {
         server.request(req.uReq);
       } catch (Exception e) {
         throw new SolrException(ErrorCode.SERVER_ERROR, "Failed synchronous update on shard " + req.node + " update: " + req.uReq , e);
+      } finally {
+        server.shutdown();
       }
+      
       return;
     }
     

solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java

@@ -553,6 +553,7 @@ public class DistributedUpdateProcessor extends UpdateRequestProcessor {
         public void run() {
           log.info("try and ask " + recoveryUrl + " to recover");
           HttpSolrServer server = new HttpSolrServer(recoveryUrl);
+          try {
             server.setSoTimeout(60000);
             server.setConnectionTimeout(15000);
             
@@ -562,7 +563,11 @@ public class DistributedUpdateProcessor extends UpdateRequestProcessor {
             try {
               server.request(recoverRequestCmd);
             } catch (Throwable t) {
-            SolrException.log(log, recoveryUrl + ": Could not tell a replica to recover", t);
+              SolrException.log(log, recoveryUrl
+                  + ": Could not tell a replica to recover", t);
+            }
+          } finally {
+            server.shutdown();
           }
         }
       };

solr/core/src/test/org/apache/solr/cloud/BasicDistributedZkTest.java

@@ -923,7 +923,7 @@ public class BasicDistributedZkTest extends AbstractFullDistribZkTestBase {
     Callable call = new Callable() {
       @Override
       public Object call() {
-        HttpSolrServer server;
+        HttpSolrServer server = null;
         try {
           server = new HttpSolrServer(baseUrl);
           server.setConnectionTimeout(15000);
@@ -944,6 +944,10 @@ public class BasicDistributedZkTest extends AbstractFullDistribZkTestBase {
         } catch (Exception e) {
           e.printStackTrace();
           //fail
+        } finally {
+          if (server != null) {
+            server.shutdown();
+          }
         }
         return null;
       }
@@ -1052,7 +1056,7 @@ public class BasicDistributedZkTest extends AbstractFullDistribZkTestBase {
       Callable call = new Callable() {
         @Override
         public Object call() {
-          HttpSolrServer server;
+          HttpSolrServer server = null;
           try {
             server = new HttpSolrServer(baseUrl);
             server.setConnectionTimeout(15000);
@@ -1067,6 +1071,10 @@ public class BasicDistributedZkTest extends AbstractFullDistribZkTestBase {
           } catch (Exception e) {
             e.printStackTrace();
             //fails
+          } finally {
+            if (server != null) {
+              server.shutdown();
+            }
           }
           return null;
         }

solr/core/src/test/org/apache/solr/cloud/CollectionsAPIDistributedZkTest.java

@@ -865,9 +865,13 @@ public class CollectionsAPIDistributedZkTest extends AbstractFullDistribZkTestBa
         while (shardIt.hasNext()) {
           Entry<String,Replica> shardEntry = shardIt.next();
           ZkCoreNodeProps coreProps = new ZkCoreNodeProps(shardEntry.getValue());
-          CoreAdminResponse mcr = CoreAdminRequest.getStatus(
+          HttpSolrServer server = new HttpSolrServer(coreProps.getBaseUrl());
-              coreProps.getCoreName(),
+          CoreAdminResponse mcr;
-              new HttpSolrServer(coreProps.getBaseUrl()));
+          try {
+            mcr = CoreAdminRequest.getStatus(coreProps.getCoreName(), server);
+          } finally {
+            server.shutdown();
+          }
           long before = mcr.getStartTime(coreProps.getCoreName()).getTime();
           urlToTime.put(coreProps.getCoreUrl(), before);
         }

solr/core/src/test/org/apache/solr/cloud/FullSolrCloudDistribCmdsTest.java

@@ -310,9 +310,10 @@ public class FullSolrCloudDistribCmdsTest extends AbstractFullDistribZkTestBase
   private void testIndexingWithSuss() throws Exception {
     ConcurrentUpdateSolrServer suss = new ConcurrentUpdateSolrServer(
         ((HttpSolrServer) clients.get(0)).getBaseURL(), 3, 1);
+    try {
       suss.setConnectionTimeout(15000);
       suss.setSoTimeout(30000);
-    for (int i=100; i<150; i++) {
+      for (int i = 100; i < 150; i++) {
         index_specific(suss, id, i);
       }
       suss.blockUntilFinished();
@@ -320,6 +321,9 @@ public class FullSolrCloudDistribCmdsTest extends AbstractFullDistribZkTestBase
       commit();
       
       checkShardConsistency();
+    } finally {
+      suss.shutdown();
+    }
   }
   
   private void testOptimisticUpdate(QueryResponse results) throws Exception {

solr/core/src/test/org/apache/solr/cloud/ShardSplitTest.java

@@ -408,12 +408,23 @@ public class ShardSplitTest extends BasicDistributedZkTest {
 
     ZkCoreNodeProps shard1_0 = getLeaderUrlFromZk(AbstractDistribZkTestBase.DEFAULT_COLLECTION, SHARD1_0);
     HttpSolrServer shard1_0Server = new HttpSolrServer(shard1_0.getCoreUrl());
-    QueryResponse response = shard1_0Server.query(query);
+    QueryResponse response;
+    try {
+      response = shard1_0Server.query(query);
+    } finally {
+      shard1_0Server.shutdown();
+    }
     long shard10Count = response.getResults().getNumFound();
 
-    ZkCoreNodeProps shard1_1 = getLeaderUrlFromZk(AbstractDistribZkTestBase.DEFAULT_COLLECTION, SHARD1_1);
+    ZkCoreNodeProps shard1_1 = getLeaderUrlFromZk(
+        AbstractDistribZkTestBase.DEFAULT_COLLECTION, SHARD1_1);
     HttpSolrServer shard1_1Server = new HttpSolrServer(shard1_1.getCoreUrl());
-    QueryResponse response2 = shard1_1Server.query(query);
+    QueryResponse response2;
+    try {
+      response2 = shard1_1Server.query(query);
+    } finally {
+      shard1_1Server.shutdown();
+    }
     long shard11Count = response2.getResults().getNumFound();
 
     logDebugHelp(docCounts, response, shard10Count, response2, shard11Count);
@@ -433,7 +444,12 @@ public class ShardSplitTest extends BasicDistributedZkTest {
     for (Replica replica : slice.getReplicas()) {
       String coreUrl = new ZkCoreNodeProps(replica).getCoreUrl();
       HttpSolrServer server = new HttpSolrServer(coreUrl);
-      QueryResponse response = server.query(query);
+      QueryResponse response;
+      try {
+        response = server.query(query);
+      } finally {
+        server.shutdown();
+      }
       numFound[c++] = response.getResults().getNumFound();
       log.info("Shard: " + shard + " Replica: {} has {} docs", coreUrl, String.valueOf(response.getResults().getNumFound()));
       assertTrue("Shard: " + shard + " Replica: " + coreUrl + " has 0 docs", response.getResults().getNumFound() > 0);

solr/core/src/test/org/apache/solr/cloud/hdfs/StressHdfsTest.java

@@ -115,7 +115,9 @@ public class StressHdfsTest extends BasicDistributedZkTest {
     
     int i = 0;
     for (SolrServer client : clients) {
-      HttpSolrServer c = new HttpSolrServer(getBaseUrl(client) + "/delete_data_dir");
+      HttpSolrServer c = new HttpSolrServer(getBaseUrl(client)
+          + "/delete_data_dir");
+      try {
         c.add(getDoc("id", i++));
         if (random().nextBoolean()) c.add(getDoc("id", i++));
         if (random().nextBoolean()) c.add(getDoc("id", i++));
@@ -131,11 +133,13 @@ public class StressHdfsTest extends BasicDistributedZkTest {
         NamedList<Object> response = c.query(
             new SolrQuery().setRequestHandler("/admin/system")).getResponse();
         NamedList<Object> coreInfo = (NamedList<Object>) response.get("core");
-      String dataDir = (String) ((NamedList<Object>) coreInfo.get("directory"))
+        String dataDir = (String) ((NamedList<Object>) coreInfo
-          .get("data");
+            .get("directory")).get("data");
         dataDirs.add(dataDir);
+      } finally {
         c.shutdown();
       }
+    }
     
     if (random().nextBoolean()) {
       cloudClient.deleteByQuery("*:*");

solr/solrj/src/test/org/apache/solr/client/solrj/SolrExceptionTest.java

@@ -42,6 +42,7 @@ public class SolrExceptionTest extends LuceneTestCase {
       SolrServer client = new HttpSolrServer("http://[ff01::114]:11235/solr/", httpClient);
       SolrQuery query = new SolrQuery("test123");
       client.query(query);
+      client.shutdown();
     } catch (SolrServerException sse) {
       gotExpectedError = true;
       /***

solr/solrj/src/test/org/apache/solr/client/solrj/TestLBHttpSolrServer.java

@@ -38,6 +38,7 @@ import org.apache.solr.client.solrj.impl.HttpClientUtil;
 import org.apache.solr.client.solrj.impl.HttpSolrServer;
 import org.apache.solr.client.solrj.impl.LBHttpSolrServer;
 import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.client.solrj.response.SolrResponseBase;
 import org.apache.solr.client.solrj.response.UpdateResponse;
 import org.apache.solr.common.SolrInputDocument;
 import org.apache.solr.common.params.ModifiableSolrParams;
@@ -107,9 +108,14 @@ public class TestLBHttpSolrServer extends LuceneTestCase {
       docs.add(doc);
     }
     HttpSolrServer solrServer = new HttpSolrServer(solrInstance.getUrl(), httpClient);
-    UpdateResponse resp = solrServer.add(docs);
+    SolrResponseBase resp;
+    try {
+      resp = solrServer.add(docs);
       assertEquals(0, resp.getStatus());
       resp = solrServer.commit();
+    } finally {
+      solrServer.shutdown();
+    }
     assertEquals(0, resp.getStatus());
   }
 

solr/solrj/src/test/org/apache/solr/client/solrj/impl/BasicHttpSolrServerTest.java

@@ -479,6 +479,7 @@ public class BasicHttpSolrServerTest extends SolrJettyTestBase {
       server.setDefaultMaxConnectionsPerHost(1);
       fail("Operation should not succeed.");
     } catch (UnsupportedOperationException e) {}
+    server.shutdown();
     client.getConnectionManager().shutdown();
   }
 

solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrServerTest.java

@@ -204,14 +204,15 @@ public class CloudSolrServerTest extends AbstractFullDistribZkTestBase {
   
   public void testShutdown() throws MalformedURLException {
     CloudSolrServer server = new CloudSolrServer("[ff01::114]:33332");
-    server.setZkConnectTimeout(100);
     try {
+      server.setZkConnectTimeout(100);
       server.connect();
       fail("Expected exception");
-    } catch(RuntimeException e) {
+    } catch (RuntimeException e) {
       assertTrue(e.getCause() instanceof TimeoutException);
-    }
+    } finally {
       server.shutdown();
     }
+  }
 
 }