You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by an...@apache.org on 2017/06/23 18:31:03 UTC

lucene-solr:master: SOLR-10915: Make builder based SolrClient constructors to be the only valid way to construct client objects and increase the visibility of builder elements to be protected so extending the builder, and the clients is possible.

Repository: lucene-solr
Updated Branches:
  refs/heads/master 05433eb70 -> e46d39bd5


SOLR-10915: Make builder based SolrClient constructors to be the only valid way to construct client objects and increase the visibility of builder elements to be protected so extending the builder, and the clients is possible.


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/e46d39bd
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/e46d39bd
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/e46d39bd

Branch: refs/heads/master
Commit: e46d39bd5a571522bb2b9ffe97e7e0bca5d9836c
Parents: 05433eb
Author: Anshum Gupta <an...@apple.com>
Authored: Thu Jun 22 15:27:34 2017 -0700
Committer: Anshum Gupta <an...@apple.com>
Committed: Fri Jun 23 11:30:50 2017 -0700

----------------------------------------------------------------------
 solr/CHANGES.txt                                |  6 +-
 .../solr/update/StreamingSolrClients.java       | 65 +++++++++-----
 .../FullThrottleStoppableIndexingThread.java    | 31 +++++--
 .../solr/client/solrj/impl/CloudSolrClient.java | 94 +++++++-------------
 .../solrj/impl/ConcurrentUpdateSolrClient.java  | 63 ++++++++-----
 .../impl/DelegationTokenHttpSolrClient.java     | 13 +++
 .../solr/client/solrj/impl/HttpSolrClient.java  | 77 +++++++++-------
 .../client/solrj/impl/LBHttpSolrClient.java     | 60 +++++++------
 .../embedded/SolrExampleStreamingTest.java      | 64 +++++++++----
 .../impl/ConcurrentUpdateSolrClientTest.java    | 36 ++++++--
 .../client/solrj/impl/LBHttpSolrClientTest.java |  2 +-
 11 files changed, 309 insertions(+), 202 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e46d39bd/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 85535c3..9e9b117 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -29,7 +29,7 @@ Upgrading from Solr 6.x
 
   ./server/scripts/cloud-scripts/zkcli.sh -zkhost 127.0.0.1:2181  -cmd clusterprop -name legacyCloud -val true
 
-* HttpClientInterceptorPlugin is now HttpClientBuilderPlugin and must work with a 
+* HttpClientInterceptorPlugin is now HttpClientBuilderPlugin and must work with a
   SolrHttpClientBuilder rather than an HttpClientConfigurer.
   
 * HttpClientUtil now allows configuring HttpClient instances via SolrHttpClientBuilder
@@ -318,6 +318,10 @@ Other Changes
   resources, they are only resolved against Solr's own or "core" class loader
   by default.  (Uwe Schindler)
 
