From c2e578ea539ddbe9ba8aad4151473a0ea8388860 Mon Sep 17 00:00:00 2001 From: iamsanjay Date: Sun, 18 Feb 2024 10:56:45 +0530 Subject: [PATCH 01/24] SOLR-16505: Switch UpdateShardHandler.getRecoveryOnlyHttpClient to Jetty HTTP2 --- solr/CHANGES.txt | 2 + .../apache/solr/cloud/RecoveryStrategy.java | 59 +++++--- .../solr/update/UpdateShardHandler.java | 23 ++- .../cloud/RecoveryStrategyStressTest.java | 135 ++++++++++++++++++ .../client/solrj/impl/Http2SolrClient.java | 5 +- 5 files changed, 197 insertions(+), 27 deletions(-) create mode 100644 solr/core/src/test/org/apache/solr/cloud/RecoveryStrategyStressTest.java diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 08f38e724b4..ff009144f98 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -97,6 +97,8 @@ Improvements * SOLR-17145: The INSTALLSHARDDATA API now includes a 'requestid' field when run asynchronously (Jason Gerlowski) +* SOLR-16505: Switch UpdateShardHandler.getRecoveryOnlyHttpClient to Jetty HTTP2 + Optimizations --------------------- * SOLR-17144: Close searcherExecutor thread per core after 1 minute (Pierre Salagnac, Christine Poerschke) diff --git a/solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java b/solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java index f26672b1960..b578af713d2 100644 --- a/solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java +++ b/solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java @@ -25,17 +25,17 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; -import org.apache.http.client.methods.HttpUriRequest; import org.apache.lucene.index.IndexCommit; import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.store.Directory; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.SolrServerException; -import org.apache.solr.client.solrj.impl.HttpSolrClient; -import org.apache.solr.client.solrj.impl.HttpSolrClient.HttpUriRequestResponse; +import org.apache.solr.client.solrj.impl.Http2SolrClient; import org.apache.solr.client.solrj.request.AbstractUpdateRequest; import org.apache.solr.client.solrj.request.CoreAdminRequest.WaitForState; import org.apache.solr.client.solrj.request.UpdateRequest; +import org.apache.solr.client.solrj.util.AsyncListener; +import org.apache.solr.client.solrj.util.Cancellable; import org.apache.solr.common.SolrException; import org.apache.solr.common.SolrException.ErrorCode; import org.apache.solr.common.cloud.DocCollection; @@ -47,7 +47,9 @@ import org.apache.solr.common.cloud.ZooKeeperException; import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.params.UpdateParams; +import org.apache.solr.common.util.ExecutorUtil; import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.SolrNamedThreadFactory; import org.apache.solr.common.util.URLUtil; import org.apache.solr.core.CoreContainer; import org.apache.solr.core.CoreDescriptor; @@ -69,6 +71,7 @@ import org.apache.solr.util.plugin.NamedListInitializedPlugin; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.slf4j.MDC; /** * This class may change in future and customisations are not supported between versions in terms of @@ -124,7 +127,7 @@ public static interface RecoveryListener { private int retries; private boolean recoveringAfterStartup; private CoreContainer cc; - private volatile HttpUriRequest prevSendPreRecoveryHttpUriRequest; + private volatile Cancellable prevSendPreRecoveryHttpUriRequest; private final Replica.Type replicaType; private CoreDescriptor coreDescriptor; @@ -176,15 +179,13 @@ public final void setRecoveringAfterStartup(boolean recoveringAfterStartup) { } /** Builds a new HttpSolrClient for use in recovery. Caller must close */ - private HttpSolrClient.Builder recoverySolrClientBuilder(String baseUrl, String leaderCoreName) { + private Http2SolrClient.Builder recoverySolrClientBuilder(String baseUrl, String leaderCoreName) { // workaround for SOLR-13605: get the configured timeouts & set them directly // (even though getRecoveryOnlyHttpClient() already has them set) final UpdateShardHandlerConfig cfg = cc.getConfig().getUpdateShardHandlerConfig(); - return (new HttpSolrClient.Builder(baseUrl) + return new Http2SolrClient.Builder(baseUrl) .withDefaultCollection(leaderCoreName) - .withConnectionTimeout(cfg.getDistributedConnectionTimeout(), TimeUnit.MILLISECONDS) - .withSocketTimeout(cfg.getDistributedSocketTimeout(), TimeUnit.MILLISECONDS) - .withHttpClient(cc.getUpdateShardHandler().getRecoveryOnlyHttpClient())); + .withHttpClient(cc.getUpdateShardHandler().getRecoveryOnlyHttpClient()); } // make sure any threads stop retrying @@ -192,7 +193,7 @@ private HttpSolrClient.Builder recoverySolrClientBuilder(String baseUrl, String public final void close() { close = true; if (prevSendPreRecoveryHttpUriRequest != null) { - prevSendPreRecoveryHttpUriRequest.abort(); + prevSendPreRecoveryHttpUriRequest.cancel(); } log.warn("Stopping recovery for core=[{}] coreNodeName=[{}]", coreName, coreZkNodeName); } @@ -631,7 +632,7 @@ public final void doSyncOrReplicateRecovery(SolrCore core) throws Exception { .getSlice(cloudDesc.getShardId()); try { - prevSendPreRecoveryHttpUriRequest.abort(); + prevSendPreRecoveryHttpUriRequest.cancel(); } catch (NullPointerException e) { // okay } @@ -890,7 +891,6 @@ public final boolean isClosed() { private final void sendPrepRecoveryCmd(String leaderBaseUrl, String leaderCoreName, Slice slice) throws SolrServerException, IOException, InterruptedException, ExecutionException { - WaitForState prepCmd = new WaitForState(); prepCmd.setCoreName(leaderCoreName); prepCmd.setNodeName(zkController.getNodeName()); @@ -911,18 +911,39 @@ private final void sendPrepRecoveryCmd(String leaderBaseUrl, String leaderCoreNa int readTimeout = conflictWaitMs + Integer.parseInt(System.getProperty("prepRecoveryReadTimeoutExtraWait", "8000")); - try (HttpSolrClient client = + var recoveryExec = + ExecutorUtil.newMDCAwareFixedThreadPool( + 1, new SolrNamedThreadFactory("sendPrepRecoveryCmd")); + try (Http2SolrClient client = recoverySolrClientBuilder( leaderBaseUrl, null) // leader core omitted since client only used for 'admin' request - .withSocketTimeout(readTimeout, TimeUnit.MILLISECONDS) + .withIdleTimeout(readTimeout, TimeUnit.MILLISECONDS) + .withExecutor(recoveryExec) .build()) { - HttpUriRequestResponse mrr = client.httpUriRequest(prepCmd); - prevSendPreRecoveryHttpUriRequest = mrr.httpUriRequest; - log.info("Sending prep recovery command to [{}]; [{}]", leaderBaseUrl, prepCmd); - - mrr.future.get(); + MDC.put("HttpSolrClient.url", baseUrl); + prevSendPreRecoveryHttpUriRequest = + client.asyncRequest( + prepCmd, + null, + new AsyncListener<>() { + @Override + public void onSuccess(NamedList entries) { + log.info( + "Prep recovery command successfully send to [{}]; [{}]", + leaderBaseUrl, + prepCmd); + } + + @Override + public void onFailure(Throwable throwable) { + log.error("Prep recovery command failed! [{}] [{}]", leaderBaseUrl, prepCmd); + } + }); + } finally { + recoveryExec.shutdown(); + MDC.remove("HttpSolrClient.url"); } } } diff --git a/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java b/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java index b3ab8cb9156..610dc66b19f 100644 --- a/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java +++ b/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java @@ -75,7 +75,8 @@ public class UpdateShardHandler implements SolrInfoBean { private final Http2SolrClient updateOnlyClient; - private final CloseableHttpClient recoveryOnlyClient; + // private final CloseableHttpClient recoveryOnlyClient; + private final Http2SolrClient recoveryOnlyClient; private final CloseableHttpClient defaultClient; @@ -87,6 +88,8 @@ public class UpdateShardHandler implements SolrInfoBean { private final InstrumentedHttpListenerFactory updateHttpListenerFactory; + private final InstrumentedHttpListenerFactory recoverHttpListenerFactory; + private SolrMetricsContext solrMetricsContext; private int socketTimeout = HttpClientUtil.DEFAULT_SO_TIMEOUT; @@ -121,9 +124,8 @@ public UpdateShardHandler(UpdateShardHandlerConfig cfg) { httpRequestExecutor = new InstrumentedHttpRequestExecutor(getMetricNameStrategy(cfg)); updateHttpListenerFactory = new InstrumentedHttpListenerFactory(getNameStrategy(cfg)); - recoveryOnlyClient = - HttpClientUtil.createClient( - clientParams, recoveryOnlyConnectionManager, false, httpRequestExecutor); + recoverHttpListenerFactory = new InstrumentedHttpListenerFactory(getNameStrategy(cfg)); + defaultClient = HttpClientUtil.createClient( clientParams, defaultConnectionManager, false, httpRequestExecutor); @@ -133,16 +135,25 @@ public UpdateShardHandler(UpdateShardHandlerConfig cfg) { DistributedUpdateProcessor.DISTRIB_FROM, DistributingUpdateProcessorFactory.DISTRIB_UPDATE_PARAM); Http2SolrClient.Builder updateOnlyClientBuilder = new Http2SolrClient.Builder(); + Http2SolrClient.Builder recoveryOnlyClientBuilder = new Http2SolrClient.Builder(); if (cfg != null) { updateOnlyClientBuilder .withConnectionTimeout(cfg.getDistributedConnectionTimeout(), TimeUnit.MILLISECONDS) .withIdleTimeout(cfg.getDistributedSocketTimeout(), TimeUnit.MILLISECONDS) .withMaxConnectionsPerHost(cfg.getMaxUpdateConnectionsPerHost()); + recoveryOnlyClientBuilder + .withConnectionTimeout(cfg.getDistributedConnectionTimeout(), TimeUnit.MILLISECONDS) + .withIdleTimeout(cfg.getDistributedSocketTimeout(), TimeUnit.MILLISECONDS) + .withMaxConnectionsPerHost(cfg.getMaxUpdateConnectionsPerHost()); } + updateOnlyClientBuilder.withTheseParamNamesInTheUrl(urlParamNames); updateOnlyClient = updateOnlyClientBuilder.build(); updateOnlyClient.addListenerFactory(updateHttpListenerFactory); + recoveryOnlyClient = recoveryOnlyClientBuilder.build(); + recoveryOnlyClient.addListenerFactory(recoverHttpListenerFactory); + ThreadFactory recoveryThreadFactory = new SolrNamedThreadFactory("recoveryExecutor"); if (cfg != null && cfg.getMaxRecoveryThreads() > 0) { if (log.isDebugEnabled()) { @@ -247,7 +258,7 @@ public Http2SolrClient getUpdateOnlyHttpClient() { } // don't introduce a bug, this client is for recovery ops only! - public HttpClient getRecoveryOnlyHttpClient() { + public Http2SolrClient getRecoveryOnlyHttpClient() { return recoveryOnlyClient; } @@ -290,7 +301,7 @@ public void close() { // do nothing } IOUtils.closeQuietly(updateOnlyClient); - HttpClientUtil.close(recoveryOnlyClient); + IOUtils.closeQuietly(recoveryOnlyClient); HttpClientUtil.close(defaultClient); defaultConnectionManager.close(); recoveryOnlyConnectionManager.close(); diff --git a/solr/core/src/test/org/apache/solr/cloud/RecoveryStrategyStressTest.java b/solr/core/src/test/org/apache/solr/cloud/RecoveryStrategyStressTest.java new file mode 100644 index 00000000000..9334333a5dd --- /dev/null +++ b/solr/core/src/test/org/apache/solr/cloud/RecoveryStrategyStressTest.java @@ -0,0 +1,135 @@ +/* + * 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.cloud; + +import java.io.IOException; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import org.apache.solr.client.solrj.SolrClient; +import org.apache.solr.client.solrj.SolrQuery; +import org.apache.solr.client.solrj.SolrServerException; +import org.apache.solr.client.solrj.impl.CloudLegacySolrClient; +import org.apache.solr.client.solrj.impl.HttpSolrClient; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.client.solrj.request.UpdateRequest; +import org.apache.solr.common.cloud.DocCollection; +import org.apache.solr.common.cloud.Replica; +import org.apache.solr.common.cloud.Slice; +import org.apache.solr.common.util.SolrNamedThreadFactory; +import org.apache.solr.embedded.JettySolrRunner; +import org.junit.BeforeClass; +import org.junit.Test; + +public class RecoveryStrategyStressTest extends SolrCloudTestCase { + + @BeforeClass + public static void setupCluster() throws Exception { + cluster = configureCluster(4).addConfig("conf", configset("cloud-minimal")).configure(); + } + + @Test + public void stressTestRecovery() throws Exception { + final String collection = "recoveryStressTest"; + CollectionAdminRequest.createCollection(collection, "conf", 1, 4) + .process(cluster.getSolrClient()); + waitForState( + "Expected a collection with one shard and two replicas", collection, clusterShape(1, 4)); + + SolrClient solrClient = + cluster.basicSolrClientBuilder().withDefaultCollection(collection).build(); + final var scheduledExecutorService = + Executors.newScheduledThreadPool(1, new SolrNamedThreadFactory("stressTestRecovery")); + try (solrClient) { + final StoppableIndexingThread indexThread = + new StoppableIndexingThread(null, solrClient, "1", true, 10, 1, true); + + final var startAndStopCount = new CountDownLatch(50); + final Thread startAndStopRandomReplicas = + new Thread( + () -> { + try { + while (startAndStopCount.getCount() > 0) { + DocCollection state = getCollectionState(collection); + Replica leader = state.getLeader("shard1"); + Replica replica = + getRandomReplica(state.getSlice("shard1"), (r) -> !leader.equals(r)); + + JettySolrRunner jetty = cluster.getReplicaJetty(replica); + jetty.stop(); + Thread.sleep(100); + jetty.start(); + startAndStopCount.countDown(); + } + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + startAndStopRandomReplicas.start(); + // index and commit doc after fixed interval of 10 sec + scheduledExecutorService.scheduleWithFixedDelay( + indexThread, 1000, 10000, TimeUnit.MILLISECONDS); + scheduledExecutorService.scheduleWithFixedDelay( + () -> { + try { + new UpdateRequest().commit(solrClient, collection); + } catch (IOException e) { + throw new RuntimeException(e); + } catch (SolrServerException e) { + throw new RuntimeException(e); + } + }, + 100, + 10000, + TimeUnit.MILLISECONDS); + + startAndStopCount.await(); + scheduledExecutorService.shutdownNow(); + // final commit to make documents visible for replicas + new UpdateRequest().commit(solrClient, collection); + } + cluster.getZkStateReader().waitForState(collection, 120, TimeUnit.SECONDS, clusterShape(1, 4)); + + // test that leader and replica have same doc count + DocCollection state = getCollectionState(collection); + assertShardConsistency(state.getSlice("shard1"), true); + } + + private void assertShardConsistency(Slice shard, boolean expectDocs) throws Exception { + List replicas = shard.getReplicas(r -> r.getState() == Replica.State.ACTIVE); + long[] numCounts = new long[replicas.size()]; + int i = 0; + for (Replica replica : replicas) { + try (var client = + new HttpSolrClient.Builder(replica.getBaseUrl()) + .withDefaultCollection(replica.getCoreName()) + .withHttpClient(((CloudLegacySolrClient) cluster.getSolrClient()).getHttpClient()) + .build()) { + numCounts[i] = + client.query(new SolrQuery("*:*").add("distrib", "false")).getResults().getNumFound(); + i++; + } + } + for (int j = 1; j < replicas.size(); j++) { + if (numCounts[j] != numCounts[j - 1]) + fail("Mismatch in counts between replicas"); // TODO improve this! + if (numCounts[j] == 0 && expectDocs) + fail("Expected docs on shard " + shard.getName() + " but found none"); + } + } +} 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 e2b847a2954..796dffb10be 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 @@ -203,6 +203,9 @@ protected Http2SolrClient(String serverBaseUrl, Builder builder) { } else { this.urlParamNames = Set.of(); } + if (builder.executor != null) { + this.executor = builder.executor; + } assert ObjectReleaseTracker.track(this); } @@ -500,7 +503,6 @@ public Cancellable asyncRequest( String collection, AsyncListener> asyncListener) { MDCCopyHelper mdcCopyHelper = new MDCCopyHelper(); - Request req; try { String url = getRequestPath(solrRequest, collection); @@ -1160,7 +1162,6 @@ private static CookieStore getDefaultCookieStore() { */ public Builder withHttpClient(Http2SolrClient http2SolrClient) { this.httpClient = http2SolrClient.httpClient; - if (this.basicAuthAuthorizationStr == null) { this.basicAuthAuthorizationStr = http2SolrClient.basicAuthAuthorizationStr; } From 3572decb16440259469b9c60d6b1eb646b2b7fe7 Mon Sep 17 00:00:00 2001 From: iamsanjay Date: Tue, 20 Feb 2024 17:04:47 +0530 Subject: [PATCH 02/24] SOLR-16505: Switch UpdateShardHandler.getRecoveryOnlyHttpClient to Jetty HTTP2 --- .../apache/solr/cloud/RecoveryStrategy.java | 10 +------- .../solr/update/UpdateShardHandler.java | 14 ++++------- .../cloud/RecoveryStrategyStressTest.java | 8 +++--- .../client/solrj/impl/Http2SolrClient.java | 25 +++++++++++-------- 4 files changed, 24 insertions(+), 33 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java b/solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java index ef3e60dcf54..d7d79d789c0 100644 --- a/solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java +++ b/solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java @@ -47,9 +47,7 @@ import org.apache.solr.common.cloud.ZooKeeperException; import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.params.UpdateParams; -import org.apache.solr.common.util.ExecutorUtil; import org.apache.solr.common.util.NamedList; -import org.apache.solr.common.util.SolrNamedThreadFactory; import org.apache.solr.common.util.URLUtil; import org.apache.solr.core.CoreContainer; import org.apache.solr.core.CoreDescriptor; @@ -178,7 +176,6 @@ public final void setRecoveringAfterStartup(boolean recoveringAfterStartup) { this.recoveringAfterStartup = recoveringAfterStartup; } - /** Builds a new HttpSolrClient for use in recovery. Caller must close */ private Http2SolrClient.Builder recoverySolrClientBuilder(String baseUrl, String leaderCoreName) { // workaround for SOLR-13605: get the configured timeouts & set them directly // (even though getRecoveryOnlyHttpClient() already has them set) @@ -915,15 +912,11 @@ private final void sendPrepRecoveryCmd(String leaderBaseUrl, String leaderCoreNa int readTimeout = conflictWaitMs + Integer.parseInt(System.getProperty("prepRecoveryReadTimeoutExtraWait", "8000")); - var recoveryExec = - ExecutorUtil.newMDCAwareFixedThreadPool( - 1, new SolrNamedThreadFactory("sendPrepRecoveryCmd")); try (Http2SolrClient client = recoverySolrClientBuilder( leaderBaseUrl, null) // leader core omitted since client only used for 'admin' request .withIdleTimeout(readTimeout, TimeUnit.MILLISECONDS) - .withExecutor(recoveryExec) .build()) { log.info("Sending prep recovery command to [{}]; [{}]", leaderBaseUrl, prepCmd); MDC.put("HttpSolrClient.url", baseUrl); @@ -935,7 +928,7 @@ private final void sendPrepRecoveryCmd(String leaderBaseUrl, String leaderCoreNa @Override public void onSuccess(NamedList entries) { log.info( - "Prep recovery command successfully send to [{}]; [{}]", + "Prep recovery command successfully sent to [{}]; [{}]", leaderBaseUrl, prepCmd); } @@ -946,7 +939,6 @@ public void onFailure(Throwable throwable) { } }); } finally { - recoveryExec.shutdown(); MDC.remove("HttpSolrClient.url"); } } diff --git a/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java b/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java index 610dc66b19f..603533040ad 100644 --- a/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java +++ b/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java @@ -75,7 +75,6 @@ public class UpdateShardHandler implements SolrInfoBean { private final Http2SolrClient updateOnlyClient; - // private final CloseableHttpClient recoveryOnlyClient; private final Http2SolrClient recoveryOnlyClient; private final CloseableHttpClient defaultClient; @@ -86,9 +85,7 @@ public class UpdateShardHandler implements SolrInfoBean { private final InstrumentedHttpRequestExecutor httpRequestExecutor; - private final InstrumentedHttpListenerFactory updateHttpListenerFactory; - - private final InstrumentedHttpListenerFactory recoverHttpListenerFactory; + private final InstrumentedHttpListenerFactory trackHttpSolrMetrics; private SolrMetricsContext solrMetricsContext; @@ -123,8 +120,7 @@ public UpdateShardHandler(UpdateShardHandlerConfig cfg) { log.debug("Created default UpdateShardHandler HTTP client with params: {}", clientParams); httpRequestExecutor = new InstrumentedHttpRequestExecutor(getMetricNameStrategy(cfg)); - updateHttpListenerFactory = new InstrumentedHttpListenerFactory(getNameStrategy(cfg)); - recoverHttpListenerFactory = new InstrumentedHttpListenerFactory(getNameStrategy(cfg)); + trackHttpSolrMetrics = new InstrumentedHttpListenerFactory(getNameStrategy(cfg)); defaultClient = HttpClientUtil.createClient( @@ -149,10 +145,10 @@ public UpdateShardHandler(UpdateShardHandlerConfig cfg) { updateOnlyClientBuilder.withTheseParamNamesInTheUrl(urlParamNames); updateOnlyClient = updateOnlyClientBuilder.build(); - updateOnlyClient.addListenerFactory(updateHttpListenerFactory); + updateOnlyClient.addListenerFactory(trackHttpSolrMetrics); recoveryOnlyClient = recoveryOnlyClientBuilder.build(); - recoveryOnlyClient.addListenerFactory(recoverHttpListenerFactory); + recoveryOnlyClient.addListenerFactory(trackHttpSolrMetrics); ThreadFactory recoveryThreadFactory = new SolrNamedThreadFactory("recoveryExecutor"); if (cfg != null && cfg.getMaxRecoveryThreads() > 0) { @@ -216,7 +212,7 @@ public String getName() { public void initializeMetrics(SolrMetricsContext parentContext, String scope) { solrMetricsContext = parentContext.getChildContext(this); String expandedScope = SolrMetricManager.mkName(scope, getCategory().name()); - updateHttpListenerFactory.initializeMetrics(solrMetricsContext, expandedScope); + trackHttpSolrMetrics.initializeMetrics(solrMetricsContext, expandedScope); defaultConnectionManager.initializeMetrics(solrMetricsContext, expandedScope); updateExecutor = MetricUtils.instrumentedExecutorService( diff --git a/solr/core/src/test/org/apache/solr/cloud/RecoveryStrategyStressTest.java b/solr/core/src/test/org/apache/solr/cloud/RecoveryStrategyStressTest.java index 9334333a5dd..06c03971a96 100644 --- a/solr/core/src/test/org/apache/solr/cloud/RecoveryStrategyStressTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/RecoveryStrategyStressTest.java @@ -16,6 +16,7 @@ */ package org.apache.solr.cloud; +import com.carrotsearch.randomizedtesting.annotations.Nightly; import java.io.IOException; import java.util.List; import java.util.concurrent.CountDownLatch; @@ -36,6 +37,7 @@ import org.junit.BeforeClass; import org.junit.Test; +@Nightly public class RecoveryStrategyStressTest extends SolrCloudTestCase { @BeforeClass @@ -50,12 +52,10 @@ public void stressTestRecovery() throws Exception { .process(cluster.getSolrClient()); waitForState( "Expected a collection with one shard and two replicas", collection, clusterShape(1, 4)); - - SolrClient solrClient = - cluster.basicSolrClientBuilder().withDefaultCollection(collection).build(); final var scheduledExecutorService = Executors.newScheduledThreadPool(1, new SolrNamedThreadFactory("stressTestRecovery")); - try (solrClient) { + try (SolrClient solrClient = + cluster.basicSolrClientBuilder().withDefaultCollection(collection).build()) { final StoppableIndexingThread indexThread = new StoppableIndexingThread(null, solrClient, "1", true, 10, 1, true); 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 796dffb10be..f395b158ee5 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 @@ -176,6 +176,16 @@ protected Http2SolrClient(String serverBaseUrl, Builder builder) { this.idleTimeoutMillis = builder.idleTimeoutMillis; + executor = builder.executor; + if (executor == null) { + BlockingArrayQueue queue = new BlockingArrayQueue<>(256, 256); + this.executor = + new ExecutorUtil.MDCAwareThreadPoolExecutor( + 32, 256, 60, TimeUnit.SECONDS, queue, new SolrNamedThreadFactory("h2sc")); + shutdownExecutor = true; + } else { + shutdownExecutor = false; + } if (builder.httpClient != null) { this.httpClient = builder.httpClient; this.closeClient = false; @@ -226,17 +236,6 @@ ProtocolHandlers getProtocolHandlers() { private HttpClient createHttpClient(Builder builder) { HttpClient httpClient; - executor = builder.executor; - if (executor == null) { - BlockingArrayQueue queue = new BlockingArrayQueue<>(256, 256); - this.executor = - new ExecutorUtil.MDCAwareThreadPoolExecutor( - 32, 256, 60, TimeUnit.SECONDS, queue, new SolrNamedThreadFactory("h2sc")); - shutdownExecutor = true; - } else { - shutdownExecutor = false; - } - SslContextFactory.Client sslContextFactory; if (builder.sslConfig == null) { sslContextFactory = getDefaultSslContextFactory(); @@ -1162,6 +1161,10 @@ private static CookieStore getDefaultCookieStore() { */ public Builder withHttpClient(Http2SolrClient http2SolrClient) { this.httpClient = http2SolrClient.httpClient; + + if (this.executor == null) { + this.executor = http2SolrClient.executor; + } if (this.basicAuthAuthorizationStr == null) { this.basicAuthAuthorizationStr = http2SolrClient.basicAuthAuthorizationStr; } From 0655f118d3a4f446f4b5f356641082a7428d49d0 Mon Sep 17 00:00:00 2001 From: iamsanjay Date: Tue, 27 Feb 2024 12:05:07 +0530 Subject: [PATCH 03/24] Using FutureTask to send PREPRECOVERY, without executor --- .../apache/solr/cloud/RecoveryStrategy.java | 34 ++++--------------- .../client/solrj/impl/Http2SolrClient.java | 28 +++++++-------- 2 files changed, 19 insertions(+), 43 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java b/solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java index d7d79d789c0..144f5f1642a 100644 --- a/solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java +++ b/solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java @@ -24,6 +24,7 @@ import java.util.List; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; +import java.util.concurrent.FutureTask; import java.util.concurrent.TimeUnit; import org.apache.lucene.index.IndexCommit; import org.apache.lucene.search.MatchAllDocsQuery; @@ -34,8 +35,6 @@ import org.apache.solr.client.solrj.request.AbstractUpdateRequest; import org.apache.solr.client.solrj.request.CoreAdminRequest.WaitForState; import org.apache.solr.client.solrj.request.UpdateRequest; -import org.apache.solr.client.solrj.util.AsyncListener; -import org.apache.solr.client.solrj.util.Cancellable; import org.apache.solr.common.SolrException; import org.apache.solr.common.SolrException.ErrorCode; import org.apache.solr.common.cloud.DocCollection; @@ -69,7 +68,6 @@ import org.apache.solr.util.plugin.NamedListInitializedPlugin; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.slf4j.MDC; /** * This class may change in future and customisations are not supported between versions in terms of @@ -125,7 +123,7 @@ public static interface RecoveryListener { private int retries; private boolean recoveringAfterStartup; private CoreContainer cc; - private volatile Cancellable prevSendPreRecoveryHttpUriRequest; + private volatile FutureTask> prevSendPreRecoveryHttpUriRequest; private final Replica.Type replicaType; private CoreDescriptor coreDescriptor; @@ -190,7 +188,7 @@ private Http2SolrClient.Builder recoverySolrClientBuilder(String baseUrl, String public final void close() { close = true; if (prevSendPreRecoveryHttpUriRequest != null) { - prevSendPreRecoveryHttpUriRequest.cancel(); + prevSendPreRecoveryHttpUriRequest.cancel(true); } log.warn("Stopping recovery for core=[{}] coreNodeName=[{}]", coreName, coreZkNodeName); } @@ -633,9 +631,10 @@ public final void doSyncOrReplicateRecovery(SolrCore core) throws Exception { .getSlice(cloudDesc.getShardId()); try { - prevSendPreRecoveryHttpUriRequest.cancel(); + prevSendPreRecoveryHttpUriRequest.cancel(true); } catch (NullPointerException e) { // okay + log.info("Failed to abort the Prep Recovery command as it has not been sent yet."); } if (isClosed()) { @@ -918,28 +917,9 @@ private final void sendPrepRecoveryCmd(String leaderBaseUrl, String leaderCoreNa null) // leader core omitted since client only used for 'admin' request .withIdleTimeout(readTimeout, TimeUnit.MILLISECONDS) .build()) { + prevSendPreRecoveryHttpUriRequest = new FutureTask<>(() -> client.request(prepCmd)); log.info("Sending prep recovery command to [{}]; [{}]", leaderBaseUrl, prepCmd); - MDC.put("HttpSolrClient.url", baseUrl); - prevSendPreRecoveryHttpUriRequest = - client.asyncRequest( - prepCmd, - null, - new AsyncListener<>() { - @Override - public void onSuccess(NamedList entries) { - log.info( - "Prep recovery command successfully sent to [{}]; [{}]", - leaderBaseUrl, - prepCmd); - } - - @Override - public void onFailure(Throwable throwable) { - log.error("Prep recovery command failed! [{}] [{}]", leaderBaseUrl, prepCmd); - } - }); - } finally { - MDC.remove("HttpSolrClient.url"); + prevSendPreRecoveryHttpUriRequest.run(); } } } 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 af2ec479704..79de13f4a26 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 @@ -176,16 +176,6 @@ protected Http2SolrClient(String serverBaseUrl, Builder builder) { this.idleTimeoutMillis = builder.idleTimeoutMillis; - executor = builder.executor; - if (executor == null) { - BlockingArrayQueue queue = new BlockingArrayQueue<>(256, 256); - this.executor = - new ExecutorUtil.MDCAwareThreadPoolExecutor( - 32, 256, 60, TimeUnit.SECONDS, queue, new SolrNamedThreadFactory("h2sc")); - shutdownExecutor = true; - } else { - shutdownExecutor = false; - } if (builder.httpClient != null) { this.httpClient = builder.httpClient; this.closeClient = false; @@ -213,9 +203,6 @@ protected Http2SolrClient(String serverBaseUrl, Builder builder) { } else { this.urlParamNames = Set.of(); } - if (builder.executor != null) { - this.executor = builder.executor; - } assert ObjectReleaseTracker.track(this); } @@ -236,6 +223,17 @@ ProtocolHandlers getProtocolHandlers() { private HttpClient createHttpClient(Builder builder) { HttpClient httpClient; + executor = builder.executor; + if (executor == null) { + BlockingArrayQueue queue = new BlockingArrayQueue<>(256, 256); + this.executor = + new ExecutorUtil.MDCAwareThreadPoolExecutor( + 32, 256, 60, TimeUnit.SECONDS, queue, new SolrNamedThreadFactory("h2sc")); + shutdownExecutor = true; + } else { + shutdownExecutor = false; + } + SslContextFactory.Client sslContextFactory; if (builder.sslConfig == null) { sslContextFactory = getDefaultSslContextFactory(); @@ -502,6 +500,7 @@ public Cancellable asyncRequest( String collection, AsyncListener> asyncListener) { MDCCopyHelper mdcCopyHelper = new MDCCopyHelper(); + Request req; try { String url = getRequestPath(solrRequest, collection); @@ -1177,9 +1176,6 @@ private static CookieStore getDefaultCookieStore() { public Builder withHttpClient(Http2SolrClient http2SolrClient) { this.httpClient = http2SolrClient.httpClient; - if (this.executor == null) { - this.executor = http2SolrClient.executor; - } if (this.basicAuthAuthorizationStr == null) { this.basicAuthAuthorizationStr = http2SolrClient.basicAuthAuthorizationStr; } From 061178287824e76413ddcf6840e8b02e1293dee8 Mon Sep 17 00:00:00 2001 From: iamsanjay Date: Thu, 29 Feb 2024 21:14:46 +0530 Subject: [PATCH 04/24] Null check for FutureTask, removed try-catch --- solr/CHANGES.txt | 2 ++ .../src/java/org/apache/solr/cloud/RecoveryStrategy.java | 9 ++------- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 74b08e2c59e..4ed49407290 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -140,6 +140,8 @@ Other Changes * SOLR-17066: GenericSolrRequest now has a `setRequiresCollection` setter that allows it to specify whether it should make use of the client-level default collection/core. (Jason Gerlowski) +* SOLR-16505: Switch internal replica recovery commands to Jetty HTTP2 (Sanjay Dutt, David Smiley) + ================== 9.5.0 ================== New Features --------------------- diff --git a/solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java b/solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java index 144f5f1642a..793c1c4271c 100644 --- a/solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java +++ b/solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java @@ -175,8 +175,6 @@ public final void setRecoveringAfterStartup(boolean recoveringAfterStartup) { } private Http2SolrClient.Builder recoverySolrClientBuilder(String baseUrl, String leaderCoreName) { - // workaround for SOLR-13605: get the configured timeouts & set them directly - // (even though getRecoveryOnlyHttpClient() already has them set) final UpdateShardHandlerConfig cfg = cc.getConfig().getUpdateShardHandlerConfig(); return new Http2SolrClient.Builder(baseUrl) .withDefaultCollection(leaderCoreName) @@ -630,11 +628,8 @@ public final void doSyncOrReplicateRecovery(SolrCore core) throws Exception { .getCollection(cloudDesc.getCollectionName()) .getSlice(cloudDesc.getShardId()); - try { + if (prevSendPreRecoveryHttpUriRequest != null) { prevSendPreRecoveryHttpUriRequest.cancel(true); - } catch (NullPointerException e) { - // okay - log.info("Failed to abort the Prep Recovery command as it has not been sent yet."); } if (isClosed()) { @@ -911,7 +906,7 @@ private final void sendPrepRecoveryCmd(String leaderBaseUrl, String leaderCoreNa int readTimeout = conflictWaitMs + Integer.parseInt(System.getProperty("prepRecoveryReadTimeoutExtraWait", "8000")); - try (Http2SolrClient client = + try (SolrClient client = recoverySolrClientBuilder( leaderBaseUrl, null) // leader core omitted since client only used for 'admin' request From 91f3c9de84fa148ab0e688353ee8255285737f72 Mon Sep 17 00:00:00 2001 From: iamsanjay Date: Fri, 1 Mar 2024 15:36:08 +0530 Subject: [PATCH 05/24] code format, added test case --- .../apache/solr/cloud/RecoveryStrategy.java | 7 +++--- .../solrj/impl/Http2SolrClientTest.java | 22 +++++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java b/solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java index 793c1c4271c..e6e6d4fa855 100644 --- a/solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java +++ b/solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Optional; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; import java.util.concurrent.FutureTask; @@ -175,6 +176,8 @@ public final void setRecoveringAfterStartup(boolean recoveringAfterStartup) { } private Http2SolrClient.Builder recoverySolrClientBuilder(String baseUrl, String leaderCoreName) { + // workaround for SOLR-13605: get the configured timeouts & set them directly + // (even though getRecoveryOnlyHttpClient() already has them set) final UpdateShardHandlerConfig cfg = cc.getConfig().getUpdateShardHandlerConfig(); return new Http2SolrClient.Builder(baseUrl) .withDefaultCollection(leaderCoreName) @@ -628,9 +631,7 @@ public final void doSyncOrReplicateRecovery(SolrCore core) throws Exception { .getCollection(cloudDesc.getCollectionName()) .getSlice(cloudDesc.getShardId()); - if (prevSendPreRecoveryHttpUriRequest != null) { - prevSendPreRecoveryHttpUriRequest.cancel(true); - } + Optional.ofNullable(prevSendPreRecoveryHttpUriRequest).ifPresent(req -> req.cancel(true)); if (isClosed()) { log.info("RecoveryStrategy has been closed"); diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java index 3c15cbd95ef..8fe3765d501 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java @@ -1112,5 +1112,27 @@ public void testBuilder() { } } + @Test + public void testIdleTimeoutWithHttpClient() { + try (Http2SolrClient oldClient = + new Http2SolrClient.Builder("baseSolrUrl") + .withIdleTimeout(5000, TimeUnit.MILLISECONDS) + .build()) { + try (Http2SolrClient onlyBaseUrlChangedClient = + new Http2SolrClient.Builder("newBaseSolrUrl").withHttpClient(oldClient).build()) { + assertEquals(oldClient.getIdleTimeout(), onlyBaseUrlChangedClient.getIdleTimeout()); + assertEquals(oldClient.getHttpClient(), onlyBaseUrlChangedClient.getHttpClient()); + } + try (Http2SolrClient idleTimeoutChangedClient = + new Http2SolrClient.Builder("baseSolrUrl") + .withHttpClient(oldClient) + .withIdleTimeout(3000, TimeUnit.MILLISECONDS) + .build()) { + assertFalse(oldClient.getIdleTimeout() == idleTimeoutChangedClient.getIdleTimeout()); + assertEquals(3000, idleTimeoutChangedClient.getIdleTimeout()); + } + } + } + /* Missed tests : - set cookies via interceptor - invariant params - compression */ } From 1c6798b0a012f47ab88ed091b9015462016dc8e8 Mon Sep 17 00:00:00 2001 From: iamsanjay Date: Sun, 3 Mar 2024 22:34:03 +0530 Subject: [PATCH 06/24] Remove comment, create method for cancel recovery --- .../java/org/apache/solr/cloud/RecoveryStrategy.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java b/solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java index e6e6d4fa855..d9453757dcc 100644 --- a/solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java +++ b/solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java @@ -176,8 +176,6 @@ public final void setRecoveringAfterStartup(boolean recoveringAfterStartup) { } private Http2SolrClient.Builder recoverySolrClientBuilder(String baseUrl, String leaderCoreName) { - // workaround for SOLR-13605: get the configured timeouts & set them directly - // (even though getRecoveryOnlyHttpClient() already has them set) final UpdateShardHandlerConfig cfg = cc.getConfig().getUpdateShardHandlerConfig(); return new Http2SolrClient.Builder(baseUrl) .withDefaultCollection(leaderCoreName) @@ -188,9 +186,7 @@ private Http2SolrClient.Builder recoverySolrClientBuilder(String baseUrl, String @Override public final void close() { close = true; - if (prevSendPreRecoveryHttpUriRequest != null) { - prevSendPreRecoveryHttpUriRequest.cancel(true); - } + cancelPrepRecoveryCmd(); log.warn("Stopping recovery for core=[{}] coreNodeName=[{}]", coreName, coreZkNodeName); } @@ -631,7 +627,7 @@ public final void doSyncOrReplicateRecovery(SolrCore core) throws Exception { .getCollection(cloudDesc.getCollectionName()) .getSlice(cloudDesc.getShardId()); - Optional.ofNullable(prevSendPreRecoveryHttpUriRequest).ifPresent(req -> req.cancel(true)); + cancelPrepRecoveryCmd(); if (isClosed()) { log.info("RecoveryStrategy has been closed"); @@ -918,4 +914,8 @@ private final void sendPrepRecoveryCmd(String leaderBaseUrl, String leaderCoreNa prevSendPreRecoveryHttpUriRequest.run(); } } + + private void cancelPrepRecoveryCmd() { + Optional.ofNullable(prevSendPreRecoveryHttpUriRequest).ifPresent(req -> req.cancel(true)); + } } From ae368b5e4357e67257b6603727972e36a1907fba Mon Sep 17 00:00:00 2001 From: iamsanjay Date: Thu, 7 Mar 2024 17:40:46 +0530 Subject: [PATCH 07/24] Update IndexFetcher Class to Use Http2SolrClient --- .../org/apache/solr/handler/IndexFetcher.java | 91 +++++++------------ .../solr/update/UpdateShardHandler.java | 13 +-- .../apache/solr/client/solrj/SolrRequest.java | 9 ++ .../client/solrj/impl/Http2SolrClient.java | 12 ++- 4 files changed, 54 insertions(+), 71 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java index 4dbe9faf44b..36d7a1da089 100644 --- a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java +++ b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java @@ -84,7 +84,6 @@ import java.util.zip.Adler32; import java.util.zip.Checksum; import java.util.zip.InflaterInputStream; -import org.apache.http.client.HttpClient; import org.apache.lucene.codecs.CodecUtil; import org.apache.lucene.index.IndexCommit; import org.apache.lucene.index.IndexWriter; @@ -97,9 +96,9 @@ import org.apache.lucene.store.IndexOutput; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.SolrServerException; +import org.apache.solr.client.solrj.impl.Http2SolrClient; import org.apache.solr.client.solrj.impl.HttpClientUtil; -import org.apache.solr.client.solrj.impl.HttpSolrClient; -import org.apache.solr.client.solrj.impl.HttpSolrClient.Builder; +import org.apache.solr.client.solrj.impl.InputStreamResponseParser; import org.apache.solr.client.solrj.request.QueryRequest; import org.apache.solr.cloud.CloudDescriptor; import org.apache.solr.cloud.ZkController; @@ -133,7 +132,7 @@ import org.apache.solr.util.RTimer; import org.apache.solr.util.RefCounted; import org.apache.solr.util.TestInjection; -import org.apache.solr.util.stats.InstrumentedHttpRequestExecutor; +import org.apache.solr.util.stats.InstrumentedHttpListenerFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -186,7 +185,7 @@ public class IndexFetcher { boolean fetchFromLeader = false; - private final HttpClient myHttpClient; + private final SolrClient solrClient; private Integer connTimeout; @@ -261,22 +260,23 @@ public String getMessage() { } } - private static HttpClient createHttpClient( - SolrCore core, - String httpBasicAuthUser, - String httpBasicAuthPassword, - boolean useCompression) { + private Http2SolrClient createHttpClient( + SolrCore core, String httpBasicAuthUser, String httpBasicAuthPassword, String leaderBaseUrl) { final ModifiableSolrParams httpClientParams = new ModifiableSolrParams(); httpClientParams.set(HttpClientUtil.PROP_BASIC_AUTH_USER, httpBasicAuthUser); httpClientParams.set(HttpClientUtil.PROP_BASIC_AUTH_PASS, httpBasicAuthPassword); - httpClientParams.set(HttpClientUtil.PROP_ALLOW_COMPRESSION, useCompression); // no metrics, just tracing - InstrumentedHttpRequestExecutor executor = new InstrumentedHttpRequestExecutor(null); - return HttpClientUtil.createClient( - httpClientParams, - core.getCoreContainer().getUpdateShardHandler().getRecoveryOnlyConnectionManager(), - true, - executor); + InstrumentedHttpListenerFactory httpListenerFactory = new InstrumentedHttpListenerFactory(null); + Http2SolrClient httpClient = + new Http2SolrClient.Builder(leaderBaseUrl) + .withHttpClient( + core.getCoreContainer().getUpdateShardHandler().getRecoveryOnlyHttpClient()) + .withBasicAuthCredentials(httpBasicAuthUser, httpBasicAuthPassword) + .withIdleTimeout(soTimeout, TimeUnit.MILLISECONDS) + .withConnectionTimeout(connTimeout, TimeUnit.MILLISECONDS) + .build(); + httpClient.addListenerFactory(httpListenerFactory); + return httpClient; } public IndexFetcher( @@ -318,12 +318,10 @@ public IndexFetcher( if (soTimeout == -1) { soTimeout = getParameter(initArgs, HttpClientUtil.PROP_SO_TIMEOUT, 120000, null); } - String httpBasicAuthUser = (String) initArgs.get(HttpClientUtil.PROP_BASIC_AUTH_USER); String httpBasicAuthPassword = (String) initArgs.get(HttpClientUtil.PROP_BASIC_AUTH_PASS); - myHttpClient = - createHttpClient( - solrCore, httpBasicAuthUser, httpBasicAuthPassword, useExternalCompression); + solrClient = + createHttpClient(solrCore, httpBasicAuthUser, httpBasicAuthPassword, leaderBaseUrl); } private void setLeaderCoreUrl(String leaderCoreUrl) { @@ -380,16 +378,10 @@ public NamedList getLatestVersion() throws IOException { params.set(CommonParams.WT, JAVABIN); params.set(CommonParams.QT, ReplicationHandler.PATH); QueryRequest req = new QueryRequest(params); - + req.setBasePath(leaderBaseUrl); // TODO modify to use shardhandler - try (SolrClient client = - new Builder(leaderBaseUrl) - .withHttpClient(myHttpClient) - .withConnectionTimeout(connTimeout, TimeUnit.MILLISECONDS) - .withSocketTimeout(soTimeout, TimeUnit.MILLISECONDS) - .build()) { - - return client.request(req, leaderCoreName); + try { + return solrClient.request(req, leaderCoreName); } catch (SolrServerException e) { throw new SolrException(ErrorCode.SERVER_ERROR, e.getMessage(), e); } @@ -407,15 +399,10 @@ private void fetchFileList(long gen) throws IOException { params.set(CommonParams.WT, JAVABIN); params.set(CommonParams.QT, ReplicationHandler.PATH); QueryRequest req = new QueryRequest(params); - + req.setBasePath(leaderBaseUrl); // TODO modify to use shardhandler - try (SolrClient client = - new HttpSolrClient.Builder(leaderBaseUrl) - .withHttpClient(myHttpClient) - .withConnectionTimeout(connTimeout, TimeUnit.MILLISECONDS) - .withSocketTimeout(soTimeout, TimeUnit.MILLISECONDS) - .build()) { - NamedList response = client.request(req, leaderCoreName); + try { + NamedList response = solrClient.request(req, leaderCoreName); List> files = (List>) response.get(CMD_GET_FILE_LIST); if (files != null) filesToDownload = Collections.synchronizedList(files); @@ -1990,17 +1977,13 @@ private FastInputStream getStream() throws IOException { NamedList response; InputStream is = null; - // TODO use shardhandler - try (SolrClient client = - new Builder(leaderBaseUrl) - .withHttpClient(myHttpClient) - .withResponseParser(null) - .withConnectionTimeout(connTimeout, TimeUnit.MILLISECONDS) - .withSocketTimeout(soTimeout, TimeUnit.MILLISECONDS) - .build()) { + try { QueryRequest req = new QueryRequest(params); - response = client.request(req, leaderCoreName); + req.setResponseParser(new InputStreamResponseParser(FILE_STREAM)); + req.setBasePath(leaderBaseUrl); + req.setExternalCompression(useExternalCompression); + response = solrClient.request(req, leaderCoreName); is = (InputStream) response.get("stream"); if (useInternalCompression) { is = new InflaterInputStream(is); @@ -2124,21 +2107,15 @@ NamedList getDetails() throws IOException, SolrServerException { params.set("follower", false); params.set(CommonParams.QT, ReplicationHandler.PATH); + QueryRequest request = new QueryRequest(params); + request.setBasePath(leaderBaseUrl); // TODO use shardhandler - try (SolrClient client = - new HttpSolrClient.Builder(leaderBaseUrl) - .withHttpClient(myHttpClient) - .withConnectionTimeout(connTimeout, TimeUnit.MILLISECONDS) - .withSocketTimeout(soTimeout, TimeUnit.MILLISECONDS) - .build()) { - QueryRequest request = new QueryRequest(params); - return client.request(request, leaderCoreName); - } + return solrClient.request(request, leaderCoreName); } public void destroy() { abortFetch(); - HttpClientUtil.close(myHttpClient); + IOUtils.closeQuietly(solrClient); } String getLeaderCoreUrl() { diff --git a/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java b/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java index 603533040ad..650a61dc41d 100644 --- a/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java +++ b/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java @@ -79,8 +79,6 @@ public class UpdateShardHandler implements SolrInfoBean { private final CloseableHttpClient defaultClient; - private final InstrumentedPoolingHttpClientConnectionManager recoveryOnlyConnectionManager; - private final InstrumentedPoolingHttpClientConnectionManager defaultConnectionManager; private final InstrumentedHttpRequestExecutor httpRequestExecutor; @@ -93,16 +91,11 @@ public class UpdateShardHandler implements SolrInfoBean { private int connectionTimeout = HttpClientUtil.DEFAULT_CONNECT_TIMEOUT; public UpdateShardHandler(UpdateShardHandlerConfig cfg) { - recoveryOnlyConnectionManager = - new InstrumentedPoolingHttpClientConnectionManager( - HttpClientUtil.getSocketFactoryRegistryProvider().getSocketFactoryRegistry()); defaultConnectionManager = new InstrumentedPoolingHttpClientConnectionManager( HttpClientUtil.getSocketFactoryRegistryProvider().getSocketFactoryRegistry()); ModifiableSolrParams clientParams = new ModifiableSolrParams(); if (cfg != null) { - recoveryOnlyConnectionManager.setMaxTotal(cfg.getMaxUpdateConnections()); - recoveryOnlyConnectionManager.setDefaultMaxPerRoute(cfg.getMaxUpdateConnectionsPerHost()); defaultConnectionManager.setMaxTotal(cfg.getMaxUpdateConnections()); defaultConnectionManager.setDefaultMaxPerRoute(cfg.getMaxUpdateConnectionsPerHost()); clientParams.set(HttpClientUtil.PROP_SO_TIMEOUT, cfg.getDistributedSocketTimeout()); @@ -271,10 +264,6 @@ public PoolingHttpClientConnectionManager getDefaultConnectionManager() { return defaultConnectionManager; } - public PoolingHttpClientConnectionManager getRecoveryOnlyConnectionManager() { - return recoveryOnlyConnectionManager; - } - /** * @return executor for recovery operations */ @@ -300,7 +289,6 @@ public void close() { IOUtils.closeQuietly(recoveryOnlyClient); HttpClientUtil.close(defaultClient); defaultConnectionManager.close(); - recoveryOnlyConnectionManager.close(); } } @@ -316,5 +304,6 @@ public int getConnectionTimeout() { public void setSecurityBuilder(HttpClientBuilderPlugin builder) { builder.setup(updateOnlyClient); + builder.setup(recoveryOnlyClient); } } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java b/solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java index 7824ec7c392..7adad55367d 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java @@ -81,6 +81,15 @@ public enum SolrClientContext { private ResponseParser responseParser; private StreamingResponseCallback callback; private Set queryParams; + private boolean useExternalCompression; + + public void setExternalCompression(boolean useCompression) { + this.useExternalCompression = useCompression; + } + + public boolean isExternalCompressionEnabled() { + return useExternalCompression; + } public SolrRequest setPreferredNodes(List nodes) { this.preferredNodes = nodes; 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 79de13f4a26..a8d15e8599c 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 @@ -203,6 +203,9 @@ protected Http2SolrClient(String serverBaseUrl, Builder builder) { } else { this.urlParamNames = Set.of(); } + if (builder.listenerFactory != null) { + this.listenerFactory.addAll(builder.listenerFactory); + } assert ObjectReleaseTracker.track(this); } @@ -630,7 +633,8 @@ static String basicAuthCredentialsToAuthorizationString(String user, String pass } private void decorateRequest(Request req, SolrRequest solrRequest, boolean isAsync) { - req.headers(headers -> headers.remove(HttpHeader.ACCEPT_ENCODING)); + if (!solrRequest.isExternalCompressionEnabled()) + req.headers(headers -> headers.remove(HttpHeader.ACCEPT_ENCODING)); if (requestTimeoutMillis > 0) { req.timeout(requestTimeoutMillis, TimeUnit.MILLISECONDS); @@ -1081,6 +1085,8 @@ public static class Builder { private boolean proxyIsSecure; private Long keyStoreReloadIntervalSecs; + private List listenerFactory; + public Builder() {} /** @@ -1175,7 +1181,6 @@ private static CookieStore getDefaultCookieStore() { */ public Builder withHttpClient(Http2SolrClient http2SolrClient) { this.httpClient = http2SolrClient.httpClient; - if (this.basicAuthAuthorizationStr == null) { this.basicAuthAuthorizationStr = http2SolrClient.basicAuthAuthorizationStr; } @@ -1197,6 +1202,9 @@ public Builder withHttpClient(Http2SolrClient http2SolrClient) { if (this.urlParamNames == null) { this.urlParamNames = http2SolrClient.urlParamNames; } + if (this.listenerFactory == null) { + this.listenerFactory = http2SolrClient.listenerFactory; + } return this; } From 9e9b5f79e8f7624f517370d9467d607ccfe774b5 Mon Sep 17 00:00:00 2001 From: iamsanjay Date: Thu, 7 Mar 2024 18:28:37 +0530 Subject: [PATCH 08/24] Adding header for compression to SolrRequests --- .../src/java/org/apache/solr/handler/IndexFetcher.java | 5 ++++- .../java/org/apache/solr/client/solrj/SolrRequest.java | 9 --------- .../apache/solr/client/solrj/impl/Http2SolrClient.java | 3 +-- 3 files changed, 5 insertions(+), 12 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java index 36d7a1da089..45a18e3eb01 100644 --- a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java +++ b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java @@ -379,6 +379,7 @@ public NamedList getLatestVersion() throws IOException { params.set(CommonParams.QT, ReplicationHandler.PATH); QueryRequest req = new QueryRequest(params); req.setBasePath(leaderBaseUrl); + if (useExternalCompression) req.addHeader("Accept-Encoding", "gzip, deflate"); // TODO modify to use shardhandler try { return solrClient.request(req, leaderCoreName); @@ -400,6 +401,7 @@ private void fetchFileList(long gen) throws IOException { params.set(CommonParams.QT, ReplicationHandler.PATH); QueryRequest req = new QueryRequest(params); req.setBasePath(leaderBaseUrl); + if (useExternalCompression) req.addHeader("Accept-Encoding", "gzip, deflate"); // TODO modify to use shardhandler try { NamedList response = solrClient.request(req, leaderCoreName); @@ -1982,7 +1984,7 @@ private FastInputStream getStream() throws IOException { QueryRequest req = new QueryRequest(params); req.setResponseParser(new InputStreamResponseParser(FILE_STREAM)); req.setBasePath(leaderBaseUrl); - req.setExternalCompression(useExternalCompression); + if (useExternalCompression) req.addHeader("Accept-Encoding", "gzip, deflate"); response = solrClient.request(req, leaderCoreName); is = (InputStream) response.get("stream"); if (useInternalCompression) { @@ -2109,6 +2111,7 @@ NamedList getDetails() throws IOException, SolrServerException { QueryRequest request = new QueryRequest(params); request.setBasePath(leaderBaseUrl); + if (useExternalCompression) request.addHeader("Accept-Encoding", "gzip, deflate"); // TODO use shardhandler return solrClient.request(request, leaderCoreName); } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java b/solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java index 7adad55367d..7824ec7c392 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java @@ -81,15 +81,6 @@ public enum SolrClientContext { private ResponseParser responseParser; private StreamingResponseCallback callback; private Set queryParams; - private boolean useExternalCompression; - - public void setExternalCompression(boolean useCompression) { - this.useExternalCompression = useCompression; - } - - public boolean isExternalCompressionEnabled() { - return useExternalCompression; - } public SolrRequest setPreferredNodes(List nodes) { this.preferredNodes = nodes; 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 a8d15e8599c..e535c6cef40 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 @@ -633,8 +633,7 @@ static String basicAuthCredentialsToAuthorizationString(String user, String pass } private void decorateRequest(Request req, SolrRequest solrRequest, boolean isAsync) { - if (!solrRequest.isExternalCompressionEnabled()) - req.headers(headers -> headers.remove(HttpHeader.ACCEPT_ENCODING)); + req.headers(headers -> headers.remove(HttpHeader.ACCEPT_ENCODING)); if (requestTimeoutMillis > 0) { req.timeout(requestTimeoutMillis, TimeUnit.MILLISECONDS); From 625a3644890080d236ac5480ca0f3b4c7ee110e7 Mon Sep 17 00:00:00 2001 From: iamsanjay Date: Tue, 12 Mar 2024 21:06:21 +0530 Subject: [PATCH 09/24] Enable testing resplication handler for externalCompression --- .../src/java/org/apache/solr/handler/IndexFetcher.java | 9 +-------- .../org/apache/solr/handler/ReplicationTestHelper.java | 3 ++- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java index 45a18e3eb01..22189a307e0 100644 --- a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java +++ b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java @@ -132,7 +132,6 @@ import org.apache.solr.util.RTimer; import org.apache.solr.util.RefCounted; import org.apache.solr.util.TestInjection; -import org.apache.solr.util.stats.InstrumentedHttpListenerFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -266,16 +265,13 @@ private Http2SolrClient createHttpClient( httpClientParams.set(HttpClientUtil.PROP_BASIC_AUTH_USER, httpBasicAuthUser); httpClientParams.set(HttpClientUtil.PROP_BASIC_AUTH_PASS, httpBasicAuthPassword); // no metrics, just tracing - InstrumentedHttpListenerFactory httpListenerFactory = new InstrumentedHttpListenerFactory(null); Http2SolrClient httpClient = new Http2SolrClient.Builder(leaderBaseUrl) .withHttpClient( core.getCoreContainer().getUpdateShardHandler().getRecoveryOnlyHttpClient()) - .withBasicAuthCredentials(httpBasicAuthUser, httpBasicAuthPassword) .withIdleTimeout(soTimeout, TimeUnit.MILLISECONDS) .withConnectionTimeout(connTimeout, TimeUnit.MILLISECONDS) .build(); - httpClient.addListenerFactory(httpListenerFactory); return httpClient; } @@ -379,7 +375,6 @@ public NamedList getLatestVersion() throws IOException { params.set(CommonParams.QT, ReplicationHandler.PATH); QueryRequest req = new QueryRequest(params); req.setBasePath(leaderBaseUrl); - if (useExternalCompression) req.addHeader("Accept-Encoding", "gzip, deflate"); // TODO modify to use shardhandler try { return solrClient.request(req, leaderCoreName); @@ -401,7 +396,6 @@ private void fetchFileList(long gen) throws IOException { params.set(CommonParams.QT, ReplicationHandler.PATH); QueryRequest req = new QueryRequest(params); req.setBasePath(leaderBaseUrl); - if (useExternalCompression) req.addHeader("Accept-Encoding", "gzip, deflate"); // TODO modify to use shardhandler try { NamedList response = solrClient.request(req, leaderCoreName); @@ -1984,7 +1978,7 @@ private FastInputStream getStream() throws IOException { QueryRequest req = new QueryRequest(params); req.setResponseParser(new InputStreamResponseParser(FILE_STREAM)); req.setBasePath(leaderBaseUrl); - if (useExternalCompression) req.addHeader("Accept-Encoding", "gzip, deflate"); + if (useExternalCompression) req.addHeader("Accept-Encoding", "gzip"); response = solrClient.request(req, leaderCoreName); is = (InputStream) response.get("stream"); if (useInternalCompression) { @@ -2111,7 +2105,6 @@ NamedList getDetails() throws IOException, SolrServerException { QueryRequest request = new QueryRequest(params); request.setBasePath(leaderBaseUrl); - if (useExternalCompression) request.addHeader("Accept-Encoding", "gzip, deflate"); // TODO use shardhandler return solrClient.request(request, leaderCoreName); } diff --git a/solr/core/src/test/org/apache/solr/handler/ReplicationTestHelper.java b/solr/core/src/test/org/apache/solr/handler/ReplicationTestHelper.java index 892dc679d3f..a9c81429467 100644 --- a/solr/core/src/test/org/apache/solr/handler/ReplicationTestHelper.java +++ b/solr/core/src/test/org/apache/solr/handler/ReplicationTestHelper.java @@ -103,7 +103,8 @@ private static void copyFile(File src, File dst, Integer port, boolean internalC if (null != port) { line = line.replace("TEST_PORT", port.toString()); } - line = line.replace("COMPRESSION", internalCompression ? "internal" : "false"); + String externalCompression = LuceneTestCase.random().nextBoolean() ? "external" : "false"; + line = line.replace("COMPRESSION", internalCompression ? "internal" : externalCompression); out.write(line); } } From 901ef513c151d691619706b68708fdb401fa3cc8 Mon Sep 17 00:00:00 2001 From: iamsanjay Date: Tue, 12 Mar 2024 21:14:46 +0530 Subject: [PATCH 10/24] Renaming method to more appropriate name --- solr/core/src/java/org/apache/solr/handler/IndexFetcher.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java index 22189a307e0..948989a8a7c 100644 --- a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java +++ b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java @@ -259,7 +259,7 @@ public String getMessage() { } } - private Http2SolrClient createHttpClient( + private Http2SolrClient createSolrClient( SolrCore core, String httpBasicAuthUser, String httpBasicAuthPassword, String leaderBaseUrl) { final ModifiableSolrParams httpClientParams = new ModifiableSolrParams(); httpClientParams.set(HttpClientUtil.PROP_BASIC_AUTH_USER, httpBasicAuthUser); @@ -317,7 +317,7 @@ public IndexFetcher( String httpBasicAuthUser = (String) initArgs.get(HttpClientUtil.PROP_BASIC_AUTH_USER); String httpBasicAuthPassword = (String) initArgs.get(HttpClientUtil.PROP_BASIC_AUTH_PASS); solrClient = - createHttpClient(solrCore, httpBasicAuthUser, httpBasicAuthPassword, leaderBaseUrl); + createSolrClient(solrCore, httpBasicAuthUser, httpBasicAuthPassword, leaderBaseUrl); } private void setLeaderCoreUrl(String leaderCoreUrl) { From 26a4c0e464fa99545ebb0954a65e8975a3f31ac1 Mon Sep 17 00:00:00 2001 From: iamsanjay Date: Wed, 13 Mar 2024 20:35:10 +0530 Subject: [PATCH 11/24] Resolve conflicts Http2SolrClient --- .../apache/solr/client/solrj/impl/Http2SolrClient.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) 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 07264bd87a9..7b8377722fa 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 @@ -139,7 +139,9 @@ protected Http2SolrClient(String serverBaseUrl, Builder builder) { this.httpClient = createHttpClient(builder); this.closeClient = true; } - + if (builder.listenerFactory != null) { + this.listenerFactory.addAll(builder.listenerFactory); + } updateDefaultMimeTypeForParser(); this.httpClient.setFollowRedirects(Boolean.TRUE.equals(builder.followRedirects)); @@ -810,6 +812,8 @@ public static class Builder protected Long keyStoreReloadIntervalSecs; + private List listenerFactory; + public Builder() { super(); } @@ -1006,6 +1010,9 @@ public Builder withHttpClient(Http2SolrClient http2SolrClient) { if (this.urlParamNames == null) { this.urlParamNames = http2SolrClient.urlParamNames; } + if (this.listenerFactory == null) { + this.listenerFactory = http2SolrClient.listenerFactory; + } return this; } From 43dda16e40454050b3bbceef9a4a693d2b9fa95a Mon Sep 17 00:00:00 2001 From: iamsanjay Date: Mon, 18 Mar 2024 11:26:09 +0530 Subject: [PATCH 12/24] Restoring the old auth of IndexFetcher --- .../core/src/java/org/apache/solr/handler/IndexFetcher.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java index 948989a8a7c..134425b983b 100644 --- a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java +++ b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java @@ -259,16 +259,16 @@ public String getMessage() { } } - private Http2SolrClient createSolrClient( + private SolrClient createSolrClient( SolrCore core, String httpBasicAuthUser, String httpBasicAuthPassword, String leaderBaseUrl) { final ModifiableSolrParams httpClientParams = new ModifiableSolrParams(); httpClientParams.set(HttpClientUtil.PROP_BASIC_AUTH_USER, httpBasicAuthUser); httpClientParams.set(HttpClientUtil.PROP_BASIC_AUTH_PASS, httpBasicAuthPassword); - // no metrics, just tracing Http2SolrClient httpClient = new Http2SolrClient.Builder(leaderBaseUrl) .withHttpClient( core.getCoreContainer().getUpdateShardHandler().getRecoveryOnlyHttpClient()) + .withBasicAuthCredentials(httpBasicAuthUser, httpBasicAuthPassword) .withIdleTimeout(soTimeout, TimeUnit.MILLISECONDS) .withConnectionTimeout(connTimeout, TimeUnit.MILLISECONDS) .build(); @@ -1950,7 +1950,7 @@ private void cleanup() { private FastInputStream getStream() throws IOException { ModifiableSolrParams params = new ModifiableSolrParams(); - // //the method is command=filecontent + //the method is command=filecontent params.set(COMMAND, CMD_GET_FILE); params.set(GENERATION, Long.toString(indexGen)); params.set(CommonParams.QT, ReplicationHandler.PATH); From 851109f245f3a7048310f5f6bb9ab50156b544a6 Mon Sep 17 00:00:00 2001 From: iamsanjay Date: Wed, 27 Mar 2024 13:15:56 +0530 Subject: [PATCH 13/24] Fix retry fetch() IndexFetcher --- .../org/apache/solr/handler/IndexFetcher.java | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java index 134425b983b..fcbdd4c1b71 100644 --- a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java +++ b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java @@ -1788,14 +1788,13 @@ private void fetch() throws Exception { try { while (true) { int result; - try (FastInputStream is = getStream()) { - // fetch packets one by one in a single request - result = fetchPackets(is); - if (result == 0 || result == NO_CONTENT) { - return; - } - // if there is an error continue. But continue from the point where it got broken + // fetch packets one by one in a single request + result = fetchPackets(); + if (result == 0 || result == NO_CONTENT) { + return; } + // if there is an error continue. But continue from the point where it got broken + } } finally { cleanup(); @@ -1811,10 +1810,10 @@ private void fetch() throws Exception { } } - private int fetchPackets(FastInputStream fis) throws Exception { + private int fetchPackets() throws Exception { byte[] intbytes = new byte[4]; byte[] longbytes = new byte[8]; - try { + try (FastInputStream fis = getStream()) { while (true) { if (stop) { stop = false; @@ -1950,7 +1949,7 @@ private void cleanup() { private FastInputStream getStream() throws IOException { ModifiableSolrParams params = new ModifiableSolrParams(); - //the method is command=filecontent + // the method is command=filecontent params.set(COMMAND, CMD_GET_FILE); params.set(GENERATION, Long.toString(indexGen)); params.set(CommonParams.QT, ReplicationHandler.PATH); From 73c5ba8b9417300fb730817e788c826534d8b2b5 Mon Sep 17 00:00:00 2001 From: iamsanjay Date: Sat, 30 Mar 2024 11:51:34 +0530 Subject: [PATCH 14/24] Avoid closing InputStream before receiving zero-length Data field --- .../org/apache/solr/handler/IndexFetcher.java | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java index fcbdd4c1b71..12196df80dc 100644 --- a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java +++ b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java @@ -1787,14 +1787,15 @@ public void fetchFile() throws Exception { private void fetch() throws Exception { try { while (true) { - int result; - // fetch packets one by one in a single request - result = fetchPackets(); - if (result == 0 || result == NO_CONTENT) { - return; + try (FastInputStream fis = getStream()) { + int result; + // fetch packets one by one in a single request + result = fetchPackets(fis); + if (result == 0 || result == NO_CONTENT) { + return; + } + // if there is an error continue. But continue from the point where it got broken } - // if there is an error continue. But continue from the point where it got broken - } } finally { cleanup(); @@ -1810,10 +1811,11 @@ private void fetch() throws Exception { } } - private int fetchPackets() throws Exception { + private int fetchPackets(FastInputStream fis) throws Exception { byte[] intbytes = new byte[4]; byte[] longbytes = new byte[8]; - try (FastInputStream fis = getStream()) { + boolean isContentReceived = false; + try { while (true) { if (stop) { stop = false; @@ -1821,13 +1823,16 @@ private int fetchPackets() throws Exception { throw new ReplicationHandlerException("User aborted replication"); } long checkSumServer = -1; + fis.readFully(intbytes); + // read the size of the packet int packetSize = readInt(intbytes); if (packetSize <= 0) { - log.warn("No content received for file: {}", fileName); + if (!isContentReceived) log.warn("No content received for file: {}", fileName); return NO_CONTENT; } + isContentReceived = true; // TODO consider recoding the remaining logic to not use/need buf[]; instead use the // internal buffer of fis if (buf.length < packetSize) { @@ -1860,7 +1865,6 @@ private int fetchPackets() throws Exception { log.debug("Fetched and wrote {} bytes of file: {}", bytesDownloaded, fileName); // errorCount is always set to zero after a successful packet errorCount = 0; - if (bytesDownloaded >= size) return 0; } } catch (ReplicationHandlerException e) { throw e; From 4c164040beccf6cf3c517b351d2fcd7ac70250c3 Mon Sep 17 00:00:00 2001 From: iamsanjay Date: Sat, 30 Mar 2024 12:49:08 +0530 Subject: [PATCH 15/24] Read till end-of-file --- solr/core/src/java/org/apache/solr/handler/IndexFetcher.java | 1 + 1 file changed, 1 insertion(+) diff --git a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java index 12196df80dc..e6103802a44 100644 --- a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java +++ b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java @@ -1829,6 +1829,7 @@ private int fetchPackets(FastInputStream fis) throws Exception { // read the size of the packet int packetSize = readInt(intbytes); if (packetSize <= 0) { + fis.read(); // read till end-of-file if (!isContentReceived) log.warn("No content received for file: {}", fileName); return NO_CONTENT; } From 19ec48977c9e2a71e0eba7083f4c8cd84ca93a65 Mon Sep 17 00:00:00 2001 From: iamsanjay Date: Sat, 30 Mar 2024 18:45:55 +0530 Subject: [PATCH 16/24] read till end-of-file --- .../src/java/org/apache/solr/handler/IndexFetcher.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java index e6103802a44..b3dabb46722 100644 --- a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java +++ b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java @@ -1817,6 +1817,9 @@ private int fetchPackets(FastInputStream fis) throws Exception { boolean isContentReceived = false; try { while (true) { + if(fis.peek() == -1){ + return NO_CONTENT; + } if (stop) { stop = false; aborted = true; @@ -1825,13 +1828,11 @@ private int fetchPackets(FastInputStream fis) throws Exception { long checkSumServer = -1; fis.readFully(intbytes); - // read the size of the packet int packetSize = readInt(intbytes); if (packetSize <= 0) { - fis.read(); // read till end-of-file if (!isContentReceived) log.warn("No content received for file: {}", fileName); - return NO_CONTENT; + continue; } isContentReceived = true; // TODO consider recoding the remaining logic to not use/need buf[]; instead use the From c54cd5b8a2300d32c334300c6524acf5f358c3ec Mon Sep 17 00:00:00 2001 From: iamsanjay Date: Thu, 18 Apr 2024 15:03:05 +0530 Subject: [PATCH 17/24] Added Test case for User managed replication with basic auth enabled --- .../org/apache/solr/handler/IndexFetcher.java | 2 +- .../conf/solrconfig-follower-auth.xml | 61 ++++ .../TestUserManagedReplicationWithAuth.java | 267 ++++++++++++++++++ 3 files changed, 329 insertions(+), 1 deletion(-) create mode 100644 solr/core/src/test-files/solr/collection1/conf/solrconfig-follower-auth.xml create mode 100644 solr/core/src/test/org/apache/solr/handler/TestUserManagedReplicationWithAuth.java diff --git a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java index 2ee20686ab3..3e5a884a05d 100644 --- a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java +++ b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java @@ -1817,7 +1817,7 @@ private int fetchPackets(FastInputStream fis) throws Exception { boolean isContentReceived = false; try { while (true) { - if(fis.peek() == -1){ + if (fis.peek() == -1) { return NO_CONTENT; } if (stop) { diff --git a/solr/core/src/test-files/solr/collection1/conf/solrconfig-follower-auth.xml b/solr/core/src/test-files/solr/collection1/conf/solrconfig-follower-auth.xml new file mode 100644 index 00000000000..1635cfb099b --- /dev/null +++ b/solr/core/src/test-files/solr/collection1/conf/solrconfig-follower-auth.xml @@ -0,0 +1,61 @@ + + + + + + ${tests.luceneMatchVersion:LATEST} + + + ${solr.data.dir:} + + + + + + + + true + + + + + + + + + + + + + + http://127.0.0.1:TEST_PORT/solr/collection1 + 00:00:01 + COMPRESSION + solr + SolrRocks + + + + + + + max-age=30, public + + + + diff --git a/solr/core/src/test/org/apache/solr/handler/TestUserManagedReplicationWithAuth.java b/solr/core/src/test/org/apache/solr/handler/TestUserManagedReplicationWithAuth.java new file mode 100644 index 00000000000..b230aad2021 --- /dev/null +++ b/solr/core/src/test/org/apache/solr/handler/TestUserManagedReplicationWithAuth.java @@ -0,0 +1,267 @@ +/* + * 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.handler; + +import static org.apache.solr.common.params.CommonParams.JAVABIN; +import static org.apache.solr.handler.ReplicationHandler.CMD_DISABLE_POLL; +import static org.apache.solr.handler.ReplicationHandler.CMD_FETCH_INDEX; +import static org.apache.solr.handler.ReplicationHandler.COMMAND; +import static org.apache.solr.handler.ReplicationTestHelper.createAndStartJetty; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.SolrTestCaseJ4.SuppressSSL; +import org.apache.solr.client.solrj.SolrClient; +import org.apache.solr.client.solrj.SolrQuery; +import org.apache.solr.client.solrj.SolrRequest; +import org.apache.solr.client.solrj.SolrResponse; +import org.apache.solr.client.solrj.SolrServerException; +import org.apache.solr.client.solrj.request.HealthCheckRequest; +import org.apache.solr.client.solrj.request.QueryRequest; +import org.apache.solr.client.solrj.request.UpdateRequest; +import org.apache.solr.client.solrj.response.QueryResponse; +import org.apache.solr.common.SolrInputDocument; +import org.apache.solr.common.params.CommonParams; +import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.embedded.JettySolrRunner; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +@SuppressSSL +public class TestUserManagedReplicationWithAuth extends SolrTestCaseJ4 { + JettySolrRunner leaderJetty, followerJetty, followerJettyWithAuth; + SolrClient leaderClient, followerClient, followerClientWithAuth; + ReplicationTestHelper.SolrInstance leader = null, follower = null, followerWithAuth = null; + + private static String user = "solr"; + private static String pass = "SolrRocks"; + private static String securityJson = + "{\n" + + "\"authentication\":{ \n" + + " \"blockUnknown\": true, \n" + + " \"class\":\"solr.BasicAuthPlugin\",\n" + + " \"credentials\":{\"solr\":\"IV0EHq1OnNrj6gvRCwvFwTrZ1+z1oBbnQdiVC3otuq0= Ndd7LKvVBAaZIF0QAVi1ekCfAJXr1GGfLtRUXhgrF8c=\"}, \n" + + " \"realm\":\"My Solr users\", \n" + + " \"forwardCredentials\": false \n" + + "},\n" + + "\"authorization\":{\n" + + " \"class\":\"solr.RuleBasedAuthorizationPlugin\",\n" + + " \"permissions\":[{\"name\":\"security-edit\",\n" + + " \"role\":\"admin\"}],\n" + + " \"user-role\":{\"solr\":\"admin\"}\n" + + "}}"; + + @Override + @Before + public void setUp() throws Exception { + super.setUp(); + systemSetPropertySolrDisableUrlAllowList("true"); + // leader with Basic auth enabled via security.json + leader = + new ReplicationTestHelper.SolrInstance( + createTempDir("solr-instance").toFile(), "leader", null); + leader.setUp(); + // Configuring basic auth for Leader + Path solrLeaderHome = Path.of(leader.getHomeDir()); + Files.write( + solrLeaderHome.resolve("security.json"), securityJson.getBytes(StandardCharsets.UTF_8)); + leaderJetty = ReplicationTestHelper.createAndStartJetty(leader); + leaderClient = + ReplicationTestHelper.createNewSolrClient( + buildUrl(leaderJetty.getLocalPort()), DEFAULT_TEST_CORENAME); + + // follower with no basic auth credentials for leader configured. + follower = + new ReplicationTestHelper.SolrInstance( + createTempDir("solr-instance").toFile(), "follower", leaderJetty.getLocalPort()); + follower.setUp(); + followerJetty = createAndStartJetty(follower); + followerClient = + ReplicationTestHelper.createNewSolrClient( + buildUrl(followerJetty.getLocalPort()), DEFAULT_TEST_CORENAME); + + // follower with basic auth credentials for leader configured in solrconfig.xml. + followerWithAuth = + new ReplicationTestHelper.SolrInstance( + createTempDir("solr-instance").toFile(), "follower-auth", leaderJetty.getLocalPort()); + followerWithAuth.setUp(); + followerJettyWithAuth = createAndStartJetty(followerWithAuth); + followerClientWithAuth = + ReplicationTestHelper.createNewSolrClient( + buildUrl(followerJettyWithAuth.getLocalPort()), DEFAULT_TEST_CORENAME); + } + + @Override + @After + public void tearDown() throws Exception { + super.tearDown(); + if (null != leaderJetty) { + leaderJetty.stop(); + leaderJetty = null; + } + if (null != followerJetty) { + followerJetty.stop(); + followerJetty = null; + } + if (null != followerJettyWithAuth) { + followerJettyWithAuth.stop(); + followerJettyWithAuth = null; + } + if (null != leaderClient) { + leaderClient.close(); + leaderClient = null; + } + if (null != followerClient) { + followerClient.close(); + followerClient = null; + } + if (null != followerClientWithAuth) { + followerClientWithAuth.close(); + followerClientWithAuth = null; + } + } + + private > T withBasicAuth(T req) { + req.setBasicAuthCredentials(user, pass); + return req; + } + + @Test + public void doTestManualFetchIndexWithAuthEnabled() throws Exception { + disablePoll(followerJetty, followerClient); + int nDocs = 500; + int docsAdded = 0; + + UpdateRequest commitReq = new UpdateRequest(); + withBasicAuth(commitReq); + for (int i = 0; docsAdded < nDocs / 2; i++, docsAdded++) { + SolrInputDocument doc = new SolrInputDocument(); + String[] fields = {"id", i + "", "name", "name = " + i}; + for (int j = 0; j < fields.length; j += 2) { + doc.addField(fields[j], fields[j + 1]); + } + UpdateRequest req = new UpdateRequest(); + withBasicAuth(req).add(doc); + req.process(leaderClient, DEFAULT_TEST_CORENAME); + if (i % 10 == 0) { + commitReq.commit(leaderClient, DEFAULT_TEST_CORENAME); + } + } + commitReq.commit(leaderClient, DEFAULT_TEST_CORENAME); + + assertEquals( + docsAdded, + queryWithBasicAuth(leaderClient, new SolrQuery("*:*")).getResults().getNumFound()); + + // Without Auth credentials fetchIndex will fail + pullIndexFromTo(leaderJetty, followerJetty, false); + assertNotEquals( + docsAdded, + queryWithBasicAuth(followerClient, new SolrQuery("*:*")).getResults().getNumFound()); + + // With Auth credentials + pullIndexFromTo(leaderJetty, followerJetty, true); + assertEquals( + docsAdded, + queryWithBasicAuth(followerClient, new SolrQuery("*:*")).getResults().getNumFound()); + } + + @Test + public void doTestAutoReplicationWithAuthEnabled() throws Exception { + int nDocs = 250; + UpdateRequest commitReq = new UpdateRequest(); + withBasicAuth(commitReq); + for (int i = 0; i < nDocs; i++) { + SolrInputDocument doc = new SolrInputDocument(); + String[] fields = {"id", i + "", "name", "name = " + i}; + for (int j = 0; j < fields.length; j += 2) { + doc.addField(fields[j], fields[j + 1]); + } + UpdateRequest req = new UpdateRequest(); + withBasicAuth(req).add(doc); + req.process(leaderClient, DEFAULT_TEST_CORENAME); + if (i % 10 == 0) { + commitReq.commit(leaderClient, DEFAULT_TEST_CORENAME); + } + } + commitReq.commit(leaderClient, DEFAULT_TEST_CORENAME); + // wait for followers to fetchIndex + Thread.sleep(5000); + // follower with auth should be healthy + HealthCheckRequest healthCheckRequestFollower = new HealthCheckRequest(); + healthCheckRequestFollower.setMaxGenerationLag(2); + assertEquals( + CommonParams.OK, + healthCheckRequestFollower + .process(followerClientWithAuth) + .getResponse() + .get(CommonParams.STATUS)); + // follower with auth should be unhealthy + healthCheckRequestFollower = new HealthCheckRequest(); + healthCheckRequestFollower.setMaxGenerationLag(2); + assertEquals( + CommonParams.FAILURE, + healthCheckRequestFollower.process(followerClient).getResponse().get(CommonParams.STATUS)); + } + + private QueryResponse queryWithBasicAuth(SolrClient client, SolrQuery q) + throws IOException, SolrServerException { + return withBasicAuth(new QueryRequest(q)).process(client); + } + + private void disablePoll(JettySolrRunner Jetty, SolrClient solrClient) + throws SolrServerException, IOException { + ModifiableSolrParams disablePollParams = new ModifiableSolrParams(); + disablePollParams.set(COMMAND, CMD_DISABLE_POLL); + disablePollParams.set(CommonParams.WT, JAVABIN); + disablePollParams.set(CommonParams.QT, ReplicationHandler.PATH); + QueryRequest req = new QueryRequest(disablePollParams); + withBasicAuth(req); + req.setBasePath(buildUrl(Jetty.getLocalPort())); + + solrClient.request(req, DEFAULT_TEST_CORENAME); + } + + private void pullIndexFromTo( + JettySolrRunner srcSolr, JettySolrRunner destSolr, boolean authEnabled) + throws SolrServerException, IOException { + String srcUrl = buildUrl(srcSolr.getLocalPort()) + "/" + DEFAULT_TEST_CORENAME; + String destUrl = buildUrl(destSolr.getLocalPort()) + "/" + DEFAULT_TEST_CORENAME; + QueryRequest req = getQueryRequestForFetchIndex(authEnabled, srcUrl); + req.setBasePath(buildUrl(destSolr.getLocalPort())); + followerClient.request(req, DEFAULT_TEST_CORENAME); + } + + private QueryRequest getQueryRequestForFetchIndex(boolean authEnabled, String srcUrl) { + ModifiableSolrParams solrParams = new ModifiableSolrParams(); + solrParams.set(COMMAND, CMD_FETCH_INDEX); + solrParams.set(CommonParams.WT, JAVABIN); + solrParams.set(CommonParams.QT, ReplicationHandler.PATH); + solrParams.set("leaderUrl", srcUrl); + solrParams.set("wait", "true"); + if (authEnabled) { + solrParams.set("httpBasicAuthUser", user); + solrParams.set("httpBasicAuthPassword", pass); + } + QueryRequest req = new QueryRequest(solrParams); + return req; + } +} From bb1c3b3dd5700b93983ed34f51f0baa6deadf85c Mon Sep 17 00:00:00 2001 From: iamsanjay Date: Mon, 22 Apr 2024 10:56:45 +0530 Subject: [PATCH 18/24] Removed isContentDownloaded and updated listener factory setting mechanism --- .../org/apache/solr/handler/IndexFetcher.java | 19 +++++++++++-------- .../client/solrj/impl/Http2SolrClient.java | 13 ++++++++++--- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java index 3e5a884a05d..5cb987b64b8 100644 --- a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java +++ b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java @@ -127,6 +127,7 @@ import org.apache.solr.search.SolrIndexSearcher; import org.apache.solr.security.AllowListUrlChecker; import org.apache.solr.update.CommitUpdateCommand; +import org.apache.solr.update.UpdateShardHandler; import org.apache.solr.util.FileUtils; import org.apache.solr.util.IndexOutputOutputStream; import org.apache.solr.util.RTimer; @@ -259,15 +260,17 @@ public String getMessage() { } } + // It's crucial not to remove the authentication credentials as they are essential for User + // managed replication. + // GitHub PR #2276 private SolrClient createSolrClient( SolrCore core, String httpBasicAuthUser, String httpBasicAuthPassword, String leaderBaseUrl) { - final ModifiableSolrParams httpClientParams = new ModifiableSolrParams(); - httpClientParams.set(HttpClientUtil.PROP_BASIC_AUTH_USER, httpBasicAuthUser); - httpClientParams.set(HttpClientUtil.PROP_BASIC_AUTH_PASS, httpBasicAuthPassword); + final UpdateShardHandler updateShardHandler = core.getCoreContainer().getUpdateShardHandler(); Http2SolrClient httpClient = new Http2SolrClient.Builder(leaderBaseUrl) - .withHttpClient( - core.getCoreContainer().getUpdateShardHandler().getRecoveryOnlyHttpClient()) + .withHttpClient(updateShardHandler.getRecoveryOnlyHttpClient()) + .withListenerFactory( + updateShardHandler.getRecoveryOnlyHttpClient().getListenerFactory()) .withBasicAuthCredentials(httpBasicAuthUser, httpBasicAuthPassword) .withIdleTimeout(soTimeout, TimeUnit.MILLISECONDS) .withConnectionTimeout(connTimeout, TimeUnit.MILLISECONDS) @@ -1814,10 +1817,12 @@ private void fetch() throws Exception { private int fetchPackets(FastInputStream fis) throws Exception { byte[] intbytes = new byte[4]; byte[] longbytes = new byte[8]; - boolean isContentReceived = false; try { while (true) { if (fis.peek() == -1) { + if (bytesDownloaded == 0) { + log.warn("No content received for file: {}", fileName); + } return NO_CONTENT; } if (stop) { @@ -1831,10 +1836,8 @@ private int fetchPackets(FastInputStream fis) throws Exception { // read the size of the packet int packetSize = readInt(intbytes); if (packetSize <= 0) { - if (!isContentReceived) log.warn("No content received for file: {}", fileName); continue; } - isContentReceived = true; // TODO consider recoding the remaining logic to not use/need buf[]; instead use the // internal buffer of fis if (buf.length < packetSize) { 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 714180fc55a..4605e82367b 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 @@ -117,6 +117,11 @@ public class Http2SolrClient extends HttpSolrClientBase { private SSLConfig sslConfig; private List listenerFactory = new ArrayList<>(); + + public List getListenerFactory() { + return listenerFactory; + } + private final AsyncTracker asyncTracker = new AsyncTracker(); private final boolean closeClient; @@ -869,6 +874,11 @@ public static class Builder protected Long keyStoreReloadIntervalSecs; + public Http2SolrClient.Builder withListenerFactory(List listenerFactory) { + this.listenerFactory = listenerFactory; + return this; + } + private List listenerFactory; public Builder() { @@ -1067,9 +1077,6 @@ public Builder withHttpClient(Http2SolrClient http2SolrClient) { if (this.urlParamNames == null) { this.urlParamNames = http2SolrClient.urlParamNames; } - if (this.listenerFactory == null) { - this.listenerFactory = http2SolrClient.listenerFactory; - } return this; } From 1b9b7fc234d64b41cfc7dc1732b9bb6e7a83ed79 Mon Sep 17 00:00:00 2001 From: iamsanjay Date: Tue, 30 Apr 2024 20:29:39 +0530 Subject: [PATCH 19/24] Change return code when downloaded successfully --- solr/core/src/java/org/apache/solr/handler/IndexFetcher.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java index 5cb987b64b8..c522ef4fce8 100644 --- a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java +++ b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java @@ -1822,8 +1822,9 @@ private int fetchPackets(FastInputStream fis) throws Exception { if (fis.peek() == -1) { if (bytesDownloaded == 0) { log.warn("No content received for file: {}", fileName); + return NO_CONTENT; } - return NO_CONTENT; + return 0; } if (stop) { stop = false; From ef92b6bf8f217e689d45a4b3600dbc11f01f1dea Mon Sep 17 00:00:00 2001 From: iamsanjay Date: Wed, 8 May 2024 10:38:33 +0530 Subject: [PATCH 20/24] tidy code --- .../apache/solr/client/solrj/impl/Http2SolrClient.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 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 d2341dd68ed..e437e2f0a18 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 @@ -115,11 +115,6 @@ public class Http2SolrClient extends HttpSolrClientBase { private SSLConfig sslConfig; private List listenerFactory = new ArrayList<>(); - - public List getListenerFactory() { - return listenerFactory; - } - private final AsyncTracker asyncTracker = new AsyncTracker(); private final boolean closeClient; @@ -154,6 +149,10 @@ public void addListenerFactory(HttpListenerFactory factory) { this.listenerFactory.add(factory); } + public List getListenerFactory() { + return listenerFactory; + } + // internal usage only HttpClient getHttpClient() { return httpClient; From 6279656e7fce15d667d4567e01f7ff9843ee38ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B2=99=E5=93=88=E6=8B=89=E9=87=8C=E7=9A=84=E4=B8=80?= =?UTF-8?q?=E7=B2=92=E6=B2=99?= Date: Wed, 8 May 2024 15:00:37 +0800 Subject: [PATCH 21/24] Update basic-authentication-plugin.adoc (#2446) for the pwd encode format, there missed a ')', which is confusing. --- .../deployment-guide/pages/basic-authentication-plugin.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solr/solr-ref-guide/modules/deployment-guide/pages/basic-authentication-plugin.adoc b/solr/solr-ref-guide/modules/deployment-guide/pages/basic-authentication-plugin.adoc index bfc998021ab..e878e902459 100644 --- a/solr/solr-ref-guide/modules/deployment-guide/pages/basic-authentication-plugin.adoc +++ b/solr/solr-ref-guide/modules/deployment-guide/pages/basic-authentication-plugin.adoc @@ -29,7 +29,7 @@ This file and where to put it is described in detail in the section xref:authent If running in cloud mode, you can use the `bin/solr auth` command-line utility to enable security for a new installation, see: `bin/solr auth --help` for more details. For Basic authentication, `security.json` must have an `authentication` block which defines the class being used for authentication. -Usernames and passwords (Format: `base64(sha256(sha256(salt+password)) base64(salt)`) could be added when the file is created, or can be added later with the Authentication API, described below. +Usernames and passwords (Format: `base64(sha256(sha256(salt+password))) base64(salt)`) could be added when the file is created, or can be added later with the Authentication API, described below. An example `security.json` showing `authentication` and `authorization` blocks is shown below to show how authentication and authorization plugins can work together: From b789a7103b7424511b285d7a795b9778d052b2a2 Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Wed, 8 May 2024 15:21:42 +0200 Subject: [PATCH 22/24] group operators together (#2450) --- .../core/src/test/org/apache/solr/util/TimeZoneUtilsTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/solr/core/src/test/org/apache/solr/util/TimeZoneUtilsTest.java b/solr/core/src/test/org/apache/solr/util/TimeZoneUtilsTest.java index 4b102a4bdda..5d085be3153 100644 --- a/solr/core/src/test/org/apache/solr/util/TimeZoneUtilsTest.java +++ b/solr/core/src/test/org/apache/solr/util/TimeZoneUtilsTest.java @@ -62,8 +62,8 @@ public void testValidIds() { // Hack: Why do some timezones have useDaylightTime() as true, but DST as 0? // It causes an exception during String.valueOf(actual/expected) - if (expected.useDaylightTime() && expected.getDSTSavings() == 0 - || actual.useDaylightTime() && actual.getDSTSavings() == 0) { + if ((expected.useDaylightTime() && expected.getDSTSavings() == 0) + || (actual.useDaylightTime() && actual.getDSTSavings() == 0)) { if (log.isWarnEnabled()) { log.warn( "Not expecting DST to be 0 for {} " + " (actual: {})", From f39e8ba5c53b39fde01f362a4bf0f621e8e416f9 Mon Sep 17 00:00:00 2001 From: Jason Gerlowski Date: Wed, 8 May 2024 11:45:09 -0400 Subject: [PATCH 23/24] SOLR-17192: Add "field-limiting" URP to catch ill-designed schemas (#2395) The URPF, `NumFieldLimitingUpdateRequestProcessorFactory`, blocks all update requests that go through `processAdd` if the core exceeds a configurable threshold of fields. The factory accepts two parameters: `maxFields` is a required integer representing the maximum field threshold, and `warnOnly` is an optional boolean that (when enabled) has the URP chain log warnings instead of blocking updates. The factory is included in the default configset, with warnOnly=false and maxFields=1000. (More lenient settings will be used on branch_9x) --------- Co-authored-by: David Smiley --- solr/CHANGES.txt | 5 + ...LimitingUpdateRequestProcessorFactory.java | 124 ++++++++++++++++++ .../conf/schema.xml | 30 +++++ .../conf/solrconfig.xml | 60 +++++++++ ...tingUpdateRequestProcessorFactoryTest.java | 84 ++++++++++++ ...UpdateRequestProcessorIntegrationTest.java | 113 ++++++++++++++++ .../configsets/_default/conf/solrconfig.xml | 6 +- .../pages/update-request-processors.adoc | 6 + .../org/apache/solr/common/util/StrUtils.java | 15 +++ 9 files changed, 442 insertions(+), 1 deletion(-) create mode 100644 solr/core/src/java/org/apache/solr/update/processor/NumFieldLimitingUpdateRequestProcessorFactory.java create mode 100644 solr/core/src/test-files/solr/configsets/cloud-minimal-field-limiting/conf/schema.xml create mode 100644 solr/core/src/test-files/solr/configsets/cloud-minimal-field-limiting/conf/solrconfig.xml create mode 100644 solr/core/src/test/org/apache/solr/update/processor/NumFieldLimitingUpdateRequestProcessorFactoryTest.java create mode 100644 solr/core/src/test/org/apache/solr/update/processor/NumFieldLimitingUpdateRequestProcessorIntegrationTest.java diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 38dad63fdb5..9313a17c730 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -100,6 +100,11 @@ New Features --------------------- * SOLR-13350: Multithreaded search execution (Ishan Chattopadhyaya, Mark Miller, Christine Poerschke, David Smiley, noble) +* SOLR-17192: Put an UpdateRequestProcessor-enforced soft-limit on the number of fields allowed in a core. The `NumFieldLimitingUpdateRequestProcessorFactory` + limit may be adjusted by raising the factory's `maxFields` setting, toggled in and out of "warning-only" mode using the `warnOnly` setting, or disabled entirely + by removing it solrconfig.xml. The limit is set at 1000 fields in the "_default" configset, but left in warning-only mode. (David Smiley, Eric Pugh, + Jason Gerlowski) + Improvements --------------------- * SOLR-16921: use -solrUrl to derive the zk host connection for bin/solr zk subcommands (Eric Pugh) diff --git a/solr/core/src/java/org/apache/solr/update/processor/NumFieldLimitingUpdateRequestProcessorFactory.java b/solr/core/src/java/org/apache/solr/update/processor/NumFieldLimitingUpdateRequestProcessorFactory.java new file mode 100644 index 00000000000..9382d25a499 --- /dev/null +++ b/solr/core/src/java/org/apache/solr/update/processor/NumFieldLimitingUpdateRequestProcessorFactory.java @@ -0,0 +1,124 @@ +/* + * 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.update.processor; + +import java.io.IOException; +import java.lang.invoke.MethodHandles; +import java.util.Locale; +import org.apache.solr.common.SolrException; +import org.apache.solr.common.util.NamedList; +import org.apache.solr.request.SolrQueryRequest; +import org.apache.solr.response.SolrQueryResponse; +import org.apache.solr.search.SolrIndexSearcher; +import org.apache.solr.update.AddUpdateCommand; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * This factory generates an UpdateRequestProcessor which fails update requests once a core has + * exceeded a configurable maximum number of fields. Meant as a safeguard to help users notice + * potentially-dangerous schema design before performance and stability problems start to occur. + * + *

The URP uses the core's {@link SolrIndexSearcher} to judge the current number of fields. + * Accordingly, it undercounts the number of fields in the core - missing all fields added since the + * previous searcher was opened. As such, the URP's request-blocking is "best effort" - it cannot be + * relied on as a precise limit on the number of fields. + * + *

Additionally, the field-counting includes all documents present in the index, including any + * deleted docs that haven't yet been purged via segment merging. Note that this can differ + * significantly from the number of fields defined in managed-schema.xml - especially when dynamic + * fields are enabled. The only way to reduce this field count is to delete documents and wait until + * the deleted documents have been removed by segment merges. Users may of course speed up this + * process by tweaking Solr's segment-merging, triggering an "optimize" operation, etc. + * + *

{@link NumFieldLimitingUpdateRequestProcessorFactory} accepts two configuration parameters: + * + *

    + *
  • maxFields - (required) The maximum number of fields before update requests + * should be aborted. Once this limit has been exceeded, additional update requests will fail + * until fields have been removed or the "maxFields" is increased. + *
  • warnOnly - (optional) If true then the URP logs verbose warnings + * about the limit being exceeded but doesn't abort update requests. Defaults to false + * if not specified + *
+ * + * @since 9.7.0 + */ +public class NumFieldLimitingUpdateRequestProcessorFactory extends UpdateRequestProcessorFactory { + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + private static final String MAXIMUM_FIELDS_PARAM = "maxFields"; + private static final String WARN_ONLY_PARAM = "warnOnly"; + + // package visibility for tests + int maximumFields; + boolean warnOnly; + + @Override + public void init(NamedList args) { + warnOnly = args.indexOf(WARN_ONLY_PARAM, 0) > 0 ? args.getBooleanArg(WARN_ONLY_PARAM) : false; + + if (args.indexOf(MAXIMUM_FIELDS_PARAM, 0) < 0) { + throw new IllegalArgumentException( + "The " + + MAXIMUM_FIELDS_PARAM + + " parameter is required for " + + getClass().getName() + + ", but no value was provided."); + } + final Object rawMaxFields = args.get(MAXIMUM_FIELDS_PARAM); + if (!(rawMaxFields instanceof Integer)) { + throw new IllegalArgumentException( + MAXIMUM_FIELDS_PARAM + " must be configured as a non-null "); + } + maximumFields = (Integer) rawMaxFields; + if (maximumFields <= 0) { + throw new IllegalArgumentException(MAXIMUM_FIELDS_PARAM + " must be a positive integer"); + } + } + + @Override + public UpdateRequestProcessor getInstance( + SolrQueryRequest req, SolrQueryResponse rsp, UpdateRequestProcessor next) { + // note: it's unusual to call req.getSearcher in a /update request but it should be fine + final int currentNumFields = req.getSearcher().getFieldInfos().size(); + if (currentNumFields <= maximumFields) { + // great; no need to insert an URP to block or log anything + return next; + } + + // Block indexing new documents + return new UpdateRequestProcessor(next) { + @Override + public void processAdd(AddUpdateCommand cmd) throws IOException { + String id = cmd.getPrintableId(); + final String messageSuffix = warnOnly ? "Blocking update of document " + id : ""; + final String message = + String.format( + Locale.ROOT, + "Current core has %d fields, exceeding the max-fields limit of %d. %s", + currentNumFields, + maximumFields, + messageSuffix); + if (warnOnly) { + log.warn(message); + } else { + throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, message); + } + } + }; + } +} diff --git a/solr/core/src/test-files/solr/configsets/cloud-minimal-field-limiting/conf/schema.xml b/solr/core/src/test-files/solr/configsets/cloud-minimal-field-limiting/conf/schema.xml new file mode 100644 index 00000000000..d6a2fa7a916 --- /dev/null +++ b/solr/core/src/test-files/solr/configsets/cloud-minimal-field-limiting/conf/schema.xml @@ -0,0 +1,30 @@ + + + + + + + + + + + + + + id + diff --git a/solr/core/src/test-files/solr/configsets/cloud-minimal-field-limiting/conf/solrconfig.xml b/solr/core/src/test-files/solr/configsets/cloud-minimal-field-limiting/conf/solrconfig.xml new file mode 100644 index 00000000000..00f1ab3714b --- /dev/null +++ b/solr/core/src/test-files/solr/configsets/cloud-minimal-field-limiting/conf/solrconfig.xml @@ -0,0 +1,60 @@ + + + + + + + + + ${solr.data.dir:} + + + + + ${tests.luceneMatchVersion:LATEST} + + + + ${solr.commitwithin.softcommit:true} + + + + + + + explicit + true + text + + + + + + ${solr.test.maxFields:1234} + + + + + + + + + + + diff --git a/solr/core/src/test/org/apache/solr/update/processor/NumFieldLimitingUpdateRequestProcessorFactoryTest.java b/solr/core/src/test/org/apache/solr/update/processor/NumFieldLimitingUpdateRequestProcessorFactoryTest.java new file mode 100644 index 00000000000..eae9fe0e7c9 --- /dev/null +++ b/solr/core/src/test/org/apache/solr/update/processor/NumFieldLimitingUpdateRequestProcessorFactoryTest.java @@ -0,0 +1,84 @@ +/* + * 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.update.processor; + +import org.apache.solr.SolrTestCase; +import org.apache.solr.common.util.NamedList; +import org.hamcrest.Matchers; +import org.junit.Before; +import org.junit.Test; + +public class NumFieldLimitingUpdateRequestProcessorFactoryTest extends SolrTestCase { + + private NumFieldLimitingUpdateRequestProcessorFactory factory = null; + + @Before + public void initFactory() { + factory = new NumFieldLimitingUpdateRequestProcessorFactory(); + } + + @Test + public void testReportsErrorIfMaximumFieldsNotProvided() { + final var initArgs = new NamedList<>(); + final IllegalArgumentException thrown = + expectThrows( + IllegalArgumentException.class, + () -> { + factory.init(initArgs); + }); + assertThat(thrown.getMessage(), Matchers.containsString("maxFields parameter is required")); + assertThat(thrown.getMessage(), Matchers.containsString("no value was provided")); + } + + @Test + public void testReportsErrorIfMaximumFieldsIsInvalid() { + final var initArgs = new NamedList<>(); + initArgs.add("maxFields", "nonIntegerValue"); + IllegalArgumentException thrown = + expectThrows( + IllegalArgumentException.class, + () -> { + factory.init(initArgs); + }); + assertThat( + thrown.getMessage(), + Matchers.containsString("maxFields must be configured as a non-null ")); + + initArgs.clear(); + initArgs.add("maxFields", Integer.valueOf(-5)); + thrown = + expectThrows( + IllegalArgumentException.class, + () -> { + factory.init(initArgs); + }); + assertThat( + thrown.getMessage(), Matchers.containsString("maxFields must be a positive integer")); + } + + @Test + public void testCorrectlyParsesAllConfigurationParams() { + final var initArgs = new NamedList<>(); + initArgs.add("maxFields", 123); + initArgs.add("warnOnly", Boolean.TRUE); + + factory.init(initArgs); + + assertEquals(123, factory.maximumFields); + assertEquals(true, factory.warnOnly); + } +} diff --git a/solr/core/src/test/org/apache/solr/update/processor/NumFieldLimitingUpdateRequestProcessorIntegrationTest.java b/solr/core/src/test/org/apache/solr/update/processor/NumFieldLimitingUpdateRequestProcessorIntegrationTest.java new file mode 100644 index 00000000000..0ebaba57215 --- /dev/null +++ b/solr/core/src/test/org/apache/solr/update/processor/NumFieldLimitingUpdateRequestProcessorIntegrationTest.java @@ -0,0 +1,113 @@ +/* + * 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.update.processor; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.UUID; +import java.util.concurrent.TimeUnit; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.cloud.SolrCloudTestCase; +import org.apache.solr.common.SolrInputDocument; +import org.hamcrest.Matchers; +import org.junit.BeforeClass; +import org.junit.Test; + +public class NumFieldLimitingUpdateRequestProcessorIntegrationTest extends SolrCloudTestCase { + + private static String COLLECTION_NAME = "collName"; + private static String FIELD_LIMITING_CS_NAME = "fieldLimitingConfig"; + + @BeforeClass + public static void setupCluster() throws Exception { + final var configPath = + TEST_PATH().resolve("configsets").resolve("cloud-minimal-field-limiting").resolve("conf"); + configureCluster(1).addConfig(FIELD_LIMITING_CS_NAME, configPath).configure(); + + final var createRequest = + CollectionAdminRequest.createCollection(COLLECTION_NAME, FIELD_LIMITING_CS_NAME, 1, 1); + createRequest.process(cluster.getSolrClient()); + cluster.waitForActiveCollection(COLLECTION_NAME, 20, TimeUnit.SECONDS, 1, 1); + } + + private void setFieldLimitTo(int value) throws Exception { + System.setProperty("solr.test.maxFields", String.valueOf(value)); + + final var reloadRequest = CollectionAdminRequest.reloadCollection(COLLECTION_NAME); + final var reloadResponse = reloadRequest.process(cluster.getSolrClient()); + assertEquals(0, reloadResponse.getStatus()); + } + + @Test + public void test() throws Exception { + setFieldLimitTo(100); + + // Add 100 new fields - should all succeed since we're under the limit until the final commit + for (int i = 0; i < 5; i++) { + addNewFieldsAndCommit(20); + } + + // Adding any additional docs should fail because we've exceeded the field limit + final var thrown = + expectThrows( + Exception.class, + () -> { + addNewFieldsAndCommit(10); + }); + assertThat( + thrown.getMessage(), Matchers.containsString("exceeding the max-fields limit of 100")); + + // After raising the limit, updates succeed again + setFieldLimitTo(150); + for (int i = 0; i < 3; i++) { + addNewFieldsAndCommit(10); + } + } + + private void addNewFieldsAndCommit(int numFields) throws Exception { + final var docList = getDocumentListToAddFields(numFields); + final var updateResponse = cluster.getSolrClient(COLLECTION_NAME).add(docList); + assertEquals(0, updateResponse.getStatus()); + cluster.getSolrClient(COLLECTION_NAME).commit(); + } + + private Collection getDocumentListToAddFields(int numFieldsToAdd) { + int fieldsAdded = 0; + final var docList = new ArrayList(); + while (fieldsAdded < numFieldsToAdd) { + final var doc = new SolrInputDocument(); + doc.addField("id", randomFieldValue()); + + final int fieldsForDoc = Math.min(numFieldsToAdd - fieldsAdded, 5); + for (int fieldCount = 0; fieldCount < fieldsForDoc; fieldCount++) { + doc.addField(randomFieldName(), randomFieldValue()); + } + fieldsAdded += fieldsForDoc; + docList.add(doc); + } + + return docList; + } + + private String randomFieldName() { + return UUID.randomUUID().toString().replace("-", "_") + "_s"; + } + + private String randomFieldValue() { + return UUID.randomUUID().toString(); + } +} diff --git a/solr/server/solr/configsets/_default/conf/solrconfig.xml b/solr/server/solr/configsets/_default/conf/solrconfig.xml index e04a4cb9a9b..5b24094179b 100644 --- a/solr/server/solr/configsets/_default/conf/solrconfig.xml +++ b/solr/server/solr/configsets/_default/conf/solrconfig.xml @@ -894,6 +894,9 @@ [^\w-\.] _ + + 1000 + @@ -937,10 +940,11 @@ pdoubles + + processor="uuid,remove-blank,field-name-mutating,max-fields,parse-boolean,parse-long,parse-double,parse-date,add-schema-fields"> diff --git a/solr/solr-ref-guide/modules/configuration-guide/pages/update-request-processors.adoc b/solr/solr-ref-guide/modules/configuration-guide/pages/update-request-processors.adoc index ee160af4136..8ac9faf031f 100644 --- a/solr/solr-ref-guide/modules/configuration-guide/pages/update-request-processors.adoc +++ b/solr/solr-ref-guide/modules/configuration-guide/pages/update-request-processors.adoc @@ -337,6 +337,12 @@ Documents processed prior to the offender are indexed by Solr; documents followi + Alternatively, the processor offers a "permissive" mode (`permissiveMode=true`) which skips the offending document and logs a warning, but doesn't abort the remainder of the batch or return an error to users. +{solr-javadocs}/core/org/apache/solr/update/processor/NumFieldLimitingUpdateRequestProcessorFactory.html[NumFieldLimitingUpdateRequestProcessorFactory]:: Fails update requests once a core has exceeded a configurable "maximum" number of fields. ++ +Solr performance can degrade and even become unstable if cores accumulate too many (e.g. more than 500) fields. The "NumFieldLimiting" URP is offered as a safeguard that helps users notice potentially-dangerous schema design and/or mis-use of dynamic fields, before these performance and stability problems would manifest. +Note that the field count an index reports can be influenced by deleted (but not yet purged) documents, and may vary from replica to replica. +In order to avoid these sort of discrepancies between replicas, use of this URP should almost always precede DistributedUpdateProcessor in when running in SolrCloud mode. + {solr-javadocs}/core/org/apache/solr/update/processor/RegexpBoostProcessorFactory.html[RegexpBoostProcessorFactory]:: A processor which will match content of "inputField" against regular expressions found in "boostFilename", and if it matches will return the corresponding boost value from the file and output this to "boostField" as a double value. {solr-javadocs}/core/org/apache/solr/update/processor/SignatureUpdateProcessorFactory.html[SignatureUpdateProcessorFactory]:: Uses a defined set of fields to generate a hash "signature" for the document. diff --git a/solr/solrj/src/java/org/apache/solr/common/util/StrUtils.java b/solr/solrj/src/java/org/apache/solr/common/util/StrUtils.java index 6c209770e42..e2992204856 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/StrUtils.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/StrUtils.java @@ -388,4 +388,19 @@ public static String stringFromReader(Reader inReader) throws IOException { return stringWriter.toString(); } } + + @SuppressWarnings("ReferenceEquality") + public static boolean equalsIgnoreCase(String left, String right) { + if (left == right) { + return true; + } + if (left == null || right == null) { + return false; + } + if (left.length() != right.length()) { + return false; + } + + return left.equalsIgnoreCase(right); + } } From 6bde3526337a756f0ba4c1f00a3bf3050b42926b Mon Sep 17 00:00:00 2001 From: David Smiley Date: Wed, 8 May 2024 17:02:01 -0400 Subject: [PATCH 24/24] CHANGES.txt --- solr/CHANGES.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 9313a17c730..a507c1026f2 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -130,6 +130,9 @@ Other Changes --------------------- * SOLR-17248: Refactor ZK related SolrCli tools to separate SolrZkClient and CloudSolrClient instantiation/usage (Lamine Idjeraoui via Eric Pugh) +* SOLR-16505: Use Jetty HTTP2 for index replication and other "recovery" operations + (Sanjay Dutt, David Smiley) + ================== 9.6.0 ================== New Features --------------------- @@ -263,8 +266,6 @@ Other Changes * SOLR-17222: QueryResponseWriterUtil.NonFlushingStream should support bulk write methods (Michael Gibney) -* SOLR-16505: Switch internal replica recovery commands to Jetty HTTP2 (Sanjay Dutt, David Smiley) - ================== 9.5.0 ================== New Features ---------------------