From d42bb9567c5ab0e2f81eb6ed6d4f3477238b95c6 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Mon, 4 Jul 2016 10:54:38 +0200 Subject: Revert "Revert "Bratseth/only one clustercontroller per host 2"" This reverts commit f9b197a293c63e824cf1062b5642b67a3f00c16e. --- .../yahoo/application/container/Processing.java | 1 + .../com/yahoo/vespa/model/AbstractService.java | 2 +- .../ClusterControllerCluster.java | 3 +- .../model/content/cluster/ContentCluster.java | 35 ++++++++++++-- .../model/provision/ModelProvisioningTest.java | 56 ++++++++++++++++++++-- .../processing/handler/ProcessingResponse.java | 16 +++---- .../rendering/AsynchronousSectionedRenderer.java | 8 ++-- 7 files changed, 95 insertions(+), 26 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 1511024364d..085067f707c 100644 --- a/application/src/main/java/com/yahoo/application/container/Processing.java +++ b/application/src/main/java/com/yahoo/application/container/Processing.java @@ -22,6 +22,7 @@ import java.io.IOException; */ @Beta public final class Processing extends ProcessingBase { + 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 13d85c3a955..187a6222648 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 controllerHosts = new ArrayList<>(); + Collection controllerHosts = new ArrayList<>(); for (Container container : containerCluster.getContainers()) { controllerHosts.add(container.getHostName() + ":" + ZK_CLIENT_PORT); } @@ -73,5 +73,6 @@ public class ClusterControllerCluster extends AbstractConfigProducer drawControllerHosts(int count, StorageGroup rootGroup, Collection containers) { List hosts = drawContentHostsRecursively(count, rootGroup); - //if (hosts.size() < count) // supply with containers TODO: Reactivate - // hosts.addAll(drawContainerHosts(count - hosts.size(), containers)); + if (hosts.size() < count) // supply with containers + 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; @@ -336,10 +339,15 @@ public class ContentCluster extends AbstractConfigProducer implements StorDistri private List drawContainerHosts(int count, Collection containerClusters) { if (containerClusters.isEmpty()) return Collections.emptyList(); - List hosts = new ArrayList<>(); + List allHosts = new ArrayList<>(); for (ContainerCluster cluster : clustersSortedByName(containerClusters)) - hosts.addAll(hostResourcesSortedByIndex(cluster)); - return hosts.subList(0, Math.min(hosts.size(), count)); + allHosts.addAll(hostResourcesSortedByIndex(cluster)); + + // Don't use the same container to supplement multiple content clusters + List hostsWithoutClusterController = + allHosts.stream().filter(h -> ! hostHasClusterController(h.getHostName(), allHosts)).collect(Collectors.toList()); + + return hostsWithoutClusterController.subList(0, Math.min(hostsWithoutClusterController.size(), count)); } private List clustersSortedByName(Collection containerModels) { @@ -356,6 +364,23 @@ 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 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 count 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 e4f405efb93..1c7940fdc8f 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(1, clusterControllers.getContainers().size()); // TODO: Expected 5 with this feature reactivated + assertEquals(5, clusterControllers.getContainers().size()); } public void testClusterControllersAreNotPlacedOnRetiredNodes() { @@ -619,17 +619,63 @@ public class ModelProvisioningTest { " " + ""; - int numberOfHosts = 5; VespaModelTester tester = new VespaModelTester(); - tester.addHosts(numberOfHosts); + tester.addHosts(5); 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(1, clusterControllers.getContainers().size()); // TODO: Expected 3 with this feature reactivated + assertEquals(3, clusterControllers.getContainers().size()); } + @Test + public void test2ContentNodesOn2ClustersWithContainerClusterProducesMixedClusterControllerCluster() throws ParseException { + String services = + "\n" + + "" + + " " + + " " + + " " + + " " + + " 2" + + " " + + " " + + " " + + " " + + " " + + " " + + " 2" + + " " + + " " + + " " + + " " + + " " + + ""; + + 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 = "\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 2ce345ad355..d9bbd30a474 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,7 +38,6 @@ 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; @@ -46,24 +45,21 @@ public class ProcessingResponse extends AsyncHttpResponse { private boolean explicitStatusSet = false; @SuppressWarnings("unchecked") - 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) { + public ProcessingResponse(int status, com.yahoo.processing.Request processingRequest, + com.yahoo.processing.Response processingResponse, + Renderer renderer, + Executor renderingExecutor, 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(final OutputStream stream, final ContentChannel channel, - final CompletionHandler completionHandler) throws IOException { + public void render(OutputStream stream, ContentChannel channel, + 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 cc186ea21ee..eeb4a2ef36d 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 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 - // calling init() and then render(). + // NOTE: Renderers are *prototype objects* - a new instance is created for each rendering by invoking + // clone(), 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 e // This MUST be created in the init() method - see comment above private Object singleThreaded; - // Rendering threads should never block. - // Burst traffic may add work faster than we can complete it, so use an unbounded queue. + // 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. // The executor SHOULD be reused across all instances having the same prototype private final ThreadPoolExecutor renderingExecutor = createExecutor(); private static ThreadPoolExecutor createExecutor() { -- cgit v1.2.3