Ensure that explicit proton tuning have precedence over default node flavor tuning.

6 files changed, 89 insertions, 29 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/ContentSearchCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/ContentSearchCluster.java
index e97d35501be..4e5234fc532 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/ContentSearchCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/ContentSearchCluster.java
@@ -210,8 +210,9 @@ public class ContentSearchCluster extends AbstractConfigProducer implements Prot
         NodeSpec spec = getNextSearchNodeSpec(parentGroup);
         SearchNode snode;
         TransactionLogServer tls;
+        Optional<Tuning> tuning = Optional.ofNullable(this.tuning);
         if (element == null) {
-            snode = SearchNode.create(parent, "" + node.getDistributionKey(), node.getDistributionKey(), spec, clusterName, node, flushOnShutdown);
+            snode = SearchNode.create(parent, "" + node.getDistributionKey(), node.getDistributionKey(), spec, clusterName, node, flushOnShutdown, tuning);
             snode.setHostResource(node.getHostResource());
             snode.initService();
 
@@ -219,7 +220,7 @@ public class ContentSearchCluster extends AbstractConfigProducer implements Prot
             tls.setHostResource(snode.getHostResource());
             tls.initService();
         } else {
-            snode = new SearchNode.Builder(""+node.getDistributionKey(), spec, clusterName, node, flushOnShutdown).build(parent, element.getXml());
+            snode = new SearchNode.Builder(""+node.getDistributionKey(), spec, clusterName, node, flushOnShutdown, tuning).build(parent, element.getXml());
             tls = new TransactionLogServer.Builder(clusterName).build(snode, element.getXml());
         }
         snode.setTls(tls);
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/search/SearchNode.java b/config-model/src/main/java/com/yahoo/vespa/model/search/SearchNode.java
index d5fc782a633..cba881bb07e 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/search/SearchNode.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/search/SearchNode.java
@@ -55,6 +55,7 @@ public class SearchNode extends AbstractService implements
     private final String clusterName;
     private TransactionLogServer tls;
     private AbstractService serviceLayerService;
