You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by mk...@apache.org on 2023/11/25 17:43:49 UTC

(solr) branch branch_9x updated: SOLR-10653: leader to replica update fail if there's a UUIDField in schema (#2078) (#2096)

This is an automated email from the ASF dual-hosted git repository.

mkhl pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git


The following commit(s) were added to refs/heads/branch_9x by this push:
     new 81d0440c136 SOLR-10653: leader to replica update fail if there's a UUIDField in schema (#2078) (#2096)
81d0440c136 is described below

commit 81d0440c1361e7a7327de40ef2b7cc1bcff20b68
Author: Mikhail Khludnev <mk...@users.noreply.github.com>
AuthorDate: Sat Nov 25 20:43:43 2023 +0300

    SOLR-10653: leader to replica update fail if there's a UUIDField in schema (#2078) (#2096)
    
    Also FieldType.createField() propagates underneath SolrException, to avoid error code erasure.
    
    (cherry picked from commit c1cfaec00d37cbca71b62b4c535c1eed5fa7c4f6)
---
 solr/CHANGES.txt                                   |   3 +
 .../src/java/org/apache/solr/schema/FieldType.java |  11 +-
 .../src/java/org/apache/solr/schema/UUIDField.java |  13 --
 .../test/org/apache/solr/schema/UUIDFieldTest.java |  17 +++
 .../apache/solr/update/UuidAtomicUpdateTest.java   | 155 +++++++++++++++++++++
 5 files changed, 177 insertions(+), 22 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index b5d9cfd31bd..b9c8c3a597d 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -53,6 +53,9 @@ Bug Fixes
 * SOLR-17045: DenseVectorField w/ vectorDimension > 1024 now work automatically with _default configset, due to
   implicit use of SchemaCodecFactory.  (hossman)
 
+* SOLR-10653: When there's a UUIDField in the schema and atomic update touches another field
+  the error occurs when leader updates replica (Mikhail Khludnev)
+
 Dependency Upgrades
 ---------------------
 * SOLR-17012: Update Apache Hadoop to 3.3.6 and Apache Curator to 5.5.0 (Kevin Risden)
diff --git a/solr/core/src/java/org/apache/solr/schema/FieldType.java b/solr/core/src/java/org/apache/solr/schema/FieldType.java
index 65cc1b06fa6..4fe06da7500 100644
--- a/solr/core/src/java/org/apache/solr/schema/FieldType.java
+++ b/solr/core/src/java/org/apache/solr/schema/FieldType.java
@@ -301,6 +301,8 @@ public abstract class FieldType extends FieldProperties {
     String val;
     try {
       val = toInternal(value.toString());
+    } catch (SolrException se) {
+      throw se; //  BAD_REQUEST to fall through
     } catch (RuntimeException e) {
       throw new SolrException(
           SolrException.ErrorCode.SERVER_ERROR,
@@ -309,15 +311,6 @@ public abstract class FieldType extends FieldProperties {
     }
     if (val == null) return null;
 
-    /*org.apache.lucene.document.FieldType newType = new org.apache.lucene.document.FieldType();
-    newType.setTokenized(field.isTokenized());
-    newType.setStored(field.stored());
-    newType.setOmitNorms(field.omitNorms());
-    newType.setIndexOptions(field.indexed() ? getIndexOptions(field, val) : IndexOptions.NONE);
-    newType.setStoreTermVectors(field.storeTermVector());
-    newType.setStoreTermVectorOffsets(field.storeTermOffsets());
-    newType.setStoreTermVectorPositions(field.storeTermPositions());
-    newType.setStoreTermVectorPayloads(field.storeTermPayloads());*/
     return createField(field.getName(), val, field);
   }
 
diff --git a/solr/core/src/java/org/apache/solr/schema/UUIDField.java b/solr/core/src/java/org/apache/solr/schema/UUIDField.java
index 94b72306622..baa591f9384 100644
--- a/solr/core/src/java/org/apache/solr/schema/UUIDField.java
+++ b/solr/core/src/java/org/apache/solr/schema/UUIDField.java
@@ -92,17 +92,4 @@ public class UUIDField extends StrField {
   public String toInternal(UUID uuid) {
     return uuid.toString().toLowerCase(Locale.ROOT);
   }
-
-  @Override
-  public UUID toObject(IndexableField f) {
-    return UUID.fromString(toExternal(f));
-  }
-
-  @Override
-  public Object toNativeType(Object val) {
-    if (val instanceof CharSequence) {
-      return UUID.fromString(val.toString());
-    }
-    return val;
-  }
 }
diff --git a/solr/core/src/test/org/apache/solr/schema/UUIDFieldTest.java b/solr/core/src/test/org/apache/solr/schema/UUIDFieldTest.java
index a7cbb4d0701..b0c96c561de 100644
--- a/solr/core/src/test/org/apache/solr/schema/UUIDFieldTest.java
+++ b/solr/core/src/test/org/apache/solr/schema/UUIDFieldTest.java
@@ -64,4 +64,21 @@ public class UUIDFieldTest extends SolrTestCaseJ4 {
     }
     assertTrue("Bad UUID check failed", ok);
   }
+
+  public void testBadRequest() {
+    try {
+      new UUIDField()
+          .createField(
+              new SchemaField(
+                  "test",
+                  new StrField() {
+                    {
+                      properties = (STORED | INDEXED);
+                    }
+                  }),
+              "bad request");
+    } catch (SolrException s) {
+      assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, s.code());
+    }
+  }
 }
