From 48bd259516b8d78c991239fe7cf3340c90f582e5 Mon Sep 17 00:00:00 2001 From: markrmiller Date: Wed, 23 May 2018 09:53:45 -0500 Subject: [PATCH] SOLR-12378: Support missing versionField on indexed docs in DocBasedVersionConstraintsURP. --- solr/CHANGES.txt | 3 ++ .../DocBasedVersionConstraintsProcessor.java | 19 ++++---- ...sedVersionConstraintsProcessorFactory.java | 20 ++++++++- .../solrconfig-externalversionconstraint.xml | 25 +++++++++++ .../TestDocBasedVersionConstraints.java | 44 +++++++++++++++++++ .../src/updating-parts-of-documents.adoc | 1 + 6 files changed, 103 insertions(+), 9 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 761d06db480..cb40f7fd295 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -119,6 +119,9 @@ New Features * SOLR-9480: A new 'relatedness()' aggregate function for JSON Faceting to enable building Semantic Knowledge Graphs. (Trey Grainger, hossman) +* SOLR-12378: Support missing versionField on indexed docs in DocBasedVersionConstraintsURP. + (Oliver Bates, Michael Braun via Mark Miller) + Bug Fixes ---------------------- 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 5bc60ec5f84..fa805d7b2ac 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 @@ -35,7 +35,6 @@ import org.apache.solr.common.params.SolrParams; import org.apache.solr.core.SolrCore; import org.apache.solr.handler.component.RealTimeGetComponent; import org.apache.solr.request.SolrQueryRequest; -import org.apache.solr.response.SolrQueryResponse; import org.apache.solr.schema.FieldType; import org.apache.solr.schema.IndexSchema; import org.apache.solr.schema.SchemaField; @@ -60,6 +59,7 @@ public class DocBasedVersionConstraintsProcessor extends UpdateRequestProcessor private final SchemaField[] userVersionFields; private final SchemaField solrVersionField; private final boolean ignoreOldUpdates; + private final boolean supportMissingVersionOnOldDocs; private final String[] deleteVersionParamNames; private final SolrCore core; @@ -72,13 +72,14 @@ public class DocBasedVersionConstraintsProcessor extends UpdateRequestProcessor public DocBasedVersionConstraintsProcessor(List versionFields, boolean ignoreOldUpdates, List deleteVersionParamNames, + boolean supportMissingVersionOnOldDocs, boolean useFieldCache, SolrQueryRequest req, - SolrQueryResponse rsp, UpdateRequestProcessor next ) { super(next); this.ignoreOldUpdates = ignoreOldUpdates; this.deleteVersionParamNames = deleteVersionParamNames.toArray(EMPTY_STR_ARR); + this.supportMissingVersionOnOldDocs = supportMissingVersionOnOldDocs; this.core = req.getCore(); this.versionFieldNames = versionFields.toArray(EMPTY_STR_ARR); IndexSchema schema = core.getLatestSchema(); @@ -123,10 +124,10 @@ public class DocBasedVersionConstraintsProcessor extends UpdateRequestProcessor return rawValue; } - private static Object[] convertFieldValuesUsingType(Object[] rawValues, SchemaField[] fields) { + private Object[] convertFieldValuesUsingType(Object[] rawValues) { Object[] returnArr = new Object[rawValues.length]; for (int i = 0; i < returnArr.length; i++) { - returnArr[i] = convertFieldValueUsingType(rawValues[i], fields[i]); + returnArr[i] = convertFieldValueUsingType(rawValues[i], userVersionFields[i]); } return returnArr; } @@ -145,7 +146,7 @@ public class DocBasedVersionConstraintsProcessor extends UpdateRequestProcessor assert null != indexedDocId; assert null != newUserVersions; - newUserVersions = convertFieldValuesUsingType(newUserVersions, userVersionFields); + newUserVersions = convertFieldValuesUsingType(newUserVersions); final DocFoundAndOldUserAndSolrVersions docFoundAndOldUserVersions; if (useFieldCache) { @@ -165,11 +166,13 @@ public class DocBasedVersionConstraintsProcessor extends UpdateRequestProcessor return versionInUpdateIsAcceptable(newUserVersions, oldUserVersions); } - private static void validateUserVersions(Object[] userVersions, String[] fieldNames, String errorMessage) { + private void validateUserVersions(Object[] userVersions, String[] fieldNames, String errorMessage) { assert userVersions.length == fieldNames.length; for (int i = 0; i < userVersions.length; i++) { Object userVersion = userVersions[i]; - if ( null == userVersion) { + if (supportMissingVersionOnOldDocs && null == userVersion) { + userVersions[i] = (Comparable) o -> -1; + } else if (null == userVersion) { // could happen if they turn this feature on after building an index // w/o the versionField, or if validating a new doc, not present. throw new SolrException(SERVER_ERROR, errorMessage + fieldNames[i]); @@ -320,7 +323,7 @@ public class DocBasedVersionConstraintsProcessor extends UpdateRequestProcessor * @return True if acceptable, false if not. */ protected boolean newUpdateComparePasses(Comparable newUserVersion, Comparable oldUserVersion, String userVersionFieldName) { - return newUserVersion.compareTo(oldUserVersion) > 0; + return oldUserVersion.compareTo(newUserVersion) < 0; } private static Object[] getObjectValues(LeafReaderContext segmentContext, 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 ff4d78a81e4..a8ee9b58482 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 @@ -80,6 +80,11 @@ import static org.apache.solr.common.SolrException.ErrorCode.SERVER_ERROR; * document version that is not great enough to be silently ignored (and return * a status 200 to the client) instead of generating a 409 Version Conflict error. * + * + *
  • supportMissingVersionOnOldDocs - This boolean parameter defaults to + * false, but if set to true allows any documents written *before* + * this feature is enabled and which are missing the versionField to be overwritten. + *
    • * * @since 4.6.0 */ @@ -90,6 +95,7 @@ public class DocBasedVersionConstraintsProcessorFactory extends UpdateRequestPro private List versionFields = null; private List deleteVersionParamNames = Collections.emptyList(); private boolean useFieldCache; + private boolean supportMissingVersionOnOldDocs = false; @Override public void init( NamedList args ) { @@ -129,6 +135,17 @@ public class DocBasedVersionConstraintsProcessorFactory extends UpdateRequestPro } ignoreOldUpdates = (Boolean) tmp; } + + // optional - defaults to false + tmp = args.remove("supportMissingVersionOnOldDocs"); + if (null != tmp) { + if (! (tmp instanceof Boolean) ) { + throw new SolrException(SERVER_ERROR, + "'supportMissingVersionOnOldDocs' must be configured as a "); + } + supportMissingVersionOnOldDocs = ((Boolean)tmp).booleanValue(); + } + super.init(args); } @@ -139,8 +156,9 @@ public class DocBasedVersionConstraintsProcessorFactory extends UpdateRequestPro return new DocBasedVersionConstraintsProcessor(versionFields, ignoreOldUpdates, deleteVersionParamNames, + supportMissingVersionOnOldDocs, useFieldCache, - req, rsp, next); + req, next); } @Override diff --git a/solr/core/src/test-files/solr/collection1/conf/solrconfig-externalversionconstraint.xml b/solr/core/src/test-files/solr/collection1/conf/solrconfig-externalversionconstraint.xml index faa0ee02a6d..cd18d1e85a9 100644 --- a/solr/core/src/test-files/solr/collection1/conf/solrconfig-externalversionconstraint.xml +++ b/solr/core/src/test-files/solr/collection1/conf/solrconfig-externalversionconstraint.xml @@ -126,5 +126,30 @@ + + + + + + + + + + + true + my_version_l + del_version + true + + + + diff --git a/solr/core/src/test/org/apache/solr/update/TestDocBasedVersionConstraints.java b/solr/core/src/test/org/apache/solr/update/TestDocBasedVersionConstraints.java index 20d64cf0d71..8580f52963a 100644 --- a/solr/core/src/test/org/apache/solr/update/TestDocBasedVersionConstraints.java +++ b/solr/core/src/test/org/apache/solr/update/TestDocBasedVersionConstraints.java @@ -478,6 +478,50 @@ public class TestDocBasedVersionConstraints extends SolrTestCaseJ4 { ExecutorUtil.shutdownAndAwaitTermination(runner); } } + + public void testMissingVersionOnOldDocs() throws Exception { + String version = "2"; + + // Write one doc with version, one doc without version using the "no version" chain + updateJ(json("[{\"id\": \"a\", \"name\": \"a1\", \"my_version_l\": " + version + "}]"), + params("update.chain", "no-external-version")); + updateJ(json("[{\"id\": \"b\", \"name\": \"b1\"}]"), params("update.chain", "no-external-version")); + assertU(commit()); + assertJQ(req("q","*:*"), "/response/numFound==2"); + assertJQ(req("q","id:a"), "/response/numFound==1"); + assertJQ(req("q","id:b"), "/response/numFound==1"); + + // Try updating both with a new version and using the enforced version chain, expect id=b to fail bc old + // doc is missing the version field + version = "3"; + updateJ(json("[{\"id\": \"a\", \"name\": \"a1\", \"my_version_l\": " + version + "}]"), + params("update.chain", "external-version-constraint")); + try { + updateJ(json("[{\"id\": \"b\", \"name\": \"b1\", \"my_version_l\": " + version + "}]"), + params("update.chain", "external-version-constraint")); + fail("Update to id=b should have failed because existing doc is missing version field"); + } catch (final SolrException ex) { + // expected + assertEquals("Doc exists in index, but has null versionField: my_version_l", ex.getMessage()); + } + assertU(commit()); + assertJQ(req("q","*:*"), "/response/numFound==2"); + assertJQ(req("qt","/get", "id", "a", "fl", "id,my_version_l"), "=={'doc':{'id':'a', 'my_version_l':3}}"); // version changed to 3 + assertJQ(req("qt","/get", "id", "b", "fl", "id,my_version_l"), "=={'doc':{'id':'b'}}"); // no version, because update failed + + // Try to update again using the external version enforcement, but allowing old docs to not have the version + // field. Expect id=a to fail because version is lower, expect id=b to succeed. + version = "1"; + updateJ(json("[{\"id\": \"a\", \"name\": \"a1\", \"my_version_l\": " + version + "}]"), + params("update.chain", "external-version-support-missing")); + System.out.println("send b"); + updateJ(json("[{\"id\": \"b\", \"name\": \"b1\", \"my_version_l\": " + version + "}]"), + params("update.chain", "external-version-support-missing")); + assertU(commit()); + assertJQ(req("q","*:*"), "/response/numFound==2"); + assertJQ(req("qt","/get", "id", "a", "fl", "id,my_version_l"), "=={'doc':{'id':'a', 'my_version_l':3}}"); + assertJQ(req("qt","/get", "id", "b", "fl", "id,my_version_l"), "=={'doc':{'id':'b', 'my_version_l':1}}"); + } private Callable delayedAdd(final String... fields) { return Executors.callable(() -> { diff --git a/solr/solr-ref-guide/src/updating-parts-of-documents.adoc b/solr/solr-ref-guide/src/updating-parts-of-documents.adoc index 2a3d0b5071b..033a0400123 100644 --- a/solr/solr-ref-guide/src/updating-parts-of-documents.adoc +++ b/solr/solr-ref-guide/src/updating-parts-of-documents.adoc @@ -295,5 +295,6 @@ The `\_version_` field used by Solr for its normal optimistic concurrency also h The value of this configuration option should be the name of a request parameter that the processor will now consider mandatory for all attempts to Delete By Id, and must be be used by clients to specify a value for the `versionField` which is greater then the existing value of the document to be deleted. When using this request param, any Delete By Id command with a high enough document version number to succeed will be internally converted into an Add Document command that replaces the existing document with a new one which is empty except for the Unique Key and `versionField` to keeping a record of the deleted version so future Add Document commands will fail if their "new" version is not high enough. If `versionField` is specified as a list, then this parameter too must be specified as a comma delimited list of the same size so that the parameters correspond with the fields. +* `supportMissingVersionOnOldDocs` - This boolean parameter defaults to `false`, but if set to `true` allows any documents written *before* this feature is enabled and which are missing the versionField to be overwritten. Please consult the {solr-javadocs}/solr-core/org/apache/solr/update/processor/DocBasedVersionConstraintsProcessorFactory.html[DocBasedVersionConstraintsProcessorFactory javadocs] and https://git1-us-west.apache.org/repos/asf?p=lucene-solr.git;a=blob;f=solr/core/src/test-files/solr/collection1/conf/solrconfig-externalversionconstraint.xml;hb=HEAD[test solrconfig.xml file] for additional information and example usages.