+    private final Optional<Tuning> tuning;
 
     public static class Builder extends VespaDomBuilder.DomConfigProducerBuilder<SearchNode> {
 
@@ -63,17 +64,19 @@ public class SearchNode extends AbstractService implements
         private final String clusterName;
         private final ContentNode contentNode;
         private final boolean flushOnShutdown;
-        public Builder(String name, NodeSpec nodeSpec, String clusterName, ContentNode node, boolean flushOnShutdown) {
+        private final Optional<Tuning> tuning;
+        public Builder(String name, NodeSpec nodeSpec, String clusterName, ContentNode node, boolean flushOnShutdown, Optional<Tuning> tuning) {
             this.name = name;
             this.nodeSpec = nodeSpec;
             this.clusterName = clusterName;
             this.contentNode = node;
             this.flushOnShutdown = flushOnShutdown;
+            this.tuning = tuning;
         }
 
         @Override
         protected SearchNode doBuild(AbstractConfigProducer ancestor, Element producerSpec) {
-            return new SearchNode(ancestor, name, contentNode.getDistributionKey(), nodeSpec, clusterName, contentNode, flushOnShutdown);
+            return new SearchNode(ancestor, name, contentNode.getDistributionKey(), nodeSpec, clusterName, contentNode, flushOnShutdown, tuning);
         }
     }
 
@@ -81,19 +84,19 @@ public class SearchNode extends AbstractService implements
      * Creates a SearchNode in elastic mode.
      */
     public static SearchNode create(AbstractConfigProducer parent, String name, int distributionKey, NodeSpec nodeSpec,
-                                    String clusterName, AbstractService serviceLayerService, boolean flushOnShutdown) {
-        return new SearchNode(parent, name, distributionKey, nodeSpec, clusterName, serviceLayerService, flushOnShutdown);
+                                    String clusterName, AbstractService serviceLayerService, boolean flushOnShutdown, Optional<Tuning> tuning) {
+        return new SearchNode(parent, name, distributionKey, nodeSpec, clusterName, serviceLayerService, flushOnShutdown, tuning);
     }
 
     private SearchNode(AbstractConfigProducer parent, String name, int distributionKey, NodeSpec nodeSpec,
-                       String clusterName, AbstractService serviceLayerService, boolean flushOnShutdown) {
-        this(parent, name, nodeSpec, clusterName, flushOnShutdown);
+                       String clusterName, AbstractService serviceLayerService, boolean flushOnShutdown, Optional<Tuning> tuning) {
+        this(parent, name, nodeSpec, clusterName, flushOnShutdown, tuning);
         this.distributionKey = distributionKey;
         this.serviceLayerService = serviceLayerService;
         setPropertiesElastic(clusterName, distributionKey);
     }
 
-    private SearchNode(AbstractConfigProducer parent, String name, NodeSpec nodeSpec, String clusterName, boolean flushOnShutdown) {
+    private SearchNode(AbstractConfigProducer parent, String name, NodeSpec nodeSpec, String clusterName, boolean flushOnShutdown, Optional<Tuning> tuning) {
         super(parent, name);
         this.nodeSpec = nodeSpec;
         this.clusterName = clusterName;
@@ -105,6 +108,7 @@ public class SearchNode extends AbstractService implements
         portsMeta.on(4).tag("http").tag("json").tag("health").tag("state");
         // Properties are set in DomSearchBuilder
         monitorService();
+        this.tuning = tuning;
     }
 
     private void setPropertiesElastic(String clusterName, int distributionKey) {
@@ -273,6 +277,9 @@ public class SearchNode extends AbstractService implements
         }
         if (getHostResource() != null && getHostResource().getFlavor().isPresent()) {
             new NodeFlavorTuning(getHostResource().getFlavor().get()).getConfig(builder);
+            if (tuning.isPresent()) {
+                tuning.get().getConfig(builder);
+            }
         }
     }
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/search/Tuning.java b/config-model/src/main/java/com/yahoo/vespa/model/search/Tuning.java
index c1650b6c3e0..83e75141fc9 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/search/Tuning.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/search/Tuning.java
@@ -105,32 +105,25 @@ public class Tuning extends AbstractConfigProducer implements PartitionsConfig.P
             public void getConfig(ProtonConfig.Builder builder) {
                 // Here, the config building gets very ugly, because we have to check for null because of autoconversion Long/long etc.
 
-                ProtonConfig.Flush.Memory.Builder memoryBuilder = new ProtonConfig.Flush.Memory.Builder();
+                ProtonConfig.Flush.Memory.Builder memoryBuilder = builder.flush.memory;
                 if (totalMaxMemoryGain != null) memoryBuilder.maxmemory(totalMaxMemoryGain);
                 if (totalDiskBloatFactor != null) memoryBuilder.diskbloatfactor(totalDiskBloatFactor);
                 if (transactionLogMaxSize != null) memoryBuilder.maxtlssize(transactionLogMaxSize);
 
-                ProtonConfig.Flush.Memory.Each.Builder eachBuilder = new ProtonConfig.Flush.Memory.Each.Builder();
+                ProtonConfig.Flush.Memory.Each.Builder eachBuilder = memoryBuilder.each;
                 if (componentMaxMemoryGain != null) eachBuilder.maxmemory(componentMaxMemoryGain);
                 if (componentDiskBloatFactor != null) eachBuilder.diskbloatfactor(componentDiskBloatFactor);
-                memoryBuilder.each(eachBuilder);
 
-                ProtonConfig.Flush.Memory.Maxage.Builder maxageBuilder = new ProtonConfig.Flush.Memory.Maxage.Builder();
+                ProtonConfig.Flush.Memory.Maxage.Builder maxageBuilder = memoryBuilder.maxage;
                 if (componentMaxage != null) maxageBuilder.time(componentMaxage);
-                memoryBuilder.maxage(maxageBuilder);
 
-                ProtonConfig.Flush.Memory.Conservative.Builder conservativeBuilder = new ProtonConfig.Flush.Memory.Conservative.Builder();
+                ProtonConfig.Flush.Memory.Conservative.Builder conservativeBuilder = memoryBuilder.conservative;
                 if (conservativeMemoryLimitFactor != null) {
                     conservativeBuilder.memorylimitfactor(conservativeMemoryLimitFactor);
                 }
                 if (conservativeDiskLimitFactor != null) {
                     conservativeBuilder.disklimitfactor(conservativeDiskLimitFactor);
                 }
-                memoryBuilder.conservative(conservativeBuilder);
-
-                builder.
-                    flush(new ProtonConfig.Flush.Builder().
-                            memory(memoryBuilder));
             }
         }
 