diff --git a/solr/core/src/test/org/apache/solr/update/UuidAtomicUpdateTest.java b/solr/core/src/test/org/apache/solr/update/UuidAtomicUpdateTest.java
new file mode 100644
index 00000000000..44ed53bdc70
--- /dev/null
+++ b/solr/core/src/test/org/apache/solr/update/UuidAtomicUpdateTest.java
@@ -0,0 +1,155 @@
+/*
+ * 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 java.io.IOException;
+import java.util.Collection;
+import java.util.Map;
+import java.util.UUID;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+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.client.solrj.response.UpdateResponse;
+import org.apache.solr.cloud.SolrCloudTestCase;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class UuidAtomicUpdateTest extends SolrCloudTestCase {
+  private static final String COMMITTED_DOC_ID = "1";
+  private static final String UNCOMMITTED_DOC_ID = "2";
+  private static final String COLLECTION = "collection1";
+  private static final int NUM_SHARDS = 1;
+  private static final int NUM_REPLICAS = 2; // bug occurs only with replica
+
+  private static final String committedUuidAfter = UUID.randomUUID().toString();
+  private static final String uncommittedUuidAfter = UUID.randomUUID().toString();
+
+  @BeforeClass
+  public static void setupCluster() throws Exception {
+    configureCluster(1).addConfig("conf", configset("cloud-dynamic")).configure();
+
+    CollectionAdminRequest.createCollection(COLLECTION, "conf", NUM_SHARDS, NUM_REPLICAS)
+        .process(cluster.getSolrClient());
+
+    cluster.waitForActiveCollection(COLLECTION, NUM_SHARDS, NUM_REPLICAS * NUM_SHARDS);
+  }
+
+  @Override
+  @Before
+  public void setUp() throws Exception {
+    super.setUp();
+    // wipe
+    new UpdateRequest().deleteByQuery("*:*").process(cluster.getSolrClient(), COLLECTION);
+    String committedUuidBefore = UUID.randomUUID().toString();
+    final SolrInputDocument committedDoc =
+        sdoc("id", COMMITTED_DOC_ID, "title_s", "title_1", "uuid", committedUuidBefore);
+    new UpdateRequest().add(committedDoc).commit(cluster.getSolrClient(), COLLECTION);
+
+    String uncommittedUuidBefore = UUID.randomUUID().toString();
+    final SolrInputDocument uncommittedDoc =
+        sdoc("id", UNCOMMITTED_DOC_ID, "title_s", "title_2", "uuid", uncommittedUuidBefore);
+    new UpdateRequest().add(uncommittedDoc).process(cluster.getSolrClient(), COLLECTION);
+  }
+
+  @Test
+  public void testUpdateCommittedTextField() throws Exception {
+    atomicSetValue(COMMITTED_DOC_ID, "title_s", "CHANGED");
+    ensureFieldHasValues(COMMITTED_DOC_ID, "title_s", "CHANGED");
+    commit();
+    ensureFieldHasValues(COMMITTED_DOC_ID, "title_s", "CHANGED");
+  }
+
+  @Test
+  public void testUpdateUncommittedTextField() throws Exception {
+    atomicSetValue(UNCOMMITTED_DOC_ID, "title_s", "CHANGED");
+    ensureFieldHasValues(UNCOMMITTED_DOC_ID, "title_s", "CHANGED");
+    commit();
+    ensureFieldHasValues(UNCOMMITTED_DOC_ID, "title_s", "CHANGED");
+  }
+
+  @Test
+  public void testUpdateCommittedUuidField() throws Exception {
+    atomicSetValue(COMMITTED_DOC_ID, "uuid", committedUuidAfter);
+    ensureFieldHasValues(COMMITTED_DOC_ID, "uuid", committedUuidAfter);
+    commit();
+    ensureFieldHasValues(COMMITTED_DOC_ID, "uuid", committedUuidAfter);
+  }
+
+  @Test
+  public void testUpdateUncommittedUuidField() throws Exception {
+    atomicSetValue(UNCOMMITTED_DOC_ID, "uuid", uncommittedUuidAfter);
+    ensureFieldHasValues(UNCOMMITTED_DOC_ID, "uuid", uncommittedUuidAfter);
+    commit();
+    ensureFieldHasValues(UNCOMMITTED_DOC_ID, "uuid", uncommittedUuidAfter);
+  }
+
+  private static void commit() throws IOException, SolrServerException {
+    new UpdateRequest().commit(cluster.getSolrClient(), COLLECTION);
+  }
+
+  private static void atomicSetValue(String docId, String fieldName, Object value)
+      throws Exception {
+    final SolrInputDocument doc = new SolrInputDocument();
+    doc.setField("id", docId);
+    doc.setField(fieldName, Map.of("set", value));
+
+    UpdateResponse updateResponse = cluster.getSolrClient().add(COLLECTION, doc);
+    assertEquals(updateResponse.toString(), 0, updateResponse.getStatus());
+    assertEquals(
+        updateResponse.toString(), NUM_REPLICAS, updateResponse.getResponseHeader().get("rf"));
+  }
+
+  private static void ensureFieldHasValues(
+      String identifyingDocId, String fieldName, Object... expectedValues) throws Exception {
+    for (Boolean tf : new Boolean[] {true, false}) {
+      final ModifiableSolrParams solrParams = new ModifiableSolrParams();
+      solrParams.set("id", identifyingDocId);
+      solrParams.set("shards.preference", "replica.leader:" + tf);
+      QueryRequest request = new QueryRequest(solrParams);
+      request.setPath("/get");
+      final QueryResponse response = request.process(cluster.getSolrClient(), COLLECTION);
+
+      final NamedList<Object> rawResponse = response.getResponse();
+      assertNotNull(rawResponse.get("doc"));
+      assertTrue(rawResponse.get("doc") instanceof SolrDocument);
+      final SolrDocument doc = (SolrDocument) rawResponse.get("doc");
+      final Collection<Object> valuesAfterUpdate = doc.getFieldValues(fieldName);
+      assertEquals(
+          "Expected field to have "
+              + expectedValues.length
+              + " values, but found "
+              + valuesAfterUpdate.size(),
+          expectedValues.length,
+          valuesAfterUpdate.size());
+      for (Object expectedValue : expectedValues) {
+        assertTrue(
+            "Expected value ["
+                + expectedValue
+                + "] was not found in field, but "
+                + valuesAfterUpdate,
+            valuesAfterUpdate.contains(expectedValue));
+      }
+    }
+  }
+}