From 9db532e1d2d1cb3aac3f85425946362aed1d63f8 Mon Sep 17 00:00:00 2001 From: Houston Putman Date: Thu, 18 Apr 2024 12:04:14 -0500 Subject: [PATCH 1/2] SOLR-17240: Fix Http2SolrClient maxConnectionsPerDestination usage --- solr/CHANGES.txt | 3 +++ .../solr/client/solrj/impl/Http2SolrClient.java | 13 +++++-------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index f609169a0bb..94c05d3ba4c 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -139,6 +139,9 @@ Improvements * SOLR-14763: Add a CompletableFuture based asynchronous API to Http2SolrClient, HttpJdkSolrClient and LBHttp2SolrClient. The previous asynchronous API is deprecated. (Rishi Sankar, James Dyer) +* SOLR-17240: Use the same logic for "maxConnectionsPerDestination" between http1 and http2 in the Http2SolrClient. + Http2 is no longer hardcoded to use 4 connections. (Houston Putman, Mark Miller) + Optimizations --------------------- * SOLR-17144: Close searcherExecutor thread per core after 1 minute (Pierre Salagnac, Christine Poerschke) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java index 40fd27b7c8e..9f861cda97b 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java @@ -158,8 +158,6 @@ ProtocolHandlers getProtocolHandlers() { } private HttpClient createHttpClient(Builder builder) { - HttpClient httpClient; - executor = builder.executor; if (executor == null) { BlockingArrayQueue queue = new BlockingArrayQueue<>(256, 256); @@ -214,10 +212,6 @@ private HttpClient createHttpClient(Builder builder) { } transport = new HttpClientTransportOverHTTP(clientConnector); - httpClient = new HttpClient(transport); - if (builder.maxConnectionsPerHost != null) { - httpClient.setMaxConnectionsPerDestination(builder.maxConnectionsPerHost); - } } else { if (log.isDebugEnabled()) { log.debug("Create Http2SolrClient with HTTP/2 transport"); @@ -225,8 +219,11 @@ private HttpClient createHttpClient(Builder builder) { HTTP2Client http2client = new HTTP2Client(clientConnector); transport = new HttpClientTransportOverHTTP2(http2client); - httpClient = new HttpClient(transport); - httpClient.setMaxConnectionsPerDestination(4); + } + + HttpClient httpClient = new HttpClient(transport); + if (builder.maxConnectionsPerHost != null) { + httpClient.setMaxConnectionsPerDestination(builder.maxConnectionsPerHost); } httpClient.setExecutor(this.executor); From db8390838d9f6e4401aceec527b57ab0cd031011 Mon Sep 17 00:00:00 2001 From: Houston Putman Date: Fri, 19 Apr 2024 13:26:27 -0700 Subject: [PATCH 2/2] Divide by the multiplexing factor. Update docs --- .../solr/client/solrj/impl/Http2SolrClient.java | 12 ++++++++++-- .../client/solrj/impl/HttpSolrClientBuilderBase.java | 5 +++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java index 9f861cda97b..a7067898e96 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java @@ -206,6 +206,8 @@ private HttpClient createHttpClient(Builder builder) { clientConnector.setSelectors(2); HttpClientTransport transport; + int maxConnectionsPerHost = + builder.maxConnectionsPerHost != null ? builder.maxConnectionsPerHost : 0; if (builder.useHttp1_1) { if (log.isDebugEnabled()) { log.debug("Create Http2SolrClient with HTTP/1.1 transport"); @@ -219,11 +221,17 @@ private HttpClient createHttpClient(Builder builder) { HTTP2Client http2client = new HTTP2Client(clientConnector); transport = new HttpClientTransportOverHTTP2(http2client); + // For HTTP2, which supports multiplexing requests over a single connection, we should reduce + // the maxConnectionsPerHost to account for the max multiplexing factor. + if (maxConnectionsPerHost > 0) { + maxConnectionsPerHost = + ((maxConnectionsPerHost - 1) / http2client.getMaxConcurrentPushedStreams()) + 1; + } } HttpClient httpClient = new HttpClient(transport); - if (builder.maxConnectionsPerHost != null) { - httpClient.setMaxConnectionsPerDestination(builder.maxConnectionsPerHost); + if (maxConnectionsPerHost > 0) { + httpClient.setMaxConnectionsPerDestination(maxConnectionsPerHost); } httpClient.setExecutor(this.executor); diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBuilderBase.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBuilderBase.java index 19025c9b6ec..9be1fc7e1f8 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBuilderBase.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBuilderBase.java @@ -106,8 +106,9 @@ public B withTheseParamNamesInTheUrl(Set urlParamNames) { } /** - * Set maxConnectionsPerHost for http1 connections, maximum number http2 connections is limited to - * 4 + * Set maxConnectionsPerHost for http1 and http2 connections. For http1 connections, this value is + * used without any change. For http2 connections, this value is ultimately divided by the max + * multiplexing rate per connection. */ @SuppressWarnings("unchecked") public B withMaxConnectionsPerHost(int max) {