From a86d2a0b3545731a1f7cc2a6e5772fa8374643d0 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Fri, 4 Sep 2026 17:35:27 -0400 Subject: [PATCH 1/2] DocBasedVersionConstraints URP: reject _version_ This processor uses _version_ for its own optimistic concurrency, setting it on every update so that its read of the existing document's version and the subsequent write are atomic. A _version_ supplied by the client -- on the document or as a request parameter -- was therefore overwritten before DistributedUpdateProcessor ever read it, so the requested precondition was silently not enforced. Adds carrying one are now rejected with a 400. Also in this change: * inform() no longer validates the tombstone document shape when deleteVersionParam is unconfigured. Tombstones are created solely by the Delete-By-Id handling that param enables, so without it the warning about uncovered required fields is noise -- and invites curating a tombstoneConfig for a code path that can never run. * Javadoc: say plainly that tombstoneConfig only has an effect alongside deleteVersionParam, and document the _version_ incompatibility. --- .../DocBasedVersionConstraintsProcessor.java | 24 ++++++++++++ ...sedVersionConstraintsProcessorFactory.java | 20 ++++++++-- .../TestDocBasedVersionConstraints.java | 37 +++++++++++++++++++ 3 files changed, 78 insertions(+), 3 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/update/processor/DocBasedVersionConstraintsProcessor.java b/solr/core/src/java/org/apache/solr/update/processor/DocBasedVersionConstraintsProcessor.java index cbec14726499..c63afcd95304 100644 --- a/solr/core/src/java/org/apache/solr/update/processor/DocBasedVersionConstraintsProcessor.java +++ b/solr/core/src/java/org/apache/solr/update/processor/DocBasedVersionConstraintsProcessor.java @@ -414,6 +414,8 @@ public void processAdd(AddUpdateCommand cmd) throws IOException { return; } + rejectClientSuppliedVersion(cmd); + final SolrInputDocument newDoc = cmd.getSolrInputDocument(); Object[] newVersions = getUserVersionsFromDocument(newDoc); validateUserVersions(newVersions, versionFieldNames, "Doc does not have versionField: "); @@ -440,6 +442,28 @@ public void processAdd(AddUpdateCommand cmd) throws IOException { } } + /** + * Rejects an update carrying Solr's native _version_. This processor overwrites that + * field to guard its own read-then-write, so a client precondition could not be honored. Consults + * the same sources, in the same order, as {@link DistributedUpdateProcessor} would. + * + *