diff --git a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
index ec10c84a335..09d2153a30f 100644
--- a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
+++ b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
@@ -1443,10 +1443,8 @@ public class ModelProvisioningTest {
     }
 
     private static Flavor createFlavorFromDiskSetting(String name, boolean fastDisk) {
-        FlavorsConfig.Flavor.Builder builder = new FlavorsConfig.Flavor.Builder();
-        builder.name(name);
-        builder.fastDisk(fastDisk);
-        return new Flavor(new FlavorsConfig.Flavor(builder));
+        return new Flavor(new FlavorsConfig.Flavor(new FlavorsConfig.Flavor.Builder().
+                name(name).fastDisk(fastDisk)));
     }
 
     private static ProtonConfig getProtonConfig(ContentSearchCluster cluster, int searchNodeIdx) {
@@ -1456,5 +1454,59 @@ public class ModelProvisioningTest {
         searchNodes.get(searchNodeIdx).getConfig(builder);
         return new ProtonConfig(builder);
     }
-    
+
+    @Test
+    public void require_that_config_override_and_explicit_proton_tuning_have_precedence_over_default_node_flavor_tuning() {
+        String services = joinLines("<?xml version='1.0' encoding='utf-8' ?>",
+                "<services>",
+                "  <content version='1.0' id='test'>",
+                "    <config name='vespa.config.search.core.proton'>",
+                "      <flush><memory><maxtlssize>2000</maxtlssize></memory></flush>",
+                "    </config>",
+                "    <documents>",
+                "      <document type='type1' mode='index'/>",
+                "    </documents>",
+                "    <nodes count='1' flavor='content-test-flavor'/>",
+                "    <engine>",
+                "      <proton>",
+                "        <tuning>",
+                "          <searchnode>",
+                "            <flushstrategy>",
+                "              <native>",
+                "                <total>",
+                "                  <maxmemorygain>1000</maxmemorygain>",
+                "                </total>",
+                "              </native>",
+                "            </flushstrategy>",
+                "          </searchnode>",
+                "        </tuning>",
+                "      </proton>",
+                "    </engine>",
+                "  </content>",
+                "</services>");
+
+        VespaModelTester tester = new VespaModelTester();
+        tester.addHosts("default", 1);
+        tester.addHosts(createFlavorFromMemoryAndDisk("content-test-flavor", 128, 100), 1);
+        VespaModel model = tester.createModel(services, true, 0);
+        ContentSearchCluster cluster = model.getContentClusters().get("test").getSearch();
+        ProtonConfig cfg = getProtonConfig(model, cluster.getSearchNodes().get(0).getConfigId());
+        assertEquals(2000, cfg.flush().memory().maxtlssize()); // from config override
+        assertEquals(1000, cfg.flush().memory().maxmemory()); // from explicit tuning
+        assertEquals((long) 16 * GB, cfg.flush().memory().each().maxmemory()); // from default node flavor tuning
+    }
+
+    private static long GB = 1024 * 1024 * 1024;
+
+    private static Flavor createFlavorFromMemoryAndDisk(String name, int memoryGb, int diskGb) {
+        return new Flavor(new FlavorsConfig.Flavor(new FlavorsConfig.Flavor.Builder().
+                name(name).minMainMemoryAvailableGb(memoryGb).minDiskAvailableGb(diskGb)));
+    }
+
+    private static ProtonConfig getProtonConfig(VespaModel model, String configId) {
+        ProtonConfig.Builder builder = new ProtonConfig.Builder();
+        model.getConfig(builder, configId);
+        return new ProtonConfig(builder);
+    }
+
 }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/search/MultilevelDispatchTest.java b/config-model/src/test/java/com/yahoo/vespa/model/search/MultilevelDispatchTest.java
