You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ro...@apache.org on 2015/03/26 11:54:33 UTC
svn commit: r1669316 - in /lucene/dev/branches/branch_5x: ./ solr/
solr/CHANGES.txt solr/solrj/
solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
Author: romseygeek
Date: Thu Mar 26 10:54:33 2015
New Revision: 1669316
URL: http://svn.apache.org/r1669316
Log:
SOLR-7203: Remove buggy no-op retries in HttpSolrClient
Modified:
lucene/dev/branches/branch_5x/ (props changed)
lucene/dev/branches/branch_5x/solr/ (props changed)
lucene/dev/branches/branch_5x/solr/CHANGES.txt (contents, props changed)
lucene/dev/branches/branch_5x/solr/solrj/ (props changed)
lucene/dev/branches/branch_5x/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
Modified: lucene/dev/branches/branch_5x/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/CHANGES.txt?rev=1669316&r1=1669315&r2=1669316&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/CHANGES.txt (original)
+++ lucene/dev/branches/branch_5x/solr/CHANGES.txt Thu Mar 26 10:54:33 2015
@@ -343,6 +343,9 @@ Other Changes
* SOLR-6673: MDC based logging of collection, shard, replica, core
(Ishan Chattopadhyaya , Noble Paul)
+* SOLR-7203: Remove buggy no-op retry code in HttpSolrClient (Alan Woodward,
+ Mark Miller, Greg Solovyev)
+
================== 5.0.0 ==================
Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release.
Modified: lucene/dev/branches/branch_5x/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java?rev=1669316&r1=1669315&r2=1669316&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java (original)
+++ lucene/dev/branches/branch_5x/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java Thu Mar 26 10:54:33 2015
@@ -21,7 +21,6 @@ import org.apache.http.Header;
import org.apache.http.HttpResponse;
import org.apache.http.HttpStatus;
import org.apache.http.NameValuePair;
-import org.apache.http.NoHttpResponseException;
import org.apache.http.client.HttpClient;
import org.apache.http.client.entity.UrlEncodedFormEntity;
import org.apache.http.client.methods.HttpEntityEnclosingRequestBase;
@@ -143,8 +142,6 @@ public class HttpSolrClient extends Solr
private volatile boolean followRedirects = false;
- private volatile int maxRetries = 0;
-
private volatile boolean useMultiPartPost;
private final boolean internalClient;
@@ -297,8 +294,7 @@ public class HttpSolrClient extends Solr
}
protected HttpRequestBase createMethod(final SolrRequest request, String collection) throws IOException, SolrServerException {
- HttpRequestBase method = null;
- InputStream is = null;
+
SolrParams params = request.getParams();
Collection<ContentStream> streams = requestWriter.getContentStreams(request);
String path = requestWriter.getPath(request);
@@ -325,158 +321,134 @@ public class HttpSolrClient extends Solr
String basePath = baseUrl;
if (collection != null)
basePath += "/" + collection;
-
- int tries = maxRetries + 1;
- try {
- while( tries-- > 0 ) {
- // Note: since we aren't do intermittent time keeping
- // ourselves, the potential non-timeout latency could be as
- // much as tries-times (plus scheduling effects) the given
- // timeAllowed.
- try {
- if( SolrRequest.METHOD.GET == request.getMethod() ) {
- if( streams != null ) {
- throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, "GET can't send streams!" );
- }
- method = new HttpGet(basePath + path + ClientUtils.toQueryString(wparams, false));
- }
- else if( SolrRequest.METHOD.POST == request.getMethod() || SolrRequest.METHOD.PUT == request.getMethod() ) {
- String url = basePath + path;
- boolean hasNullStreamName = false;
- if (streams != null) {
- for (ContentStream cs : streams) {
- if (cs.getName() == null) {
- hasNullStreamName = true;
- break;
- }
- }
- }
- boolean isMultipart = ((this.useMultiPartPost && SolrRequest.METHOD.POST == request.getMethod())
- || ( streams != null && streams.size() > 1 )) && !hasNullStreamName;
+ if (SolrRequest.METHOD.GET == request.getMethod()) {
+ if (streams != null) {
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "GET can't send streams!");
+ }
+ return new HttpGet(basePath + path + ClientUtils.toQueryString(wparams, false));
+ }
- LinkedList<NameValuePair> postOrPutParams = new LinkedList<>();
- if (streams == null || isMultipart) {
- // send server list and request list as query string params
- ModifiableSolrParams queryParams = calculateQueryParams(this.queryParams, wparams);
- queryParams.add(calculateQueryParams(request.getQueryParams(), wparams));
- String fullQueryUrl = url + ClientUtils.toQueryString( queryParams, false );
- HttpEntityEnclosingRequestBase postOrPut = SolrRequest.METHOD.POST == request.getMethod() ?
- new HttpPost(fullQueryUrl) : new HttpPut(fullQueryUrl);
- if (!isMultipart) {
- postOrPut.addHeader("Content-Type",
- "application/x-www-form-urlencoded; charset=UTF-8");
- }
+ if (SolrRequest.METHOD.POST == request.getMethod() || SolrRequest.METHOD.PUT == request.getMethod()) {
- List<FormBodyPart> parts = new LinkedList<>();
- Iterator<String> iter = wparams.getParameterNamesIterator();
- while (iter.hasNext()) {
- String p = iter.next();
- String[] vals = wparams.getParams(p);
- if (vals != null) {
- for (String v : vals) {
- if (isMultipart) {
- parts.add(new FormBodyPart(p, new StringBody(v, StandardCharsets.UTF_8)));
- } else {
- postOrPutParams.add(new BasicNameValuePair(p, v));
- }
- }
- }
- }
+ String url = basePath + path;
+ boolean hasNullStreamName = false;
+ if (streams != null) {
+ for (ContentStream cs : streams) {
+ if (cs.getName() == null) {
+ hasNullStreamName = true;
+ break;
+ }
+ }
+ }
+ boolean isMultipart = ((this.useMultiPartPost && SolrRequest.METHOD.POST == request.getMethod())
+ || (streams != null && streams.size() > 1)) && !hasNullStreamName;
- if (isMultipart && streams != null) {
- for (ContentStream content : streams) {
- String contentType = content.getContentType();
- if(contentType==null) {
- contentType = BinaryResponseParser.BINARY_CONTENT_TYPE; // default
- }
- String name = content.getName();
- if(name==null) {
- name = "";
- }
- parts.add(new FormBodyPart(name,
- new InputStreamBody(
- content.getStream(),
- contentType,
- content.getName())));
- }
- }
-
- if (parts.size() > 0) {
- MultipartEntity entity = new MultipartEntity(HttpMultipartMode.STRICT);
- for(FormBodyPart p: parts) {
- entity.addPart(p);
- }
- postOrPut.setEntity(entity);
- } else {
- //not using multipart
- postOrPut.setEntity(new UrlEncodedFormEntity(postOrPutParams, StandardCharsets.UTF_8));
- }
+ LinkedList<NameValuePair> postOrPutParams = new LinkedList<>();
+ if (streams == null || isMultipart) {
+ // send server list and request list as query string params
+ ModifiableSolrParams queryParams = calculateQueryParams(this.queryParams, wparams);
+ queryParams.add(calculateQueryParams(request.getQueryParams(), wparams));
+ String fullQueryUrl = url + ClientUtils.toQueryString(queryParams, false);
+ HttpEntityEnclosingRequestBase postOrPut = SolrRequest.METHOD.POST == request.getMethod() ?
+ new HttpPost(fullQueryUrl) : new HttpPut(fullQueryUrl);
+ if (!isMultipart) {
+ postOrPut.addHeader("Content-Type",
+ "application/x-www-form-urlencoded; charset=UTF-8");
+ }
- method = postOrPut;
- }
- // It is has one stream, it is the post body, put the params in the URL
- else {
- String pstr = ClientUtils.toQueryString(wparams, false);
- HttpEntityEnclosingRequestBase postOrPut = SolrRequest.METHOD.POST == request.getMethod() ?
- new HttpPost(url + pstr) : new HttpPut(url + pstr);
-
- // Single stream as body
- // Using a loop just to get the first one
- final ContentStream[] contentStream = new ContentStream[1];
- for (ContentStream content : streams) {
- contentStream[0] = content;
- break;
- }
- if (contentStream[0] instanceof RequestWriter.LazyContentStream) {
- postOrPut.setEntity(new InputStreamEntity(contentStream[0].getStream(), -1) {
- @Override
- public Header getContentType() {
- return new BasicHeader("Content-Type", contentStream[0].getContentType());
- }
-
- @Override
- public boolean isRepeatable() {
- return false;
- }
-
- });
+ List<FormBodyPart> parts = new LinkedList<>();
+ Iterator<String> iter = wparams.getParameterNamesIterator();
+ while (iter.hasNext()) {
+ String p = iter.next();
+ String[] vals = wparams.getParams(p);
+ if (vals != null) {
+ for (String v : vals) {
+ if (isMultipart) {
+ parts.add(new FormBodyPart(p, new StringBody(v, StandardCharsets.UTF_8)));
} else {
- postOrPut.setEntity(new InputStreamEntity(contentStream[0].getStream(), -1) {
- @Override
- public Header getContentType() {
- return new BasicHeader("Content-Type", contentStream[0].getContentType());
- }
-
- @Override
- public boolean isRepeatable() {
- return false;
- }
- });
+ postOrPutParams.add(new BasicNameValuePair(p, v));
}
- method = postOrPut;
}
}
- else {
- throw new SolrServerException("Unsupported method: "+request.getMethod() );
- }
}
- catch( NoHttpResponseException r ) {
- method = null;
- if(is != null) {
- is.close();
- }
- // If out of tries then just rethrow (as normal error).
- if (tries < 1) {
- throw r;
+
+ if (isMultipart && streams != null) {
+ for (ContentStream content : streams) {
+ String contentType = content.getContentType();
+ if (contentType == null) {
+ contentType = BinaryResponseParser.BINARY_CONTENT_TYPE; // default
+ }
+ String name = content.getName();
+ if (name == null) {
+ name = "";
+ }
+ parts.add(new FormBodyPart(name,
+ new InputStreamBody(
+ content.getStream(),
+ contentType,
+ content.getName())));
}
}
+
+ if (parts.size() > 0) {
+ MultipartEntity entity = new MultipartEntity(HttpMultipartMode.STRICT);
+ for (FormBodyPart p : parts) {
+ entity.addPart(p);
+ }
+ postOrPut.setEntity(entity);
+ } else {
+ //not using multipart
+ postOrPut.setEntity(new UrlEncodedFormEntity(postOrPutParams, StandardCharsets.UTF_8));
+ }
+
+ return postOrPut;
+ }
+ // It is has one stream, it is the post body, put the params in the URL
+ else {
+ String pstr = ClientUtils.toQueryString(wparams, false);
+ HttpEntityEnclosingRequestBase postOrPut = SolrRequest.METHOD.POST == request.getMethod() ?
+ new HttpPost(url + pstr) : new HttpPut(url + pstr);
+
+ // Single stream as body
+ // Using a loop just to get the first one
+ final ContentStream[] contentStream = new ContentStream[1];
+ for (ContentStream content : streams) {
+ contentStream[0] = content;
+ break;
+ }
+ if (contentStream[0] instanceof RequestWriter.LazyContentStream) {
+ postOrPut.setEntity(new InputStreamEntity(contentStream[0].getStream(), -1) {
+ @Override
+ public Header getContentType() {
+ return new BasicHeader("Content-Type", contentStream[0].getContentType());
+ }
+
+ @Override
+ public boolean isRepeatable() {
+ return false;
+ }
+
+ });
+ } else {
+ postOrPut.setEntity(new InputStreamEntity(contentStream[0].getStream(), -1) {
+ @Override
+ public Header getContentType() {
+ return new BasicHeader("Content-Type", contentStream[0].getContentType());
+ }
+
+ @Override
+ public boolean isRepeatable() {
+ return false;
+ }
+ });
+ }
+ return postOrPut;
}
- } catch (IOException ex) {
- throw new SolrServerException("error reading streams", ex);
}
-
- return method;
+
+ throw new SolrServerException("Unsupported method: " + request.getMethod());
+
}
protected NamedList<Object> executeMethod(HttpRequestBase method, final ResponseParser processor) throws SolrServerException {
@@ -706,21 +678,9 @@ public class HttpSolrClient extends Solr
}
/**
- * Set maximum number of retries to attempt in the event of transient errors.
- * <p>
- * Maximum number of retries to attempt in the event of transient errors.
- * Default: 0 (no) retries. No more than 1 recommended.
- * </p>
- * @param maxRetries
- * No more than 1 recommended
+ * @deprecated retries should be implemented in client code, and should be considered carefully per-request
*/
- public void setMaxRetries(int maxRetries) {
- if (maxRetries > 1) {
- log.warn("HttpSolrServer: maximum Retries " + maxRetries
- + " > 1. Maximum recommended retries is 1.");
- }
- this.maxRetries = maxRetries;
- }
+ public void setMaxRetries(int maxRetries) { }
public void setRequestWriter(RequestWriter requestWriter) {
this.requestWriter = requestWriter;