Merge pull request #17533 from vespa-engine/balder/add-restart-detection-to-dictionary

Add restart handling for changes to dictionary.

4 files changed, 40 insertions, 3 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/VespaRestartAction.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/VespaRestartAction.java
index ca3a408b2e0..69cd022d939 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/VespaRestartAction.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/VespaRestartAction.java
@@ -6,7 +6,6 @@ import com.yahoo.config.model.api.ServiceInfo;
 import com.yahoo.config.provision.ClusterSpec;
 
 import java.util.List;
-import java.util.Optional;
 
 /**
  * Represents an action to restart services in order to handle a config change.
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/AttributeChangeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/AttributeChangeValidator.java
index 0c3354d2af5..7a17fe5e80a 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/AttributeChangeValidator.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/AttributeChangeValidator.java
@@ -6,6 +6,8 @@ import com.yahoo.documentmodel.NewDocumentType;
 import com.yahoo.searchdefinition.derived.AttributeFields;
 import com.yahoo.searchdefinition.derived.IndexSchema;
 import com.yahoo.searchdefinition.document.Attribute;
+import com.yahoo.searchdefinition.document.Case;
+import com.yahoo.searchdefinition.document.Dictionary;
 import com.yahoo.searchdefinition.document.HnswIndexParams;
 import com.yahoo.vespa.model.application.validation.change.VespaConfigChangeAction;
 import com.yahoo.vespa.model.application.validation.change.VespaRestartAction;
@@ -83,14 +85,25 @@ public class AttributeChangeValidator {
         return attribute.hnswIndexParams().isPresent();
     }
 
+    private static Dictionary.Type extractDictionaryType(Attribute attr) {
+        Dictionary dict = attr.getDictionary();
+        return dict != null ? dict.getType() : Dictionary.Type.BTREE;
+    }
+
+    private static Case extractDictionaryCase(Attribute attr) {
+        Dictionary dict = attr.getDictionary();
+        return dict != null ? dict.getMatch() : Case.UNCASED;
+    }
+
     private List<VespaConfigChangeAction> validateAttributeSettings() {
         List<VespaConfigChangeAction> result = new ArrayList<>();
         for (Attribute nextAttr : nextFields.attributes()) {
             Attribute currAttr = currentFields.getAttribute(nextAttr.getName());
             if (currAttr != null) {
-                // These validations can be removed as there are they are 1-1 with attributes.def which has restart annotations for them.
                 validateAttributeSetting(id, currAttr, nextAttr, Attribute::isFastSearch, "fast-search", result);
                 validateAttributeSetting(id, currAttr, nextAttr, Attribute::isFastAccess, "fast-access", result);
+                validateAttributeSetting(id, currAttr, nextAttr, AttributeChangeValidator::extractDictionaryType, "dictionary: btree/hash", result);
+                validateAttributeSetting(id, currAttr, nextAttr, AttributeChangeValidator::extractDictionaryCase, "dictionary: cased/uncased", result);
                 validateAttributeSetting(id, currAttr, nextAttr, Attribute::isHuge, "huge", result);
                 validateAttributeSetting(id, currAttr, nextAttr, Attribute::densePostingListThreshold, "dense-posting-list-threshold", result);
                 validateAttributeSetting(id, currAttr, nextAttr, Attribute::isEnabledOnlyBitVector, "rank: filter", result);
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ConfigChangeTestUtils.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ConfigChangeTestUtils.java
index 2157839ef5c..7f92d2f409c 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ConfigChangeTestUtils.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ConfigChangeTestUtils.java
@@ -6,7 +6,6 @@ import com.yahoo.config.model.api.ConfigChangeAction;
 import com.yahoo.config.model.api.ServiceInfo;
 import com.yahoo.config.provision.ClusterSpec;
 
-import java.time.Instant;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.stream.Collectors;
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/AttributeChangeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/AttributeChangeValidatorTest.java
index e89f0c0a9cd..cd8afa404da 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/AttributeChangeValidatorTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/AttributeChangeValidatorTest.java
@@ -3,6 +3,7 @@ package com.yahoo.vespa.model.application.validation.change.search;
 
 import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.vespa.model.application.validation.change.VespaConfigChangeAction;
+import org.junit.Ignore;
 import org.junit.Test;
 
 import java.util.List;
@@ -69,6 +70,22 @@ public class AttributeChangeValidatorTest {
     }
 
     @Test
+    public void changing_btree2hash_require_restart() throws Exception {
+        new Fixture("field f1 type long { indexing: attribute\n attribute: fast-search\n dictionary: btree}",
+                "field f1 type long { indexing: attribute\n attribute: fast-search\n dictionary: hash }").
+                assertValidation(newRestartAction(ClusterSpec.Id.from("test"),
+                        "Field 'f1' changed: change property 'dictionary: btree/hash' from 'BTREE' to 'HASH'"));
+    }
+
+    @Test
+    public void changing_hash2btree_require_restart() throws Exception {
+        new Fixture("field f1 type long { indexing: attribute\n attribute: fast-search\n dictionary: hash}",
+                "field f1 type long { indexing: attribute\n attribute: fast-search\n dictionary: btree }").
+                assertValidation(newRestartAction(ClusterSpec.Id.from("test"),
+                        "Field 'f1' changed: change property 'dictionary: btree/hash' from 'HASH' to 'BTREE'"));
+    }
+
+    @Test
     public void changing_fast_access_require_restart() throws Exception {
         new Fixture("field f1 type string { indexing: attribute \n attribute: fast-access }",
                 "field f1 type string { indexing: attribute }").
@@ -76,6 +93,15 @@ public class AttributeChangeValidatorTest {
                                                   "Field 'f1' changed: remove attribute 'fast-access'"));
     }
 
+    // TODO Not testable until one dictionary type supports both cased/uncased
+    @Ignore
+    public void changing_uncased2cased_require_restart() throws Exception {
+        new Fixture("field f1 type string { indexing: attribute\n attribute: fast-search\n dictionary { hash\ncased}\nmatch:cased}",
+                "field f1 type string { indexing: attribute\n attribute: fast-search\n dictionary{ btree} }").
+                assertValidation(newRestartAction(ClusterSpec.Id.from("test"),
+                        "Field 'f1' changed: change property 'dictionary: cased/uncased' from 'CASED' to 'UNCASED'"));
+    }
+
     @Test
     public void changing_huge_require_restart() throws Exception {
         new Fixture("field f1 type string { indexing: attribute }",