index 3c6b8fdcc10..ffab1d2af28 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/search/MultilevelDispatchTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/search/MultilevelDispatchTest.java
@@ -290,7 +290,7 @@ public class MultilevelDispatchTest {
         List<SearchNode> searchNodes = new ArrayList<>();
         MockRoot root = new MockRoot("");
         for (int i = 0; i < numNodes; ++i) {
-            searchNodes.add(SearchNode.create(root, "mynode" + i, i, new NodeSpec(0, i), "mycluster", null, false));
+            searchNodes.add(SearchNode.create(root, "mynode" + i, i, new NodeSpec(0, i), "mycluster", null, false, Optional.empty()));
         }
         return searchNodes;
     }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/search/test/SearchNodeTest.java b/config-model/src/test/java/com/yahoo/vespa/model/search/test/SearchNodeTest.java
index 964d0bbe4e9..564ac2f2cf7 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/search/test/SearchNodeTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/search/test/SearchNodeTest.java
@@ -1,6 +1,7 @@
 // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.model.search.test;
 
+import com.yahoo.config.model.producer.AbstractConfigProducer;
 import com.yahoo.config.model.test.MockRoot;
 import com.yahoo.vespa.config.search.core.ProtonConfig;
 import com.yahoo.vespa.defaults.Defaults;
@@ -13,6 +14,8 @@ import org.hamcrest.CoreMatchers;
 import org.junit.Assert;
 import org.junit.Test;
 
+import java.util.Optional;
+
 import static junit.framework.TestCase.assertEquals;
 import static junit.framework.TestCase.assertFalse;
 import static junit.framework.TestCase.assertTrue;
@@ -45,10 +48,14 @@ public class SearchNodeTest {
         root.freezeModelTopology();
     }
 
+    private static SearchNode createSearchNode(AbstractConfigProducer parent, String name, int distributionKey, NodeSpec nodeSpec, boolean flushOnShutDown) {
+        return SearchNode.create(parent, name, distributionKey, nodeSpec, "mycluster", null, flushOnShutDown, Optional.empty());
+    }
+
     @Test
     public void requireThatBasedirIsCorrectForElasticMode() {
         MockRoot root = new MockRoot("");
-        SearchNode node = SearchNode.create(root, "mynode", 3, new NodeSpec(7, 5), "mycluster", null, false);
+        SearchNode node = createSearchNode(root, "mynode", 3, new NodeSpec(7, 5), false);
         prepare(root, node);
         assertBaseDir(Defaults.getDefaults().vespaHome() + "var/db/vespa/search/cluster.mycluster/n3", node);
     }
@@ -56,7 +63,7 @@ public class SearchNodeTest {
     @Test
     public void requireThatPreShutdownCommandIsEmptyWhenNotActivated() {
         MockRoot root = new MockRoot("");
-        SearchNode node = SearchNode.create(root, "mynode", 3, new NodeSpec(7, 5), "mycluster", null, false);
+        SearchNode node = createSearchNode(root, "mynode", 3, new NodeSpec(7, 5), false);
         node.setHostResource(new HostResource(new Host(node, "mynbode")));
         node.initService();
         assertFalse(node.getPreShutdownCommand().isPresent());
@@ -65,7 +72,7 @@ public class SearchNodeTest {
     @Test
     public void requireThatPreShutdownCommandUsesPrepareRestartWhenActivated() {
         MockRoot root = new MockRoot("");
-        SearchNode node = SearchNode.create(root, "mynode2", 4, new NodeSpec(7, 5), "mycluster", null, true);
+        SearchNode node = createSearchNode(root, "mynode2", 4, new NodeSpec(7, 5), true);
         node.setHostResource(new HostResource(new Host(node, "mynbode2")));
         node.initService();
         assertTrue(node.getPreShutdownCommand().isPresent());