Merge pull request #286 from yahoo/revert-283-bratseth/only-one-clustercontroller-per-host-2

Revert "Bratseth/only one clustercontroller per host 2"

7 files changed, 26 insertions, 95 deletions

diff --git a/application/src/main/java/com/yahoo/application/container/Processing.java b/application/src/main/java/com/yahoo/application/container/Processing.java
index 085067f707c..1511024364d 100644
--- a/application/src/main/java/com/yahoo/application/container/Processing.java
+++ b/application/src/main/java/com/yahoo/application/container/Processing.java
@@ -22,7 +22,6 @@ import java.io.IOException;
 */
 @Beta
 public final class Processing extends ProcessingBase<Request, Response, Processor> {
-
     private final ProcessingHandler handler;
 
     Processing(ProcessingHandler handler) {
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java b/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java
index 187a6222648..13d85c3a955 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java
@@ -114,7 +114,7 @@ public abstract class AbstractService extends AbstractConfigProducer<AbstractCon
         }
         if (hostResource == null) {
             throw new RuntimeException("No host found for service '" + getServiceName() + "'. " +
-                                       "The hostalias is probably missing from hosts.xml.");
+                    "The hostalias is probably missing from hosts.xml.");
         }
         id = getIndex(hostResource);
         ports = hostResource.allocateService(this, getInstanceWantedPort(userPort));
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerCluster.java
index 3847c32dfb2..b82f58fbb40 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerCluster.java
@@ -43,7 +43,7 @@ public class ClusterControllerCluster extends AbstractConfigProducer<ContainerCl
 
     @Override
     public void getConfig(ZookeepersConfig.Builder builder) {
-        Collection<String> controllerHosts = new ArrayList<>();
+        final Collection<String> controllerHosts = new ArrayList<>();
         for (Container container : containerCluster.getContainers()) {
             controllerHosts.add(container.getHostName() + ":" + ZK_CLIENT_PORT);
         }
@@ -73,6 +73,5 @@ public class ClusterControllerCluster extends AbstractConfigProducer<ContainerCl
             }
         }
     }
-
 }
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java
index 669320165cc..8350ac3fc79 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java
@@ -5,7 +5,6 @@ import com.google.common.base.Preconditions;
 import com.google.common.collect.Sets;
 import com.yahoo.config.model.ConfigModelUtils;
 import com.yahoo.config.model.producer.AbstractConfigProducerRoot;
-import com.yahoo.config.model.provision.Host;
 import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.vespa.config.content.MessagetyperouteselectorpolicyConfig;
@@ -17,9 +16,7 @@ import com.yahoo.documentapi.messagebus.protocol.DocumentProtocol;
 import com.yahoo.metrics.MetricsmanagerConfig;
 import com.yahoo.config.model.producer.AbstractConfigProducer;
 import com.yahoo.vespa.model.HostResource;
-import com.yahoo.vespa.model.Service;
 import com.yahoo.vespa.model.admin.Admin;
-import com.yahoo.vespa.model.admin.Configserver;
 import com.yahoo.vespa.model.admin.Metric;
 import com.yahoo.vespa.model.admin.MetricsConsumer;
 import com.yahoo.vespa.model.admin.MonitoringSystem;
