diff options
56 files changed, 861 insertions, 275 deletions
diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorFlowTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorFlowTestCase.java index 38515c36690..cba931e81f0 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorFlowTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorFlowTestCase.java @@ -319,7 +319,7 @@ public class RankingExpressionWithTensorFlowTestCase { @Test public void testFunctionGeneration() { final String name = "mnist_saved"; - final String expression = "join(join(reduce(join(join(join(imported_ml_function_" + name + "_dnn_hidden2_add, reduce(rename(constant(" + name + "_dnn_hidden2_Const), d0, d2), sum, d2), f(a,b)(a * b)), imported_ml_function_" + name + "_dnn_hidden2_add, f(a,b)(max(a,b))), constant(" + name + "_dnn_outputs_weights_read), f(a,b)(a * b)), sum, d2), constant(" + name + "_dnn_outputs_bias_read), f(a,b)(a + b)), tensor(d0[1])(1.0), f(a,b)(a * b))"; + final String expression = "join(join(reduce(join(join(join(imported_ml_function_" + name + "_dnn_hidden2_add, reduce(constant(" + name + "_dnn_hidden2_Const), sum, d2), f(a,b)(a * b)), imported_ml_function_" + name + "_dnn_hidden2_add, f(a,b)(max(a,b))), constant(" + name + "_dnn_outputs_weights_read), f(a,b)(a * b)), sum, d2), constant(" + name + "_dnn_outputs_bias_read), f(a,b)(a + b)), tensor(d0[1])(1.0), f(a,b)(a * b))"; final String functionExpression1 = "join(reduce(join(reduce(rename(input, (d0, d1), (d0, d4)), sum, d0), constant(" + name + "_dnn_hidden1_weights_read), f(a,b)(a * b)), sum, d4), constant(" + name + "_dnn_hidden1_bias_read), f(a,b)(a + b))"; final String functionExpression2 = "join(reduce(join(join(join(imported_ml_function_" + name + "_dnn_hidden1_add, 0.009999999776482582, f(a,b)(a * b)), imported_ml_function_" + name + "_dnn_hidden1_add, f(a,b)(max(a,b))), constant(" + name + "_dnn_hidden2_weights_read), f(a,b)(a * b)), sum, d3), constant(" + name + "_dnn_hidden2_bias_read), f(a,b)(a + b))"; @@ -349,7 +349,7 @@ public class RankingExpressionWithTensorFlowTestCase { " rank-profile my_profile_child inherits my_profile {\n" + " }"; - final String expression = "join(join(reduce(join(join(join(imported_ml_function_" + name + "_dnn_hidden2_add, reduce(rename(constant(" + name + "_dnn_hidden2_Const), d0, d2), sum, d2), f(a,b)(a * b)), imported_ml_function_" + name + "_dnn_hidden2_add, f(a,b)(max(a,b))), constant(" + name + "_dnn_outputs_weights_read), f(a,b)(a * b)), sum, d2), constant(" + name + "_dnn_outputs_bias_read), f(a,b)(a + b)), tensor(d0[1])(1.0), f(a,b)(a * b))"; + final String expression = "join(join(reduce(join(join(join(imported_ml_function_" + name + "_dnn_hidden2_add, reduce(constant(" + name + "_dnn_hidden2_Const), sum, d2), f(a,b)(a * b)), imported_ml_function_" + name + "_dnn_hidden2_add, f(a,b)(max(a,b))), constant(" + name + "_dnn_outputs_weights_read), f(a,b)(a * b)), sum, d2), constant(" + name + "_dnn_outputs_bias_read), f(a,b)(a + b)), tensor(d0[1])(1.0), f(a,b)(a * b))"; final String functionExpression1 = "join(reduce(join(reduce(rename(input, (d0, d1), (d0, d4)), sum, d0), constant(" + name + "_dnn_hidden1_weights_read), f(a,b)(a * b)), sum, d4), constant(" + name + "_dnn_hidden1_bias_read), f(a,b)(a + b))"; final String functionExpression2 = "join(reduce(join(join(join(imported_ml_function_" + name + "_dnn_hidden1_add, 0.009999999776482582, f(a,b)(a * b)), imported_ml_function_" + name + "_dnn_hidden1_add, f(a,b)(max(a,b))), constant(" + name + "_dnn_hidden2_weights_read), f(a,b)(a * b)), sum, d3), constant(" + name + "_dnn_hidden2_bias_read), f(a,b)(a + b))"; diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizer.java index 15e52e48c3a..86fee1ab9bc 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizer.java @@ -127,7 +127,7 @@ public class MultiTenantRpcAuthorizer implements RpcAuthorizer { } throw new AuthorizationException( String.format( - "Peer is not allowed to access config for owned by %s. Peer is owned by %s", + "Peer is not allowed to access config owned by %s. Peer is owned by %s", resolvedApplication.toShortString(), peerOwner.toShortString())); } default: @@ -149,7 +149,9 @@ public class MultiTenantRpcAuthorizer implements RpcAuthorizer { if (filesOwnedByApplication.contains(requestedFile)) { return; // allowed to access } - throw new AuthorizationException(String.format("Peer is not allowed to access file %s. Peer is owned by %s", requestedFile.value(), peerOwner.toShortString())); + throw new AuthorizationException( + String.format("Peer is not allowed to access file reference %s. Peer is owned by %s. File references owned by this application: %s", + requestedFile.value(), peerOwner.toShortString(), filesOwnedByApplication)); default: throw new AuthorizationException(String.format("'%s' nodes are not allowed to access files", peerIdentity.nodeType())); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizerTest.java index a1d4f28cb74..9f5a297103d 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizerTest.java @@ -148,7 +148,7 @@ public class MultiTenantRpcAuthorizerTest { Request fileRequest = createFileRequest(new FileReference("other-file-reference")); - exceptionRule.expectMessage("Peer is not allowed to access file other-file-reference"); + exceptionRule.expectMessage("Peer is not allowed to access file reference other-file-reference. Peer is owned by mytenant.myapplication. File references owned by this application: [file 'myfilereference']"); exceptionRule.expectCause(instanceOf(AuthorizationException.class)); authorizer.authorizeFileRequest(fileRequest) @@ -168,7 +168,7 @@ public class MultiTenantRpcAuthorizerTest { Request configRequest = createConfigRequest(new ConfigKey<>("name", "configid", "namespace"), HOSTNAME); - exceptionRule.expectMessage("Peer is not allowed to access config for owned by mytenant.myapplication. Peer is owned by malice.malice-app"); + exceptionRule.expectMessage("Peer is not allowed to access config owned by mytenant.myapplication. Peer is owned by malice.malice-app"); exceptionRule.expectCause(instanceOf(AuthorizationException.class)); authorizer.authorizeConfigRequest(configRequest) diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java b/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java index e341b66e0f8..d7760ac9c91 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java @@ -13,6 +13,7 @@ import com.yahoo.jdisc.handler.ResponseDispatch; import com.yahoo.jdisc.handler.ResponseHandler; import com.yahoo.jdisc.http.HttpHeaders; import com.yahoo.metrics.MetricsPresentationConfig; +import org.json.JSONArray; import org.json.JSONException; import org.json.JSONObject; @@ -81,33 +82,47 @@ public class MetricsPacketsHandler extends AbstractRequestHandler { @Override protected Iterable<ByteBuffer> responseContent() { - return Collections.singleton(ByteBuffer.wrap(buildMetricOutput())); + return Collections.singleton(ByteBuffer.wrap(buildMetricOutput(request.getUri().getQuery()))); } }.dispatch(handler); return null; } - private byte[] buildMetricOutput() { + private byte[] buildMetricOutput(String query) { try { - String output = getStatusPacket() + getAllMetricsPackets() + "\n"; + if (query != null && query.equals("array-formatted")) { + return getMetricsArray(); + } + String output = jsonToString(getStatusPacket()) + getAllMetricsPackets() + "\n"; return output.getBytes(StandardCharsets.UTF_8); } catch (JSONException e) { throw new RuntimeException("Bad JSON construction.", e); } } + private byte[] getMetricsArray() throws JSONException { + JSONObject root = new JSONObject(); + JSONArray jsonArray = new JSONArray(); + jsonArray.put(getStatusPacket()); + getPacketsForSnapshot(getSnapshot(), applicationName, timer.currentTimeMillis()) + .forEach(jsonArray::put); + root.put("metrics", jsonArray); + return jsonToString(root) + .getBytes(StandardCharsets.UTF_8); + } + /** * Exactly one status packet is added to the response. */ - private String getStatusPacket() throws JSONException { + private JSONObject getStatusPacket() throws JSONException { JSONObject packet = new JSONObjectWithLegibleException(); packet.put(APPLICATION_KEY, applicationName); StateMonitor.Status status = monitor.status(); packet.put(STATUS_CODE_KEY, status.ordinal()); packet.put(STATUS_MSG_KEY, status.name()); - return jsonToString(packet); + return packet; } private String jsonToString(JSONObject jsonObject) throws JSONException { diff --git a/container-core/src/test/java/com/yahoo/container/jdisc/state/MetricsPacketsHandlerTest.java b/container-core/src/test/java/com/yahoo/container/jdisc/state/MetricsPacketsHandlerTest.java index 92330345b50..e933f042cec 100644 --- a/container-core/src/test/java/com/yahoo/container/jdisc/state/MetricsPacketsHandlerTest.java +++ b/container-core/src/test/java/com/yahoo/container/jdisc/state/MetricsPacketsHandlerTest.java @@ -122,6 +122,21 @@ public class MetricsPacketsHandlerTest extends StateHandlerTestBase { assertEquals(3, packets.size()); } + @Test + public void get_metrics_in_json_array() throws Exception { + metric.add("counter", 1, null); + incrementCurrentTimeAndAssertSnapshot(SNAPSHOT_INTERVAL); + String response = requestAsString("http://localhost/metrics-packets?array-formatted"); + List<JsonNode> responseJson = toJsonPackets(response); + assertEquals(1, responseJson.size()); + JsonNode metricsNode = responseJson.get(0).get(METRICS_KEY); + assertEquals(2, metricsNode.size()); + + JsonNode counterPacket = metricsNode.get(1); + assertCountMetric(counterPacket, "counter.count", 1); + + } + private List<JsonNode> incrementTimeAndGetJsonPackets() throws Exception { incrementCurrentTimeAndAssertSnapshot(SNAPSHOT_INTERVAL); String response = requestAsString("http://localhost/metrics-packets"); diff --git a/container-search/abi-spec.json b/container-search/abi-spec.json index fa2ea8cdbe1..facf894272d 100644 --- a/container-search/abi-spec.json +++ b/container-search/abi-spec.json @@ -2048,7 +2048,6 @@ "abstract" ], "methods": [ - "public void statusIsKnown(java.lang.Object)", "public abstract void working(java.lang.Object)", "public abstract void failed(java.lang.Object)", "public abstract void ping(java.lang.Object, java.util.concurrent.Executor)", @@ -2081,6 +2080,7 @@ "public java.lang.Object getNode()", "public void failed(com.yahoo.search.result.ErrorMessage)", "public void responded()", + "public java.lang.Boolean isKnownWorking()", "protected synchronized void setWorking(boolean, java.lang.String)" ], "fields": [] diff --git a/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterSearcher.java b/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterSearcher.java index b0d9c2b0002..37d8e316302 100644 --- a/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterSearcher.java @@ -86,8 +86,6 @@ public class ClusterSearcher extends Searcher { VipStatus vipStatus) { super(id); - Dispatcher dispatcher = Dispatcher.create(id.stringValue(), dispatchConfig, clusterInfoConfig.nodeCount(), vipStatus, metric); - int searchClusterIndex = clusterConfig.clusterId(); clusterModelName = clusterConfig.clusterName(); QrSearchersConfig.Searchcluster searchClusterConfig = getSearchClusterConfigFromClusterName(qrsConfig, clusterModelName); @@ -98,8 +96,7 @@ public class ClusterSearcher extends Searcher { .setLogRaw(false).setLogMean(true)); maxQueryTimeout = ParameterParser.asMilliSeconds(clusterConfig.maxQueryTimeout(), DEFAULT_MAX_QUERY_TIMEOUT); - maxQueryCacheTimeout = ParameterParser.asMilliSeconds(clusterConfig.maxQueryCacheTimeout(), - DEFAULT_MAX_QUERY_CACHE_TIMEOUT); + maxQueryCacheTimeout = ParameterParser.asMilliSeconds(clusterConfig.maxQueryCacheTimeout(), DEFAULT_MAX_QUERY_CACHE_TIMEOUT); SummaryParameters docSumParams = new SummaryParameters(qrsConfig .com().yahoo().prelude().fastsearch().FastSearcher().docsum() @@ -115,12 +112,12 @@ public class ClusterSearcher extends Searcher { } if (searchClusterConfig.indexingmode() == STREAMING) { - VdsStreamingSearcher searcher = vdsCluster(fs4ResourcePool.getServerId(), - searchClusterIndex, - searchClusterConfig, docSumParams, - documentDbConfig); + VdsStreamingSearcher searcher = vdsCluster(fs4ResourcePool.getServerId(), searchClusterIndex, + searchClusterConfig, docSumParams, documentDbConfig); addBackendSearcher(searcher); + vipStatus.addToRotation(searcher.getName()); } else { + Dispatcher dispatcher = Dispatcher.create(id.stringValue(), dispatchConfig, clusterInfoConfig.nodeCount(), vipStatus, metric); for (int dispatcherIndex = 0; dispatcherIndex < searchClusterConfig.dispatcher().size(); dispatcherIndex++) { try { if ( ! isRemote(searchClusterConfig.dispatcher(dispatcherIndex).host())) { @@ -129,13 +126,12 @@ public class ClusterSearcher extends Searcher { addBackendSearcher(searcher); } } catch (UnknownHostException e) { - e.printStackTrace(); throw new RuntimeException(e); } } } if ( server == null ) { - throw new IllegalStateException("ClusterSearcher should have a top level dispatch."); + throw new IllegalStateException("ClusterSearcher should have backend."); } } 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..dd01d895963 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 @@ -54,10 +54,12 @@ public abstract class BaseNodeMonitor<T> { /** * Returns whether this node is currently in a state suitable - * for receiving traffic. As far as we know, that is + * for receiving traffic (default true) */ public boolean isWorking() { return isWorking; } + /** @deprecated Not used */ + @Deprecated // TODO: Remove on Vespa 8 public boolean isQuarantined() { return isQuarantined; } /** 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..22c7f59872c 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 @@ -22,7 +22,7 @@ import java.util.logging.Logger; */ public class ClusterMonitor<T> { - private MonitorConfiguration configuration = new MonitorConfiguration(); + private final MonitorConfiguration configuration = new MonitorConfiguration(); private static Logger log = Logger.getLogger(ClusterMonitor.class.getName()); @@ -33,7 +33,7 @@ public class ClusterMonitor<T> { private volatile boolean shutdown = false; /** A map from Node to corresponding MonitoredNode */ - private final Map<T, BaseNodeMonitor<T>> nodeMonitors = Collections.synchronizedMap(new java.util.LinkedHashMap<>()); + private final Map<T, TrafficNodeMonitor<T>> nodeMonitors = Collections.synchronizedMap(new java.util.LinkedHashMap<>()); public ClusterMonitor(NodeManager<T> manager) { nodeManager = manager; @@ -56,8 +56,7 @@ public class ClusterMonitor<T> { * @param internal whether or not this node is internal to this cluster */ public void add(T node, boolean internal) { - BaseNodeMonitor<T> monitor = new TrafficNodeMonitor<>(node, configuration, internal); - nodeMonitors.put(node, monitor); + nodeMonitors.put(node, new TrafficNodeMonitor<>(node, configuration, internal)); } /** @@ -69,24 +68,20 @@ public class ClusterMonitor<T> { /** Called from ClusterSearcher/NodeManager when a node failed */ public synchronized void failed(T node, ErrorMessage error) { - nodeManager.statusIsKnown(node); - BaseNodeMonitor<T> monitor = nodeMonitors.get(node); - boolean wasWorking = monitor.isWorking(); + TrafficNodeMonitor<T> monitor = nodeMonitors.get(node); + Boolean wasWorking = monitor.isKnownWorking(); monitor.failed(error); - if (wasWorking && !monitor.isWorking()) { + if (wasWorking != monitor.isKnownWorking()) nodeManager.failed(node); - } } /** Called when a node responded */ public synchronized void responded(T node) { - nodeManager.statusIsKnown(node); - BaseNodeMonitor<T> monitor = nodeMonitors.get(node); - boolean wasFailing =! monitor.isWorking(); + TrafficNodeMonitor<T> monitor = nodeMonitors.get(node); + Boolean wasWorking = monitor.isKnownWorking(); monitor.responded(); - if (wasFailing && monitor.isWorking()) { + if (wasWorking != monitor.isKnownWorking()) nodeManager.working(monitor.getNode()); - } } /** 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<T> { - /** 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/cluster/TrafficNodeMonitor.java b/container-search/src/main/java/com/yahoo/search/cluster/TrafficNodeMonitor.java index ea881ad8b48..830014bca46 100644 --- a/container-search/src/main/java/com/yahoo/search/cluster/TrafficNodeMonitor.java +++ b/container-search/src/main/java/com/yahoo/search/cluster/TrafficNodeMonitor.java @@ -36,6 +36,7 @@ public class TrafficNodeMonitor<T> extends BaseNodeMonitor<T> { @Override public void failed(ErrorMessage error) { respondedAt = now(); + atStartUp = false; if (error.getCode() == Error.BACKEND_COMMUNICATION_ERROR.code) { setWorking(false, "Connection failure: " + error.toString()); @@ -60,9 +61,14 @@ public class TrafficNodeMonitor<T> extends BaseNodeMonitor<T> { setWorking(true,"Responds correctly"); } + /** + * Returns whether this node is currently is a state suitable for receiving traffic, or null if not known + */ + public Boolean isKnownWorking() { return atStartUp ? null : isWorking; } + /** Thread-safely changes the state of this node if required */ - protected synchronized void setWorking(boolean working,String explanation) { - if (this.isWorking==working) return; // Old news + protected synchronized void setWorking(boolean working, String explanation) { + if (this.isWorking == working) return; // Old news if (explanation==null) { explanation=""; diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java b/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java index 31af74a39b2..78c50254a84 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java @@ -48,21 +48,24 @@ public abstract class InvokerFactory { * @return Optional containing the SearchInvoker or <i>empty</i> if some node in the * list is invalid and the remaining coverage is not sufficient */ - public Optional<SearchInvoker> createSearchInvoker(VespaBackEndSearcher searcher, Query query, OptionalInt groupId, List<Node> nodes, - boolean acceptIncompleteCoverage) { + public Optional<SearchInvoker> createSearchInvoker(VespaBackEndSearcher searcher, + Query query, + OptionalInt groupId, + List<Node> nodes, + boolean acceptIncompleteCoverage) { List<SearchInvoker> invokers = new ArrayList<>(nodes.size()); Set<Integer> failed = null; for (Node node : nodes) { boolean nodeAdded = false; - if (node.isWorking()) { + if (node.isWorking() != Boolean.FALSE) { Optional<SearchInvoker> invoker = createNodeSearchInvoker(searcher, query, node); - if(invoker.isPresent()) { + if (invoker.isPresent()) { invokers.add(invoker.get()); nodeAdded = true; } } - if (!nodeAdded) { + if ( ! nodeAdded) { if (failed == null) { failed = new HashSet<>(); } 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..0e4e87b9a6a 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 @@ -54,7 +54,7 @@ public class Group { public int workingNodes() { int nodesUp = 0; for (Node node : nodes) { - if (node.isWorking()) { + if (node.isWorking() == Boolean.TRUE) { nodesUp++; } } @@ -64,7 +64,7 @@ public class Group { void aggregateActiveDocuments() { long activeDocumentsInGroup = 0; for (Node node : nodes) { - if (node.isWorking()) { + if (node.isWorking() == Boolean.TRUE) { activeDocumentsInGroup += node.getActiveDocuments(); } } 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..b47f2fefa5b 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 @@ -46,16 +46,15 @@ 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.statusIsKnown.lazySet(true); + this.working.lazySet(working); + } - /** Returns whether this node is currently responding to requests */ - public boolean isWorking() { return working.get(); } + /** Returns whether this node is currently responding to requests, or null if status is not known */ + public Boolean isWorking() { + return statusIsKnown.get() ? working.get() : null; + } /** Updates the active documents on this node */ public void setActiveDocuments(long activeDocuments) { 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 6f775e7218e..a55a970e8ff 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 @@ -95,16 +95,13 @@ public class SearchCluster implements NodeManager<Node> { this.pingFactory = pingFactory; for (var group : orderedGroups) { - for (var node : group.nodes()) { - // cluster monitor will only call working() when the - // node transitions from down to up, so we need to - // register the initial (working) state here: - working(node); + for (var node : group.nodes()) clusterMonitor.add(node, true); - } } } + ClusterMonitor<Node> clusterMonitor() { return clusterMonitor; } + private static Optional<Node> findLocalCorpusDispatchTarget(String selfHostname, int searchClusterSize, int containerClusterSize, @@ -199,30 +196,27 @@ public class SearchCluster implements NodeManager<Node> { Group localSearchGroup = groups.get(localCorpusDispatchTarget.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(); + // Only use direct dispatch if the local search node is not down + if ( localCorpusDispatchTarget.get().isWorking() == Boolean.FALSE) return Optional.empty(); return localCorpusDispatchTarget; } - /** Called by the cluster monitor whenever we get information (positive or negative) about a node */ - @Override - public void statusIsKnown(Node node) { - node.setStatusIsKnown(); + private void updateWorkingState(Node node, boolean isWorking) { + node.setWorking(isWorking); + updateVipStatusOnNodeChange(node, isWorking); } /** Called by the cluster monitor when node state changes to working */ @Override public void working(Node node) { - node.setWorking(true); - updateVipStatusOnNodeChange(node, true); + updateWorkingState(node, true); } /** Called by the cluster monitor when node state changes to failed */ @Override public void failed(Node node) { - node.setWorking(false); - updateVipStatusOnNodeChange(node, true); + updateWorkingState(node, false); } private void updateSufficientCoverage(Group group, boolean sufficientCoverage) { @@ -232,41 +226,38 @@ public class SearchCluster implements NodeManager<Node> { 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); + private void updateVipStatusOnNodeChange(Node node, boolean nodeIsWorking) { + if (localCorpusDispatchTarget.isEmpty()) { // consider entire cluster + if (hasInformationAboutAllNodes()) + setInRotationOnlyIf(hasWorkingNodes()); } - else if (localCorpusDispatchTarget.isEmpty() && hasInformationAboutAllNodes()) { - if (hasWorkingNodes()) - vipStatus.addToRotation(clusterId); - else - vipStatus.removeFromRotation(clusterId); + else if (usesLocalCorpusIn(node)) { // follow the status of this node + setInRotationOnlyIf(nodeIsWorking); } } private void updateVipStatusOnCoverageChange(Group group, boolean sufficientCoverage) { - boolean isInRotation = vipStatus.isInRotation(); - if (usesLocalCorpusIn(group)) { // follow the status of the local corpus - if (sufficientCoverage) - vipStatus.addToRotation(clusterId); - else - vipStatus.removeFromRotation(clusterId); + if ( localCorpusDispatchTarget.isEmpty()) { // consider entire cluster + // VIP status does not depend on coverage } - else if ( localCorpusDispatchTarget.isEmpty()) { - if (isInRotation && sufficientCoverage) - vipStatus.addToRotation(clusterId); + else if (usesLocalCorpusIn(group)) { // follow the status of this group + setInRotationOnlyIf(sufficientCoverage); } } + private void setInRotationOnlyIf(boolean inRotation) { + if (inRotation) + vipStatus.addToRotation(clusterId); + else + vipStatus.removeFromRotation(clusterId); + } + private boolean hasInformationAboutAllNodes() { - return nodesByHost.values().stream().allMatch(Node::getStatusIsKnown); + return nodesByHost.values().stream().allMatch(node -> node.isWorking() != null); } private boolean hasWorkingNodes() { - return nodesByHost.values().stream().anyMatch(Node::isWorking); + return nodesByHost.values().stream().anyMatch(node -> node.isWorking() != Boolean.FALSE ); } private boolean usesLocalCorpusIn(Node node) { diff --git a/container-search/src/main/java/com/yahoo/search/searchchain/model/VespaSearchers.java b/container-search/src/main/java/com/yahoo/search/searchchain/model/VespaSearchers.java index 1657b45b1b4..cf4f5f360ad 100644 --- a/container-search/src/main/java/com/yahoo/search/searchchain/model/VespaSearchers.java +++ b/container-search/src/main/java/com/yahoo/search/searchchain/model/VespaSearchers.java @@ -8,18 +8,20 @@ import com.yahoo.component.chain.dependencies.Dependencies; import com.yahoo.component.chain.model.ChainedComponentModel; import com.yahoo.search.Searcher; import com.yahoo.search.searchchain.model.federation.FederationSearcherModel; -import com.yahoo.search.searchchain.model.federation.FederationSearcherModel.TargetSpec; -import org.apache.commons.collections.CollectionUtils; - -import java.util.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; /** * Defines the searcher models used in the vespa and native search chains, except for federation. * * @author Tony Vaagenes */ -@SuppressWarnings({"rawtypes", "deprecation", "unchecked"}) +@SuppressWarnings({"rawtypes", "unchecked"}) public class VespaSearchers { public static final Collection<ChainedComponentModel> vespaSearcherModels = toSearcherModels( @@ -58,7 +60,7 @@ public class VespaSearchers { private static FederationSearcherModel federationSearcherModel() { return new FederationSearcherModel(new ComponentSpecification("federation"), Dependencies.emptyDependencies(), - Collections.<TargetSpec>emptyList(), true); + Collections.emptyList(), true); } private static boolean allAdded(Collection<ChainedComponentModel> searcherModels, Set<ComponentId> componentIds) { diff --git a/container-search/src/main/java/com/yahoo/search/searchchain/model/federation/LocalProviderSpec.java b/container-search/src/main/java/com/yahoo/search/searchchain/model/federation/LocalProviderSpec.java index 316baa26198..0dcfab92a8e 100644 --- a/container-search/src/main/java/com/yahoo/search/searchchain/model/federation/LocalProviderSpec.java +++ b/container-search/src/main/java/com/yahoo/search/searchchain/model/federation/LocalProviderSpec.java @@ -20,7 +20,6 @@ import net.jcip.annotations.Immutable; */ @Immutable public class LocalProviderSpec { - @SuppressWarnings("unchecked") public static final Collection<ChainedComponentModel> searcherModels = toSearcherModels( com.yahoo.prelude.querytransform.CJKSearcher.class, @@ -63,7 +62,7 @@ public class LocalProviderSpec { } @SafeVarargs - private static final Collection<ChainedComponentModel> toSearcherModels(Class<? extends Searcher>... searchers) { + private static Collection<ChainedComponentModel> toSearcherModels(Class<? extends Searcher>... searchers) { List<ChainedComponentModel> searcherModels = new ArrayList<>(); for (Class<? extends Searcher> c : searchers) { diff --git a/container-search/src/main/java/com/yahoo/search/yql/VespaSerializer.java b/container-search/src/main/java/com/yahoo/search/yql/VespaSerializer.java index f326903bff5..30d0bc6f0e1 100644 --- a/container-search/src/main/java/com/yahoo/search/yql/VespaSerializer.java +++ b/container-search/src/main/java/com/yahoo/search/yql/VespaSerializer.java @@ -645,6 +645,7 @@ public class VespaSerializer { } private static class SameElementSerializer extends Serializer { + @Override void onExit(StringBuilder destination, Item item) { } @@ -654,7 +655,6 @@ public class VespaSerializer { } static boolean serialize(StringBuilder destination, Item item, boolean includeField) { - SameElementItem sameElement = (SameElementItem) item; if (includeField) { @@ -669,10 +669,11 @@ public class VespaSerializer { Item current = sameElement.getItem(i); if (current instanceof WordItem) { new WordSerializer().serialize(destination, current); + } else if (current instanceof IntItem) { + new NumberSerializer().serialize(destination, current); } else { - throw new IllegalArgumentException( - "Serializing of " + current.getClass().getSimpleName() - + " in same_element is not implemented, please report this as a bug."); + throw new IllegalArgumentException("Serializing of " + current.getClass().getSimpleName() + + " in same_element is not implemented, please report this as a bug."); } } destination.append(')'); diff --git a/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java b/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java index 9ea7276583b..3e6e88c72c1 100644 --- a/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java @@ -5,6 +5,7 @@ import com.yahoo.cloud.config.ClusterInfoConfig; import com.yahoo.component.ComponentId; import com.yahoo.container.QrConfig; import com.yahoo.container.QrSearchersConfig; +import com.yahoo.container.handler.ClustersStatus; import com.yahoo.container.handler.VipStatus; import com.yahoo.container.protect.Error; import com.yahoo.prelude.IndexFacts; @@ -264,8 +265,7 @@ public class ClusterSearcherTestCase { Set<String> documentTypes = new LinkedHashSet<>(docTypesList); ClusterSearcher cluster = new ClusterSearcher(documentTypes); try { - cluster.addBackendSearcher(new MyMockSearcher( - expectAttributePrefetch)); + cluster.addBackendSearcher(new MyMockSearcher(expectAttributePrefetch)); cluster.setValidRankProfile("default", documentTypes); cluster.addValidRankProfile("testprofile", "type1"); return new Execution(cluster, Execution.Context.createContextStub()); @@ -513,19 +513,23 @@ public class ClusterSearcherTestCase { assertEquals(3, result.getTotalHitCount()); } - private static ClusterSearcher createSearcher(Double maxQueryTimeout, - Double maxQueryCacheTimeout) { + private static ClusterSearcher createSearcher(String clusterName, Double maxQueryTimeout, Double maxQueryCacheTimeout, + boolean streamingMode, VipStatus vipStatus) + { QrSearchersConfig.Builder qrSearchersConfig = new QrSearchersConfig.Builder(); - QrSearchersConfig.Searchcluster.Builder searchClusterConfig = - new QrSearchersConfig.Searchcluster.Builder().name("test-cluster"); + QrSearchersConfig.Searchcluster.Builder searchClusterConfig = new QrSearchersConfig.Searchcluster.Builder(); + searchClusterConfig.name(clusterName); + if (streamingMode) { + searchClusterConfig.indexingmode(QrSearchersConfig.Searchcluster.Indexingmode.Enum.STREAMING); + searchClusterConfig.searchdef("streaming_sd"); + } qrSearchersConfig.searchcluster(searchClusterConfig); - QrSearchersConfig.Searchcluster.Dispatcher.Builder dispatcherConfig = - new QrSearchersConfig.Searchcluster.Dispatcher.Builder(); + QrSearchersConfig.Searchcluster.Dispatcher.Builder dispatcherConfig = new QrSearchersConfig.Searchcluster.Dispatcher.Builder(); dispatcherConfig.host("localhost"); dispatcherConfig.port(0); searchClusterConfig.dispatcher(dispatcherConfig); - ClusterConfig.Builder clusterConfig = new ClusterConfig.Builder().clusterName("test-cluster"); + ClusterConfig.Builder clusterConfig = new ClusterConfig.Builder().clusterName(clusterName); if (maxQueryTimeout != null) clusterConfig.maxQueryTimeout(maxQueryTimeout); if (maxQueryCacheTimeout != null) @@ -543,7 +547,7 @@ public class ClusterSearcherTestCase { Statistics.nullImplementation, new MockMetric(), new FS4ResourcePool(new QrConfig.Builder().build()), - new VipStatus()); + vipStatus); } private static ClusterInfoConfig createClusterInfoConfig() { @@ -558,7 +562,9 @@ public class ClusterSearcherTestCase { Execution exec; Query query; QueryTimeoutFixture(Double maxQueryTimeout, Double maxQueryCacheTimeout) { - searcher = createSearcher(maxQueryTimeout, maxQueryCacheTimeout); + String clusterName = "test-cluster"; + VipStatus vipStatus = new VipStatus(new QrSearchersConfig.Builder().searchcluster(new QrSearchersConfig.Searchcluster.Builder().name(clusterName)).build(), new ClustersStatus()); + searcher = createSearcher(clusterName, maxQueryTimeout, maxQueryCacheTimeout, false, vipStatus); exec = new Execution(searcher, Execution.Context.createContextStub()); query = new Query("?query=hello&restrict=type1"); } @@ -568,6 +574,15 @@ public class ClusterSearcherTestCase { } @Test + public void testThatVipStatusIsSetUpForStreamingSearch() { + String clusterName = "test-cluster"; + VipStatus vipStatus = new VipStatus(new QrSearchersConfig.Builder().searchcluster(new QrSearchersConfig.Searchcluster.Builder().name(clusterName)).build(), new ClustersStatus()); + assertFalse(vipStatus.isInRotation()); + ClusterSearcher searcher = createSearcher(clusterName, 1.0, 10.0, true, vipStatus); + assertTrue(vipStatus.isInRotation()); + } + + @Test public void testThatQueryTimeoutIsCappedWithDefaultMax() { QueryTimeoutFixture f = new QueryTimeoutFixture(null, null); f.query.setTimeout(600001); diff --git a/container-search/src/test/java/com/yahoo/search/cluster/test/ClusteredConnectionTestCase.java b/container-search/src/test/java/com/yahoo/search/cluster/test/ClusteredConnectionTestCase.java index 84c10991293..aa57e7903f9 100644 --- a/container-search/src/test/java/com/yahoo/search/cluster/test/ClusteredConnectionTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/cluster/test/ClusteredConnectionTestCase.java @@ -114,7 +114,7 @@ public class ClusteredConnectionTestCase { connection0.setInService(false); forcePing(myBackend); r=new Execution(myBackend, Execution.Context.createContextStub()).search(new SimpleQuery(0)); - assertEquals("No backends in service. Try later",r.hits().getError().getMessage()); + assertEquals("No backends in service. Try later", r.hits().getError().getMessage()); connection2.setInService(true); connection1.setInService(true); diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTest.java index bbaf512534a..f29d6ddf324 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTest.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTest.java @@ -8,24 +8,45 @@ import com.yahoo.prelude.Pong; import com.yahoo.search.cluster.ClusterMonitor; import com.yahoo.search.dispatch.MockSearchCluster; import com.yahoo.search.result.ErrorMessage; +import org.jetbrains.annotations.NotNull; import org.junit.Test; import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.concurrent.Callable; +import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +/** + * @author baldersheim + */ public class SearchClusterTest { static class State { + class MyExecutor implements Executor { + private final List<Runnable> list = new ArrayList<>(); + @Override + public void execute(@NotNull Runnable command) { + list.add(command); + } + void run() { + for (Runnable runnable : list) { + runnable.run(); + } + list.clear(); + } + } final String clusterId; final int nodesPerGroup; final VipStatus vipStatus; - final SearchCluster sc; + final SearchCluster searchCluster; final List<AtomicInteger> numDocsPerNode; List<AtomicInteger> pingCounts; State(String clusterId, int nodesPergroup, String ... nodeNames) { @@ -35,9 +56,6 @@ public class SearchClusterTest { this.clusterId = clusterId; this.nodesPerGroup = nodesPergroup; vipStatus = new VipStatus(new QrSearchersConfig.Builder().searchcluster(new QrSearchersConfig.Searchcluster.Builder().name(clusterId)).build(), new ClustersStatus()); - assertFalse(vipStatus.isInRotation()); - vipStatus.addToRotation(clusterId); - assertTrue(vipStatus.isInRotation()); numDocsPerNode = new ArrayList<>(nodeNames.size()); pingCounts = new ArrayList<>(nodeNames.size()); List<Node> nodes = new ArrayList<>(nodeNames.size()); @@ -49,12 +67,12 @@ public class SearchClusterTest { numDocsPerNode.add(new AtomicInteger(1)); pingCounts.add(new AtomicInteger(0)); } - sc = new SearchCluster(clusterId, MockSearchCluster.createDispatchConfig(nodes),nodes.size() / nodesPergroup, vipStatus); + searchCluster = new SearchCluster(clusterId, MockSearchCluster.createDispatchConfig(nodes), nodes.size() / nodesPergroup, vipStatus); } void startMonitoring() { - sc.startClusterMonitoring(new Factory(nodesPerGroup, numDocsPerNode, pingCounts)); + searchCluster.startClusterMonitoring(new Factory(nodesPerGroup, numDocsPerNode, pingCounts)); } - private static int getMaxValue(List<AtomicInteger> list) { + static private int getMaxValue(List<AtomicInteger> list) { int max = list.get(0).get(); for (AtomicInteger v : list) { if (v.get() > max) { @@ -72,10 +90,13 @@ public class SearchClusterTest { } return min; } - private static void waitAtLeast(int atLeast, List<AtomicInteger> list) { + private void waitAtLeast(int atLeast, List<AtomicInteger> list) { while (getMinValue(list) < atLeast) { + ExecutorService executor = Executors.newCachedThreadPool(); + searchCluster.clusterMonitor().ping(executor); + executor.shutdown(); try { - Thread.sleep(100); + executor.awaitTermination(60, TimeUnit.SECONDS); } catch (InterruptedException e) {} } } @@ -118,19 +139,25 @@ public class SearchClusterTest { } @Test - public void requireThatVipStatusIsDefaultUp() { + public void requireThatVipStatusIsDefaultDownButComesUpAfterPinging() { State test = new State("cluster.1", 2, "a", "b"); + assertTrue(test.searchCluster.localCorpusDispatchTarget().isEmpty()); + + assertFalse(test.vipStatus.isInRotation()); + test.startMonitoring(); + test.waitOneFullPingRound(); assertTrue(test.vipStatus.isInRotation()); - assertTrue(test.sc.localCorpusDispatchTarget().isEmpty()); } @Test public void requireThatZeroDocsAreFine() { State test = new State("cluster.1", 2,"a", "b"); + test.startMonitoring(); + test.waitOneFullPingRound(); + assertTrue(test.vipStatus.isInRotation()); - assertTrue(test.sc.localCorpusDispatchTarget().isEmpty()); + assertTrue(test.searchCluster.localCorpusDispatchTarget().isEmpty()); - test.startMonitoring(); test.numDocsPerNode.get(0).set(-1); test.numDocsPerNode.get(1).set(-1); test.waitOneFullPingRound(); @@ -141,21 +168,41 @@ public class SearchClusterTest { } @Test - public void requireThatVipStatusIsDefaultUpWithLocalDispatch() { + public void requireThatVipStatusIsDefaultDownWithLocalDispatch() { State test = new State("cluster.1", 1, HostName.getLocalhost(), "b"); + assertTrue(test.searchCluster.localCorpusDispatchTarget().isPresent()); + + assertFalse(test.vipStatus.isInRotation()); + test.startMonitoring(); + test.waitOneFullPingRound(); assertTrue(test.vipStatus.isInRotation()); - assertTrue(test.sc.localCorpusDispatchTarget().isPresent()); + } + + @Test + public void requireThatVipStatusIsDefaultDownWithOnlySingleLocalDispatch() { + State test = new State("cluster.1", 1, HostName.getLocalhost()); + assertTrue(test.searchCluster.localCorpusDispatchTarget().isPresent()); + + assertFalse(test.vipStatus.isInRotation()); + test.startMonitoring(); + test.waitOneFullPingRound(); + assertTrue(test.vipStatus.isInRotation()); + test.numDocsPerNode.get(0).set(-1); + test.waitOneFullPingRound(); + assertFalse(test.vipStatus.isInRotation()); } @Test public void requireThatVipStatusDownWhenLocalIsDown() { State test = new State("cluster.1",1,HostName.getLocalhost(), "b"); - assertTrue(test.vipStatus.isInRotation()); - assertTrue(test.sc.localCorpusDispatchTarget().isPresent()); test.startMonitoring(); test.waitOneFullPingRound(); assertTrue(test.vipStatus.isInRotation()); + assertTrue(test.searchCluster.localCorpusDispatchTarget().isPresent()); + + test.waitOneFullPingRound(); + assertTrue(test.vipStatus.isInRotation()); test.numDocsPerNode.get(0).set(-1); test.waitOneFullPingRound(); assertFalse(test.vipStatus.isInRotation()); @@ -183,10 +230,11 @@ public class SearchClusterTest { private void verifyThatVipStatusDownRequireAllNodesDown(int numGroups, int nodesPerGroup) { List<String> nodeNames = generateNodeNames(numGroups, nodesPerGroup); State test = new State("cluster.1", nodesPerGroup, nodeNames); + test.startMonitoring(); + test.waitOneFullPingRound(); assertTrue(test.vipStatus.isInRotation()); - assertTrue(test.sc.localCorpusDispatchTarget().isEmpty()); + assertTrue(test.searchCluster.localCorpusDispatchTarget().isEmpty()); - test.startMonitoring(); test.waitOneFullPingRound(); assertTrue(test.vipStatus.isInRotation()); @@ -214,13 +262,15 @@ public class SearchClusterTest { } return nodeNames; } + private void verifyThatVipStatusUpRequireOnlyOneOnlineNode(int numGroups, int nodesPerGroup) { List<String> nodeNames = generateNodeNames(numGroups, nodesPerGroup); State test = new State("cluster.1", nodesPerGroup, nodeNames); + test.startMonitoring(); + test.waitOneFullPingRound(); assertTrue(test.vipStatus.isInRotation()); - assertTrue(test.sc.localCorpusDispatchTarget().isEmpty()); + assertTrue(test.searchCluster.localCorpusDispatchTarget().isEmpty()); - test.startMonitoring(); for (int i=0; i < test.numDocsPerNode.size()-1; i++) { test.numDocsPerNode.get(i).set(-1); } diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 5893e473965..7087a58da21 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.flags; import com.yahoo.vespa.defaults.Defaults; -import com.yahoo.vespa.flags.custom.NodeResources; import com.yahoo.vespa.flags.custom.PreprovisionCapacity; import java.util.List; @@ -10,7 +9,6 @@ import java.util.Optional; import java.util.TreeMap; import static com.yahoo.vespa.flags.FetchVector.Dimension.APPLICATION_ID; -import static com.yahoo.vespa.flags.FetchVector.Dimension.CLUSTER_TYPE; import static com.yahoo.vespa.flags.FetchVector.Dimension.HOSTNAME; import static com.yahoo.vespa.flags.FetchVector.Dimension.NODE_TYPE; @@ -136,12 +134,6 @@ public class Flags { "Takes effect on next iteration of HostProvisionMaintainer.", APPLICATION_ID); - public static final UnboundJacksonFlag<NodeResources> DEFAULT_RESOURCES = defineJacksonFlag( - "default-resources", null, NodeResources.class, - "Node resources that will be used when not specified in services.xml", - "Takes effect on next deployment", - CLUSTER_TYPE); - public static final UnboundBooleanFlag USE_HTTPS_LOAD_BALANCER_UPSTREAM = defineFeatureFlag( "use-https-load-balancer-upstream", false, "Use https between load balancer and upstream containers", diff --git a/model-integration/src/main/java/ai/vespa/rankingexpression/importer/operations/Const.java b/model-integration/src/main/java/ai/vespa/rankingexpression/importer/operations/Const.java index fc59ad35ef8..3ad5cb1d19f 100644 --- a/model-integration/src/main/java/ai/vespa/rankingexpression/importer/operations/Const.java +++ b/model-integration/src/main/java/ai/vespa/rankingexpression/importer/operations/Const.java @@ -20,7 +20,6 @@ import java.util.Optional; public class Const extends IntermediateOperation { private final AttributeMap attributeMap; - private OrderedTensorType standardNamingType; // using standard naming convention: d0, d1, ... public Const(String modelName, String nodeName, @@ -30,7 +29,6 @@ public class Const extends IntermediateOperation { super(modelName, nodeName, inputs); this.attributeMap = attributeMap; this.type = type.rename(vespaName() + "_"); - standardNamingType = OrderedTensorType.standardType(type); setConstantValue(value()); } @@ -55,13 +53,7 @@ public class Const extends IntermediateOperation { } else { expressionNode = new ReferenceNode(Reference.simple("constant", vespaName())); } - TensorFunction output = new TensorFunctionNode.TensorFunctionExpressionNode(expressionNode); - if ( ! standardNamingType.equals(type)) { - List<String> renameFrom = standardNamingType.dimensionNames(); - List<String> renameTo = type.dimensionNames(); - output = new Rename(output, renameFrom, renameTo); - } - return output; + return new TensorFunctionNode.TensorFunctionExpressionNode(expressionNode); } /** Constant names are prefixed by "modelName_" to avoid name conflicts between models */ diff --git a/model-integration/src/main/java/ai/vespa/rankingexpression/importer/tensorflow/AttributeConverter.java b/model-integration/src/main/java/ai/vespa/rankingexpression/importer/tensorflow/AttributeConverter.java index ecb67f93d69..f2c6dfd9069 100644 --- a/model-integration/src/main/java/ai/vespa/rankingexpression/importer/tensorflow/AttributeConverter.java +++ b/model-integration/src/main/java/ai/vespa/rankingexpression/importer/tensorflow/AttributeConverter.java @@ -57,7 +57,7 @@ class AttributeConverter implements IntermediateOperation.AttributeMap { if (attributeMap.containsKey(key)) { AttrValue attrValue = attributeMap.get(key); if (attrValue.getValueCase() == AttrValue.ValueCase.TENSOR) { - return Optional.of(new TensorValue(TensorConverter.toVespaTensor(attrValue.getTensor(), type.type()))); + return Optional.of(new TensorValue(TensorConverter.toVespaTensor(attrValue.getTensor(), type))); } } return get(key); diff --git a/model-integration/src/main/java/ai/vespa/rankingexpression/importer/tensorflow/TensorConverter.java b/model-integration/src/main/java/ai/vespa/rankingexpression/importer/tensorflow/TensorConverter.java index 6ab7a69e469..95727acb5b4 100644 --- a/model-integration/src/main/java/ai/vespa/rankingexpression/importer/tensorflow/TensorConverter.java +++ b/model-integration/src/main/java/ai/vespa/rankingexpression/importer/tensorflow/TensorConverter.java @@ -46,12 +46,12 @@ public class TensorConverter { return builder.build(); } - static Tensor toVespaTensor(TensorProto tensorProto, TensorType type) { - IndexedTensor.BoundBuilder builder = (IndexedTensor.BoundBuilder)Tensor.Builder.of(type); + static Tensor toVespaTensor(TensorProto tensorProto, OrderedTensorType type) { Values values = readValuesOf(tensorProto); - if (values.size() == 0) // Might be stored as "tensor_content" instead - return toVespaTensor(readTensorContentOf(tensorProto)); - + if (values.size() == 0) { // Might be stored as "tensor_content" instead + return toVespaTensor(readTensorContentOf(tensorProto), type); + } + IndexedTensor.BoundBuilder builder = (IndexedTensor.BoundBuilder)Tensor.Builder.of(type.type()); for (int i = 0; i < values.size(); ++i) builder.cellByDirectIndex(i, values.get(i)); return builder.build(); diff --git a/model-integration/src/test/java/ai/vespa/rankingexpression/importer/tensorflow/DropoutImportTestCase.java b/model-integration/src/test/java/ai/vespa/rankingexpression/importer/tensorflow/DropoutImportTestCase.java index f38403bfbd4..b9d767774be 100644 --- a/model-integration/src/test/java/ai/vespa/rankingexpression/importer/tensorflow/DropoutImportTestCase.java +++ b/model-integration/src/test/java/ai/vespa/rankingexpression/importer/tensorflow/DropoutImportTestCase.java @@ -34,7 +34,7 @@ public class DropoutImportTestCase { ImportedMlFunction function = signature.outputFunction("y", "y"); assertNotNull(function); - assertEquals("join(join(imported_ml_function_test_outputs_BiasAdd, reduce(rename(constant(test_outputs_Const), d0, d1), sum, d1), f(a,b)(a * b)), imported_ml_function_test_outputs_BiasAdd, f(a,b)(max(a,b)))", + assertEquals("join(join(imported_ml_function_test_outputs_BiasAdd, reduce(constant(test_outputs_Const), sum, d1), f(a,b)(a * b)), imported_ml_function_test_outputs_BiasAdd, f(a,b)(max(a,b)))", function.expression()); model.assertEqualResult("X", "outputs/Maximum"); assertEquals("{X=tensor(d0[],d1[784])}", function.argumentTypes().toString()); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java index f21231236d4..445d056b8e3 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java @@ -1,4 +1,4 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision; import com.google.common.collect.ImmutableList; @@ -67,6 +67,13 @@ public class NodeList implements Iterable<Node> { return filter(node -> node.allocation().isPresent() && node.allocation().get().membership().cluster().type().equals(type)); } + /** Returns the subset of nodes that are currently changing their Vespa version */ + public NodeList changingVersion() { + return filter(node -> node.status().vespaVersion().isPresent() && + node.allocation().isPresent() && + !node.status().vespaVersion().get().equals(node.allocation().get().membership().cluster().vespaVersion())); + } + /** Returns the subset of nodes assigned to the given cluster */ public NodeList cluster(ClusterSpec.Id cluster) { return filter(node -> node.allocation().isPresent() && node.allocation().get().membership().cluster().id().equals(cluster)); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java index 39b0422901e..02161caead6 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java @@ -1,4 +1,4 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.maintenance; import com.google.inject.Inject; @@ -46,6 +46,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent { private final Optional<LoadBalancerExpirer> loadBalancerExpirer; private final Optional<DynamicProvisioningMaintainer> dynamicProvisioningMaintainer; private final CapacityReportMaintainer capacityReportMaintainer; + private final OsUpgradeActivator osUpgradeActivator; @Inject public NodeRepositoryMaintenance(NodeRepository nodeRepository, Deployer deployer, InfraDeployer infraDeployer, @@ -80,6 +81,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent { dynamicProvisioningMaintainer = provisionServiceProvider.getHostProvisioner().map(hostProvisioner -> new DynamicProvisioningMaintainer(nodeRepository, durationFromEnv("host_provisioner_interval").orElse(defaults.dynamicProvisionerInterval), hostProvisioner, flagSource)); capacityReportMaintainer = new CapacityReportMaintainer(nodeRepository, metric, durationFromEnv("capacity_report_interval").orElse(defaults.capacityReportInterval)); + osUpgradeActivator = new OsUpgradeActivator(nodeRepository, defaults.osUpgradeActivatorInterval); // The DuperModel is filled with infrastructure applications by the infrastructure provisioner, so explicitly run that now infrastructureProvisioner.maintain(); @@ -102,6 +104,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent { infrastructureProvisioner.deconstruct(); loadBalancerExpirer.ifPresent(Maintainer::deconstruct); dynamicProvisioningMaintainer.ifPresent(Maintainer::deconstruct); + osUpgradeActivator.deconstruct(); } private static Optional<Duration> durationFromEnv(String envVariable) { @@ -145,6 +148,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent { private final Duration infrastructureProvisionInterval; private final Duration loadBalancerExpirerInterval; private final Duration dynamicProvisionerInterval; + private final Duration osUpgradeActivatorInterval; private final NodeFailer.ThrottlePolicy throttlePolicy; @@ -164,6 +168,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent { loadBalancerExpirerInterval = Duration.ofMinutes(10); reservationExpiry = Duration.ofMinutes(20); // Need to be long enough for deployment to be finished for all config model versions dynamicProvisionerInterval = Duration.ofMinutes(5); + osUpgradeActivatorInterval = Duration.ofMinutes(5); if (zone.environment().equals(Environment.prod) && ! zone.system().isCd()) { inactiveExpiry = Duration.ofHours(4); // enough time for the application owner to discover and redeploy diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OsUpgradeActivator.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OsUpgradeActivator.java new file mode 100644 index 00000000000..e197689eda2 --- /dev/null +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OsUpgradeActivator.java @@ -0,0 +1,37 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.provision.maintenance; + +import com.yahoo.config.provision.NodeType; +import com.yahoo.vespa.hosted.provision.NodeRepository; + +import java.time.Duration; + +/** + * This maintainer (de)activates OS upgrades according to Vespa upgrade status of nodes in this repository. + * + * If a node is upgrading to a new Vespa version, any ongoing OS upgrade will be paused for all nodes of that type. OS + * upgrades will resume once all nodes of that type have completed their Vespa upgrade. + * + * @author mpolden + */ +public class OsUpgradeActivator extends Maintainer { + + public OsUpgradeActivator(NodeRepository nodeRepository, Duration interval) { + super(nodeRepository, interval); + } + + @Override + protected void maintain() { + for (var nodeType : NodeType.values()) { + if (!nodeType.isDockerHost()) continue; + var active = canUpgradeOsOf(nodeType); + nodeRepository().osVersions().setActive(nodeType, active); + } + } + + /** Returns whether to allow OS upgrade of nodes of given type */ + private boolean canUpgradeOsOf(NodeType type) { + return nodeRepository().list().nodeType(type).changingVersion().asList().isEmpty(); + } + +} diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersion.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersion.java index 571356b0a34..99945ce46e8 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersion.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersion.java @@ -44,4 +44,9 @@ public class OsVersion { return Objects.hash(version, active); } + @Override + public String toString() { + return "OS version " + version + " [active: " + active + "]"; + } + } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java index bc738400c45..a2d84bc7379 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java @@ -1,4 +1,4 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.os; import com.google.common.base.Supplier; @@ -18,6 +18,9 @@ import java.util.logging.Logger; /** * Thread-safe class that manages target OS versions for nodes in this repository. * + * A version target is initially inactive. Activation decision is taken by + * {@link com.yahoo.vespa.hosted.provision.maintenance.OsUpgradeActivator}. + * * The target OS version for each node type is set through the /nodes/v2/upgrade REST API. * * @author mpolden @@ -45,6 +48,11 @@ public class OsVersions { this.db = db; this.cacheTtl = cacheTtl; createCache(); + + // Read and write all versions to make sure they are stored in the latest version of the serialized format + try (var lock = db.lockOsVersions()) { + db.writeOsVersions(db.readOsVersions()); + } } private void createCache() { @@ -65,6 +73,7 @@ public class OsVersions { /** Remove OS target for given node type. Nodes of this type will stop receiving wanted OS version in their * node object */ public void removeTarget(NodeType nodeType) { + require(nodeType); try (Lock lock = db.lockOsVersions()) { Map<NodeType, OsVersion> osVersions = db.readOsVersions(); osVersions.remove(nodeType); @@ -76,9 +85,7 @@ public class OsVersions { /** Set the target OS version for nodes of given type */ public void setTarget(NodeType nodeType, Version newTarget, boolean force) { - if (!nodeType.isDockerHost()) { - throw new IllegalArgumentException("Setting target OS version for " + nodeType + " nodes is unsupported"); - } + require(nodeType); if (newTarget.isEmpty()) { throw new IllegalArgumentException("Invalid target version: " + newTarget.toFullString()); } @@ -96,11 +103,33 @@ public class OsVersions { + oldTarget.get().version()); } - osVersions.put(nodeType, new OsVersion(newTarget, true)); + osVersions.put(nodeType, new OsVersion(newTarget, false)); db.writeOsVersions(osVersions); createCache(); // Throw away current cache log.info("Set OS target version for " + nodeType + " nodes to " + newTarget.toFullString()); } } + /** Activate or deactivate target for given node type. This is used for resuming or pausing an OS upgrade. */ + public void setActive(NodeType nodeType, boolean active) { + require(nodeType); + try (Lock lock = db.lockOsVersions()) { + var osVersions = db.readOsVersions(); + var currentVersion = osVersions.get(nodeType); + if (currentVersion == null) return; // No target version set for this type + if (currentVersion.active() == active) return; // No change + + osVersions.put(nodeType, new OsVersion(currentVersion.version(), active)); + db.writeOsVersions(osVersions); + createCache(); // Throw away current cache + log.info((active ? "Activated" : "Deactivated") + " OS target version for " + nodeType + " nodes"); + } + } + + private static void require(NodeType nodeType) { + if (!nodeType.isDockerHost()) { + throw new IllegalArgumentException("Node type '" + nodeType + "' does not support OS upgrades"); + } + } + } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/OsVersionsSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/OsVersionsSerializer.java index 4104a31886a..26e59040b95 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/OsVersionsSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/OsVersionsSerializer.java @@ -29,9 +29,11 @@ public class OsVersionsSerializer { public static byte[] toJson(Map<NodeType, OsVersion> versions) { var slime = new Slime(); var object = slime.setObject(); - // TODO(mpolden): Write active status here once all readers can handle it - versions.forEach((nodeType, osVersion) -> object.setString(NodeSerializer.toString(nodeType), - osVersion.version().toFullString())); + versions.forEach((nodeType, osVersion) -> { + var versionObject = object.setObject(NodeSerializer.toString(nodeType)); + versionObject.setString(VERSION_FIELD, osVersion.version().toFullString()); + versionObject.setBool(ACTIVE_FIELD, osVersion.active()); + }); try { return SlimeUtils.toJsonBytes(slime); } catch (IOException e) { @@ -45,11 +47,11 @@ public class OsVersionsSerializer { inspector.traverse((ObjectTraverser) (key, value) -> { Version version; boolean active; - // TODO(mpolden): Remove fallback after next version if (value.type() == Type.OBJECT) { version = Version.fromString(value.field(VERSION_FIELD).asString()); active = value.field(ACTIVE_FIELD).asBool(); } else { + // TODO(mpolden): Remove support for legacy format after September 2019 version = Version.fromString(value.asString()); active = true; } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java index 5bc3703c11c..98d06f7e01a 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java @@ -6,10 +6,6 @@ import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.Zone; -import com.yahoo.vespa.flags.FetchVector; -import com.yahoo.vespa.flags.FlagSource; -import com.yahoo.vespa.flags.Flags; -import com.yahoo.vespa.flags.JacksonFlag; import java.util.Arrays; import java.util.Locale; @@ -23,11 +19,12 @@ import java.util.Optional; public class CapacityPolicies { private final Zone zone; - private final JacksonFlag<com.yahoo.vespa.flags.custom.NodeResources> defaultResourcesFlag; + /* Deployments must match 1-to-1 the advertised resources of a physical host */ + private final boolean isUsingAdvertisedResources; - public CapacityPolicies(Zone zone, FlagSource flagSource) { + public CapacityPolicies(Zone zone) { this.zone = zone; - this.defaultResourcesFlag = Flags.DEFAULT_RESOURCES.bindTo(flagSource); + this.isUsingAdvertisedResources = zone.region().value().contains("aws-"); } public int decideSize(Capacity requestedCapacity, ClusterSpec.Type clusterType) { @@ -46,9 +43,7 @@ public class CapacityPolicies { public NodeResources decideNodeResources(Optional<NodeResources> requestedResources, ClusterSpec cluster) { if (requestedResources.isPresent()) assertMinimumResources(requestedResources.get(), cluster); - NodeResources resources = requestedResources - .or(() -> flagNodeResources(cluster.type())) - .orElse(defaultNodeResources(cluster.type())); + NodeResources resources = requestedResources.orElse(defaultNodeResources(cluster.type())); // Allow slow disks in zones which are not performance sensitive if (zone.system().isCd() || zone.environment() == Environment.dev || zone.environment() == Environment.test) @@ -71,16 +66,16 @@ public class CapacityPolicies { minMemoryGb, cluster.type().name(), cluster.id().value(), resources.memoryGb())); } - private Optional<NodeResources> flagNodeResources(ClusterSpec.Type clusterType) { - return Optional.ofNullable(defaultResourcesFlag.with(FetchVector.Dimension.CLUSTER_TYPE, clusterType.name()).value()) - .map(r -> new NodeResources(r.vcpu(), r.memoryGb(), r.diskGb(), r.bandwidthGbps(), NodeResources.DiskSpeed.valueOf(r.diskSpeed()))); - } - private NodeResources defaultNodeResources(ClusterSpec.Type clusterType) { - if (clusterType == ClusterSpec.Type.admin) - return new NodeResources(0.5, 2, 50, 0.3); + if (clusterType == ClusterSpec.Type.admin) { + return isUsingAdvertisedResources ? + new NodeResources(0.5, 4, 50, 0.3) : + new NodeResources(0.5, 2, 50, 0.3); + } - return new NodeResources(1.5, 8, 50, 0.3); + return isUsingAdvertisedResources ? + new NodeResources(2.0, 8, 50, 0.3) : + new NodeResources(1.5, 8, 50, 0.3); } /** diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java index 7c6fdbe6fa5..97b615d493f 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java @@ -58,7 +58,7 @@ public class NodeRepositoryProvisioner implements Provisioner { public NodeRepositoryProvisioner(NodeRepository nodeRepository, Zone zone, ProvisionServiceProvider provisionServiceProvider, FlagSource flagSource) { this.nodeRepository = nodeRepository; - this.capacityPolicies = new CapacityPolicies(zone, flagSource); + this.capacityPolicies = new CapacityPolicies(zone); this.zone = zone; this.loadBalancerProvisioner = provisionServiceProvider.getLoadBalancerService().map(lbService -> new LoadBalancerProvisioner(nodeRepository, lbService, flagSource)); this.preparer = new Preparer(nodeRepository, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OsUpgradeActivatorTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OsUpgradeActivatorTest.java new file mode 100644 index 00000000000..158951969eb --- /dev/null +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OsUpgradeActivatorTest.java @@ -0,0 +1,127 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.provision.maintenance; + +import com.yahoo.component.Version; +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ClusterMembership; +import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.config.provision.NodeType; +import com.yahoo.vespa.hosted.provision.Node; +import com.yahoo.vespa.hosted.provision.node.Allocation; +import com.yahoo.vespa.hosted.provision.node.Status; +import com.yahoo.vespa.hosted.provision.os.OsVersion; +import com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester; +import org.junit.Test; + +import java.time.Duration; +import java.util.Comparator; +import java.util.List; +import java.util.Optional; +import java.util.stream.Stream; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +/** + * @author mpolden + */ +public class OsUpgradeActivatorTest { + + private final ProvisioningTester tester = new ProvisioningTester.Builder().build(); + + @Test + public void activates_upgrade() { + var osVersions = tester.nodeRepository().osVersions(); + var osUpgradeActivator = new OsUpgradeActivator(tester.nodeRepository(), Duration.ofDays(1)); + var version0 = Version.fromString("7.0"); + + // Create infrastructure nodes + var configHostApplication = ApplicationId.from("hosted-vespa", "configserver-host", "default"); + var configHostNodes = tester.makeReadyNodes(3, "default", NodeType.confighost); + tester.prepareAndActivateInfraApplication(configHostApplication, NodeType.confighost, version0); + + var tenantHostApplication = ApplicationId.from("hosted-vespa", "tenant-host", "default"); + var tenantHostNodes = tester.makeReadyNodes(3, "default", NodeType.host); + tester.prepareAndActivateInfraApplication(tenantHostApplication, NodeType.host, version0); + + // All nodes are on initial version + assertEquals(version0, minWantedVersion(NodeType.confighost, NodeType.host)); + completeUpgradeOf(configHostNodes); + completeUpgradeOf(tenantHostNodes); + assertEquals("All nodes are on initial version", version0, minCurrentVersion(NodeType.confighost, NodeType.host)); + + // New OS target version is set + var osVersion0 = Version.fromString("8.0"); + osVersions.setTarget(NodeType.host, osVersion0, false); + osVersions.setTarget(NodeType.confighost, osVersion0, false); + + // New OS version is activated as there is no ongoing Vespa upgrade + osUpgradeActivator.maintain(); + assertTrue("OS version " + osVersion0 + " is active", isOsVersionActive(NodeType.confighost, NodeType.host)); + + // Tenant hosts start upgrading to next Vespa version + var version1 = Version.fromString("7.1"); + tester.prepareAndActivateInfraApplication(tenantHostApplication, NodeType.host, version1); + assertEquals("Wanted version of " + NodeType.host + " is raised", version1, + minWantedVersion(NodeType.host)); + + // Activator pauses upgrade for tenant hosts only + osUpgradeActivator.maintain(); + assertTrue("OS version " + osVersion0 + " is active", isOsVersionActive(NodeType.confighost)); + assertFalse("OS version " + osVersion0 + " is inactive", isOsVersionActive(NodeType.host)); + + // Tenant hosts complete their Vespa upgrade + completeUpgradeOf(tenantHostNodes); + assertEquals("Tenant hosts upgraded", version1, minCurrentVersion(NodeType.host)); + + // Activator resumes OS upgrade of tenant hosts + osUpgradeActivator.run(); + assertTrue("OS version " + osVersion0 + " is active", isOsVersionActive(NodeType.confighost, NodeType.host)); + } + + private boolean isOsVersionActive(NodeType... types) { + var active = true; + for (var type : types) { + active &= tester.nodeRepository().osVersions().targetFor(type).map(OsVersion::active).orElse(false); + } + return active; + } + + private void completeUpgradeOf(List<Node> nodes) { + for (var node : nodes) { + try (var lock = tester.nodeRepository().lock(node)) { + node = tester.nodeRepository().getNode(node.hostname()).get(); + node = node.with(node.status().withVespaVersion(node.allocation().get().membership().cluster().vespaVersion())); + tester.nodeRepository().write(node, lock); + } + } + } + + private Stream<Node> streamNodes(NodeType... types) { + Stream<Node> stream = Stream.empty(); + for (var type : types) { + stream = Stream.concat(stream, tester.nodeRepository().getNodes(type).stream()); + } + return stream; + } + + private Version minCurrentVersion(NodeType... types) { + return streamNodes(types).map(Node::status) + .map(Status::vespaVersion) + .flatMap(Optional::stream) + .min(Comparator.naturalOrder()) + .orElse(Version.emptyVersion); + } + + private Version minWantedVersion(NodeType... types) { + return streamNodes(types).map(Node::allocation) + .flatMap(Optional::stream) + .map(Allocation::membership) + .map(ClusterMembership::cluster) + .map(ClusterSpec::vespaVersion) + .min(Comparator.naturalOrder()) + .orElse(Version.emptyVersion); + } + +} diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java index 06f9dcfae68..070db08f090 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java @@ -1,4 +1,4 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.os; import com.yahoo.component.Version; @@ -7,7 +7,6 @@ import com.yahoo.vespa.hosted.provision.NodeRepositoryTester; import org.junit.Test; import java.time.Duration; -import java.util.Map; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -21,38 +20,41 @@ import static org.junit.Assert.fail; */ public class OsVersionsTest { - private final OsVersions versions = new OsVersions( - new NodeRepositoryTester().nodeRepository().database(), - Duration.ofDays(1) // Long TTL to avoid timed expiry during test - ); - @Test public void test_versions() { + var versions = new OsVersions(new NodeRepositoryTester().nodeRepository().database(), Duration.ofDays(1)); + assertTrue("No versions set", versions.targets().isEmpty()); assertSame("Caches empty target versions", versions.targets(), versions.targets()); // Upgrade OS - Version version1 = Version.fromString("7.1"); - versions.setTarget(NodeType.host, version1, false); - Map<NodeType, OsVersion> targetVersions = versions.targets(); + var version1 = new OsVersion(Version.fromString("7.1"), false); + versions.setTarget(NodeType.host, version1.version(), false); + var targetVersions = versions.targets(); assertSame("Caches target versions", targetVersions, versions.targets()); - assertEquals(version1, versions.targetFor(NodeType.host).get().version()); + assertEquals(version1, versions.targetFor(NodeType.host).get()); // Upgrade OS again - Version version2 = Version.fromString("7.2"); - versions.setTarget(NodeType.host, version2, false); + var version2 = new OsVersion(Version.fromString("7.2"), false); + versions.setTarget(NodeType.host, version2.version(), false); assertNotSame("Cache invalidated", targetVersions, versions.targets()); - assertEquals(version2, versions.targetFor(NodeType.host).get().version()); + assertEquals(version2, versions.targetFor(NodeType.host).get()); + + // Target can be (de)activated + versions.setActive(NodeType.host, true); + assertTrue("Target version deactivated", versions.targetFor(NodeType.host).get().active()); + versions.setActive(NodeType.host, false); + assertFalse("Target version deactivated", versions.targetFor(NodeType.host).get().active()); // Downgrading fails try { - versions.setTarget(NodeType.host, version1, false); + versions.setTarget(NodeType.host, version1.version(), false); fail("Expected exception"); } catch (IllegalArgumentException ignored) {} // Forcing downgrade succeeds - versions.setTarget(NodeType.host, version1, true); - assertEquals(version1, versions.targetFor(NodeType.host).get().version()); + versions.setTarget(NodeType.host, version1.version(), true); + assertEquals(version1, versions.targetFor(NodeType.host).get()); // Target can be removed versions.removeTarget(NodeType.host); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/OsVersionsSerializerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/OsVersionsSerializerTest.java index 4aec5b8370e..b41958b36db 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/OsVersionsSerializerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/OsVersionsSerializerTest.java @@ -9,14 +9,13 @@ import org.junit.Test; import java.nio.charset.StandardCharsets; import java.util.Map; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; /** * @author mpolden */ public class OsVersionsSerializerTest { - // TODO(mpolden): Remove once no longer supported @Test public void legacy_format() { var json = "{\"host\":\"1.2.3\",\"proxyhost\":\"4.5.6\",\"confighost\":\"7.8.9\"}"; @@ -33,28 +32,14 @@ public class OsVersionsSerializerTest { } @Test - public void read_future_format() { - var json = "{\n" + - " \"host\": {\n" + - " \"version\": \"1.2.3\",\n" + - " \"active\": false\n" + - " " + - "},\n" + - " \"proxyhost\": {\n" + - " \"version\": \"4.5.6\",\n" + - " \"active\": true\n" + - " },\n" + - " \"confighost\": {\n" + - " \"version\": \"7.8.9\",\n" + - " \"active\": true\n" + - " }\n" + - "}"; - var versions = OsVersionsSerializer.fromJson(json.getBytes(StandardCharsets.UTF_8)); - assertEquals(Map.of( - NodeType.host, new OsVersion(Version.fromString("1.2.3"), false), - NodeType.proxyhost, new OsVersion(Version.fromString("4.5.6"), true), + public void serialization() { + var versions = Map.of( + NodeType.host, new OsVersion(Version.fromString("1.2.3"), true), + NodeType.proxyhost, new OsVersion(Version.fromString("4.5.6"), false), NodeType.confighost, new OsVersion(Version.fromString("7.8.9"), true) - ), versions); + ); + var serialized = OsVersionsSerializer.fromJson(OsVersionsSerializer.toJson(versions)); + assertEquals(serialized, versions); } } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java index dee32513457..ef1ad1e76eb 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java @@ -1,4 +1,4 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.provisioning; import com.yahoo.component.Version; @@ -91,7 +91,7 @@ public class ProvisioningTester { this.orchestrator = orchestrator; ProvisionServiceProvider provisionServiceProvider = new MockProvisionServiceProvider(loadBalancerService, hostProvisioner); this.provisioner = new NodeRepositoryProvisioner(nodeRepository, zone, provisionServiceProvider, flagSource); - this.capacityPolicies = new CapacityPolicies(zone, flagSource); + this.capacityPolicies = new CapacityPolicies(zone); this.provisionLogger = new NullProvisionLogger(); this.loadBalancerService = loadBalancerService; } @@ -156,13 +156,17 @@ public class ProvisioningTester { assertEquals(toHostNames(hosts), toHostNames(nodeRepository.getNodes(application, Node.State.active))); } - public void prepareAndActivateInfraApplication(ApplicationId application, NodeType nodeType) { - ClusterSpec cluster = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from(nodeType.toString()), Version.fromString("6.42"), false); + public void prepareAndActivateInfraApplication(ApplicationId application, NodeType nodeType, Version version) { + ClusterSpec cluster = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from(nodeType.toString()), version, false); Capacity capacity = Capacity.fromRequiredNodeType(nodeType); List<HostSpec> hostSpecs = prepare(application, cluster, capacity, 1, true); activate(application, hostSpecs); } + public void prepareAndActivateInfraApplication(ApplicationId application, NodeType nodeType) { + prepareAndActivateInfraApplication(application, nodeType, Version.fromString("6.42")); + } + public void deactivate(ApplicationId applicationId) { NestedTransaction deactivateTransaction = new NestedTransaction(); nodeRepository.deactivate(applicationId, deactivateTransaction); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java index 729ded3234c..cad885104f3 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java @@ -1,4 +1,4 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.restapi.v2; import com.yahoo.application.Networking; @@ -8,7 +8,11 @@ import com.yahoo.application.container.handler.Response; import com.yahoo.config.provision.NodeType; import com.yahoo.io.IOUtils; import com.yahoo.text.Utf8; +import com.yahoo.vespa.config.SlimeUtils; +import com.yahoo.vespa.hosted.provision.NodeRepository; +import com.yahoo.vespa.hosted.provision.maintenance.OsUpgradeActivator; import com.yahoo.vespa.hosted.provision.testutils.ContainerConfig; +import com.yahoo.vespa.hosted.provision.testutils.MockNodeRepository; import org.junit.After; import org.junit.Before; import org.junit.ComparisonFailure; @@ -17,6 +21,7 @@ import org.junit.Test; import java.io.File; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.time.Duration; import java.util.Arrays; import java.util.List; import java.util.regex.Pattern; @@ -680,7 +685,7 @@ public class RestApiTest { Utf8.toBytes("{\"osVersion\": \"7.5.2\"}"), Request.Method.PATCH), 400, - "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Setting target OS version for config nodes is unsupported\"}"); + "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Node type 'config' does not support OS upgrades\"}"); // Attempt to downgrade OS assertResponse(new Request("http://localhost:8080/nodes/v2/upgrade/confighost", @@ -731,6 +736,11 @@ public class RestApiTest { Request.Method.PATCH), "{\"message\":\"Set osVersion to 7.5.2 for nodes of type host\"}"); + // Activate target + var nodeRepository = (NodeRepository) container.components().getComponent(MockNodeRepository.class.getName()); + var osUpgradeActivator = new OsUpgradeActivator(nodeRepository, Duration.ofDays(1)); + osUpgradeActivator.run(); + // Other node type does not return wanted OS version Response r = container.handleRequest(new Request("http://localhost:8080/nodes/v2/node/host1.yahoo.com")); assertFalse("Response omits wantedOsVersions field", r.getBodyAsString().contains("wantedOsVersion")); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/maintenance.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/maintenance.json index 28881717e7c..cfb39e7e5b1 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/maintenance.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/maintenance.json @@ -31,6 +31,9 @@ "name": "OperatorChangeApplicationMaintainer" }, { + "name": "OsUpgradeActivator" + }, + { "name": "PeriodicApplicationMaintainer" }, { diff --git a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp index 55d9655d805..72591d340a9 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp @@ -26,6 +26,8 @@ using namespace search::grouping; using search::DocumentMetaData; using search::LidUsageStats; using search::FeatureSet; +using search::StructFieldMapper; +using search::MatchingElements; using search::attribute::IAttributeContext; using search::fef::MatchDataLayout; using search::fef::MatchData; @@ -368,4 +370,17 @@ Matcher::getRankFeatures(const DocsumRequest & req, ISearchContext & searchCtx, return getFeatureSet(req, searchCtx, attrCtx, sessionMgr, false); } +MatchingElements +Matcher::get_matching_elements(const DocsumRequest &req, ISearchContext &search_ctx, + IAttributeContext &attr_ctx, SessionManager &session_manager, + const StructFieldMapper &field_mapper) +{ + (void) req; + (void) search_ctx; + (void) attr_ctx; + (void) session_manager; + (void) field_mapper; + return MatchingElements(); +} + } diff --git a/searchcore/src/vespa/searchcore/proton/matching/matcher.h b/searchcore/src/vespa/searchcore/proton/matching/matcher.h index f234b76008e..3a1fad927b7 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/matcher.h +++ b/searchcore/src/vespa/searchcore/proton/matching/matcher.h @@ -10,6 +10,8 @@ #include <vespa/searchcore/proton/matching/querylimiter.h> #include <vespa/searchcommon/attribute/i_attribute_functor.h> #include <vespa/searchlib/common/featureset.h> +#include <vespa/searchlib/common/struct_field_mapper.h> +#include <vespa/searchlib/common/matching_elements.h> #include <vespa/searchlib/common/resultset.h> #include <vespa/searchlib/queryeval/blueprint.h> #include <vespa/searchlib/fef/fef.h> @@ -49,6 +51,8 @@ private: using DocsumRequest = search::engine::DocsumRequest; using Properties = search::fef::Properties; using my_clock = std::chrono::steady_clock; + using StructFieldMapper = search::StructFieldMapper; + using MatchingElements = search::MatchingElements; IndexEnvironment _indexEnv; search::fef::BlueprintFactory _blueprintFactory; search::fef::RankSetup::SP _rankSetup; @@ -156,6 +160,22 @@ public: IAttributeContext & attrCtx, SessionManager &sessionManager); /** + * Perform partial matching for the documents in the given docsum request + * to identify which struct field elements the query matched. + * + * @param req the docsum request + * @param search_ctx abstract view of searchable data + * @param attr_ctx abstract view of attribute data + * @param session_manager multilevel grouping session and query cache + * @param field_mapper knows which fields to collect information + * about and how they relate to each other + * @return matching elements + **/ + MatchingElements get_matching_elements(const DocsumRequest &req, ISearchContext &search_ctx, + IAttributeContext &attr_ctx, SessionManager &session_manager, + const StructFieldMapper &field_mapper); + + /** * @return true if this rankprofile has summary-features enabled **/ bool canProduceSummaryFeatures() const { return ! _rankSetup->getSummaryFeatures().empty(); } diff --git a/searchlib/CMakeLists.txt b/searchlib/CMakeLists.txt index 9e997443343..449580e577b 100644 --- a/searchlib/CMakeLists.txt +++ b/searchlib/CMakeLists.txt @@ -100,9 +100,11 @@ vespa_define_module( src/tests/common/bitvector src/tests/common/foregroundtaskexecutor src/tests/common/location + src/tests/common/matching_elements src/tests/common/packets src/tests/common/resultset src/tests/common/sequencedtaskexecutor + src/tests/common/struct_field_mapper src/tests/common/summaryfeatures src/tests/diskindex/bitvector src/tests/diskindex/diskindex diff --git a/searchlib/src/tests/common/matching_elements/CMakeLists.txt b/searchlib/src/tests/common/matching_elements/CMakeLists.txt new file mode 100644 index 00000000000..cd1d3560c15 --- /dev/null +++ b/searchlib/src/tests/common/matching_elements/CMakeLists.txt @@ -0,0 +1,9 @@ +# Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +vespa_add_executable(searchlib_common_matching_elements_test_app TEST + SOURCES + matching_elements_test.cpp + DEPENDS + searchlib + gtest +) +vespa_add_test(NAME searchlib_common_matching_elements_test_app COMMAND searchlib_common_matching_elements_test_app) diff --git a/searchlib/src/tests/common/matching_elements/matching_elements_test.cpp b/searchlib/src/tests/common/matching_elements/matching_elements_test.cpp new file mode 100644 index 00000000000..e23460b83af --- /dev/null +++ b/searchlib/src/tests/common/matching_elements/matching_elements_test.cpp @@ -0,0 +1,45 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include <vespa/vespalib/gtest/gtest.h> +#include <vespa/searchlib/common/matching_elements.h> + +using namespace search; + +namespace { + +std::vector<uint32_t> vec(const std::initializer_list<uint32_t> list) { + return std::vector<uint32_t>(list); +} + +} + +struct MatchingElementsTest : ::testing::Test { + MatchingElements matches; + MatchingElementsTest() : matches() { + matches.add_matching_elements(1, "foo", vec({1, 3, 5})); + matches.add_matching_elements(1, "bar", vec({2, 4, 6})); + matches.add_matching_elements(2, "foo", vec({1, 2, 3})); + matches.add_matching_elements(2, "bar", vec({4, 5, 6})); + matches.add_matching_elements(2, "foo", vec({2, 3, 5})); + matches.add_matching_elements(2, "bar", vec({2, 4, 5})); + } + ~MatchingElementsTest() = default; +}; + + +TEST_F(MatchingElementsTest, require_that_added_matches_can_be_looked_up) { + EXPECT_EQ(matches.get_matching_elements(1, "foo"), vec({1, 3, 5})); + EXPECT_EQ(matches.get_matching_elements(1, "bar"), vec({2, 4, 6})); +} + +TEST_F(MatchingElementsTest, require_that_added_matches_are_merged) { + EXPECT_EQ(matches.get_matching_elements(2, "foo"), vec({1, 2, 3, 5})); + EXPECT_EQ(matches.get_matching_elements(2, "bar"), vec({2, 4, 5, 6})); +} + +TEST_F(MatchingElementsTest, require_that_nonexisting_lookup_gives_empty_result) { + EXPECT_EQ(matches.get_matching_elements(1, "bogus"), vec({})); + EXPECT_EQ(matches.get_matching_elements(7, "foo"), vec({})); +} + +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchlib/src/tests/common/struct_field_mapper/CMakeLists.txt b/searchlib/src/tests/common/struct_field_mapper/CMakeLists.txt new file mode 100644 index 00000000000..f5712d22989 --- /dev/null +++ b/searchlib/src/tests/common/struct_field_mapper/CMakeLists.txt @@ -0,0 +1,9 @@ +# Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +vespa_add_executable(searchlib_common_struct_field_mapper_test_app TEST + SOURCES + struct_field_mapper_test.cpp + DEPENDS + searchlib + gtest +) +vespa_add_test(NAME searchlib_common_struct_field_mapper_test_app COMMAND searchlib_common_struct_field_mapper_test_app) diff --git a/searchlib/src/tests/common/struct_field_mapper/struct_field_mapper_test.cpp b/searchlib/src/tests/common/struct_field_mapper/struct_field_mapper_test.cpp new file mode 100644 index 00000000000..c5368111859 --- /dev/null +++ b/searchlib/src/tests/common/struct_field_mapper/struct_field_mapper_test.cpp @@ -0,0 +1,52 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include <vespa/vespalib/gtest/gtest.h> +#include <vespa/searchlib/common/struct_field_mapper.h> + +using namespace search; + +namespace { + +std::string str(const vespalib::string &s) { return std::string(s.data(), s.size()); } + +} + +struct StructFieldMapperTest : ::testing::Test { + StructFieldMapper mapper; + StructFieldMapperTest() : mapper() { + mapper.add_mapping("foo", "foo.a"); + mapper.add_mapping("foo", "foo.b"); + mapper.add_mapping("bar", "bar.x"); + mapper.add_mapping("bar", "bar.y"); + } + ~StructFieldMapperTest() = default; +}; + +TEST_F(StructFieldMapperTest, require_that_struct_field_can_be_identified) { + EXPECT_TRUE(mapper.is_struct_field("foo")); + EXPECT_TRUE(mapper.is_struct_field("bar")); + EXPECT_TRUE(!mapper.is_struct_field("foo.a")); + EXPECT_TRUE(!mapper.is_struct_field("bar.x")); + EXPECT_TRUE(!mapper.is_struct_field("bogus")); +} + +TEST_F(StructFieldMapperTest, require_that_struct_subfield_can_be_identified) { + EXPECT_TRUE(!mapper.is_struct_subfield("foo")); + EXPECT_TRUE(!mapper.is_struct_subfield("bar")); + EXPECT_TRUE(mapper.is_struct_subfield("foo.a")); + EXPECT_TRUE(mapper.is_struct_subfield("bar.x")); + EXPECT_TRUE(!mapper.is_struct_subfield("bogus")); +} + +TEST_F(StructFieldMapperTest, require_that_struct_subfield_maps_to_enclosing_struct_field_name) { + EXPECT_EQ(str(mapper.get_struct_field("foo.a")), str("foo")); + EXPECT_EQ(str(mapper.get_struct_field("foo.b")), str("foo")); + EXPECT_EQ(str(mapper.get_struct_field("bar.x")), str("bar")); + EXPECT_EQ(str(mapper.get_struct_field("bar.y")), str("bar")); +} + +TEST_F(StructFieldMapperTest, require_that_nonexisting_struct_subfield_maps_to_empty_string) { + EXPECT_EQ(str(mapper.get_struct_field("bogus")), str("")); +} + +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchlib/src/vespa/searchlib/common/CMakeLists.txt b/searchlib/src/vespa/searchlib/common/CMakeLists.txt index 9abb6c42c8d..4f0b241e98f 100644 --- a/searchlib/src/vespa/searchlib/common/CMakeLists.txt +++ b/searchlib/src/vespa/searchlib/common/CMakeLists.txt @@ -18,6 +18,7 @@ vespa_add_library(searchlib_common OBJECT location.cpp locationiterators.cpp mapnames.cpp + matching_elements.cpp packets.cpp partialbitvector.cpp resultset.cpp @@ -28,6 +29,7 @@ vespa_add_library(searchlib_common OBJECT sortdata.cpp sortresults.cpp sortspec.cpp + struct_field_mapper.cpp threaded_compactable_lid_space.cpp tunefileinfo.cpp DEPENDS diff --git a/searchlib/src/vespa/searchlib/common/matching_elements.cpp b/searchlib/src/vespa/searchlib/common/matching_elements.cpp new file mode 100644 index 00000000000..1a4653e267b --- /dev/null +++ b/searchlib/src/vespa/searchlib/common/matching_elements.cpp @@ -0,0 +1,31 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "matching_elements.h" +#include <algorithm> + +namespace search { + +MatchingElements::MatchingElements() = default; +MatchingElements::~MatchingElements() = default; + +void +MatchingElements::add_matching_elements(uint32_t docid, const vespalib::string &struct_field_name, const std::vector<uint32_t> &elements) +{ + auto &list = _map[key_t(docid, struct_field_name)]; + std::vector<uint32_t> new_list; + std::set_union(list.begin(), list.end(), elements.begin(), elements.end(), std::back_inserter(new_list)); + list = std::move(new_list); +} + +const std::vector<uint32_t> & +MatchingElements::get_matching_elements(uint32_t docid, const vespalib::string &struct_field_name) const +{ + static const std::vector<uint32_t> empty; + auto res = _map.find(key_t(docid, struct_field_name)); + if (res == _map.end()) { + return empty; + } + return res->second; +} + +} // namespace search diff --git a/searchlib/src/vespa/searchlib/common/matching_elements.h b/searchlib/src/vespa/searchlib/common/matching_elements.h new file mode 100644 index 00000000000..9299191e83a --- /dev/null +++ b/searchlib/src/vespa/searchlib/common/matching_elements.h @@ -0,0 +1,31 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once +#include <utility> +#include <map> +#include <vector> +#include <vespa/vespalib/stllike/string.h> + +namespace search { + +/** + * Keeps track of which elements matched the query for a set of struct + * fields across multiple documents. + **/ +class MatchingElements +{ +private: + using key_t = std::pair<uint32_t, vespalib::string>; + using value_t = std::vector<uint32_t>; + + std::map<key_t, value_t> _map; + +public: + MatchingElements(); + ~MatchingElements(); + + void add_matching_elements(uint32_t docid, const vespalib::string &struct_field_name, const std::vector<uint32_t> &elements); + const std::vector<uint32_t> &get_matching_elements(uint32_t docid, const vespalib::string &struct_field_name) const; +}; + +} // namespace search diff --git a/searchlib/src/vespa/searchlib/common/struct_field_mapper.cpp b/searchlib/src/vespa/searchlib/common/struct_field_mapper.cpp new file mode 100644 index 00000000000..849cfd06ade --- /dev/null +++ b/searchlib/src/vespa/searchlib/common/struct_field_mapper.cpp @@ -0,0 +1,10 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "struct_field_mapper.h" + +namespace search { + +StructFieldMapper::StructFieldMapper() = default; +StructFieldMapper::~StructFieldMapper() = default; + +} // namespace search diff --git a/searchlib/src/vespa/searchlib/common/struct_field_mapper.h b/searchlib/src/vespa/searchlib/common/struct_field_mapper.h new file mode 100644 index 00000000000..07951db99f7 --- /dev/null +++ b/searchlib/src/vespa/searchlib/common/struct_field_mapper.h @@ -0,0 +1,47 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include <vespa/vespalib/stllike/string.h> +#include <set> +#include <map> + +namespace search { + +/** + * Keeps track of a set of struct field names and enables mapping the + * full name of struct subfields into the name of the enclosing struct + * field. + **/ +class StructFieldMapper +{ +private: + std::set<vespalib::string> _struct_fields; + std::map<vespalib::string,vespalib::string> _struct_subfields; + +public: + StructFieldMapper(); + ~StructFieldMapper(); + void add_mapping(const vespalib::string &struct_field_name, + const vespalib::string &struct_subfield_name) + { + _struct_fields.insert(struct_field_name); + _struct_subfields[struct_subfield_name] = struct_field_name; + } + bool is_struct_field(const vespalib::string &field_name) const { + return (_struct_fields.count(field_name) > 0); + } + bool is_struct_subfield(const vespalib::string &field_name) const { + return (_struct_subfields.find(field_name) != _struct_subfields.end()); + } + const vespalib::string &get_struct_field(const vespalib::string &struct_subfield_name) const { + static const vespalib::string empty; + auto res = _struct_subfields.find(struct_subfield_name); + if (res == _struct_subfields.end()) { + return empty; + } + return res->second; + } +}; + +} // namespace search diff --git a/storage/src/tests/distributor/statecheckerstest.cpp b/storage/src/tests/distributor/statecheckerstest.cpp index 7282d2e7d2a..0c1ec1e77de 100644 --- a/storage/src/tests/distributor/statecheckerstest.cpp +++ b/storage/src/tests/distributor/statecheckerstest.cpp @@ -158,6 +158,7 @@ struct StateCheckersTest : Test, DistributorTestUtil { struct CheckerParams { std::string _bucketInfo; std::string _clusterState {"distributor:1 storage:2"}; + std::string _pending_cluster_state; std::string _expect; static const PendingMessage NO_OP_BLOCKER; const PendingMessage* _blockerMessage {&NO_OP_BLOCKER}; @@ -182,6 +183,10 @@ struct StateCheckersTest : Test, DistributorTestUtil { _clusterState = state; return *this; } + CheckerParams& pending_cluster_state(const std::string& state) { + _pending_cluster_state = state; + return *this; + } CheckerParams& blockerMessage(const PendingMessage& blocker) { _blockerMessage = &blocker; return *this; @@ -208,6 +213,11 @@ struct StateCheckersTest : Test, DistributorTestUtil { addNodesToBucketDB(bid, params._bucketInfo); setRedundancy(params._redundancy); enableDistributorClusterState(params._clusterState); + if (!params._pending_cluster_state.empty()) { + auto cmd = std::make_shared<api::SetSystemStateCommand>(lib::ClusterState(params._pending_cluster_state)); + _distributor->onDown(cmd); + tick(); // Trigger command processing and pending state setup. + } NodeMaintenanceStatsTracker statsTracker; StateChecker::Context c( getExternalOperationHandler(), getDistributorBucketSpace(), statsTracker, makeDocumentBucket(bid)); @@ -640,10 +650,8 @@ TEST_F(StateCheckersTest, synchronize_and_move) { runAndVerify<SynchronizeAndMoveStateChecker>( CheckerParams().expect( "[Synchronizing buckets with different checksums " - "node(idx=0,crc=0x1,docs=1/1,bytes=1/1,trusted=false," - "active=false,ready=false), " - "node(idx=1,crc=0x2,docs=2/2,bytes=2/2,trusted=false," - "active=false,ready=false)] " + "node(idx=0,crc=0x1,docs=1/1,bytes=1/1,trusted=false,active=false,ready=false), " + "node(idx=1,crc=0x2,docs=2/2,bytes=2/2,trusted=false,active=false,ready=false)] " "(scheduling pri MEDIUM)") .bucketInfo("0=1,1=2") .includeSchedulingPriority(true)); @@ -698,12 +706,9 @@ TEST_F(StateCheckersTest, synchronize_and_move) { runAndVerify<SynchronizeAndMoveStateChecker>( CheckerParams() .expect("[Synchronizing buckets with different checksums " - "node(idx=0,crc=0x3,docs=3/3,bytes=3/3,trusted=false," - "active=false,ready=false), " - "node(idx=1,crc=0x3,docs=3/3,bytes=3/3,trusted=false," - "active=false,ready=false), " - "node(idx=2,crc=0x0,docs=0/0,bytes=0/0,trusted=false," - "active=false,ready=false)]") + "node(idx=0,crc=0x3,docs=3/3,bytes=3/3,trusted=false,active=false,ready=false), " + "node(idx=1,crc=0x3,docs=3/3,bytes=3/3,trusted=false,active=false,ready=false), " + "node(idx=2,crc=0x0,docs=0/0,bytes=0/0,trusted=false,active=false,ready=false)]") .bucketInfo("0=3,1=3,2=0") .clusterState("distributor:1 storage:3")); @@ -712,14 +717,10 @@ TEST_F(StateCheckersTest, synchronize_and_move) { runAndVerify<SynchronizeAndMoveStateChecker>( CheckerParams() .expect("[Synchronizing buckets with different checksums " - "node(idx=0,crc=0x2,docs=3/3,bytes=4/4,trusted=false," - "active=false,ready=false), " - "node(idx=1,crc=0x1,docs=2/2,bytes=3/3,trusted=true," - "active=false,ready=false), " - "node(idx=2,crc=0x1,docs=2/2,bytes=3/3,trusted=true," - "active=false,ready=false), " - "node(idx=3,crc=0x1,docs=2/2,bytes=3/3,trusted=true," - "active=false,ready=false)] " + "node(idx=0,crc=0x2,docs=3/3,bytes=4/4,trusted=false,active=false,ready=false), " + "node(idx=1,crc=0x1,docs=2/2,bytes=3/3,trusted=true,active=false,ready=false), " + "node(idx=2,crc=0x1,docs=2/2,bytes=3/3,trusted=true,active=false,ready=false), " + "node(idx=3,crc=0x1,docs=2/2,bytes=3/3,trusted=true,active=false,ready=false)] " "(pri 120) (scheduling pri MEDIUM)") .bucketInfo("0=2/3/4,1=1/2/3/t,2=1/2/3/t,3=1/2/3/t") .clusterState("distributor:1 storage:5") @@ -750,6 +751,25 @@ TEST_F(StateCheckersTest, synchronize_and_move) { .clusterState("distributor:1 storage:4")); } +// Upon entering a cluster state transition edge the distributor will +// prune all replicas from its DB that are on nodes that are unavailable +// in the _pending_ state. As long as this state is pending, the _current_ +// state will include these nodes as available. But since replicas for +// the unavailable node(s) have been pruned away, started merges that +// involve these nodes as part of their chain are doomed to fail. +TEST_F(StateCheckersTest, do_not_schedule_merges_when_included_node_is_unavailable_in_pending_state) { + runAndVerify<SynchronizeAndMoveStateChecker>( + CheckerParams() + .expect("NO OPERATIONS GENERATED") + .redundancy(3) + .bucketInfo("1=1,2=1") // Node 0 pruned from DB since it's s:m in state 2 + .clusterState("version:1 distributor:2 storage:3") + // We change the distributor set as well as the content node set. Just setting a node + // into maintenance does not trigger a pending state since it does not require any + // bucket info fetches from any of the nodes. + .pending_cluster_state("version:2 distributor:1 storage:3 .0.s:m")); +} + TEST_F(StateCheckersTest, do_not_merge_inconsistently_split_buckets) { // No merge generated if buckets are inconsistently split. // This matches the case where a bucket has been split into 2 on one diff --git a/storage/src/vespa/storage/distributor/statechecker.cpp b/storage/src/vespa/storage/distributor/statechecker.cpp index 12dbd7f3c52..f7ef1122692 100644 --- a/storage/src/vespa/storage/distributor/statechecker.cpp +++ b/storage/src/vespa/storage/distributor/statechecker.cpp @@ -66,6 +66,7 @@ StateChecker::Context::Context(const DistributorComponent& c, : bucket(bucket_), siblingBucket(c.getSibling(bucket.getBucketId())), systemState(distributorBucketSpace.getClusterState()), + pending_cluster_state(c.getDistributor().pendingClusterStateOrNull(bucket_.getBucketSpace())), distributorConfig(c.getDistributor().getConfig()), distribution(distributorBucketSpace.getDistribution()), gcTimeCalculator(c.getDistributor().getBucketIdHasher(), @@ -75,12 +76,11 @@ StateChecker::Context::Context(const DistributorComponent& c, db(distributorBucketSpace.getBucketDatabase()), stats(statsTracker) { - idealState = - distribution.getIdealStorageNodes(systemState, bucket.getBucketId()); + idealState = distribution.getIdealStorageNodes(systemState, bucket.getBucketId()); unorderedIdealState.insert(idealState.begin(), idealState.end()); } -StateChecker::Context::~Context() {} +StateChecker::Context::~Context() = default; std::string StateChecker::Context::toString() const @@ -99,12 +99,7 @@ StateChecker::Context::toString() const return ss.str(); } -StateChecker::StateChecker() -{ -} - -StateChecker::~StateChecker() -{ -} +StateChecker::StateChecker() = default; +StateChecker::~StateChecker() = default; } diff --git a/storage/src/vespa/storage/distributor/statechecker.h b/storage/src/vespa/storage/distributor/statechecker.h index 9c0fe2ef53c..5b9ce065f99 100644 --- a/storage/src/vespa/storage/distributor/statechecker.h +++ b/storage/src/vespa/storage/distributor/statechecker.h @@ -64,6 +64,7 @@ public: // Common const lib::ClusterState& systemState; + const lib::ClusterState* pending_cluster_state; // nullptr if no state is pending. const DistributorConfiguration& distributorConfig; const lib::Distribution& distribution; diff --git a/storage/src/vespa/storage/distributor/statecheckers.cpp b/storage/src/vespa/storage/distributor/statecheckers.cpp index 0a0abb8d417..dab2025bfbb 100644 --- a/storage/src/vespa/storage/distributor/statecheckers.cpp +++ b/storage/src/vespa/storage/distributor/statecheckers.cpp @@ -584,18 +584,29 @@ SplitInconsistentStateChecker::check(StateChecker::Context& c) } namespace { + bool containsMaintenanceNode(const std::vector<uint16_t>& ideal, const StateChecker::Context& c) { - for (uint32_t i = 0; i < ideal.size(); i++) { - if (c.systemState.getNodeState(lib::Node(lib::NodeType::STORAGE, - ideal[i])).getState() - == lib::State::MAINTENANCE) - { + for (uint16_t n : ideal) { + lib::Node node(lib::NodeType::STORAGE, n); + if (c.systemState.getNodeState(node).getState() == lib::State::MAINTENANCE) { return true; } } + return false; +} +bool ideal_node_is_unavailable_in_pending_state(const StateChecker::Context& c) { + if (!c.pending_cluster_state) { + return false; + } + for (uint16_t n : c.idealState) { + lib::Node node(lib::NodeType::STORAGE, n); + if (!c.pending_cluster_state->getNodeState(node).getState().oneOf("uir")){ + return true; + } + } return false; } @@ -715,7 +726,7 @@ private: uint8_t _priority; }; -MergeNodes::~MergeNodes() {} +MergeNodes::~MergeNodes() = default; bool presentInIdealState(const StateChecker::Context& c, uint16_t node) @@ -830,6 +841,9 @@ SynchronizeAndMoveStateChecker::check(StateChecker::Context& c) if (containsMaintenanceNode(c.idealState, c)) { return Result::noMaintenanceNeeded(); } + if (ideal_node_is_unavailable_in_pending_state(c)) { + return Result::noMaintenanceNeeded(); + } if (allCopiesAreInvalid(c)) { return Result::noMaintenanceNeeded(); } |