You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by is...@apache.org on 2019/02/18 09:03:43 UTC
[lucene-solr] branch master updated: SOLR-11876: In-place updates
fail during resolution if required fields are present
This is an automated email from the ASF dual-hosted git repository.
ishan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/master by this push:
new 6a0f7b2 SOLR-11876: In-place updates fail during resolution if required fields are present
6a0f7b2 is described below
commit 6a0f7b251de104d9ce1dfa6b18821715929fe76b
Author: Ishan Chattopadhyaya <is...@apache.org>
AuthorDate: Mon Feb 18 14:33:28 2019 +0530
SOLR-11876: In-place updates fail during resolution if required fields are present
---
solr/CHANGES.txt | 3 ++
.../handler/component/RealTimeGetComponent.java | 25 ++++++++++-
.../org/apache/solr/update/DocumentBuilder.java | 4 +-
.../processor/AtomicUpdateDocumentMerger.java | 2 +-
.../conf/schema-inplace-required-field.xml | 35 ++++++++++++++++
.../update/TestInPlaceUpdatesRequiredField.java | 48 ++++++++++++++++++++++
6 files changed, 112 insertions(+), 5 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 182c006..7624d4f 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -61,6 +61,9 @@ Bug Fixes
* SOLR-13229: Cleanup replicasMetTragicEvent after all types of exception (Tomás Fernández Löbbe)
+* SOLR-11876: In-place update fails when resolving from Tlog if schema has a required field (Justin Deoliveira, janhoy,
+ Ishan Chattopadhyaya)
+
Improvements
----------------------
* SOLR-12999: Index replication could delete segments before downloading segments from master if there is not enough
diff --git a/solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java b/solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java
index 6ca8ad1..f49a555 100644
--- a/solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java
+++ b/solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java
@@ -82,6 +82,7 @@ import org.apache.solr.update.IndexFingerprint;
import org.apache.solr.update.PeerSync;
import org.apache.solr.update.PeerSyncWithLeader;
import org.apache.solr.update.UpdateLog;
+import org.apache.solr.update.processor.AtomicUpdateDocumentMerger;
import org.apache.solr.util.RefCounted;
import org.apache.solr.util.TestInjection;
import org.slf4j.Logger;
@@ -426,7 +427,15 @@ public class RealTimeGetComponent extends SearchComponent
} else { // i.e. lastPrevPointer==0
assert lastPrevPointer == 0;
// We have successfully resolved the document based off the tlogs
- return toSolrDoc(partialDoc, core.getLatestSchema());
+
+ // determine whether we can use the in place document, if the caller specified onlyTheseFields
+ // and those fields are all supported for in-place updates
+ IndexSchema schema = core.getLatestSchema();
+ boolean forInPlaceUpdate = onlyTheseFields != null
+ && onlyTheseFields.stream().map(schema::getField)
+ .allMatch(f -> null!=f && AtomicUpdateDocumentMerger.isSupportedFieldForInPlaceUpdate(f));
+
+ return toSolrDoc(partialDoc, schema, forInPlaceUpdate);
}
}
@@ -773,9 +782,21 @@ public class RealTimeGetComponent extends SearchComponent
* @lucene.experimental
*/
public static SolrDocument toSolrDoc(SolrInputDocument sdoc, IndexSchema schema) {
+ return toSolrDoc(sdoc, schema, false);
+ }
+
+ /**
+ * Converts a SolrInputDocument to SolrDocument, using an IndexSchema instance.
+ *
+ * @param sdoc The input document.
+ * @param schema The index schema.
+ * @param forInPlaceUpdate Whether the document is being used for an in place update,
+ * see {@link DocumentBuilder#toDocument(SolrInputDocument, IndexSchema, boolean, boolean)}
+ */
+ public static SolrDocument toSolrDoc(SolrInputDocument sdoc, IndexSchema schema, boolean forInPlaceUpdate) {
// TODO what about child / nested docs?
// TODO: do something more performant than this double conversion
- Document doc = DocumentBuilder.toDocument(sdoc, schema);
+ Document doc = DocumentBuilder.toDocument(sdoc, schema, forInPlaceUpdate, true);
// copy the stored fields only
Document out = new Document();
diff --git a/solr/core/src/java/org/apache/solr/update/DocumentBuilder.java b/solr/core/src/java/org/apache/solr/update/DocumentBuilder.java
index 2fd0266..86f555c 100644
--- a/solr/core/src/java/org/apache/solr/update/DocumentBuilder.java
+++ b/solr/core/src/java/org/apache/solr/update/DocumentBuilder.java
@@ -229,8 +229,8 @@ public class DocumentBuilder {
// Now validate required fields or add default values
// fields with default values are defacto 'required'
- // Note: We don't need to add default fields if this document is to be used for
- // in-place updates, since this validation and population of default fields would've happened
+ // Note: We don't need to add required fields if this document is to be used for
+ // in-place updates, since this validation and population of required fields would've happened
// during the full indexing initially.
if (!forInPlaceUpdate) {
for (SchemaField field : schema.getRequiredFields()) {
diff --git a/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java b/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java
index 1069c33..3b142b3 100644
--- a/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java
+++ b/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java
@@ -148,7 +148,7 @@ public class AtomicUpdateDocumentMerger {
* Note: If an update command has updates to only supported fields (and _version_ is also supported),
* only then is such an update command executed as an in-place update.
*/
- private static boolean isSupportedFieldForInPlaceUpdate(SchemaField schemaField) {
+ public static boolean isSupportedFieldForInPlaceUpdate(SchemaField schemaField) {
return !(schemaField.indexed() || schemaField.stored() || !schemaField.hasDocValues() ||
schemaField.multiValued() || !(schemaField.getType() instanceof NumericValueFieldType));
}
diff --git a/solr/core/src/test-files/solr/collection1/conf/schema-inplace-required-field.xml b/solr/core/src/test-files/solr/collection1/conf/schema-inplace-required-field.xml
new file mode 100644
index 0000000..6782cb5
--- /dev/null
+++ b/solr/core/src/test-files/solr/collection1/conf/schema-inplace-required-field.xml
@@ -0,0 +1,35 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<!--
+ 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.
+ -->
+<schema name="inplace-updates" version="1.6">
+
+ <uniqueKey>id</uniqueKey>
+ <field name="id" type="string" indexed="true" stored="true" docValues="true"/>
+ <field name="_version_" type="long" indexed="false" stored="false" docValues="true" />
+
+ <field name="name" type="string" indexed="true" stored="true" required="true"/>
+ <field name="inplace_updatable_int" type="int" indexed="false" stored="false" docValues="true" />
+
+ <fieldType name="string" class="solr.StrField" multiValued="false" indexed="false" stored="false" docValues="false" />
+ <fieldType name="long" class="${solr.tests.LongFieldType}" multiValued="false" indexed="false" stored="false" docValues="false"/>
+ <fieldType name="int" class="${solr.tests.IntegerFieldType}" multiValued="false" indexed="false" stored="false" docValues="false"/>
+
+ <!-- cruft needed by the solrconfig used in our tests for startup, but not used in the tests -->
+ <field name="signatureField" type="string" indexed="true" stored="false"/>
+ <dynamicField name="*_sS" type="string" indexed="true" stored="true"/>
+
+</schema>
\ No newline at end of file
diff --git a/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesRequiredField.java b/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesRequiredField.java
new file mode 100644
index 0000000..a7f4da3
--- /dev/null
+++ b/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesRequiredField.java
@@ -0,0 +1,48 @@
+/*
+ * 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;
+
+import org.apache.solr.SolrTestCaseJ4;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import static org.apache.solr.update.TestInPlaceUpdatesStandalone.addAndAssertVersion;
+
+public class TestInPlaceUpdatesRequiredField extends SolrTestCaseJ4 {
+
+ @BeforeClass
+ public static void beforeClass() throws Exception {
+ initCore("solrconfig-tlog.xml", "schema-inplace-required-field.xml");
+ }
+
+ @Test
+ public void testUpdateFromTlog() throws Exception {
+ long version1 = addAndGetVersion(sdoc("id", "1", "name", "first", "inplace_updatable_int", 1), null);
+ assertU(commit("softCommit", "false"));
+ assertQ(req("q", "*:*"), "//*[@numFound='1']");
+
+ // do an in place update that hits off the tlog
+ version1 = addAndAssertVersion(version1, "id", "1", "inplace_updatable_int", map("inc", 1));
+ version1 = addAndAssertVersion(version1, "id", "1", "inplace_updatable_int", map("inc", 1));
+ version1 = addAndAssertVersion(version1, "id", "1", "inplace_updatable_int", map("inc", 1)); // new value should be 4
+ assertU(commit("softCommit", "false"));
+ assertQ(req("q", "id:1", "fl", "*,[docid]"),
+ "//result/doc[1]/int[@name='inplace_updatable_int'][.='4']",
+ "//result/doc[1]/long[@name='_version_'][.='"+version1+"']");
+ }
+}