This is an automated email from the ASF dual-hosted git repository.
gerlowskija pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new 733c41a6ada SOLR-17758: Fix limiting-URP "warnOnly" mode (#3348)
733c41a6ada is described below
commit 733c41a6adae2786c367a1ecd05dcba2d09eab66
Author: Jason Gerlowski <[email protected]>
AuthorDate: Mon May 19 10:22:43 2025 -0400
SOLR-17758: Fix limiting-URP "warnOnly" mode (#3348)
Due to a bug in the initial implementation, the "warnOnly" mode of the
NumFieldLimiting "URP" never worked as designed. The "warn" codepath
failed to call `processAdd` for the next URP in the chain. This makes
the "warn only" behavior indistinguishable from the "hard limit"
behavior from a user perspective.
This commit fixes this bug. It also adds integration testing for
several "warn only" mode related scenarios, to prevent against future
regressions.
---
solr/CHANGES.txt | 3 +
...FieldLimitingUpdateRequestProcessorFactory.java | 5 +-
.../conf/solrconfig.xml | 1 +
...itingUpdateRequestProcessorIntegrationTest.java | 70 ++++++++++++++++++----
4 files changed, 64 insertions(+), 15 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index a88fa2c22b5..d3d7f82d142 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -288,6 +288,9 @@ Bug Fixes
* SOLR-17761: Global circuit breakers can no longer trigger
ConcurrentModificationException's on Solr startup (Jason Gerlowski)
+* SOLR-17758: The NumFieldLimitingUpdateRequestProcessor's "warnOnly" mode has
been fixed, and now processes documents even
+ when the limit has been exceeded. (Jason Gerlowski, Rahul Goswami)
+
Dependency Upgrades
---------------------
* SOLR-17471: Upgrade Lucene to 9.12.1. (Pierre Salagnac, Christine Poerschke)
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
index 9382d25a499..c6117613318 100644
---
a/solr/core/src/java/org/apache/solr/update/processor/NumFieldLimitingUpdateRequestProcessorFactory.java
+++
b/solr/core/src/java/org/apache/solr/update/processor/NumFieldLimitingUpdateRequestProcessorFactory.java
@@ -69,7 +69,7 @@ public class NumFieldLimitingUpdateRequestProcessorFactory
extends UpdateRequest
@Override
public void init(NamedList<?> args) {
- warnOnly = args.indexOf(WARN_ONLY_PARAM, 0) > 0 ?
args.getBooleanArg(WARN_ONLY_PARAM) : false;
+ 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(
@@ -105,7 +105,7 @@ public class NumFieldLimitingUpdateRequestProcessorFactory
extends UpdateRequest
@Override
public void processAdd(AddUpdateCommand cmd) throws IOException {
String id = cmd.getPrintableId();
- final String messageSuffix = warnOnly ? "Blocking update of document "
+ id : "";
+ final String messageSuffix = !warnOnly ? "Blocking update of document:
" + id : "";
final String message =
String.format(
Locale.ROOT,
@@ -115,6 +115,7 @@ public class NumFieldLimitingUpdateRequestProcessorFactory
extends UpdateRequest
messageSuffix);
if (warnOnly) {
log.warn(message);
+ super.processAdd(cmd);
} else {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
message);
}
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
index 00f1ab3714b..38c4491c154 100644
---
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
@@ -47,6 +47,7 @@
<updateRequestProcessorChain name="add-unknown-fields-to-the-schema"
default="true">
<processor class="solr.NumFieldLimitingUpdateRequestProcessorFactory"
name="max-fields">
<int name="maxFields">${solr.test.maxFields:1234}</int>
+ <bool name="warnOnly">${solr.test.fieldLimit.warnOnly:false}</bool>
</processor>
<processor class="solr.LogUpdateProcessorFactory"/>
<processor class="solr.DistributedUpdateProcessorFactory"/>
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
index 0ebaba57215..d67dd497462 100644
---
a/solr/core/src/test/org/apache/solr/update/processor/NumFieldLimitingUpdateRequestProcessorIntegrationTest.java
+++
b/solr/core/src/test/org/apache/solr/update/processor/NumFieldLimitingUpdateRequestProcessorIntegrationTest.java
@@ -24,6 +24,7 @@ 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.Before;
import org.junit.BeforeClass;
import org.junit.Test;
@@ -38,20 +39,31 @@ public class
NumFieldLimitingUpdateRequestProcessorIntegrationTest extends SolrC
TEST_PATH().resolve("configsets").resolve("cloud-minimal-field-limiting").resolve("conf");
configureCluster(1).addConfig(FIELD_LIMITING_CS_NAME,
configPath).configure();
+ System.setProperty("solr.test.fieldLimit.warnOnly", "false");
+ System.setProperty("solr.test.maxFields", String.valueOf(100));
+ }
+
+ @Before
+ @Override
+ public void setUp() throws Exception {
+ super.setUp();
+
+ System.setProperty("solr.test.fieldLimit.warnOnly", "false");
+ System.setProperty("solr.test.maxFields", String.valueOf(100));
+
+ // Collection might already exist if test is being run multiple times
+ final var collections =
CollectionAdminRequest.listCollections(cluster.getSolrClient());
+ if (collections.contains(COLLECTION_NAME)) {
+ final var deleteRequest =
CollectionAdminRequest.deleteCollection(COLLECTION_NAME);
+ deleteRequest.processAndWait(cluster.getSolrClient(), DEFAULT_TIMEOUT);
+ }
+
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);
@@ -62,6 +74,28 @@ public class
NumFieldLimitingUpdateRequestProcessorIntegrationTest extends SolrC
}
// Adding any additional docs should fail because we've exceeded the field
limit
+ ensureNewFieldCreationTriggersExpectedFailure();
+
+ // Switch to warn only mode and ensure that updates succeed again
+ setWarnOnly(true);
+ ensureNewFieldsCanBeCreated();
+
+ // Switch back to "hard" limit mode and ensure things fail again
+ setWarnOnly(false);
+ ensureNewFieldCreationTriggersExpectedFailure();
+
+ // Raise the limit and ensure that updates succeed again
+ setFieldLimitTo(300);
+ ensureNewFieldsCanBeCreated();
+ }
+
+ private void ensureNewFieldsCanBeCreated() throws Exception {
+ for (int i = 0; i < 3; i++) {
+ addNewFieldsAndCommit(10);
+ }
+ }
+
+ private void ensureNewFieldCreationTriggersExpectedFailure() throws
Exception {
final var thrown =
expectThrows(
Exception.class,
@@ -70,12 +104,22 @@ public class
NumFieldLimitingUpdateRequestProcessorIntegrationTest extends SolrC
});
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 setWarnOnly(boolean warnOnly) throws Exception {
+ System.setProperty("solr.test.fieldLimit.warnOnly",
String.valueOf(warnOnly));
+
+ final var reloadRequest =
CollectionAdminRequest.reloadCollection(COLLECTION_NAME);
+ final var reloadResponse = reloadRequest.process(cluster.getSolrClient());
+ assertEquals(0, reloadResponse.getStatus());
+ }
+
+ 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());
}
private void addNewFieldsAndCommit(int numFields) throws Exception {