NOTE: Perhaps we could do either-or, or combine both somehow. + */ + private static void rejectClientSuppliedVersion(AddUpdateCommand cmd) { + if (cmd.getVersion() == 0 + && cmd.getSolrInputDocument().getField(CommonParams.VERSION_FIELD) == null + && cmd.getReq().getParams().get(CommonParams.VERSION_FIELD) == null) { + return; + } + throw new SolrException( + BAD_REQUEST, + "Optimistic concurrency via " + + CommonParams.VERSION_FIELD + + " is not supported when DocBasedVersionConstraintsProcessorFactory is configured," + + " since it uses that field itself. Express version constraints with the configured" + + " versionField(s) instead."); + } + private static void logOverlyFailedRetries(int i, UpdateCommand cmd) { // Log a warning every 256 retries.... even a few retries should normally be very unusual. if ((i & 0xff) == 0xff) { diff --git a/solr/core/src/java/org/apache/solr/update/processor/DocBasedVersionConstraintsProcessorFactory.java b/solr/core/src/java/org/apache/solr/update/processor/DocBasedVersionConstraintsProcessorFactory.java index 90307f9dc381..f70e05cf6c2e 100644 --- a/solr/core/src/java/org/apache/solr/update/processor/DocBasedVersionConstraintsProcessorFactory.java +++ b/solr/core/src/java/org/apache/solr/update/processor/DocBasedVersionConstraintsProcessorFactory.java @@ -76,10 +76,20 @@ * false, but if set to true allows any documents written *before* this * feature is enabled and which are missing the versionField to be overwritten. *

  • tombstoneConfig - a list of field names to values to add to the created - * tombstone document. In general is not a good idea to populate tombsone documents with - * anything other than the minimum required fields so that it doean't match queries + * tombstone document. Only has any effect in combination with deleteVersionParam + * , since tombstones are created solely by the Delete By Id handling that param + * enables; without it this init param is inert. In general, it is not a good idea to populate + * tombstone documents with anything beyond the minimum required fields, so that they don't + * match queries. * * + *

    This processor is incompatible with Solr's native optimistic concurrency. It uses the + * _version_ field for itself, setting it on every update so that its read of the + * existing document's version and the subsequent write are atomic, retrying on conflict. A + * _version_ supplied by the client — on the document or as a request parameter — could + * therefore not be honored, so an Add carrying one is rejected with a 400. Express ordering through + * the per-document versionField values instead. + * * @since 4.6.0 */ public class DocBasedVersionConstraintsProcessorFactory extends UpdateRequestProcessorFactory @@ -205,7 +215,11 @@ public void inform(SolrCore core) { } } - canCreateTombstoneDocument(core.getLatestSchema()); + if (!deleteVersionParamNames.isEmpty()) { + // Tombstones are only produced by the Delete-By-Id handling that deleteVersionParam enables, + // so without it there is no tombstone shape to validate. + canCreateTombstoneDocument(core.getLatestSchema()); + } } /** diff --git a/solr/core/src/test/org/apache/solr/update/processor/TestDocBasedVersionConstraints.java b/solr/core/src/test/org/apache/solr/update/processor/TestDocBasedVersionConstraints.java index 5b69ae0d37c2..101cdc6ab205 100644 --- a/solr/core/src/test/org/apache/solr/update/processor/TestDocBasedVersionConstraints.java +++ b/solr/core/src/test/org/apache/solr/update/processor/TestDocBasedVersionConstraints.java @@ -53,6 +53,43 @@ public void before() { assertU(commit()); } + /** + * The processor uses {@code _version_} for its own optimistic concurrency, so a client-supplied + * one cannot be honored and must fail rather than be silently ignored. + */ + public void testNativeVersionIsRejected() throws Exception { + try (ErrorLogMuter muter = ErrorLogMuter.regex("Optimistic concurrency via _version_")) { + // on the document + SolrException ex = + expectThrows( + SolrException.class, + () -> + updateJ( + jsonAdd(sdoc("id", "aaa", "my_version_l", "1001", "_version_", "-1")), + params("update.chain", "external-version-constraint"))); + assertEquals(400, ex.code()); + + // as a request param + ex = + expectThrows( + SolrException.class, + () -> + updateJ( + jsonAdd(sdoc("id", "aaa", "my_version_l", "1001")), + params("update.chain", "external-version-constraint", "_version_", "-1"))); + assertEquals(400, ex.code()); + + assertEquals(2, muter.getCount()); + } + + // sanity check: the same add without a version still works + updateJ( + jsonAdd(sdoc("id", "aaa", "my_version_l", "1001")), + params("update.chain", "external-version-constraint")); + assertU(commit()); + assertJQ(req("q", "id:aaa"), "/response/numFound==1"); + } + public void testSimpleUpdates() throws Exception { // skip low version against committed data From e07853cdf124b38fa4f0740733743210aa6262cc Mon Sep 17 00:00:00 2001 From: David Smiley Date: Tue, 8 Sep 2026 00:11:16 -0400 Subject: [PATCH 2/2] changelog --- ...-docbasedversionconstraints-reject-native-version.yml | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 changelog/unreleased/PR#4877-docbasedversionconstraints-reject-native-version.yml diff --git a/changelog/unreleased/PR#4877-docbasedversionconstraints-reject-native-version.yml b/changelog/unreleased/PR#4877-docbasedversionconstraints-reject-native-version.yml new file mode 100644 index 000000000000..7f2936c39f33 --- /dev/null +++ b/changelog/unreleased/PR#4877-docbasedversionconstraints-reject-native-version.yml @@ -0,0 +1,9 @@ +title: > + DocBasedVersionConstraintsProcessor now rejects a client-supplied _version_ with a 400 instead of silently ignoring it. + And it no longer validates the tombstone document shape when deleteVersionParam isn't configured +type: changed +authors: + - name: David Smiley +links: + - name: PR#4877 + url: https://github.com/apache/solr/pull/4877 \ No newline at end of file