janhoy commented on code in PR #2259: URL: https://github.com/apache/solr/pull/2259#discussion_r1506760404
########## solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrJdkClient.java: ########## @@ -0,0 +1,449 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.client.solrj.impl; + +import java.io.IOException; +import java.io.InputStream; +import java.io.PipedInputStream; +import java.io.PipedOutputStream; +import java.lang.invoke.MethodHandles; +import java.net.CookieHandler; +import java.net.InetSocketAddress; +import java.net.ProxySelector; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.net.http.HttpTimeoutException; +import java.time.Duration; +import java.time.temporal.ChronoUnit; +import java.util.Collection; +import java.util.Locale; +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.TimeUnit; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Collectors; +import javax.net.ssl.SSLContext; +import org.apache.solr.client.solrj.ResponseParser; +import org.apache.solr.client.solrj.SolrRequest; +import org.apache.solr.client.solrj.SolrServerException; +import org.apache.solr.client.solrj.request.RequestWriter; +import org.apache.solr.client.solrj.util.ClientUtils; +import org.apache.solr.common.SolrException; +import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.common.util.ContentStream; +import org.apache.solr.common.util.ExecutorUtil; +import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.ObjectReleaseTracker; +import org.apache.solr.common.util.SolrNamedThreadFactory; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class HttpSolrJdkClient extends HttpSolrClientBase { + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + + private static final String USER_AGENT = + "Solr[" + MethodHandles.lookup().lookupClass().getName() + "] 1.0"; + + protected HttpClient httpClient; + + protected ExecutorService executor; + + private boolean shutdownExecutor; + + protected HttpSolrJdkClient(String serverBaseUrl, HttpSolrJdkClient.Builder builder) { + super(serverBaseUrl, builder); + + HttpClient.Redirect followRedirects = + Boolean.TRUE.equals(builder.followRedirects) + ? HttpClient.Redirect.NORMAL + : HttpClient.Redirect.NEVER; + HttpClient.Builder b = HttpClient.newBuilder().followRedirects(followRedirects); + if (builder.sslContext != null) { + b.sslContext(builder.sslContext); + } + + if (builder.executor != null) { + this.executor = builder.executor; + this.shutdownExecutor = false; + } else { + BlockingQueue<Runnable> queue = new LinkedBlockingQueue<>(1024); + this.executor = + new ExecutorUtil.MDCAwareThreadPoolExecutor( + 4, + 256, + 60, + TimeUnit.SECONDS, + queue, + new SolrNamedThreadFactory(this.getClass().getSimpleName())); + this.shutdownExecutor = true; + } + b.executor(this.executor); + + if (builder.useHttp1_1) { + b.version(HttpClient.Version.HTTP_1_1); + } + + if (builder.cookieHandler != null) { + b.cookieHandler(builder.cookieHandler); + } + + if (builder.proxyHost != null) { + if (builder.proxyIsSocks4) { + log.warn( + "Socks4 is likely not supported by this client. See https://bugs.openjdk.org/browse/JDK-8214516";); + } + b.proxy(ProxySelector.of(new InetSocketAddress(builder.proxyHost, builder.proxyPort))); + } + this.httpClient = b.build(); + updateDefaultMimeTypeForParser(); + + // This is a workaround for the case where the first request using + // this client: + // + // (1) no SSL/TLS (2) using POST with stream and (3) using Http/2 + // + // The JDK Http Client will send an upgrade request over Http/1 + // along with request content in the same request. However, + // the Jetty Server underpinning Solr does not accept this. + // + // By sending a ping before any real requests occur, the client + // knows if Solr can accept Http/2, and no additional + // upgrade requests will be sent. + // + // See https://bugs.openjdk.org/browse/JDK-8287589 + // See https://github.com/jetty/jetty.project/issues/9998#issuecomment-1614216870 + // + try { + ping(); Review Comment: > Second, I am calling "ping" in the constructor, which I felt was a lesser evil than having to check a volatile boolean with each request to see if "ping" was called yet. It would be somewhat unexpected to have the client start sending calls to the server by just constructing it. May it even cause real issues since startup order between a customer client application and the solr server would then become significant? E.g. what happens if the client is constructed while Solr is unavailable, then this ping call fails (no harm due to the try-catch), and then solr is started before the first query/update is sent. Then I suppose JDK http client will try the request upgrade again since it still does not know the server. And we'd have no second chance to force a ping ahead of time... -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