@@ -322,8 +319,8 @@ public class ContentCluster extends AbstractConfigProducer implements StorDistri
 
         private List<HostResource> drawControllerHosts(int count, StorageGroup rootGroup, Collection<ContainerModel> containers) {
             List<HostResource> hosts = drawContentHostsRecursively(count, rootGroup);
-            if (hosts.size() < count) // supply with containers
-                hosts.addAll(drawContainerHosts(count - hosts.size(), containers));
+            //if (hosts.size() < count) // supply with containers TODO: Reactivate
+            //    hosts.addAll(drawContainerHosts(count - hosts.size(), containers));
             if (hosts.size() % 2 == 0) // ZK clusters of even sizes are less available (even in the size=2 case)
                 hosts = hosts.subList(0, hosts.size()-1);
             return hosts;
@@ -339,15 +336,10 @@ public class ContentCluster extends AbstractConfigProducer implements StorDistri
         private List<HostResource> drawContainerHosts(int count, Collection<ContainerModel> containerClusters) {
             if (containerClusters.isEmpty()) return Collections.emptyList();
 
-            List<HostResource> allHosts = new ArrayList<>();
+            List<HostResource> hosts = new ArrayList<>();
             for (ContainerCluster cluster : clustersSortedByName(containerClusters))
-                allHosts.addAll(hostResourcesSortedByIndex(cluster));
-
-            // Don't use the same container to supplement multiple content clusters
-            List<HostResource> hostsWithoutClusterController =
-                    allHosts.stream().filter(h -> ! hostHasClusterController(h.getHostName(), allHosts)).collect(Collectors.toList());
-
-            return hostsWithoutClusterController.subList(0, Math.min(hostsWithoutClusterController.size(), count));
+                hosts.addAll(hostResourcesSortedByIndex(cluster));
+            return hosts.subList(0, Math.min(hosts.size(), count));
         }
         
         private List<ContainerCluster> clustersSortedByName(Collection<ContainerModel> containerModels) {
@@ -364,23 +356,6 @@ public class ContentCluster extends AbstractConfigProducer implements StorDistri
                     .collect(Collectors.toList());
         }
 
-        /** Returns whether any host having the given hostname has a cluster controller */
-        private boolean hostHasClusterController(String hostname, List<HostResource> allHosts) {
-            for (HostResource host : allHosts) {
-                if ( ! host.getHostName().equals(hostname)) continue;
-                if (! hasClusterController(host)) continue;
-                return true;
-            }
-            return false;
-        }
-
-        private boolean hasClusterController(HostResource host) {
-            for (Service service : host.getServices())
-                if (service instanceof ClusterControllerContainer)
-                    return true;
-            return false;
-        }
-
         /**
          * Draw <code>count</code> nodes from as many different content groups below this as possible.
          * This will only achieve maximum spread in the case where the groups are balanced and never on the same
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 1c7940fdc8f..e4f405efb93 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
@@ -458,7 +458,7 @@ public class ModelProvisioningTest {
 
         ContentCluster cluster = model.getContentClusters().get("bar");
         ContainerCluster clusterControllers = cluster.getClusterControllers();
-        assertEquals(5, clusterControllers.getContainers().size());
+        assertEquals(1, clusterControllers.getContainers().size()); // TODO: Expected 5 with this feature reactivated
     }
 
     public void testClusterControllersAreNotPlacedOnRetiredNodes() {
@@ -619,63 +619,17 @@ public class ModelProvisioningTest {
                 "  </content>" +
                 "</services>";
 
+        int numberOfHosts = 5;
         VespaModelTester tester = new VespaModelTester();
-        tester.addHosts(5);
+        tester.addHosts(numberOfHosts);
         VespaModel model = tester.createModel(services, true);
+        assertThat(model.getRoot().getHostSystem().getHosts().size(), is(numberOfHosts));
 
         ContentCluster cluster = model.getContentClusters().get("bar");
         ContainerCluster clusterControllers = cluster.getClusterControllers();
-        assertEquals(3, clusterControllers.getContainers().size());
+        assertEquals(1, clusterControllers.getContainers().size()); // TODO: Expected 3 with this feature reactivated
     }
 
-    @Test
-    public void test2ContentNodesOn2ClustersWithContainerClusterProducesMixedClusterControllerCluster() throws ParseException {
-        String services =
-                "<?xml version='1.0' encoding='utf-8' ?>\n" +
-                "<services>" +
-                "  <container version='1.0' id='container'>" +
-                "     <nodes count='3' flavor='container-node'/>" +
-                "  </container>" +
-                "  <content version='1.0' id='content1'>" +
-                "     <redundancy>2</redundancy>" +
-                "     <documents>" +
-                "       <document type='type1' mode='index'/>" +
-                "     </documents>" +
-                "     <nodes count='2' flavor='content1-node'/>" +
-                "  </content>" +
-                "  <content version='1.0' id='content2'>" +
-                "     <redundancy>2</redundancy>" +
-                "     <documents>" +
-                "       <document type='type1' mode='index'/>" +
-                "     </documents>" +
-                "     <nodes count='2' flavor='content2-node'/>" +
-                "  </content>" +
-                "</services>";
-
-        VespaModelTester tester = new VespaModelTester();
-        // use different flavors to make the test clearer
-        tester.addHosts("container-node", 3);
-        tester.addHosts("content1-node",  2);
-        tester.addHosts("content2-node",  2);
-        VespaModel model = tester.createModel(services, true);
-
-        ContentCluster cluster1 = model.getContentClusters().get("content1");
-        ContainerCluster clusterControllers1 = cluster1.getClusterControllers();
-        assertEquals(3, clusterControllers1.getContainers().size());
-        assertEquals("content1-node0",  clusterControllers1.getContainers().get(0).getHostName());
-        assertEquals("content1-node1",  clusterControllers1.getContainers().get(1).getHostName());
-        assertEquals("container-node0", clusterControllers1.getContainers().get(2).getHostName());
-
-        ContentCluster cluster2 = model.getContentClusters().get("content2");
-        ContainerCluster clusterControllers2 = cluster2.getClusterControllers();
-        assertEquals(3, clusterControllers2.getContainers().size());
-        assertEquals("content2-node0",  clusterControllers2.getContainers().get(0).getHostName());
-        assertEquals("content2-node1",  clusterControllers2.getContainers().get(1).getHostName());
-        assertEquals("We do not pick the container used to supplement another cluster",
-                     "container-node1", clusterControllers2.getContainers().get(2).getHostName());
-    }
-
-    @Test
     public void testExplicitDedicatedClusterControllers() {
         String services =
                 "<?xml version='1.0' encoding='utf-8' ?>\n" +
diff --git a/container-core/src/main/java/com/yahoo/processing/handler/ProcessingResponse.java b/container-core/src/main/java/com/yahoo/processing/handler/ProcessingResponse.java
index d9bbd30a474..2ce345ad355 100644
--- a/container-core/src/main/java/com/yahoo/processing/handler/ProcessingResponse.java
+++ b/container-core/src/main/java/com/yahoo/processing/handler/ProcessingResponse.java
@@ -38,6 +38,7 @@ public class ProcessingResponse extends AsyncHttpResponse {
 
     private final com.yahoo.processing.Request processingRequest;
     private final com.yahoo.processing.Response processingResponse;
+    private final Executor renderingExecutor;
     private final Execution execution;
     private final Renderer renderer;
 
@@ -45,21 +46,24 @@ public class ProcessingResponse extends AsyncHttpResponse {
     private boolean explicitStatusSet = false;
 
     @SuppressWarnings("unchecked")
-    public ProcessingResponse(int status, com.yahoo.processing.Request processingRequest,
-                              com.yahoo.processing.Response processingResponse,
-                              Renderer renderer,
-                              Executor renderingExecutor, Execution execution) {
+    public ProcessingResponse(
+            int status,
+            final com.yahoo.processing.Request processingRequest,
+            final com.yahoo.processing.Response processingResponse,
+            final Renderer renderer,
+            final Executor renderingExecutor, final Execution execution) {
         super(status);
         this.processingRequest = processingRequest;
         this.processingResponse = processingResponse;
+        this.renderingExecutor = renderingExecutor;
         this.execution = execution;
         this.renderer = renderer;
     }
 
     @SuppressWarnings("unchecked")
     @Override
-    public void render(OutputStream stream, ContentChannel channel, 
-                       CompletionHandler completionHandler) throws IOException {
+    public void render(final OutputStream stream, final ContentChannel channel,
+                       final CompletionHandler completionHandler) throws IOException {
         if (renderer instanceof AsynchronousRenderer) {
             AsynchronousRenderer asyncRenderer = (AsynchronousRenderer)renderer;
             asyncRenderer.setNetworkWiring(channel, completionHandler);
diff --git a/container-core/src/main/java/com/yahoo/processing/rendering/AsynchronousSectionedRenderer.java b/container-core/src/main/java/com/yahoo/processing/rendering/AsynchronousSectionedRenderer.java
index eeb4a2ef36d..cc186ea21ee 100644
--- a/container-core/src/main/java/com/yahoo/processing/rendering/AsynchronousSectionedRenderer.java
+++ b/container-core/src/main/java/com/yahoo/processing/rendering/AsynchronousSectionedRenderer.java
@@ -93,8 +93,8 @@ public abstract class AsynchronousSectionedRenderer<RESPONSE extends Response> e
 
     private static final Logger logger = Logger.getLogger(AsynchronousSectionedRenderer.class.getName());
 
-    // NOTE: Renderers are *prototype objects* - a new instance is created for each rendering by invoking 
-    // clone(), init() and then render().
+    // NOTE: Renderers are *prototype objects* - a new instance is created for each rendering by invoking clone
+    // calling init() and then render().
     // Hence any field which is not reinitialized in init() or render() will be *reused* in all rendering operations
     // across all threads!
 
@@ -110,8 +110,8 @@ public abstract class AsynchronousSectionedRenderer<RESPONSE extends Response> e
     // This MUST be created in the init() method - see comment above
     private Object singleThreaded;
 
-    // Rendering threads should never block so use one thread per core.
-    // We should complete any work we have already started so use an unbounded queue.
+    // Rendering threads should never block.
+    // Burst traffic may add work faster than we can complete it, so use an unbounded queue.
     // The executor SHOULD be reused across all instances having the same prototype
     private final ThreadPoolExecutor renderingExecutor = createExecutor();
     private static ThreadPoolExecutor createExecutor() {