From 1593db489f48dfb6b15043aa1617b43a73d70b36 Mon Sep 17 00:00:00 2001 From: Håkon Hallingstad Date: Thu, 19 Sep 2019 17:13:19 +0200 Subject: Revert "Bratseth/vip logic" --- .../com/yahoo/search/cluster/BaseNodeMonitor.java | 12 +- .../com/yahoo/search/cluster/ClusterMonitor.java | 4 +- .../java/com/yahoo/search/cluster/NodeManager.java | 3 - .../java/com/yahoo/search/dispatch/Dispatcher.java | 5 +- .../yahoo/search/dispatch/searchcluster/Group.java | 9 +- .../yahoo/search/dispatch/searchcluster/Node.java | 12 +- .../search/dispatch/searchcluster/PingFactory.java | 2 - .../dispatch/searchcluster/SearchCluster.java | 136 +++++++++------------ .../com/yahoo/search/dispatch/DispatcherTest.java | 2 +- .../yahoo/search/dispatch/MockSearchCluster.java | 2 +- 10 files changed, 73 insertions(+), 114 deletions(-) (limited to 'container-search') diff --git a/container-search/src/main/java/com/yahoo/search/cluster/BaseNodeMonitor.java b/container-search/src/main/java/com/yahoo/search/cluster/BaseNodeMonitor.java index d21ef35bcc2..59674d25402 100644 --- a/container-search/src/main/java/com/yahoo/search/cluster/BaseNodeMonitor.java +++ b/container-search/src/main/java/com/yahoo/search/cluster/BaseNodeMonitor.java @@ -26,25 +26,25 @@ public abstract class BaseNodeMonitor { /** The object representing the monitored node */ protected T node; - protected boolean isWorking = true; + protected boolean isWorking=true; /** Whether this node is quarantined for unstability */ - protected boolean isQuarantined = false; + protected boolean isQuarantined=false; /** The last time this node failed, in ms */ - protected long failedAt = 0; + protected long failedAt=0; /** The last time this node responded (failed or succeeded), in ms */ - protected long respondedAt = 0; + protected long respondedAt=0; /** The last time this node responded successfully */ - protected long succeededAt = 0; + protected long succeededAt=0; /** The configuration of this monitor */ protected MonitorConfiguration configuration; /** Is the node we monitor part of an internal Vespa cluster or not */ - private boolean internal; + private boolean internal=false; public BaseNodeMonitor(boolean internal) { this.internal=internal; diff --git a/container-search/src/main/java/com/yahoo/search/cluster/ClusterMonitor.java b/container-search/src/main/java/com/yahoo/search/cluster/ClusterMonitor.java index b79f6f49c19..ac0c8375f04 100644 --- a/container-search/src/main/java/com/yahoo/search/cluster/ClusterMonitor.java +++ b/container-search/src/main/java/com/yahoo/search/cluster/ClusterMonitor.java @@ -24,7 +24,7 @@ public class ClusterMonitor { private MonitorConfiguration configuration = new MonitorConfiguration(); - private static Logger log = Logger.getLogger(ClusterMonitor.class.getName()); + private static Logger log=Logger.getLogger(ClusterMonitor.class.getName()); private NodeManager nodeManager; @@ -69,7 +69,6 @@ public class ClusterMonitor { /** Called from ClusterSearcher/NodeManager when a node failed */ public synchronized void failed(T node, ErrorMessage error) { - nodeManager.statusIsKnown(node); BaseNodeMonitor monitor = nodeMonitors.get(node); boolean wasWorking = monitor.isWorking(); monitor.failed(error); @@ -80,7 +79,6 @@ public class ClusterMonitor { /** Called when a node responded */ public synchronized void responded(T node) { - nodeManager.statusIsKnown(node); BaseNodeMonitor monitor = nodeMonitors.get(node); boolean wasFailing =! monitor.isWorking(); monitor.responded(); diff --git a/container-search/src/main/java/com/yahoo/search/cluster/NodeManager.java b/container-search/src/main/java/com/yahoo/search/cluster/NodeManager.java index ef10680a4ae..9b20139e3c5 100644 --- a/container-search/src/main/java/com/yahoo/search/cluster/NodeManager.java +++ b/container-search/src/main/java/com/yahoo/search/cluster/NodeManager.java @@ -11,9 +11,6 @@ import java.util.concurrent.Executor; */ public interface NodeManager { - /** Called when we gain evidence about whether or not a node is working */ - default void statusIsKnown(T node) { } - /** Called when a failed node is working (ready for production) again */ void working(T node); diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java b/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java index af34fc3e106..58f73ea52cc 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java @@ -139,7 +139,7 @@ public class Dispatcher extends AbstractComponent { return invoker; } - /** Builds an invoker based on searchpath */ + // build invoker based on searchpath private Optional getSearchPathInvoker(Query query, VespaBackEndSearcher searcher) { String searchPath = query.getModel().getSearchPath(); if (searchPath == null) return Optional.empty(); @@ -156,7 +156,7 @@ public class Dispatcher extends AbstractComponent { } private Optional getInternalInvoker(Query query, VespaBackEndSearcher searcher) { - Optional directNode = searchCluster.localCorpusDispatchTarget(); + Optional directNode = searchCluster.directDispatchTarget(); if (directNode.isPresent()) { Node node = directNode.get(); query.trace(false, 2, "Dispatching directly to ", node); @@ -202,5 +202,4 @@ public class Dispatcher extends AbstractComponent { metric.add(INTERNAL_METRIC, 1, metricContext); } } - } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Group.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Group.java index 93cbd77cef7..bafc72b9b43 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Group.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Group.java @@ -77,11 +77,6 @@ public class Group { return this.activeDocuments.get(); } - public boolean isFullCoverageStatusChanged(boolean hasFullCoverageNow) { - boolean previousState = hasFullCoverage.getAndSet(hasFullCoverageNow); - return previousState != hasFullCoverageNow; - } - @Override public String toString() { return "search group " + id; } @@ -95,4 +90,8 @@ public class Group { return ((Group) other).id == this.id; } + public boolean isFullCoverageStatusChanged(boolean hasFullCoverageNow) { + boolean previousState = hasFullCoverage.getAndSet(hasFullCoverageNow); + return previousState != hasFullCoverageNow; + } } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Node.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Node.java index 0dd933fc24e..a71ce0354f9 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Node.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Node.java @@ -19,7 +19,6 @@ public class Node { private final int fs4port; final int group; - private final AtomicBoolean statusIsKnown = new AtomicBoolean(false); private final AtomicBoolean working = new AtomicBoolean(true); private final AtomicLong activeDocuments = new AtomicLong(0); @@ -46,13 +45,9 @@ public class Node { /** Returns the id of this group this node belongs to */ public int group() { return group; } - /** Note that we know the status of this node */ - public void setStatusIsKnown() { statusIsKnown.lazySet(true); } - - /** Returns whether we know the status of this node */ - public boolean getStatusIsKnown() { return statusIsKnown.get(); } - - public void setWorking(boolean working) { this.working.lazySet(working); } + public void setWorking(boolean working) { + this.working.lazySet(working); + } /** Returns whether this node is currently responding to requests */ public boolean isWorking() { return working.get(); } @@ -82,5 +77,4 @@ public class Node { @Override public String toString() { return "search node " + hostname + ":" + fs4port + " in group " + group; } - } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/PingFactory.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/PingFactory.java index 285a1fcd57e..c9f722ef79b 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/PingFactory.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/PingFactory.java @@ -6,7 +6,5 @@ import com.yahoo.search.cluster.ClusterMonitor; import java.util.concurrent.Callable; public interface PingFactory { - Callable createPinger(Node node, ClusterMonitor monitor); - } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java index 2bcac6e2ce4..c091e211aca 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java @@ -55,7 +55,7 @@ public class SearchCluster implements NodeManager { * if it only queries this cluster when the local node cannot be used, to avoid unnecessary * cross-node network traffic. */ - private final Optional localCorpusDispatchTarget; + private final Optional directDispatchTarget; public SearchCluster(String clusterId, DispatchConfig dispatchConfig, int containerClusterSize, VipStatus vipStatus) { this.clusterId = clusterId; @@ -82,11 +82,8 @@ public class SearchCluster implements NodeManager { nodesByHostBuilder.put(node.hostname(), node); this.nodesByHost = nodesByHostBuilder.build(); - this.localCorpusDispatchTarget = findLocalCorpusDispatchTarget(HostName.getLocalhost(), - size, - containerClusterSize, - nodesByHost, - groups); + this.directDispatchTarget = findDirectDispatchTarget(HostName.getLocalhost(), size, containerClusterSize, + nodesByHost, groups); this.clusterMonitor = new ClusterMonitor<>(this); } @@ -105,11 +102,11 @@ public class SearchCluster implements NodeManager { } } - private static Optional findLocalCorpusDispatchTarget(String selfHostname, - int searchClusterSize, - int containerClusterSize, - ImmutableMultimap nodesByHost, - ImmutableMap groups) { + private static Optional findDirectDispatchTarget(String selfHostname, + int searchClusterSize, + int containerClusterSize, + ImmutableMultimap nodesByHost, + ImmutableMap groups) { // A search node in the search cluster in question is configured on the same host as the currently running container. // It has all the data <==> No other nodes in the search cluster have the same group id as this node. // That local search node responds. @@ -189,101 +186,70 @@ public class SearchCluster implements NodeManager { } /** - * Returns the single, local node we should dispatch queries directly to, + * Returns the recipient we should dispatch queries directly to (bypassing fdispatch), * or empty if we should not dispatch directly. */ - public Optional localCorpusDispatchTarget() { - if ( localCorpusDispatchTarget.isEmpty()) return Optional.empty(); + public Optional directDispatchTarget() { + if ( directDispatchTarget.isEmpty()) return Optional.empty(); // Only use direct dispatch if the local group has sufficient coverage - Group localSearchGroup = groups.get(localCorpusDispatchTarget.get().group()); + Group localSearchGroup = groups.get(directDispatchTarget.get().group()); if ( ! localSearchGroup.hasSufficientCoverage()) return Optional.empty(); // Only use direct dispatch if the local search node is up - if ( ! localCorpusDispatchTarget.get().isWorking()) return Optional.empty(); + if ( ! directDispatchTarget.get().isWorking()) return Optional.empty(); - return localCorpusDispatchTarget; + return directDispatchTarget; } - /** Called by the cluster monitor whenever we get information (positive or negative) about a node */ - @Override - public void statusIsKnown(Node node) { - node.setStatusIsKnown(); - } - - /** Called by the cluster monitor when node state changes to working */ + /** Used by the cluster monitor to manage node status */ @Override public void working(Node node) { node.setWorking(true); - updateVipStatusOnNodeChange(node, true); + + if (usesDirectDispatchTo(node)) + vipStatus.addToRotation(clusterId); } - /** Called by the cluster monitor when node state changes to failed */ + /** Used by the cluster monitor to manage node status */ @Override public void failed(Node node) { node.setWorking(false); - updateVipStatusOnNodeChange(node, true); - } - private void updateSufficientCoverage(Group group, boolean sufficientCoverage) { - if (sufficientCoverage == group.hasSufficientCoverage()) return; // no change - - group.setHasSufficientCoverage(sufficientCoverage); - updateVipStatusOnCoverageChange(group, sufficientCoverage); - } - - private void updateVipStatusOnNodeChange(Node node, boolean working) { - if (usesLocalCorpusIn(node)) { // follow the status of the local corpus - if (working) - vipStatus.addToRotation(clusterId); - else - vipStatus.removeFromRotation(clusterId); - } - else { - if ( ! hasInformationAboutAllNodes()) return; - - if (hasWorkingNodes()) - vipStatus.addToRotation(clusterId); - else - vipStatus.removeFromRotation(clusterId); - } + // Take ourselves out if we usually dispatch only to our own host + if (usesDirectDispatchTo(node)) + vipStatus.removeFromRotation(clusterId); } - private void updateVipStatusOnCoverageChange(Group group, boolean sufficientCoverage) { + private void updateSufficientCoverage(Group group, boolean sufficientCoverage) { + // update VIP status if we direct dispatch to this group and coverage status changed boolean isInRotation = vipStatus.isInRotation(); - if (usesLocalCorpusIn(group)) { // follow the status of the local corpus - if (sufficientCoverage) - vipStatus.addToRotation(clusterId); - else - vipStatus.removeFromRotation(clusterId); - } - else { - if ( ! isInRotation && sufficientCoverage) - vipStatus.addToRotation(clusterId); + boolean hasChanged = sufficientCoverage != group.hasSufficientCoverage(); + boolean isDirectDispatchGroupAndChange = usesDirectDispatchTo(group) && hasChanged; + group.setHasSufficientCoverage(sufficientCoverage); + if ((!isInRotation || isDirectDispatchGroupAndChange) && sufficientCoverage) { + // We will set this cluster in rotation if + // - not already in rotation and one group has sufficient coverage. + vipStatus.addToRotation(clusterId); + } else if (isDirectDispatchGroupAndChange) { + // We will take it out of rotation if the group is mandatory (direct dispatch to this group) + vipStatus.removeFromRotation(clusterId); } } - private boolean hasInformationAboutAllNodes() { - return nodesByHost.values().stream().allMatch(Node::getStatusIsKnown); - } - - private boolean hasWorkingNodes() { - return nodesByHost.values().stream().anyMatch(Node::isWorking); - } - - private boolean usesLocalCorpusIn(Node node) { - return localCorpusDispatchTarget.isPresent() && localCorpusDispatchTarget.get().equals(node); + private boolean usesDirectDispatchTo(Node node) { + return directDispatchTarget.isPresent() && directDispatchTarget.get().equals(node); } - private boolean usesLocalCorpusIn(Group group) { - return localCorpusDispatchTarget.isPresent() && localCorpusDispatchTarget.get().group() == group.id(); + private boolean usesDirectDispatchTo(Group group) { + return directDispatchTarget.isPresent() && directDispatchTarget.get().group() == group.id(); } /** Used by the cluster monitor to manage node status */ @Override public void ping(Node node, Executor executor) { - if (pingFactory == null) return; // not initialized yet - + if (pingFactory == null) // not initialized yet + return; FutureTask futurePong = new FutureTask<>(pingFactory.createPinger(node, clusterMonitor)); executor.execute(futurePong); Pong pong = getPong(futurePong, node); @@ -306,10 +272,9 @@ public class SearchCluster implements NodeManager { // group will always be marked sufficient for use. updateSufficientCoverage(group, true); boolean fullCoverage = isGroupCoverageSufficient(group.workingNodes(), group.nodes().size(), group.getActiveDocuments(), - group.getActiveDocuments()); + group.getActiveDocuments()); trackGroupCoverageChanges(0, group, fullCoverage, group.getActiveDocuments()); } - private void pingIterationCompletedMultipleGroups() { int numGroups = orderedGroups.size(); // Update active documents per group and use it to decide if the group should be active @@ -334,7 +299,13 @@ public class SearchCluster implements NodeManager { trackGroupCoverageChanges(i, group, sufficientCoverage, averageDocumentsInOtherGroups); } } - + private boolean areAllNodesDownInAllgroups() { + for(int i = 0; i < groups.size(); i++) { + Group group = orderedGroups.get(i); + if (group.workingNodes() > 0) return false; + } + return true; + } /** * Update statistics after a round of issuing pings. * Note that this doesn't wait for pings to return, so it will typically accumulate data from @@ -348,6 +319,9 @@ public class SearchCluster implements NodeManager { } else { pingIterationCompletedMultipleGroups(); } + if ( areAllNodesDownInAllgroups() ) { + vipStatus.removeFromRotation(clusterId); + } } private boolean isGroupCoverageSufficient(int workingNodes, int nodesInGroup, long activeDocuments, long averageDocumentsInOtherGroups) { @@ -423,11 +397,11 @@ public class SearchCluster implements NodeManager { if (changed) { int requiredNodes = groupSize() - dispatchConfig.maxNodesDownPerGroup(); if (fullCoverage) { - log.info(() -> String.format("Group %d is now good again (%d/%d active docs, coverage %d/%d)", - index, group.getActiveDocuments(), averageDocuments, group.workingNodes(), groupSize())); + log.info(() -> String.format("Group %d is now good again (%d/%d active docs, coverage %d/%d)", index, + group.getActiveDocuments(), averageDocuments, group.workingNodes(), groupSize())); } else { - log.warning(() -> String.format("Coverage of group %d is only %d/%d (requires %d)", - index, group.workingNodes(), groupSize(), requiredNodes)); + log.warning(() -> String.format("Coverage of group %d is only %d/%d (requires %d)", index, group.workingNodes(), groupSize(), + requiredNodes)); } } } diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/DispatcherTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/DispatcherTest.java index 310f536f961..03a417c9bbb 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/DispatcherTest.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/DispatcherTest.java @@ -69,7 +69,7 @@ public class DispatcherTest { public void requireThatDispatcherSupportsSingleNodeDirectDispatch() { SearchCluster cl = new MockSearchCluster("1", 0, 0) { @Override - public Optional localCorpusDispatchTarget() { + public Optional directDispatchTarget() { return Optional.of(new Node(1, "test", 123, 1)); } }; diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java b/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java index f4bfa766328..7ee62ae9978 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java @@ -89,7 +89,7 @@ public class MockSearchCluster extends SearchCluster { } @Override - public Optional localCorpusDispatchTarget() { + public Optional directDispatchTarget() { return Optional.empty(); } -- cgit v1.2.3