+* SOLR-10915: Make builder based SolrClient constructors to be the only valid way to construct client objects and
+  increase the visibility of builder elements to be protected so extending the builder, and the clients is possible.
+  (Jason Gerlowski, Anshum Gupta)
+
 ==================  6.7.0 ==================
 
 Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release.

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e46d39bd/solr/core/src/java/org/apache/solr/update/StreamingSolrClients.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/update/StreamingSolrClients.java b/solr/core/src/java/org/apache/solr/update/StreamingSolrClients.java
index 7c630f4..316aa45 100644
--- a/solr/core/src/java/org/apache/solr/update/StreamingSolrClients.java
+++ b/solr/core/src/java/org/apache/solr/update/StreamingSolrClients.java
@@ -72,7 +72,13 @@ public class StreamingSolrClients {
       // NOTE: increasing to more than 1 threadCount for the client could cause updates to be reordered
       // on a greater scale since the current behavior is to only increase the number of connections/Runners when
       // the queue is more than half full.
-      client = new ErrorReportingConcurrentUpdateSolrClient(url, httpClient, 100, runnerCount, updateExecutor, true, req);
+      client = new ErrorReportingConcurrentUpdateSolrClient.Builder(url, req, errors)
+          .withHttpClient(httpClient)
+          .withQueueSize(100)
+          .withThreadCount(runnerCount)
+          .withExecutorService(updateExecutor)
+          .alwaysStreamDeletes()
+          .build();
       client.setPollQueueTime(Integer.MAX_VALUE); // minimize connections created
       client.setParser(new BinaryResponseParser());
       client.setRequestWriter(new BinaryRequestWriter());
@@ -115,31 +121,48 @@ public class StreamingSolrClients {
   public ExecutorService getUpdateExecutor() {
     return updateExecutor;
   }
+}
+
+class ErrorReportingConcurrentUpdateSolrClient extends ConcurrentUpdateSolrClient {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+  private final SolrCmdDistributor.Req req;
+  private final List<Error> errors;
+  
+  public ErrorReportingConcurrentUpdateSolrClient(Builder builder) {
+    super(builder);
+    this.req = builder.req;
+    this.errors = builder.errors;
+  }
   
-  class ErrorReportingConcurrentUpdateSolrClient extends ConcurrentUpdateSolrClient {
-    private final SolrCmdDistributor.Req req;
+  @Override
+  public void handleError(Throwable ex) {
+    req.trackRequestResult(null, false);
+    log.error("error", ex);
+    Error error = new Error();
+    error.e = (Exception) ex;
+    if (ex instanceof SolrException) {
+      error.statusCode = ((SolrException) ex).code();
+    }
+    error.req = req;
+    errors.add(error);
+  }
+  @Override
+  public void onSuccess(HttpResponse resp) {
+    req.trackRequestResult(resp, true);
+  }
+  
+  static class Builder extends ConcurrentUpdateSolrClient.Builder {
+    protected SolrCmdDistributor.Req req;
+    protected List<Error> errors;
     
-    public ErrorReportingConcurrentUpdateSolrClient(String solrServerUrl, HttpClient client, int queueSize,
-        int threadCount, ExecutorService es, boolean streamDeletes, SolrCmdDistributor.Req req) {
-      super(solrServerUrl, client, queueSize, threadCount, es, streamDeletes);
+    public Builder(String baseSolrUrl, SolrCmdDistributor.Req req, List<Error> errors) {
+      super(baseSolrUrl);
       this.req = req;
+      this.errors = errors;
     }
     
-    @Override
-    public void handleError(Throwable ex) {
-      req.trackRequestResult(null, false);
-      log.error("error", ex);
-      Error error = new Error();
-      error.e = (Exception) ex;
-      if (ex instanceof SolrException) {
-        error.statusCode = ((SolrException) ex).code();
-      }
-      error.req = req;
-      errors.add(error);
-    }
-    @Override
-    public void onSuccess(HttpResponse resp) {
-      req.trackRequestResult(resp, true);
+    public ErrorReportingConcurrentUpdateSolrClient build() {
+      return new ErrorReportingConcurrentUpdateSolrClient(this);
     }
   }
 }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e46d39bd/solr/core/src/test/org/apache/solr/cloud/FullThrottleStoppableIndexingThread.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/FullThrottleStoppableIndexingThread.java b/solr/core/src/test/org/apache/solr/cloud/FullThrottleStoppableIndexingThread.java
index 909f39a..c33ad4f 100644
--- a/solr/core/src/test/org/apache/solr/cloud/FullThrottleStoppableIndexingThread.java
+++ b/solr/core/src/test/org/apache/solr/cloud/FullThrottleStoppableIndexingThread.java
@@ -22,7 +22,6 @@ import java.util.List;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 
-import org.apache.http.client.HttpClient;
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.lucene.util.LuceneTestCase;
 import org.apache.solr.client.solrj.SolrClient;
@@ -55,7 +54,11 @@ class FullThrottleStoppableIndexingThread extends StoppableIndexingThread {
     setDaemon(true);
     this.clients = clients;
 
-    cusc = new ErrorLoggingConcurrentUpdateSolrClient(((HttpSolrClient) clients.get(0)).getBaseURL(), httpClient, 8, 2);
+    cusc = new ErrorLoggingConcurrentUpdateSolrClient.Builder(((HttpSolrClient) clients.get(0)).getBaseURL())
+        .withHttpClient(httpClient)
+        .withQueueSize(8)
+        .withThreadCount(2)
+        .build();
     cusc.setConnectionTimeout(10000);
     cusc.setSoTimeout(clientSoTimeout);
   }
@@ -114,8 +117,11 @@ class FullThrottleStoppableIndexingThread extends StoppableIndexingThread {
         clientIndex = 0;
       }
       cusc.shutdownNow();
-      cusc = new ErrorLoggingConcurrentUpdateSolrClient(((HttpSolrClient) clients.get(clientIndex)).getBaseURL(),
-          httpClient, 30, 3);
+      cusc = new ErrorLoggingConcurrentUpdateSolrClient.Builder(((HttpSolrClient) clients.get(clientIndex)).getBaseURL())
+          .withHttpClient(httpClient)
+          .withQueueSize(30)
+          .withThreadCount(3)
+          .build();
     }
   }
   
@@ -143,14 +149,25 @@ class FullThrottleStoppableIndexingThread extends StoppableIndexingThread {
   }
   
   static class ErrorLoggingConcurrentUpdateSolrClient extends ConcurrentUpdateSolrClient {
-    @SuppressWarnings("deprecation")
-    public ErrorLoggingConcurrentUpdateSolrClient(String serverUrl, HttpClient httpClient, int queueSize, int threadCount) {
-      super(serverUrl, httpClient, queueSize, threadCount, null, false);
+    public ErrorLoggingConcurrentUpdateSolrClient(Builder builder) {
+      super(builder);
     }
+    
     @Override
     public void handleError(Throwable ex) {
       log.warn("cusc error", ex);
     }
+    
+    static class Builder extends ConcurrentUpdateSolrClient.Builder {
+
+      public Builder(String baseSolrUrl) {
+        super(baseSolrUrl);
+      }
+      
+      public ErrorLoggingConcurrentUpdateSolrClient build() {
+        return new ErrorLoggingConcurrentUpdateSolrClient(this);
+      }
+    }
   }
   
 }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e46d39bd/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java
----------------------------------------------------------------------
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 f0684f8..6cd5caf 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
@@ -233,77 +233,44 @@ public class CloudSolrClient extends SolrClient {
       retriedAt = System.nanoTime();
     }
   }
-
+  
   /**
    * Create a new client object that connects to Zookeeper and is always aware
    * of the SolrCloud state. If there is a fully redundant Zookeeper quorum and
    * SolrCloud has enough replicas for every shard in a collection, there is no
    * single point of failure. Updates will be sent to shard leaders by default.
    *
-   * @param zkHosts
-   *          A Java Collection (List, Set, etc) of HOST:PORT strings, one for
-   *          each host in the zookeeper ensemble. Note that with certain
-   *          Collection types like HashSet, the order of hosts in the final
-   *          connect string may not be in the same order you added them.
-   *          Provide only one of solrUrls or zkHosts.
-   * @param chroot
-   *          A chroot value for zookeeper, starting with a forward slash. If no
-   *          chroot is required, use null.
-   * @param solrUrls
-   *          A list of Solr URLs to configure the underlying {@link HttpClusterStateProvider}, which will
-   *          use of the these URLs to fetch the list of live nodes for this Solr cluster.  URL's must point to the
-   *          root Solr path ("/solr"). Provide only one of solrUrls or zkHosts.
-   * @param httpClient
-   *          the {@link HttpClient} instance to be used for all requests. The provided httpClient should use a
-   *          multi-threaded connection manager.  If null, a default HttpClient will be used.
-   * @param lbSolrClient
-   *          LBHttpSolrClient instance for requests.  If null, a default LBHttpSolrClient will be used.
-   * @param lbHttpSolrClientBuilder
-   *          LBHttpSolrClient builder to construct the LBHttpSolrClient. If null, a default builder will be used.
-   * @param updatesToLeaders
-   *          If true, sends updates to shard leaders.
-   * @param directUpdatesToLeadersOnly
-   *          If true, sends direct updates to shard leaders only.
+   * @param builder a {@link CloudSolrClient.Builder} with the options used to create the client.
    */
-  private CloudSolrClient(Collection<String> zkHosts,
-                          String chroot,
-                          List<String> solrUrls,
-                          HttpClient httpClient,
-                          LBHttpSolrClient lbSolrClient,
-                          LBHttpSolrClient.Builder lbHttpSolrClientBuilder,
-                          boolean updatesToLeaders,
-                          boolean directUpdatesToLeadersOnly,
-                          ClusterStateProvider stateProvider
-
-  ) {
-    if (stateProvider == null) {
-      if (zkHosts != null && solrUrls != null) {
+  protected CloudSolrClient(Builder builder) {
+    if (builder.stateProvider == null) {
+      if (builder.zkHosts != null && builder.solrUrls != null) {
         throw new IllegalArgumentException("Both zkHost(s) & solrUrl(s) have been specified. Only specify one.");
       }
-      if (zkHosts != null) {
-        this.stateProvider = new ZkClientClusterStateProvider(zkHosts, chroot);
-      } else if (solrUrls != null && !solrUrls.isEmpty()) {
+      if (builder.zkHosts != null) {
+        this.stateProvider = new ZkClientClusterStateProvider(builder.zkHosts, builder.zkChroot);
+      } else if (builder.solrUrls != null && !builder.solrUrls.isEmpty()) {
         try {
-          this.stateProvider = new HttpClusterStateProvider(solrUrls, httpClient);
+          this.stateProvider = new HttpClusterStateProvider(builder.solrUrls, builder.httpClient);
         } catch (Exception e) {
           throw new RuntimeException("Couldn't initialize a HttpClusterStateProvider (is/are the "
-              + "Solr server(s), "  + solrUrls + ", down?)", e);
+              + "Solr server(s), "  + builder.solrUrls + ", down?)", e);
         }
       } else {
         throw new IllegalArgumentException("Both zkHosts and solrUrl cannot be null.");
       }
     } else {
-      this.stateProvider = stateProvider;
+      this.stateProvider = builder.stateProvider;
     }
-    this.clientIsInternal = httpClient == null;
-    this.shutdownLBHttpSolrServer = lbSolrClient == null;
-    if(lbHttpSolrClientBuilder != null) lbSolrClient = lbHttpSolrClientBuilder.build();
-    if(lbSolrClient != null) httpClient = lbSolrClient.getHttpClient();
-    this.myClient = httpClient == null ? HttpClientUtil.createClient(null) : httpClient;
-    if (lbSolrClient == null) lbSolrClient = createLBHttpSolrClient(myClient);
-    this.lbClient = lbSolrClient;
-    this.updatesToLeaders = updatesToLeaders;
-    this.directUpdatesToLeadersOnly = directUpdatesToLeadersOnly;
+    this.clientIsInternal = builder.httpClient == null;
+    this.shutdownLBHttpSolrServer = builder.loadBalancedSolrClient == null;
+    if(builder.lbClientBuilder != null) builder.loadBalancedSolrClient = builder.lbClientBuilder.build();
+    if(builder.loadBalancedSolrClient != null) builder.httpClient = builder.loadBalancedSolrClient.getHttpClient();
+    this.myClient = (builder.httpClient == null) ? HttpClientUtil.createClient(null) : builder.httpClient;
+    if (builder.loadBalancedSolrClient == null) builder.loadBalancedSolrClient = createLBHttpSolrClient(myClient);
+    this.lbClient = builder.loadBalancedSolrClient;
+    this.updatesToLeaders = builder.shardLeadersOnly;
+    this.directUpdatesToLeadersOnly = builder.directUpdatesToLeadersOnly;
   }
 
   /**Sets the cache ttl for DocCollection Objects cached  . This is only applicable for collections which are persisted outside of clusterstate.json
@@ -1372,15 +1339,15 @@ public class CloudSolrClient extends SolrClient {
    * Constructs {@link CloudSolrClient} instances from provided configuration.
    */
   public static class Builder {
-    private Collection<String> zkHosts;
-    private List<String> solrUrls;
-    private HttpClient httpClient;
-    private String zkChroot;
-    private LBHttpSolrClient loadBalancedSolrClient;
-    private LBHttpSolrClient.Builder lbClientBuilder;
-    private boolean shardLeadersOnly;
-    private boolean directUpdatesToLeadersOnly;
-    private ClusterStateProvider stateProvider;
+    protected Collection<String> zkHosts;
+    protected List<String> solrUrls;
+    protected HttpClient httpClient;
+    protected String zkChroot;
+    protected LBHttpSolrClient loadBalancedSolrClient;
+    protected LBHttpSolrClient.Builder lbClientBuilder;
+    protected boolean shardLeadersOnly;
+    protected boolean directUpdatesToLeadersOnly;
+    protected ClusterStateProvider stateProvider;
 
 
     public Builder() {
@@ -1536,8 +1503,7 @@ public class CloudSolrClient extends SolrClient {
           throw new IllegalArgumentException("Both zkHosts and solrUrl cannot be null.");
         }
       }
-      return new CloudSolrClient(zkHosts, zkChroot, solrUrls, httpClient, loadBalancedSolrClient, lbClientBuilder,
-          shardLeadersOnly, directUpdatesToLeadersOnly, stateProvider);
+      return new CloudSolrClient(this);
     }
   }
 }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e46d39bd/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClient.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClient.java
index f169a60..fead54f 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClient.java
@@ -97,33 +97,52 @@ public class ConcurrentUpdateSolrClient extends SolrClient {
   
   /**
    * Uses the supplied HttpClient to send documents to the Solr server.
+   * 
+   * @deprecated use {@link ConcurrentUpdateSolrClient#ConcurrentUpdateSolrClient(Builder)} instead, as it is a more extension/subclassing-friendly alternative
    */
+  @Deprecated
   protected ConcurrentUpdateSolrClient(String solrServerUrl,
                                        HttpClient client, int queueSize, int threadCount,
                                        ExecutorService es, boolean streamDeletes) {
-    this.internalHttpClient = (client == null);
-    this.client = new HttpSolrClient.Builder(solrServerUrl)
+    this((streamDeletes) ?
+        new Builder(solrServerUrl)
         .withHttpClient(client)
+        .withQueueSize(queueSize)
+        .withThreadCount(threadCount)
+        .withExecutorService(es)
+        .alwaysStreamDeletes() :
+          new Builder(solrServerUrl)
+          .withHttpClient(client)
+          .withQueueSize(queueSize)
+          .withThreadCount(threadCount)
+          .withExecutorService(es)
+          .neverStreamDeletes());
+  }
+  
+  protected ConcurrentUpdateSolrClient(Builder builder) {
+    this.internalHttpClient = (builder.httpClient == null);
+    this.client = new HttpSolrClient.Builder(builder.baseSolrUrl)
+        .withHttpClient(builder.httpClient)
         .build();
     this.client.setFollowRedirects(false);
-    queue = new LinkedBlockingQueue<>(queueSize);
-    this.threadCount = threadCount;
-    runners = new LinkedList<>();
-    this.streamDeletes = streamDeletes;
+    this.queue = new LinkedBlockingQueue<>(builder.queueSize);
+    this.threadCount = builder.threadCount;
+    this.runners = new LinkedList<>();
+    this.streamDeletes = builder.streamDeletes;
     
-    if (es != null) {
-      scheduler = es;
-      shutdownExecutor = false;
+    if (builder.executorService != null) {
+      this.scheduler = builder.executorService;
+      this.shutdownExecutor = false;
     } else {
-      scheduler = ExecutorUtil.newMDCAwareCachedThreadPool(new SolrjNamedThreadFactory("concurrentUpdateScheduler"));
-      shutdownExecutor = true;
+      this.scheduler = ExecutorUtil.newMDCAwareCachedThreadPool(new SolrjNamedThreadFactory("concurrentUpdateScheduler"));
+      this.shutdownExecutor = true;
     }
     
     if (log.isDebugEnabled()) {
-      pollInterrupts = new AtomicInteger();
-      pollExits = new AtomicInteger();
-      blockLoops = new AtomicInteger();
-      emptyQueueLoops = new AtomicInteger();
+      this.pollInterrupts = new AtomicInteger();
+      this.pollExits = new AtomicInteger();
+      this.blockLoops = new AtomicInteger();
+      this.emptyQueueLoops = new AtomicInteger();
     }
   }
 
@@ -743,12 +762,12 @@ public class ConcurrentUpdateSolrClient extends SolrClient {
    * Constructs {@link ConcurrentUpdateSolrClient} instances from provided configuration.
    */
   public static class Builder {
-    private String baseSolrUrl;
-    private HttpClient httpClient;
-    private int queueSize;
-    private int threadCount;
-    private ExecutorService executorService;
-    private boolean streamDeletes;
+    protected String baseSolrUrl;
+    protected HttpClient httpClient;
+    protected int queueSize;
+    protected int threadCount;
+    protected ExecutorService executorService;
+    protected boolean streamDeletes;
 
     /**
      * Create a Builder object, based on the provided Solr URL.
@@ -838,7 +857,7 @@ public class ConcurrentUpdateSolrClient extends SolrClient {
         throw new IllegalArgumentException("Cannot create HttpSolrClient without a valid baseSolrUrl!");
       }
       
-      return new ConcurrentUpdateSolrClient(baseSolrUrl, httpClient, queueSize, threadCount, executorService, streamDeletes);
+      return new ConcurrentUpdateSolrClient(this);
     }
   }
 }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e46d39bd/solr/solrj/src/java/org/apache/solr/client/solrj/impl/DelegationTokenHttpSolrClient.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/DelegationTokenHttpSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/DelegationTokenHttpSolrClient.java
index fc83391..e878110 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/DelegationTokenHttpSolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/DelegationTokenHttpSolrClient.java
@@ -38,7 +38,11 @@ public class DelegationTokenHttpSolrClient extends HttpSolrClient {
    * Package protected constructor for use by 
    * {@linkplain org.apache.solr.client.solrj.impl.HttpSolrClient.Builder}.
    * @lucene.internal
+   * 
+   * @deprecated use {@link DelegationTokenHttpSolrClient#DelegationTokenHttpSolrClient(HttpSolrClient.Builder)} instead, as it is a more
+   * extension/subclassing-friendly alternative
    */
+  @Deprecated
   DelegationTokenHttpSolrClient(String baseURL,
                                 HttpClient client,
                                 ResponseParser parser,
@@ -52,6 +56,11 @@ public class DelegationTokenHttpSolrClient extends HttpSolrClient {
     invariantParams = new ModifiableSolrParams();
     invariantParams.set(DELEGATION_TOKEN_PARAM, delegationToken);
   }
+  
+  protected DelegationTokenHttpSolrClient(Builder builder) {
+    super(builder);
+    setQueryParams(new TreeSet<>(Arrays.asList(DELEGATION_TOKEN_PARAM)));
+  }
 
   /**
    * This constructor is defined at "protected" scope. Ideally applications should
@@ -63,7 +72,11 @@ public class DelegationTokenHttpSolrClient extends HttpSolrClient {
    * @param parser Response parser instance to use to decode response from Solr server
    * @param allowCompression Should compression be allowed ?
    * @param invariantParams The parameters which should be passed with every request.
+   * 
+   * @deprecated use {@link DelegationTokenHttpSolrClient#DelegationTokenHttpSolrClient(HttpSolrClient.Builder)} instead, as it is a more
+   * extension/subclassing-friendly alternative
    */
+  @Deprecated
   protected DelegationTokenHttpSolrClient(String baseURL,
       HttpClient client,
       ResponseParser parser,

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e46d39bd/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
index e73e08b..f944d7c 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
@@ -143,8 +143,40 @@ public class HttpSolrClient extends SolrClient {
   private volatile Integer connectionTimeout;
   private volatile Integer soTimeout;
   
+  /**
+   * @deprecated use {@link HttpSolrClient#HttpSolrClient(Builder)} instead, as it is a more extension/subclassing-friendly alternative
+   */
+  @Deprecated
   protected HttpSolrClient(String baseURL, HttpClient client, ResponseParser parser, boolean allowCompression) {
-    this.baseUrl = baseURL;
+    this(new Builder(baseURL)
+        .withHttpClient(client)
+        .withResponseParser(parser)
+        .allowCompression(allowCompression));
+  }
+
+  /**
+   * The constructor.
+   *
+   * @param baseURL The base url to communicate with the Solr server
+   * @param client Http client instance to use for communication
+   * @param parser Response parser instance to use to decode response from Solr server
+   * @param allowCompression Should compression be allowed ?
+   * @param invariantParams The parameters which should be included with every request.
+   * 
+   * @deprecated use {@link HttpSolrClient#HttpSolrClient(Builder)} instead, as it is a more extension/subclassing-friendly alternative
+   */
+  @Deprecated
+  protected HttpSolrClient(String baseURL, HttpClient client, ResponseParser parser, boolean allowCompression,
+      ModifiableSolrParams invariantParams) {
+    this(new Builder(baseURL)
+        .withHttpClient(client)
+        .withResponseParser(parser)
+        .allowCompression(allowCompression)
+        .withInvariantParams(invariantParams));
+  }
+  
+  protected HttpSolrClient(Builder builder) {
+    this.baseUrl = builder.baseSolrUrl;
     if (baseUrl.endsWith("/")) {
       baseUrl = baseUrl.substring(0, baseUrl.length() - 1);
     }
@@ -154,34 +186,19 @@ public class HttpSolrClient extends SolrClient {
               + baseUrl);
     }
     
-    if (client != null) {
-      httpClient = client;
-      internalClient = false;
+    if (builder.httpClient != null) {
+      this.httpClient = builder.httpClient;
+      this.internalClient = false;
     } else {
-      internalClient = true;
+      this.internalClient = true;
       ModifiableSolrParams params = new ModifiableSolrParams();
       params.set(HttpClientUtil.PROP_FOLLOW_REDIRECTS, followRedirects);
-      params.set(HttpClientUtil.PROP_ALLOW_COMPRESSION, allowCompression);
+      params.set(HttpClientUtil.PROP_ALLOW_COMPRESSION, builder.compression);
       httpClient = HttpClientUtil.createClient(params);
     }
     
-    this.parser = parser;
-  }
-
-  /**
-   * The consturctor.
-   *
-   * @param baseURL The base url to communicate with the Solr server
-   * @param client Http client instance to use for communication
-   * @param parser Response parser instance to use to decode response from Solr server
-   * @param allowCompression Should compression be allowed ?
-   * @param invariantParams The parameters which should be included with every request.
-   */
-  protected HttpSolrClient(String baseURL, HttpClient client, ResponseParser parser, boolean allowCompression,
-      ModifiableSolrParams invariantParams) {
-    this(baseURL, client, parser, allowCompression);
-
-    this.invariantParams = invariantParams;
+    this.parser = builder.responseParser;
+    this.invariantParams = builder.invariantParams;
   }
 
   public Set<String> getQueryParams() {
@@ -803,11 +820,11 @@ public class HttpSolrClient extends SolrClient {
    * Constructs {@link HttpSolrClient} instances from provided configuration.
    */
   public static class Builder {
-    private String baseSolrUrl;
-    private HttpClient httpClient;
-    private ResponseParser responseParser;
-    private boolean compression;
-    private ModifiableSolrParams invariantParams = new ModifiableSolrParams();
+    protected String baseSolrUrl;
+    protected HttpClient httpClient;
+    protected ResponseParser responseParser;
+    protected boolean compression;
+    protected ModifiableSolrParams invariantParams = new ModifiableSolrParams();
 
     public Builder() {
       this.responseParser = new BinaryResponseParser();
@@ -924,9 +941,9 @@ public class HttpSolrClient extends SolrClient {
       }
 
       if (this.invariantParams.get(DelegationTokenHttpSolrClient.DELEGATION_TOKEN_PARAM) == null) {
-        return new HttpSolrClient(baseSolrUrl, httpClient, responseParser, compression, invariantParams);
+        return new HttpSolrClient(this);
       } else {
-        return new DelegationTokenHttpSolrClient(baseSolrUrl, httpClient, responseParser, compression, invariantParams);
+        return new DelegationTokenHttpSolrClient(this);
       }
     }
   }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e46d39bd/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java
index dbb99ce..b96c935 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java
@@ -239,33 +239,43 @@ public class LBHttpSolrClient extends SolrClient {
 
   /**
    * The provided httpClient should use a multi-threaded connection manager
+   *
+   * @deprecated use {@link LBHttpSolrClient#LBHttpSolrClient(Builder)} instead, as it is a more extension/subclassing-friendly alternative
    */
+  @Deprecated
   protected LBHttpSolrClient(HttpSolrClient.Builder httpSolrClientBuilder,
                           HttpClient httpClient, String... solrServerUrl) {
-    clientIsInternal = httpClient == null;
-    this.httpSolrClientBuilder = httpSolrClientBuilder;
-    httpClient = constructClient(null);
-    this.httpClient = httpClient;
-    if (solrServerUrl != null) {
-      for (String s : solrServerUrl) {
-        ServerWrapper wrapper = new ServerWrapper(makeSolrClient(s));
-        aliveServers.put(wrapper.getKey(), wrapper);
-      }
-    }
-    updateAliveList();
+    this(new Builder()
+        .withHttpSolrClientBuilder(httpSolrClientBuilder)
+        .withHttpClient(httpClient)
+        .withBaseSolrUrls(solrServerUrl));
   }
 
   /**
    * The provided httpClient should use a multi-threaded connection manager
+   *
+   * @deprecated use {@link LBHttpSolrClient#LBHttpSolrClient(Builder)} instead, as it is a more extension/subclassing-friendly alternative
    */
+  @Deprecated
   protected LBHttpSolrClient(HttpClient httpClient, ResponseParser parser, String... solrServerUrl) {
-    clientIsInternal = (httpClient == null);
-    this.httpClient = httpClient == null ? constructClient(solrServerUrl) : httpClient;
-    this.parser = parser;
+    this(new Builder()
+        .withBaseSolrUrls(solrServerUrl)
+        .withResponseParser(parser)
+        .withHttpClient(httpClient));
+  }
+
+  protected LBHttpSolrClient(Builder builder) {
+    this.clientIsInternal = builder.httpClient == null;
+    this.httpSolrClientBuilder = builder.httpSolrClientBuilder;
+    this.httpClient = builder.httpClient == null ? constructClient(builder.baseSolrUrls.toArray(new String[builder.baseSolrUrls.size()])) : builder.httpClient;
     
-    for (String s : solrServerUrl) {
-      ServerWrapper wrapper = new ServerWrapper(makeSolrClient(s));
-      aliveServers.put(wrapper.getKey(), wrapper);
+    this.parser = builder.responseParser;
+
+    if (! builder.baseSolrUrls.isEmpty()) {
+      for (String s : builder.baseSolrUrls) {
+        ServerWrapper wrapper = new ServerWrapper(makeSolrClient(s));
+        aliveServers.put(wrapper.getKey(), wrapper);
+      }
     }
     updateAliveList();
   }
@@ -852,10 +862,10 @@ public class LBHttpSolrClient extends SolrClient {
    * Constructs {@link LBHttpSolrClient} instances from provided configuration.
    */
   public static class Builder {
-    private final List<String> baseSolrUrls;
-    private HttpClient httpClient;
-    private ResponseParser responseParser;
-    private HttpSolrClient.Builder httpSolrClientBuilder;
+    protected final List<String> baseSolrUrls;
+    protected HttpClient httpClient;
+    protected ResponseParser responseParser;
+    protected HttpSolrClient.Builder httpSolrClientBuilder;
 
     public Builder() {
       this.baseSolrUrls = new ArrayList<>();
@@ -953,13 +963,7 @@ public class LBHttpSolrClient extends SolrClient {
      * Create a {@link HttpSolrClient} based on provided configuration.
      */
     public LBHttpSolrClient build() {
-      final String[] baseUrlArray = new String[baseSolrUrls.size()];
-      String[] solrServerUrls = baseSolrUrls.toArray(baseUrlArray);
-      if (httpSolrClientBuilder != null) {
-        return new LBHttpSolrClient(httpSolrClientBuilder, httpClient, solrServerUrls);
-      } else {
-        return new LBHttpSolrClient(httpClient, responseParser, solrServerUrls);
-      }
+      return new LBHttpSolrClient(this);
     }
   }
 }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e46d39bd/solr/solrj/src/test/org/apache/solr/client/solrj/embedded/SolrExampleStreamingTest.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/embedded/SolrExampleStreamingTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/embedded/SolrExampleStreamingTest.java
index c2314f8..35ab898 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/embedded/SolrExampleStreamingTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/embedded/SolrExampleStreamingTest.java
@@ -39,25 +39,10 @@ import java.util.List;
 @Slow
 public class SolrExampleStreamingTest extends SolrExampleTests {
 
-  protected Throwable handledException = null;
-
   @BeforeClass
   public static void beforeTest() throws Exception {
     createJetty(legacyExampleCollection1SolrHome());
   }
-  
-  public class ErrorTrackingConcurrentUpdateSolrClient extends ConcurrentUpdateSolrClient {
-    public Throwable lastError = null;
-
-    public ErrorTrackingConcurrentUpdateSolrClient(String solrServerUrl, int queueSize, int threadCount) {
-      super(solrServerUrl, null, queueSize, threadCount, null, false);
-    }
-    
-    @Override
-    public void handleError(Throwable ex) {
-      handledException = lastError = ex;
-    }
-  }
 
   @Override
   public SolrClient createNewSolrClient()
@@ -66,7 +51,10 @@ public class SolrExampleStreamingTest extends SolrExampleTests {
       // setup the server...
       String url = jetty.getBaseUrl().toString() + "/collection1";
       // smaller queue size hits locks more often
-      ConcurrentUpdateSolrClient concurrentClient = new ErrorTrackingConcurrentUpdateSolrClient( url, 2, 5 );
+      ConcurrentUpdateSolrClient concurrentClient = new ErrorTrackingConcurrentUpdateSolrClient.Builder(url)
+          .withQueueSize(2)
+          .withThreadCount(5)
+          .build();
       concurrentClient.setParser(new XMLResponseParser());
       concurrentClient.setRequestWriter(new RequestWriter());
       return concurrentClient;
@@ -81,7 +69,10 @@ public class SolrExampleStreamingTest extends SolrExampleTests {
     // SOLR-3903
     final List<Throwable> failures = new ArrayList<>();
     final String serverUrl = jetty.getBaseUrl().toString() + "/collection1";
-    try (ConcurrentUpdateSolrClient concurrentClient = new FailureRecordingConcurrentUpdateSolrClient(serverUrl, 2, 2)) {
+    try (ConcurrentUpdateSolrClient concurrentClient = new FailureRecordingConcurrentUpdateSolrClient.Builder(serverUrl)
+        .withQueueSize(2)
+        .withThreadCount(2)
+        .build()) {
       int docId = 42;
       for (UpdateRequest.ACTION action : EnumSet.allOf(UpdateRequest.ACTION.class)) {
         for (boolean waitSearch : Arrays.asList(true, false)) {
@@ -108,14 +99,49 @@ public class SolrExampleStreamingTest extends SolrExampleTests {
   static class FailureRecordingConcurrentUpdateSolrClient extends ConcurrentUpdateSolrClient {
     private final List<Throwable> failures = new ArrayList<>();
     
-    public FailureRecordingConcurrentUpdateSolrClient(String serverUrl, int queueSize, int numThreads) {
-      super(serverUrl, null, queueSize, numThreads, null, false);
+    public FailureRecordingConcurrentUpdateSolrClient(Builder builder) {
+      super(builder);
     }
     
     @Override
     public void handleError(Throwable ex) {
       failures.add(ex);
     }
+    
+    static class Builder extends ConcurrentUpdateSolrClient.Builder {
+      public Builder(String baseSolrUrl) {
+        super(baseSolrUrl);
+      }
+   
+      @Override
+      public FailureRecordingConcurrentUpdateSolrClient build() {
+        return new FailureRecordingConcurrentUpdateSolrClient(this);
+      }
+    }
   }
+  
+  public static class ErrorTrackingConcurrentUpdateSolrClient extends ConcurrentUpdateSolrClient {
+    public Throwable lastError = null;
+    
+    public ErrorTrackingConcurrentUpdateSolrClient(Builder builder) {
+      super(builder);
+    }
+        
+    @Override
+    public void handleError(Throwable ex) {
+      lastError = ex;
+    }
+    
+    static class Builder extends ConcurrentUpdateSolrClient.Builder {
 
+      public Builder(String baseSolrUrl) {
+        super(baseSolrUrl);
+      }
+      
+      @Override
+      public ErrorTrackingConcurrentUpdateSolrClient build() {
+        return new ErrorTrackingConcurrentUpdateSolrClient(this);
+      }
+    }
+  }
 }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e46d39bd/solr/solrj/src/test/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClientTest.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClientTest.java
index 4b061d5..44afccd 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClientTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClientTest.java
@@ -148,8 +148,10 @@ public class ConcurrentUpdateSolrClientTest extends SolrJettyTestBase {
     final StringBuilder errors = new StringBuilder();     
     
     @SuppressWarnings("serial")
-    ConcurrentUpdateSolrClient concurrentClient = new OutcomeCountingConcurrentUpdateSolrClient(serverUrl, cussQueueSize,
-        cussThreadCount, successCounter, errorCounter, errors);
+    ConcurrentUpdateSolrClient concurrentClient = new OutcomeCountingConcurrentUpdateSolrClient.Builder(serverUrl, successCounter, errorCounter, errors)
+      .withQueueSize(cussQueueSize)
+      .withThreadCount(cussThreadCount)
+      .build();
     
     concurrentClient.setPollQueueTime(0);
     
@@ -309,13 +311,12 @@ public class ConcurrentUpdateSolrClientTest extends SolrJettyTestBase {
     private final AtomicInteger successCounter;
     private final AtomicInteger failureCounter;
     private final StringBuilder errors;
-    public OutcomeCountingConcurrentUpdateSolrClient(String serverUrl, int queueSize, int threadCount,
-        AtomicInteger successCounter, AtomicInteger failureCounter, StringBuilder errors) {
-      super(serverUrl, null, queueSize, threadCount, null, false);
-      
-      this.successCounter = successCounter;
-      this.failureCounter = failureCounter;
-      this.errors = errors;
+    
+    public OutcomeCountingConcurrentUpdateSolrClient(Builder builder) {
+      super(builder);
+      this.successCounter = builder.successCounter;
+      this.failureCounter = builder.failureCounter;
+      this.errors = builder.errors;
     }
     
     @Override
@@ -327,5 +328,22 @@ public class ConcurrentUpdateSolrClientTest extends SolrJettyTestBase {
     public void onSuccess(HttpResponse resp) {
       successCounter.incrementAndGet();
     }
+    
+    static class Builder extends ConcurrentUpdateSolrClient.Builder {
+      protected final AtomicInteger successCounter;
+      protected final AtomicInteger failureCounter;
+      protected final StringBuilder errors;
+
+      public Builder(String baseSolrUrl, AtomicInteger successCounter, AtomicInteger failureCounter, StringBuilder errors) {
+        super(baseSolrUrl);
+        this.successCounter = successCounter;
+        this.failureCounter = failureCounter;
+        this.errors = errors;
+      }
+      
+      public OutcomeCountingConcurrentUpdateSolrClient build() {
+        return new OutcomeCountingConcurrentUpdateSolrClient(this);
+      }
+    }
   }
 }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e46d39bd/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttpSolrClientTest.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttpSolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttpSolrClientTest.java
index 94af2ca..34804c4 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttpSolrClientTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttpSolrClientTest.java
@@ -40,7 +40,7 @@ public class LBHttpSolrClientTest {
   public void testLBHttpSolrClientHttpClientResponseParserStringArray() throws IOException {
     CloseableHttpClient httpClient = HttpClientUtil.createClient(new ModifiableSolrParams());
     try (
-         LBHttpSolrClient testClient = new LBHttpSolrClient(httpClient, (ResponseParser) null);
+         LBHttpSolrClient testClient = new LBHttpSolrClient.Builder().withHttpClient(httpClient).withResponseParser(null).build();
          HttpSolrClient httpSolrClient = testClient.makeSolrClient("http://127.0.0.1:8080")) {
       assertNull("Generated server should have null parser.", httpSolrClient.getParser());
     } finally {