diff options
84 files changed, 1374 insertions, 575 deletions
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java index 7881ef70ebb..5e7c0d9a7a0 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java @@ -219,6 +219,13 @@ public class VespaMetricSet { metrics.add(new Metric("requestsOverQuota.rate")); metrics.add(new Metric("requestsOverQuota.count")); + metrics.add(new Metric("relevance.at_1.average")); + metrics.add(new Metric("relevance.at_1.count")); + metrics.add(new Metric("relevance.at_3.average")); + metrics.add(new Metric("relevance.at_3.count")); + metrics.add(new Metric("relevance.at_10.average")); + metrics.add(new Metric("relevance.at_10.count")); + // Errors from qrserver metrics.add(new Metric("error.timeout.rate")); metrics.add(new Metric("error.backends_oos.rate")); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2Builder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2Builder.java index 0c9f747b767..9b420c68e0b 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2Builder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2Builder.java @@ -54,6 +54,10 @@ public class DomAdminV2Builder extends DomAdminBuilderBase { ModelElement adminElement = new ModelElement(adminE); addLogForwarders(adminElement.getChild("logforwarding"), admin); + + if (adminElement.getChild("filedistribution") != null) { + deployState.getDeployLogger().log(LogLevel.WARNING, "'filedistribution' element is deprecated and ignored"); + } } private List<Configserver> parseConfigservers(DeployState deployState, Admin admin, Element adminE) { diff --git a/config-provisioning/src/main/resources/configdefinitions/node-repository.def b/config-provisioning/src/main/resources/configdefinitions/node-repository.def index 4af8806fbbb..8ea9265aa23 100644 --- a/config-provisioning/src/main/resources/configdefinitions/node-repository.def +++ b/config-provisioning/src/main/resources/configdefinitions/node-repository.def @@ -6,6 +6,3 @@ namespace=config.provisioning dockerImage string default="dummyImage" useCuratorClientCache bool default=false - -# Comma separated list of hostnames that are authorized for all config server REST APIs -hostnameWhitelist string default="" diff --git a/configd/src/apps/sentinel/config-handler.cpp b/configd/src/apps/sentinel/config-handler.cpp index 4ab8b00916d..d9f4d1f8bd7 100644 --- a/configd/src/apps/sentinel/config-handler.cpp +++ b/configd/src/apps/sentinel/config-handler.cpp @@ -384,6 +384,7 @@ ConfigHandler::handleCmd(const Cmd& cmd) char retbuf[65536]; size_t left = 65536; size_t pos = 0; + retbuf[pos] = 0; for (ServiceMap::iterator it(_services.begin()), mt(_services.end()); it != mt; it++) { Service *service = it->second.get(); const SentinelConfig::Service& config = service->serviceConfig(); @@ -397,6 +398,7 @@ ConfigHandler::handleCmd(const Cmd& cmd) left -= sz; if (left <= 0) break; } + retbuf[65535] = 0; cmd.retValue(retbuf); } break; diff --git a/configdefinitions/src/vespa/configserver.def b/configdefinitions/src/vespa/configserver.def index de8f1b7cac8..75d11ef46df 100644 --- a/configdefinitions/src/vespa/configserver.def +++ b/configdefinitions/src/vespa/configserver.def @@ -67,6 +67,7 @@ maxDurationOfBootstrap long default=7200 # How long to sleep before redeploying again if it fails (in seconds) sleepTimeWhenRedeployingFails long default=30 -# Feature Flags (poor man's feature flags, to be overridden in configserver-config.xml if needed) +# Features (to be overridden in configserver-config.xml if needed) buildMinimalSetOfConfigModels bool default=true throwIfBootstrappingTenantRepoFails bool default=true +canReturnEmptySentinelConfig bool default=false diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/GetConfigProcessor.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/GetConfigProcessor.java index a7c04f33d5f..bbdef71129a 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/GetConfigProcessor.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/GetConfigProcessor.java @@ -84,7 +84,7 @@ class GetConfigProcessor implements Runnable { // If we are certain that this request is from a node that no longer belongs to this application, // fabricate an empty request to cause the sentinel to stop all running services - if (rpcServer.isHostedVespa() && rpcServer.allTenantsLoaded() && !tenant.isPresent() && isSentinelConfigRequest(request)) { + if (rpcServer.canReturnEmptySentinelConfig() && rpcServer.allTenantsLoaded() && tenant.isEmpty() && isSentinelConfigRequest(request)) { returnEmpty(request); return null; } @@ -164,6 +164,7 @@ class GetConfigProcessor implements Runnable { } private void returnEmpty(JRTServerConfigRequest request) { + log.log(LogLevel.INFO, "Returning empty sentinel config for request from " + request.getClientHostName()); ConfigPayload emptyPayload = ConfigPayload.empty(); String configMd5 = ConfigUtils.getMd5(emptyPayload); ConfigResponse config = SlimeConfigResponse.fromConfigPayload(emptyPayload, null, 0, false, configMd5); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/RpcServer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/RpcServer.java index 061a3b3302b..1df72522310 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/RpcServer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/RpcServer.java @@ -84,6 +84,7 @@ public class RpcServer implements Runnable, ReloadListener, TenantListener { private Spec spec; private final boolean useRequestVersion; private final boolean hostedVespa; + private final boolean canReturnEmptySentinelConfig; private static final Logger log = Logger.getLogger(RpcServer.class.getName()); @@ -136,6 +137,7 @@ public class RpcServer implements Runnable, ReloadListener, TenantListener { hostRegistry = hostRegistries.getTenantHostRegistry(); this.useRequestVersion = config.useVespaVersionInRequest(); this.hostedVespa = config.hostedVespa(); + this.canReturnEmptySentinelConfig = config.canReturnEmptySentinelConfig(); this.fileServer = fileServer; downloader = fileServer.downloader(); setUpHandlers(); @@ -441,6 +443,10 @@ public class RpcServer implements Runnable, ReloadListener, TenantListener { /** Returns true if this rpc server is currently running in a hosted Vespa configuration */ public boolean isHostedVespa() { return hostedVespa; } + + /** Returns true if empty sentinel config can be returned when a request from a host that is + * not part of an application asks for sentinel config */ + public boolean canReturnEmptySentinelConfig() { return canReturnEmptySentinelConfig; } MetricUpdaterFactory metricUpdaterFactory() { return metricUpdaterFactory; diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/GetConfigProcessorTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/GetConfigProcessorTest.java deleted file mode 100644 index 28f111a04d2..00000000000 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/GetConfigProcessorTest.java +++ /dev/null @@ -1,128 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.server.rpc; - -import com.yahoo.cloud.config.SentinelConfig; -import com.yahoo.config.provision.TenantName; -import com.yahoo.text.Utf8; -import com.yahoo.text.Utf8Array; -import com.yahoo.text.Utf8String; -import com.yahoo.vespa.config.ConfigKey; - -import com.yahoo.vespa.config.protocol.CompressionInfo; -import com.yahoo.vespa.config.protocol.CompressionType; -import com.yahoo.vespa.config.protocol.ConfigResponse; -import com.yahoo.vespa.config.protocol.DefContent; -import com.yahoo.vespa.config.protocol.JRTClientConfigRequestV3; -import com.yahoo.vespa.config.protocol.JRTServerConfigRequest; -import com.yahoo.vespa.config.protocol.JRTServerConfigRequestV3; -import com.yahoo.vespa.config.protocol.Trace; -import com.yahoo.vespa.config.server.tenant.MockTenantProvider; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.TemporaryFolder; - -import static org.junit.Assert.assertFalse; - -import java.io.IOException; -import java.io.OutputStream; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; -import java.util.Optional; - -import static org.hamcrest.core.Is.is; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; - -/** - * @author Ulf Lilleengen - */ -public class GetConfigProcessorTest { - - @Rule - public TemporaryFolder temporaryFolder = new TemporaryFolder(); - - @Test - public void testSentinelConfig() throws IOException { - MockRpc rpc = new MockRpc(13337, false, temporaryFolder.newFolder()); - rpc.response = new MockConfigResponse("foo"); // should be a sentinel config, but it does not matter for this test - - // one tenant, which has host1 assigned - boolean pretendToHaveLoadedApplications = true; - TenantName testTenant = TenantName.from("test"); - rpc.onTenantCreate(testTenant, new MockTenantProvider(pretendToHaveLoadedApplications)); - rpc.hostsUpdated(testTenant, Collections.singleton("host1")); - - { // a config is returned normally - JRTServerConfigRequest req = createV3SentinelRequest("host1"); - GetConfigProcessor proc = new GetConfigProcessor(rpc, req, false); - proc.run(); - assertTrue(rpc.tryResolveConfig); - assertTrue(rpc.tryRespond); - assertThat(rpc.errorCode, is(0)); - } - - rpc.resetChecks(); - // host1 is replaced by host2 for this tenant - rpc.hostsUpdated(testTenant, Collections.singleton("host2")); - - { // this causes us to get an empty config instead of normal config resolution - JRTServerConfigRequest req = createV3SentinelRequest("host1"); - GetConfigProcessor proc = new GetConfigProcessor(rpc, req, false); - proc.run(); - assertFalse(rpc.tryResolveConfig); // <-- no normal config resolution happening - assertTrue(rpc.tryRespond); - assertThat(rpc.errorCode, is(0)); - } - } - - private static JRTServerConfigRequest createV3SentinelRequest(String fromHost) { - final ConfigKey<?> configKey = new ConfigKey<>(SentinelConfig.CONFIG_DEF_NAME, "myid", SentinelConfig.CONFIG_DEF_NAMESPACE); - return JRTServerConfigRequestV3.createFromRequest(JRTClientConfigRequestV3. - createWithParams(configKey, DefContent.fromList(Arrays.asList(SentinelConfig.CONFIG_DEF_SCHEMA)), - fromHost, "", 0, 100, Trace.createDummy(), CompressionType.UNCOMPRESSED, - Optional.empty()).getRequest()); - } - - private class MockConfigResponse implements ConfigResponse { - - private final String line; - public MockConfigResponse(String line) { - this.line = line; - } - - @Override - public Utf8Array getPayload() { - return new Utf8String(""); - } - - @Override - public List<String> getLegacyPayload() { - return Arrays.asList(line); - } - - @Override - public long getGeneration() { - return 1; - } - - @Override - public boolean isInternalRedeploy() { return false; } - - @Override - public String getConfigMd5() { - return "mymd5"; - } - - @Override - public void serialize(OutputStream os, CompressionType uncompressed) throws IOException { - os.write(Utf8.toBytes(line)); - } - - @Override - public CompressionInfo getCompressionInfo() { - return CompressionInfo.uncompressed(); - } - } - -} diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcServerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcServerTest.java index 972446ca267..c8bc9364922 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcServerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcServerTest.java @@ -2,7 +2,9 @@ package com.yahoo.vespa.config.server.rpc; import com.google.common.base.Joiner; +import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.cloud.config.LbServicesConfig; +import com.yahoo.cloud.config.SentinelConfig; import com.yahoo.config.SimpletypesConfig; import com.yahoo.config.codegen.DefParser; import com.yahoo.config.codegen.InnerCNode; @@ -55,11 +57,12 @@ public class RpcServerTest { testPrintStatistics(tester); testGetConfig(tester); testEnabled(tester); - testEmptyConfigHostedVespa(tester); + testApplicationNotLoadedErrorWhenAppDeleted(tester); + testEmptySentinelConfigWhenAppDeletedOnHostedVespa(); } } - private void testEmptyConfigHostedVespa(RpcTester tester) throws InterruptedException, IOException { + private void testApplicationNotLoadedErrorWhenAppDeleted(RpcTester tester) throws InterruptedException, IOException { tester.rpcServer().onTenantDelete(TenantName.defaultName()); tester.rpcServer().onTenantsLoaded(); JRTClientConfigRequest clientReq = createSimpleRequest(); @@ -74,6 +77,28 @@ public class RpcServerTest { assertTrue(clientReq.validateResponse()); } + @Test + public void testEmptySentinelConfigWhenAppDeletedOnHostedVespa() throws IOException, InterruptedException { + ConfigserverConfig.Builder configBuilder = new ConfigserverConfig.Builder().canReturnEmptySentinelConfig(true); + try (RpcTester tester = new RpcTester(temporaryFolder, configBuilder)) { + tester.rpcServer().onTenantDelete(TenantName.defaultName()); + tester.rpcServer().onTenantsLoaded(); + JRTClientConfigRequest clientReq = createSentinelRequest(); + + // Should get empty sentinel config when on hosted vespa + tester.performRequest(clientReq.getRequest()); + assertTrue(clientReq.validateResponse()); + assertEquals(0, clientReq.errorCode()); + + ConfigPayload payload = ConfigPayload.fromUtf8Array(clientReq.getNewPayload().getData()); + assertNotNull(payload); + SentinelConfig.Builder builder = new SentinelConfig.Builder(); + new ConfigPayloadApplier<>(builder).applyPayload(payload); + SentinelConfig config = new SentinelConfig(builder); + assertEquals(0, config.service().size()); + } + } + private JRTClientConfigRequest createSimpleRequest() { ConfigKey<?> key = new ConfigKey<>(SimpletypesConfig.class, ""); JRTClientConfigRequest clientReq = createRequest(new RawConfig(key, SimpletypesConfig.getDefMd5())); @@ -81,6 +106,13 @@ public class RpcServerTest { return clientReq; } + private JRTClientConfigRequest createSentinelRequest() { + ConfigKey<?> key = new ConfigKey<>(SentinelConfig.class, ""); + JRTClientConfigRequest clientReq = createRequest(new RawConfig(key, SentinelConfig.getDefMd5())); + assertTrue(clientReq.validateParameters()); + return clientReq; + } + private void testEnabled(RpcTester tester) throws IOException, SAXException { tester.generationCounter().increment(); Application app = new Application(new VespaModel(MockApplicationPackage.createEmpty()), diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcTester.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcTester.java index 3849fc899de..43a2c01f26a 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcTester.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcTester.java @@ -51,19 +51,25 @@ public class RpcTester implements AutoCloseable { private Thread t; private Supervisor sup; private Spec spec; - private int port; private List<Integer> allocatedPorts; private final TemporaryFolder temporaryFolder; + private final ConfigserverConfig configserverConfig; RpcTester(TemporaryFolder temporaryFolder) throws InterruptedException, IOException { + this(temporaryFolder, new ConfigserverConfig.Builder()); + } + + RpcTester(TemporaryFolder temporaryFolder, ConfigserverConfig.Builder configBuilder) throws InterruptedException, IOException { this.temporaryFolder = temporaryFolder; allocatedPorts = new ArrayList<>(); - port = allocatePort(); + int port = allocatePort(); spec = createSpec(port); tenantProvider = new MockTenantProvider(); generationCounter = new MemoryGenerationCounter(); + configBuilder.rpcport(port); + configserverConfig = new ConfigserverConfig(configBuilder); createAndStartRpcServer(); assertFalse(hostLivenessTracker.lastRequestFrom(myHostname).isPresent()); } @@ -81,11 +87,7 @@ public class RpcTester implements AutoCloseable { } void createAndStartRpcServer() throws IOException { - ConfigserverConfig configserverConfig = new ConfigserverConfig(new ConfigserverConfig.Builder()); - rpcServer = new RpcServer(new ConfigserverConfig(new ConfigserverConfig.Builder() - .rpcport(port) - .numRpcThreads(1) - .maxgetconfigclients(1)), + rpcServer = new RpcServer(configserverConfig, new SuperModelRequestHandler(new TestConfigDefinitionRepo(), configserverConfig, new SuperModelManager( 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 c9a156faede..db724b41657 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 @@ -320,7 +320,7 @@ public class ClusterSearcher extends Searcher { doFill(searcher, result, summaryClass, execution); } else { if (result.hits().getErrorHit() == null) { - result.hits().addError(ErrorMessage.createTimeout("No time left to get summaries")); + result.hits().addError(ErrorMessage.createTimeout("No time left to get summaries, query timeout was " + query.getTimeout() + " ms")); } } } else { diff --git a/container-search/src/main/java/com/yahoo/prelude/statistics/StatisticsSearcher.java b/container-search/src/main/java/com/yahoo/prelude/statistics/StatisticsSearcher.java index 3606e01ffe5..086ceb514fa 100644 --- a/container-search/src/main/java/com/yahoo/prelude/statistics/StatisticsSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/statistics/StatisticsSearcher.java @@ -15,6 +15,8 @@ import com.yahoo.search.Searcher; import com.yahoo.search.result.Coverage; import com.yahoo.search.result.ErrorHit; import com.yahoo.search.result.ErrorMessage; +import com.yahoo.search.result.Hit; +import com.yahoo.search.result.HitGroup; import com.yahoo.search.searchchain.Execution; import com.yahoo.search.searchchain.PhaseNames; import com.yahoo.statistics.Counter; @@ -23,6 +25,8 @@ import com.yahoo.statistics.Value; import java.util.HashMap; import java.util.Map; import java.util.Optional; +import java.util.PriorityQueue; +import java.util.Queue; import java.util.logging.Level; import static com.yahoo.container.protect.Error.*; @@ -57,6 +61,9 @@ public class StatisticsSearcher extends Searcher { private static final String DOCS_COVERED_METRIC = "documents_covered"; private static final String DOCS_TOTAL_METRIC = "documents_total"; private static final String DEGRADED_METRIC = "degraded_queries"; + private static final String RELEVANCE_AT_1_METRIC = "relevance.at_1"; + private static final String RELEVANCE_AT_3_METRIC = "relevance.at_3"; + private static final String RELEVANCE_AT_10_METRIC = "relevance.at_10"; private final Counter queries; // basic counter private final Counter failedQueries; // basic counter @@ -79,6 +86,7 @@ public class StatisticsSearcher extends Searcher { private Map<String, Metric.Context> chainContexts = new CopyOnWriteHashMap<>(); private Map<String, Metric.Context> statePageOnlyContexts = new CopyOnWriteHashMap<>(); private Map<String, Map<DegradedReason, Metric.Context>> degradedReasonContexts = new CopyOnWriteHashMap<>(); + private Map<String, Map<String, Metric.Context>> relevanceContexts = new CopyOnWriteHashMap<>(); private java.util.Timer scheduler = new java.util.Timer(true); private class PeakQpsReporter extends java.util.TimerTask { @@ -184,6 +192,29 @@ public class StatisticsSearcher extends Searcher { return DegradedReason.non_ideal_state; } + private Metric.Context createRelevanceMetricContext(String chainName, String rankProfile) { + Map<String, String> dimensions = new HashMap<>(); + dimensions.put("chain", chainName); + dimensions.put("rankProfile", rankProfile); + return metric.createContext(dimensions); + } + + private Metric.Context getRelevanceMetricContext(Execution execution, Query query) { + String chain = execution.chain().getId().stringValue(); + String rankProfile = query.getRanking().getProfile(); + Map<String, Metric.Context> chainContext = relevanceContexts.get(chain); // CopyOnWriteHashMap - don't use computeIfAbsent + if (chainContext == null) { + chainContext = new CopyOnWriteHashMap<>(); + relevanceContexts.put(chain, chainContext); + } + Metric.Context metricContext = chainContext.get(rankProfile); + if (metricContext == null) { + metricContext = createRelevanceMetricContext(chain, rankProfile); + chainContext.put(rankProfile, metricContext); + } + return metricContext; + } + /** * Generate statistics for the query passing through this Searcher * 1) Add 1 to total query count @@ -242,12 +273,12 @@ public class StatisticsSearcher extends Searcher { metric.add(EMPTY_RESULTS_METRIC, 1, metricContext); } - // Update running averages - //setAverages(); + addRelevanceMetrics(query, execution, result); return result; } + private void logQuery(com.yahoo.search.Query query) { // Don't parse the query if it's not necessary for the logging Query.toString triggers parsing if (getLogger().isLoggable(Level.FINER)) { @@ -342,5 +373,49 @@ public class StatisticsSearcher extends Searcher { return context; } + /** + * Effectively flattens the hits, and measures relevance @ 1, 3, and 10 + */ + private void addRelevanceMetrics(Query query, Execution execution, Result result) { + Queue<Double> topScores = findTopRelevanceScores(10, result.hits()); + if (topScores.isEmpty()) { + return; + } + Metric.Context metricContext = getRelevanceMetricContext(execution, query); + setRelevanceMetric(10, RELEVANCE_AT_10_METRIC, topScores, metricContext); // min-queue: lowest values are polled first + setRelevanceMetric(3, RELEVANCE_AT_3_METRIC, topScores, metricContext); + setRelevanceMetric(1, RELEVANCE_AT_1_METRIC, topScores, metricContext); + } + + private static Queue<Double> findTopRelevanceScores(int n, HitGroup hits) { + PriorityQueue<Double> heap = new PriorityQueue<>(n); + for (var iterator = hits.unorderedDeepIterator(); iterator.hasNext(); ) { + Hit hit = iterator.next(); + if (hit instanceof ErrorHit || hit.getRelevance() == null) { + continue; + } + double score = hit.getRelevance().getScore(); + if (Double.isNaN(score)) { + continue; + } + if (heap.size() < n) { + heap.add(score); + } else if (score > heap.peek()) { + heap.remove(); + heap.add(score); + } + } + return heap; + } + + private void setRelevanceMetric(int pos, String name, Queue<Double> minQueue, Metric.Context context) { + while (minQueue.size() > pos) { + minQueue.remove(); + } + if (minQueue.size() == pos) { + metric.set(name, minQueue.poll(), context); + } + } + } diff --git a/container-search/src/test/java/com/yahoo/search/federation/test/FederationTester.java b/container-search/src/test/java/com/yahoo/search/federation/test/FederationTester.java index 794fa9349f3..bfe51c15d46 100644 --- a/container-search/src/test/java/com/yahoo/search/federation/test/FederationTester.java +++ b/container-search/src/test/java/com/yahoo/search/federation/test/FederationTester.java @@ -53,7 +53,7 @@ class FederationTester { public Result search() { Query query = new Query(); query.setTimeout(60 * 1000); - return search(new Query()); + return search(query); } public Result search(Query query) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index f51bccff5d1..d53d3137991 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -227,7 +227,7 @@ public class ApplicationController { if (asList(id.tenant()).stream().noneMatch(application -> application.id().application().equals(id.application()))) com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId.validate(id.application().value()); - Optional<Tenant> tenant = controller.tenants().tenant(id.tenant()); + Optional<Tenant> tenant = controller.tenants().get(id.tenant()); if ( ! tenant.isPresent()) throw new IllegalArgumentException("Could not create '" + id + "': This tenant does not exist"); if (get(id).isPresent()) @@ -555,7 +555,7 @@ public class ApplicationController { if ( ! application.get().deployments().isEmpty()) throw new IllegalArgumentException("Could not delete '" + application + "': It has active deployments"); - Tenant tenant = controller.tenants().tenant(id.tenant()).get(); + Tenant tenant = controller.tenants().get(id.tenant()).get(); if (tenant instanceof AthenzTenant && ! token.isPresent()) throw new IllegalArgumentException("Could not delete '" + application + "': No Okta Access Token provided"); @@ -728,7 +728,7 @@ public class ApplicationController { public void verifyApplicationIdentityConfiguration(TenantName tenantName, ApplicationPackage applicationPackage, Optional<AthenzIdentity> deployingIdentity) { applicationPackage.deploymentSpec().athenzDomain() .ifPresent(identityDomain -> { - Optional<Tenant> tenant = controller.tenants().tenant(tenantName); + Optional<Tenant> tenant = controller.tenants().get(tenantName); if(!tenant.isPresent()) { throw new IllegalArgumentException("Tenant does not exist"); } else { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java index 590b18f929d..e69e28b6432 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java @@ -6,88 +6,142 @@ import com.yahoo.vespa.athenz.api.AthenzDomain; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.api.identifiers.Property; import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; -import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; +import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; +import com.yahoo.vespa.hosted.controller.tenant.BillingInfo; +import com.yahoo.vespa.hosted.controller.tenant.CloudTenant; import com.yahoo.vespa.hosted.controller.tenant.Tenant; import com.yahoo.vespa.hosted.controller.tenant.UserTenant; -import java.util.Objects; import java.util.Optional; -import java.util.function.Consumer; + +import static java.util.Objects.requireNonNull; /** * A tenant that has been locked for modification. Provides methods for modifying a tenant's fields. * * @author mpolden + * @author jonmv */ -public class LockedTenant { - - private final Lock lock; - private final TenantName name; - private AthenzDomain domain; - private Property property; - private Optional<PropertyId> propertyId; - private final Optional<Contact> contact; - private final boolean isAthenzTenant; - - /** - * Should never be constructed directly. - * - * Use {@link TenantController#lockIfPresent(TenantName, Consumer)} or - * {@link TenantController#lockOrThrow(TenantName, Consumer)} - */ - LockedTenant(AthenzTenant tenant, Lock lock) { - this(lock, tenant.name(), tenant.domain(), tenant.property(), tenant.propertyId(), tenant.contact()); - } +public abstract class LockedTenant { - LockedTenant(UserTenant tenant, Lock lock) { - this(lock, tenant.name(), tenant.contact()); - } + final TenantName name; - private LockedTenant(Lock lock, TenantName name, AthenzDomain domain, Property property, - Optional<PropertyId> propertyId, Optional<Contact> contact) { - this.lock = Objects.requireNonNull(lock, "lock must be non-null"); - this.name = Objects.requireNonNull(name, "name must be non-null"); - this.domain = Objects.requireNonNull(domain, "domain must be non-null"); - this.property = Objects.requireNonNull(property, "property must be non-null"); - this.propertyId = Objects.requireNonNull(propertyId, "propertyId must be non-null"); - this.contact = Objects.requireNonNull(contact, "contact must be non-null"); - this.isAthenzTenant = true; + private LockedTenant(TenantName name) { + this.name = requireNonNull(name); } - private LockedTenant(Lock lock, TenantName name, Optional<Contact> contact) { - this.lock = Objects.requireNonNull(lock, "lock must be non-null"); - this.name = Objects.requireNonNull(name, "name must be non-null"); - this.contact = Objects.requireNonNull(contact, "contact must be non-null"); - this.isAthenzTenant = false; + static LockedTenant of(Tenant tenant, Lock lock) { + switch (tenant.type()) { + case athenz: return new Athenz((AthenzTenant) tenant); + case user: return new User((UserTenant) tenant); + case cloud: return new Cloud((CloudTenant) tenant); + default: throw new IllegalArgumentException("Unexpected tenant type '" + tenant.getClass().getName() + "'."); + } } /** Returns a read-only copy of this */ - public Tenant get() { - if (isAthenzTenant) return new AthenzTenant(name, domain, property, propertyId, contact); - else return new UserTenant(name, contact); - } + public abstract Tenant get(); - public LockedTenant with(AthenzDomain domain) { - return new LockedTenant(lock, name, domain, property, propertyId, contact); + @Override + public String toString() { + return "tenant '" + name + "'"; } - public LockedTenant with(Property property) { - return new LockedTenant(lock, name, domain, property, propertyId, contact); - } - public LockedTenant with(PropertyId propertyId) { - return new LockedTenant(lock, name, domain, property, Optional.of(propertyId), contact); + /** A locked AthenzTenant. */ + public static class Athenz extends LockedTenant { + + private final AthenzDomain domain; + private final Property property; + private final Optional<PropertyId> propertyId; + private final Optional<Contact> contact; + + private Athenz(TenantName name, AthenzDomain domain, Property property, Optional<PropertyId> propertyId, Optional<Contact> contact) { + super(name); + this.domain = domain; + this.property = property; + this.propertyId = propertyId; + this.contact = contact; + } + + private Athenz(AthenzTenant tenant) { + this(tenant.name(), tenant.domain(), tenant.property(), tenant.propertyId(), tenant.contact()); + } + + @Override + public AthenzTenant get() { + return new AthenzTenant(name, domain, property, propertyId, contact); + } + + public Athenz with(AthenzDomain domain) { + return new Athenz(name, domain, property, propertyId, contact); + } + + public Athenz with(Property property) { + return new Athenz(name, domain, property, propertyId, contact); + } + + public Athenz with(PropertyId propertyId) { + return new Athenz(name, domain, property, Optional.of(propertyId), contact); + } + + public Athenz with(Contact contact) { + return new Athenz(name, domain, property, propertyId, Optional.of(contact)); + } + } - public LockedTenant with(Contact contact) { - if (isAthenzTenant) return new LockedTenant(lock, name, domain, property, propertyId, Optional.of(contact)); - return new LockedTenant(lock, name, Optional.of(contact)); + + /** A locked UserTenant. */ + public static class User extends LockedTenant { + + private final Optional<Contact> contact; + + private User(TenantName name, Optional<Contact> contact) { + super(name); + this.contact = contact; + } + + private User(UserTenant tenant) { + this(tenant.name(), tenant.contact()); + } + + @Override + public UserTenant get() { + return new UserTenant(name, contact); + } + + public User with(Contact contact) { + return new User(name, Optional.of(contact)); + } + } - @Override - public String toString() { - return "tenant '" + name + "'"; + + /** A locked CloudTenant. */ + public static class Cloud extends LockedTenant { + + private final BillingInfo billingInfo; + + private Cloud(TenantName name, BillingInfo billingInfo) { + super(name); + this.billingInfo = billingInfo; + } + + private Cloud(CloudTenant tenant) { + this(tenant.name(), tenant.billingInfo()); + } + + @Override + public CloudTenant get() { + return new CloudTenant(name, billingInfo); + } + + public Cloud with(BillingInfo billingInfo) { + return new Cloud(name, billingInfo); + } + } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java index 78099fac34e..e274fc2fe87 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java @@ -60,7 +60,7 @@ public class TenantController { for (TenantName name : curator.readTenantNames()) { try (Lock lock = lock(name)) { // Get while holding lock so that we know we're operating on a current version - Optional<Tenant> optionalTenant = tenant(name); + Optional<Tenant> optionalTenant = get(name); if (!optionalTenant.isPresent()) continue; // Deleted while updating, skip Tenant tenant = optionalTenant.get(); @@ -96,27 +96,27 @@ public class TenantController { .collect(Collectors.toList()); } - /** - * Lock a tenant for modification and apply action. Only valid for Athenz tenants as it's the only type that - * accepts modification. - */ - public void lockIfPresent(TenantName name, Consumer<LockedTenant> action) { + /** Locks a tenant for modification and applies the given action. */ + public <T extends LockedTenant> void lockIfPresent(TenantName name, Class<T> token, Consumer<T> action) { try (Lock lock = lock(name)) { - tenant(name).map(tenant -> { - tenant = tenant instanceof AthenzTenant ? (AthenzTenant) tenant : (UserTenant) tenant; - if (tenant instanceof AthenzTenant) return new LockedTenant((AthenzTenant) tenant, lock); - else return new LockedTenant((UserTenant) tenant, lock); - }).ifPresent(action); + get(name).map(tenant -> LockedTenant.of(tenant, lock)) + .map(token::cast) + .ifPresent(action); } } /** Lock a tenant for modification and apply action. Throws if the tenant does not exist */ - public void lockOrThrow(TenantName name, Consumer<LockedTenant> action) { + public <T extends LockedTenant> void lockOrThrow(TenantName name, Class<T> token, Consumer<T> action) { try (Lock lock = lock(name)) { - action.accept(new LockedTenant(requireAthenzTenant(name), lock)); + action.accept(token.cast(LockedTenant.of(require(name), lock))); } } + /** Returns the tenant with the given name, or throws. */ + public Tenant require(TenantName name) { + return get(name).orElseThrow(() -> new IllegalArgumentException("No such tenant '" + name + "'.")); + } + /** Replace and store any previous version of given tenant */ public void store(LockedTenant tenant) { curator.writeTenant(tenant.get()); @@ -156,18 +156,20 @@ public class TenantController { } /** Find tenant by name */ - public Optional<Tenant> tenant(TenantName name) { + public Optional<Tenant> get(TenantName name) { return curator.readTenant(name); } /** Find tenant by name */ - public Optional<Tenant> tenant(String name) { - return tenant(TenantName.from(name)); + public Optional<Tenant> get(String name) { + return get(TenantName.from(name)); } /** Find Athenz tenant by name */ public Optional<AthenzTenant> athenzTenant(TenantName name) { - return curator.readAthenzTenant(name); + return curator.readTenant(name) + .filter(AthenzTenant.class::isInstance) + .map(AthenzTenant.class::cast); } /** Returns Athenz tenant with name or throws if no such tenant exists */ @@ -176,8 +178,8 @@ public class TenantController { } /** Update Athenz domain for tenant. Returns the updated tenant which must be explicitly stored */ - public LockedTenant withDomain(LockedTenant tenant, AthenzDomain newDomain, OktaAccessToken token) { - AthenzTenant athenzTenant = (AthenzTenant) tenant.get(); + public LockedTenant.Athenz withDomain(LockedTenant.Athenz tenant, AthenzDomain newDomain, OktaAccessToken token) { + AthenzTenant athenzTenant = tenant.get(); AthenzDomain existingDomain = athenzTenant.domain(); if (existingDomain.equals(newDomain)) return tenant; Optional<Tenant> existingTenantWithNewDomain = tenantIn(newDomain); @@ -219,10 +221,10 @@ public class TenantController { } private void requireNonExistent(TenantName name) { - if (tenant(name).isPresent() || + if (get(name).isPresent() || // Underscores are allowed in existing Athenz tenant names, but tenants with - and _ cannot co-exist. E.g. // my-tenant cannot be created if my_tenant exists. - tenant(dashToUnderscore(name.value())).isPresent()) { + get(dashToUnderscore(name.value())).isPresent()) { throw new IllegalArgumentException("Tenant '" + name + "' already exists"); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationPackage.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationPackage.java index 131164c5c5e..78500d26865 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationPackage.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationPackage.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.application; +import com.google.common.collect.ImmutableMap; import com.google.common.hash.Hashing; import com.yahoo.component.Version; import com.yahoo.config.application.api.DeploymentSpec; @@ -13,9 +14,11 @@ import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStreamReader; import java.io.Reader; +import java.io.UncheckedIOException; import java.time.Instant; import java.util.Objects; import java.util.Optional; +import java.util.Set; /** * A representation of the content of an application package. @@ -43,10 +46,11 @@ public class ApplicationPackage { public ApplicationPackage(byte[] zippedContent) { this.zippedContent = Objects.requireNonNull(zippedContent, "The application package content cannot be null"); this.contentHash = Hashing.sha1().hashBytes(zippedContent).toString(); - this.deploymentSpec = extractFile("deployment.xml", zippedContent).map(DeploymentSpec::fromXml).orElse(DeploymentSpec.empty); - this.validationOverrides = extractFile("validation-overrides.xml", zippedContent).map(ValidationOverrides::fromXml).orElse(ValidationOverrides.empty); - Optional<Inspector> buildMetaObject = extractFileBytes("build-meta.json", zippedContent) - .map(SlimeUtils::jsonToSlime).map(Slime::get); + + Files files = Files.extract(Set.of("deployment.xml", "validation-overrides.xml", "build-meta.json"), zippedContent); + this.deploymentSpec = files.getAsReader("deployment.xml").map(DeploymentSpec::fromXml).orElse(DeploymentSpec.empty); + this.validationOverrides = files.getAsReader("validation-overrides.xml").map(ValidationOverrides::fromXml).orElse(ValidationOverrides.empty); + Optional<Inspector> buildMetaObject = files.get("build-meta.json").map(SlimeUtils::jsonToSlime).map(Slime::get); this.compileVersion = buildMetaObject.map(object -> Version.fromString(object.field("compileVersion").asString())); this.buildTime = buildMetaObject.map(object -> Instant.ofEpochMilli(object.field("buildTime").asLong())); } @@ -75,21 +79,51 @@ public class ApplicationPackage { /** Returns the time this package was built, if known. */ public Optional<Instant> buildTime() { return buildTime; } - private static Optional<byte[]> extractFileBytes(String fileName, byte[] zippedContent) { - try (ByteArrayInputStream stream = new ByteArrayInputStream(zippedContent)) { - ZipStreamReader reader = new ZipStreamReader(stream); - for (ZipStreamReader.ZipEntryWithContent entry : reader.entries()) - if (entry.zipEntry().getName().equals(fileName) || entry.zipEntry().getName().equals("application/" + fileName)) // TODO: Remove application/ directory support - return Optional.of(entry.content()); - return Optional.empty(); + + private static class Files { + + /** Max size of each extracted file */ + private static final int maxSize = 10 * 1024 * 1024; // 10 MiB + + // TODO: Vespa 8: Remove application/ directory support + private static final String applicationDir = "application/"; + + private final ImmutableMap<String, byte[]> files; + + private Files(ImmutableMap<String, byte[]> files) { + this.files = files; + } + + public static Files extract(Set<String> filesToExtract, byte[] zippedContent) { + ImmutableMap.Builder<String, byte[]> builder = ImmutableMap.builder(); + try (ByteArrayInputStream stream = new ByteArrayInputStream(zippedContent)) { + ZipStreamReader reader = new ZipStreamReader(stream, + (name) -> filesToExtract.contains(withoutLegacyDir(name)), + maxSize); + for (ZipStreamReader.ZipEntryWithContent entry : reader.entries()) { + builder.put(withoutLegacyDir(entry.zipEntry().getName()), entry.content()); + } + } catch (IOException e) { + throw new UncheckedIOException("Exception reading application package", e); + } + return new Files(builder.build()); } - catch (IOException e) { - throw new IllegalArgumentException("Exception reading application package", e); + + /** Get content of given file name */ + public Optional<byte[]> get(String name) { + return Optional.ofNullable(files.get(name)); + } + + /** Get reader for the content of given file name */ + public Optional<Reader> getAsReader(String name) { + return get(name).map(ByteArrayInputStream::new).map(InputStreamReader::new); + } + + private static String withoutLegacyDir(String name) { + if (name.startsWith(applicationDir)) return name.substring(applicationDir.length()); + return name; } - } - private static Optional<Reader> extractFile(String fileName, byte[] zippedContent) { - return extractFileBytes(fileName, zippedContent).map(ByteArrayInputStream::new).map(InputStreamReader::new); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ZipStreamReader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ZipStreamReader.java index 69c846f2562..7576bcae8ee 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ZipStreamReader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ZipStreamReader.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.controller.application; import com.google.common.collect.ImmutableList; @@ -6,7 +6,11 @@ import com.google.common.collect.ImmutableList; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; +import java.io.UncheckedIOException; +import java.nio.file.Path; +import java.util.Arrays; import java.util.List; +import java.util.function.Predicate; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; @@ -14,50 +18,66 @@ import java.util.zip.ZipInputStream; * @author bratseth */ public class ZipStreamReader { - + private final ImmutableList<ZipEntryWithContent> entries; + private final int maxEntrySizeInBytes; - public ZipStreamReader(InputStream input) { + public ZipStreamReader(InputStream input, Predicate<String> entryNameMatcher, int maxEntrySizeInBytes) { + this.maxEntrySizeInBytes = maxEntrySizeInBytes; try (ZipInputStream zipInput = new ZipInputStream(input)) { ImmutableList.Builder<ZipEntryWithContent> builder = new ImmutableList.Builder<>(); ZipEntry zipEntry; - while (null != (zipEntry = zipInput.getNextEntry())) + while (null != (zipEntry = zipInput.getNextEntry())) { + if (!entryNameMatcher.test(requireName(zipEntry.getName()))) continue; builder.add(new ZipEntryWithContent(zipEntry, readContent(zipInput))); + } entries = builder.build(); - } - catch (IOException e) { - throw new IllegalArgumentException("IO error reading zip content", e); + } catch (IOException e) { + throw new UncheckedIOException("IO error reading zip content", e); } } - + private byte[] readContent(ZipInputStream zipInput) { try (ByteArrayOutputStream bis = new ByteArrayOutputStream()) { byte[] buffer = new byte[2048]; int read; - while ( -1 != (read = zipInput.read(buffer))) + long size = 0; + while ( -1 != (read = zipInput.read(buffer))) { + size += read; + if (size > maxEntrySizeInBytes) { + throw new IllegalArgumentException("Entry in zip content exceeded size limit of " + + maxEntrySizeInBytes + " bytes"); + } bis.write(buffer, 0, read); + } return bis.toByteArray(); - } - catch (IOException e) { - throw new IllegalArgumentException("Failed reading from zipped content", e); + } catch (IOException e) { + throw new UncheckedIOException("Failed reading from zipped content", e); } } - + public List<ZipEntryWithContent> entries() { return entries; } - + + private static String requireName(String name) { + IllegalArgumentException e = new IllegalArgumentException("Unexpected non-normalized path found in zip content"); + if (Arrays.asList(name.split("/")).contains("..")) throw e; + if (!name.equals(Path.of(name).normalize().toString())) throw e; + return name; + } + public static class ZipEntryWithContent { - + private final ZipEntry zipEntry; private final byte[] content; - + public ZipEntryWithContent(ZipEntry zipEntry, byte[] content) { this.zipEntry = zipEntry; this.content = content; } - + public ZipEntry zipEntry() { return zipEntry; } public byte[] content() { return content; } - + } - + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java index d5c84c38e91..a3b1aab8f40 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java @@ -52,11 +52,13 @@ public class ApplicationOwnershipConfirmer extends Maintainer { .forEach(application -> { try { Tenant tenant = tenantOf(application.id()); - Optional<IssueId> ourIssueId = application.ownershipIssueId(); - Contact contact = tenant.contact().orElseThrow(RuntimeException::new); - User assignee = determineAssignee(tenant, application); - ourIssueId = ownershipIssues.confirmOwnership(ourIssueId, application.id(), assignee, contact); - ourIssueId.ifPresent(issueId -> store(issueId, application.id())); + tenant.contact().ifPresent(contact -> { // TODO jvenstad: Makes sense to require, and run this only in main? + ownershipIssues.confirmOwnership(application.ownershipIssueId(), + application.id(), + determineAssignee(tenant, application), + contact) + .ifPresent(newIssueId -> store(newIssueId, application.id())); + }); } catch (RuntimeException e) { // Catch errors due to wrong data in the controller, or issues client timeout. log.log(Level.INFO, "Exception caught when attempting to file an issue for '" + application.id() + "': " + Exceptions.toMessageString(e)); @@ -70,11 +72,8 @@ public class ApplicationOwnershipConfirmer extends Maintainer { for (Application application : controller().applications().asList()) application.ownershipIssueId().ifPresent(issueId -> { try { - Optional<Contact> contact = Optional.of(application.id()) - .map(this::tenantOf) - .filter(t -> t instanceof AthenzTenant) - .flatMap(Tenant::contact); - ownershipIssues.ensureResponse(issueId, contact); + Tenant tenant = tenantOf(application.id()); + ownershipIssues.ensureResponse(issueId, tenant.type() == Tenant.Type.athenz ? tenant.contact() : Optional.empty()); } catch (RuntimeException e) { log.log(Level.INFO, "Exception caught when attempting to escalate issue with id '" + issueId + "': " + Exceptions.toMessageString(e)); @@ -105,7 +104,7 @@ public class ApplicationOwnershipConfirmer extends Maintainer { } private Tenant tenantOf(ApplicationId applicationId) { - return controller().tenants().tenant(applicationId.tenant()) + return controller().tenants().get(applicationId.tenant()) .orElseThrow(() -> new IllegalStateException("No tenant found for application " + applicationId)); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java index a02f28e371d..7dfbc135df9 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java @@ -1,13 +1,12 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.maintenance; -import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; -import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; -import com.yahoo.vespa.hosted.controller.api.integration.organization.ContactRetriever; import com.yahoo.config.provision.SystemName; import com.yahoo.log.LogLevel; import com.yahoo.vespa.hosted.controller.Controller; -import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; +import com.yahoo.vespa.hosted.controller.LockedTenant; +import com.yahoo.vespa.hosted.controller.TenantController; +import com.yahoo.vespa.hosted.controller.api.integration.organization.ContactRetriever; import com.yahoo.vespa.hosted.controller.tenant.Tenant; import com.yahoo.yolean.Exceptions; @@ -35,14 +34,19 @@ public class ContactInformationMaintainer extends Maintainer { @Override protected void maintain() { - for (Tenant tenant : controller().tenants().asList()) { + TenantController tenants = controller().tenants(); + for (Tenant tenant : tenants.asList()) { try { - Optional<PropertyId> tenantPropertyId = Optional.empty(); - if (tenant instanceof AthenzTenant) { - tenantPropertyId = ((AthenzTenant) tenant).propertyId(); + switch (tenant.type()) { + case athenz: tenants.lockIfPresent(tenant.name(), LockedTenant.Athenz.class, lockedTenant -> + tenants.store(lockedTenant.with(contactRetriever.getContact(lockedTenant.get().propertyId())))); + return; + case user: tenants.lockIfPresent(tenant.name(), LockedTenant.User.class, lockedTenant -> + tenants.store(lockedTenant.with(contactRetriever.getContact(Optional.empty())))); + return; + case cloud: return; + default: throw new IllegalArgumentException("Unexpected tenant type '" + tenant.type() + "'."); } - Contact contact = contactRetriever.getContact(tenantPropertyId); - controller().tenants().lockIfPresent(tenant.name(), lockedTenant -> controller().tenants().store(lockedTenant.with(contact))); } catch (Exception e) { log.log(LogLevel.WARNING, "Failed to update contact information for " + tenant + ": " + Exceptions.toMessageString(e) + ". Retrying in " + @@ -51,5 +55,4 @@ public class ContactInformationMaintainer extends Maintainer { } } - } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporter.java index 3f5c2d1f317..48e2702b9e0 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporter.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporter.java @@ -6,8 +6,6 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.SystemName; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; -import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; -import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; import com.yahoo.vespa.hosted.controller.api.integration.organization.DeploymentIssues; import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId; import com.yahoo.vespa.hosted.controller.api.integration.organization.User; @@ -106,7 +104,7 @@ public class DeploymentIssueReporter extends Maintainer { } private Tenant ownerOf(ApplicationId applicationId) { - return controller().tenants().tenant(applicationId.tenant()) + return controller().tenants().get(applicationId.tenant()) .orElseThrow(() -> new IllegalStateException("No tenant found for application " + applicationId)); } @@ -118,10 +116,12 @@ public class DeploymentIssueReporter extends Maintainer { private void fileDeploymentIssueFor(ApplicationId applicationId) { try { Tenant tenant = ownerOf(applicationId); - User asignee = tenant instanceof UserTenant ? userFor(tenant) : null; - Optional<IssueId> ourIssueId = controller().applications().require(applicationId).deploymentJobs().issueId(); - IssueId issueId = deploymentIssues.fileUnlessOpen(ourIssueId, applicationId, asignee, tenant.contact().get()); - store(applicationId, issueId); + tenant.contact().ifPresent(contact -> { + User assignee = tenant.type() == Tenant.Type.user ? userFor(tenant) : null; + Optional<IssueId> ourIssueId = controller().applications().require(applicationId).deploymentJobs().issueId(); + IssueId issueId = deploymentIssues.fileUnlessOpen(ourIssueId, applicationId, assignee, contact); + store(applicationId, issueId); + }); } catch (RuntimeException e) { // Catch errors due to wrong data in the controller, or issues client timeout. log.log(Level.INFO, "Exception caught when attempting to file an issue for '" + applicationId + "': " + Exceptions.toMessageString(e)); @@ -132,11 +132,10 @@ public class DeploymentIssueReporter extends Maintainer { private void escalateInactiveDeploymentIssues(Collection<Application> applications) { applications.forEach(application -> application.deploymentJobs().issueId().ifPresent(issueId -> { try { - AthenzTenant tenant = Optional.of(application.id()) - .map(this::ownerOf) - .filter(t -> t instanceof AthenzTenant) - .map(AthenzTenant.class::cast).orElseThrow(RuntimeException::new); - deploymentIssues.escalateIfInactive(issueId, maxInactivity, tenant.contact()); + Tenant tenant = ownerOf(application.id()); + deploymentIssues.escalateIfInactive(issueId, + maxInactivity, + tenant.type() == Tenant.Type.athenz ? tenant.contact() : Optional.empty()); } catch (RuntimeException e) { log.log(Level.INFO, "Exception caught when attempting to escalate issue with id '" + issueId + "': " + Exceptions.toMessageString(e)); @@ -148,4 +147,5 @@ public class DeploymentIssueReporter extends Maintainer { controller().applications().lockIfPresent(id, application -> controller().applications().store(application.withDeploymentIssueId(issueId))); } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java index 1648040fc2b..ec34585a950 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java @@ -21,9 +21,7 @@ import com.yahoo.vespa.hosted.controller.application.RoutingPolicy; import com.yahoo.vespa.hosted.controller.auditlog.AuditLog; import com.yahoo.vespa.hosted.controller.deployment.Run; import com.yahoo.vespa.hosted.controller.deployment.Step; -import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; import com.yahoo.vespa.hosted.controller.tenant.Tenant; -import com.yahoo.vespa.hosted.controller.tenant.UserTenant; import com.yahoo.vespa.hosted.controller.versions.OsVersion; import com.yahoo.vespa.hosted.controller.versions.OsVersionStatus; import com.yahoo.vespa.hosted.controller.versions.VersionStatus; @@ -312,19 +310,8 @@ public class CuratorDb { curator.set(tenantPath(tenant.name()), asJson(tenantSerializer.toSlime(tenant))); } - public Optional<UserTenant> readUserTenant(TenantName name) { - return readSlime(tenantPath(name)).map(tenantSerializer::userTenantFrom); - } - - public Optional<AthenzTenant> readAthenzTenant(TenantName name) { - return readSlime(tenantPath(name)).map(tenantSerializer::athenzTenantFrom); - } - public Optional<Tenant> readTenant(TenantName name) { - if (name.value().startsWith(Tenant.userPrefix)) { - return readUserTenant(name).map(Tenant.class::cast); - } - return readAthenzTenant(name).map(Tenant.class::cast); + return readSlime(tenantPath(name)).map(tenantSerializer::tenantFrom); } public List<Tenant> readTenants() { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java index 245cb0f4dae..91a01435b68 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java @@ -1,7 +1,6 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.persistence; - import com.yahoo.config.provision.TenantName; import com.yahoo.slime.ArrayTraverser; import com.yahoo.slime.Cursor; @@ -13,6 +12,8 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.Property; import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; +import com.yahoo.vespa.hosted.controller.tenant.BillingInfo; +import com.yahoo.vespa.hosted.controller.tenant.CloudTenant; import com.yahoo.vespa.hosted.controller.tenant.Tenant; import com.yahoo.vespa.hosted.controller.tenant.UserTenant; @@ -29,6 +30,7 @@ import java.util.Optional; public class TenantSerializer { private static final String nameField = "name"; + private static final String typeField = "type"; private static final String athenzDomainField = "athenzDomain"; private static final String propertyField = "property"; private static final String propertyIdField = "propertyId"; @@ -40,58 +42,91 @@ public class TenantSerializer { private static final String personField = "person"; private static final String queueField = "queue"; private static final String componentField = "component"; + private static final String billingInfoField = "billingInfo"; + private static final String customerIdField = "customerId"; + private static final String productCodeField = "productCode"; public Slime toSlime(Tenant tenant) { - if (tenant instanceof AthenzTenant) return toSlime((AthenzTenant) tenant); - return toSlime((UserTenant) tenant); + Slime slime = new Slime(); + Cursor tenantObject = slime.setObject(); + tenantObject.setString(nameField, tenant.name().value()); + tenantObject.setString(typeField, valueOf(tenant.type())); + + switch (tenant.type()) { + case athenz: toSlime((AthenzTenant) tenant, tenantObject); break; + case user: toSlime((UserTenant) tenant, tenantObject); break; + case cloud: toSlime((CloudTenant) tenant, tenantObject); break; + default: throw new IllegalArgumentException("Unexpected tenant type '" + tenant.type() + "'."); + } + return slime; } - private Slime toSlime(AthenzTenant tenant) { - Slime slime = new Slime(); - Cursor root = slime.setObject(); - root.setString(nameField, tenant.name().value()); - root.setString(athenzDomainField, tenant.domain().getName()); - root.setString(propertyField, tenant.property().id()); - tenant.propertyId().ifPresent(propertyId -> root.setString(propertyIdField, propertyId.id())); + private void toSlime(AthenzTenant tenant, Cursor tenantObject) { + tenantObject.setString(athenzDomainField, tenant.domain().getName()); + tenantObject.setString(propertyField, tenant.property().id()); + tenant.propertyId().ifPresent(propertyId -> tenantObject.setString(propertyIdField, propertyId.id())); tenant.contact().ifPresent(contact -> { - Cursor contactCursor = root.setObject(contactField); + Cursor contactCursor = tenantObject.setObject(contactField); writeContact(contact, contactCursor); }); - return slime; } - private Slime toSlime(UserTenant tenant) { - Slime slime = new Slime(); - Cursor root = slime.setObject(); - root.setString(nameField, tenant.name().value()); + private void toSlime(UserTenant tenant, Cursor tenantObject) { tenant.contact().ifPresent(contact -> { - Cursor contactCursor = root.setObject(contactField); + Cursor contactCursor = tenantObject.setObject(contactField); writeContact(contact, contactCursor); }); - return slime; } - public AthenzTenant athenzTenantFrom(Slime slime) { - Inspector root = slime.get(); - TenantName name = TenantName.from(root.field(nameField).asString()); - AthenzDomain domain = new AthenzDomain(root.field(athenzDomainField).asString()); - Property property = new Property(root.field(propertyField).asString()); - Optional<PropertyId> propertyId = SlimeUtils.optionalString(root.field(propertyIdField)).map(PropertyId::new); - Optional<Contact> contact = contactFrom(root.field(contactField)); + private void toSlime(CloudTenant tenant, Cursor root) { + toSlime(tenant.billingInfo(), root.setObject(billingInfoField)); + } + + private void toSlime(BillingInfo billingInfo, Cursor billingInfoObject) { + billingInfoObject.setString(customerIdField, billingInfo.customerId()); + billingInfoObject.setString(productCodeField, billingInfo.productCode()); + } + + public Tenant tenantFrom(Slime slime) { + Inspector tenantObject = slime.get(); + Tenant.Type type; + if (tenantObject.field(typeField).valid()) + type = typeOf(tenantObject.field(typeField).asString()); + else // TODO jvenstad: Remove once all tenants are stored on updated format. + type = tenantObject.field(nameField).asString().startsWith(Tenant.userPrefix) ? Tenant.Type.user : Tenant.Type.athenz; + + switch (type) { + case athenz: return athenzTenantFrom(tenantObject); + case user: return userTenantFrom(tenantObject); + case cloud: return cloudTenantFrom(tenantObject); + default: throw new IllegalArgumentException("Unexpected tenant type '" + type + "'."); + } + } + + private AthenzTenant athenzTenantFrom(Inspector tenantObject) { + TenantName name = TenantName.from(tenantObject.field(nameField).asString()); + AthenzDomain domain = new AthenzDomain(tenantObject.field(athenzDomainField).asString()); + Property property = new Property(tenantObject.field(propertyField).asString()); + Optional<PropertyId> propertyId = SlimeUtils.optionalString(tenantObject.field(propertyIdField)).map(PropertyId::new); + Optional<Contact> contact = contactFrom(tenantObject.field(contactField)); return new AthenzTenant(name, domain, property, propertyId, contact); } - public UserTenant userTenantFrom(Slime slime) { - Inspector root = slime.get(); - TenantName name = TenantName.from(root.field(nameField).asString()); - Optional<Contact> contact = contactFrom(root.field(contactField)); + private UserTenant userTenantFrom(Inspector tenantObject) { + TenantName name = TenantName.from(tenantObject.field(nameField).asString()); + Optional<Contact> contact = contactFrom(tenantObject.field(contactField)); return new UserTenant(name, contact); } + private CloudTenant cloudTenantFrom(Inspector tenantObject) { + TenantName name = TenantName.from(tenantObject.field(nameField).asString()); + BillingInfo billingInfo = billingInfoFrom(tenantObject.field(billingInfoField)); + return new CloudTenant(name, billingInfo); + } + private Optional<Contact> contactFrom(Inspector object) { - if (!object.valid()) { - return Optional.empty(); - } + if ( ! object.valid()) return Optional.empty(); + URI contactUrl = URI.create(object.field(contactUrlField).asString()); URI propertyUrl = URI.create(object.field(propertyUrlField).asString()); URI issueTrackerUrl = URI.create(object.field(issueTrackerUrlField).asString()); @@ -132,4 +167,27 @@ public class TenantSerializer { return personLists; } + private BillingInfo billingInfoFrom(Inspector billingInfoObject) { + return new BillingInfo(billingInfoObject.field(customerIdField).asString(), + billingInfoObject.field(productCodeField).asString()); + } + + private static Tenant.Type typeOf(String value) { + switch (value) { + case "athenz": return Tenant.Type.athenz; + case "user": return Tenant.Type.user; + case "cloud": return Tenant.Type.cloud; + default: throw new IllegalArgumentException("Unknown tenant type '" + value + "'."); + } + } + + private static String valueOf(Tenant.Type type) { + switch (type) { + case athenz: return "athenz"; + case user: return "user"; + case cloud: return "cloud"; + default: throw new IllegalArgumentException("Unexpected tenant type '" + type + "'."); + } + } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index 6fc5cc645e8..37c0847f167 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -30,6 +30,7 @@ import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.AlreadyExistsException; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.LockedTenant; import com.yahoo.vespa.hosted.controller.NotExistsException; import com.yahoo.vespa.hosted.controller.api.ActivateResult; import com.yahoo.vespa.hosted.controller.api.application.v4.ApplicationResource; @@ -333,7 +334,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { } private HttpResponse tenant(String tenantName, HttpRequest request) { - return controller.tenants().tenant(TenantName.from(tenantName)) + return controller.tenants().get(TenantName.from(tenantName)) .map(tenant -> tenant(tenant, request, true)) .orElseGet(() -> ErrorResponse.notFoundError("Tenant '" + tenantName + "' does not exist")); } @@ -756,7 +757,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { Inspector requestData = toSlime(request.getData()).get(); OktaAccessToken token = requireOktaAccessToken(request, "Could not update " + tenantName); - controller.tenants().lockOrThrow(tenant.get().name(), lockedTenant -> { + controller.tenants().lockOrThrow(tenant.get().name(), LockedTenant.Athenz.class, lockedTenant -> { lockedTenant = lockedTenant.with(new Property(mandatory("property", requestData).asString())); lockedTenant = controller.tenants().withDomain( lockedTenant, @@ -983,7 +984,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { } private HttpResponse deleteTenant(String tenantName, HttpRequest request) { - Optional<Tenant> tenant = controller.tenants().tenant(tenantName); + Optional<Tenant> tenant = controller.tenants().get(tenantName); if ( ! tenant.isPresent()) return ErrorResponse.notFoundError("Could not delete tenant '" + tenantName + "': Tenant not found"); // NOTE: The Jersey implementation would silently ignore this @@ -1099,7 +1100,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { } private Tenant getTenantOrThrow(String tenantName) { - return controller.tenants().tenant(tenantName) + return controller.tenants().get(tenantName) .orElseThrow(() -> new NotExistsException(new TenantId(tenantName))); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilter.java index 77e626509b3..3d0e21617c5 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilter.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilter.java @@ -158,7 +158,7 @@ public class ControllerAuthorizationFilter extends CorsRequestFilterBase { } private void verifyIsTenantAdmin(AthenzPrincipal principal, TenantName name) { - tenantController.tenant(name) + tenantController.get(name) .ifPresent(tenant -> { if (!isTenantAdmin(principal.getIdentity(), tenant)) { throw new ForbiddenException("Tenant admin or Vespa operator role required"); @@ -182,7 +182,7 @@ public class ControllerAuthorizationFilter extends CorsRequestFilterBase { private void verifyIsTenantPipelineOperator(AthenzPrincipal principal, TenantName name, ApplicationName application) { - tenantController.tenant(name) + tenantController.get(name) .ifPresent(tenant -> verifyIsTenantPipelineOperator(principal.getIdentity(), tenant, application)); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/AthenzTenant.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/AthenzTenant.java index abe09090761..f8edeee5939 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/AthenzTenant.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/AthenzTenant.java @@ -75,4 +75,10 @@ public class AthenzTenant extends Tenant { } return name; } + + @Override + public Type type() { + return Type.athenz; + } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/BillingInfo.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/BillingInfo.java new file mode 100644 index 00000000000..0eeb331b59f --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/BillingInfo.java @@ -0,0 +1,54 @@ +package com.yahoo.vespa.hosted.controller.tenant; + +import java.util.Objects; +import java.util.StringJoiner; + +import static java.util.Objects.requireNonNull; + +/** + * Information pertinent to billing a tenant for use of hosted Vespa services. + * + * @author jonmv + */ +public class BillingInfo { + + private final String customerId; + private final String productCode; + + /** Creates a new BillingInfo with the given data. Assumes data has already been validated. */ + public BillingInfo(String customerId, String productCode) { + this.customerId = requireNonNull(customerId); + this.productCode = requireNonNull(productCode); + } + + public String customerId() { + return customerId; + } + + public String productCode() { + return productCode; + } + + @Override + public String toString() { + return new StringJoiner(", ", BillingInfo.class.getSimpleName() + "[", "]") + .add("customerId='" + customerId + "'") + .add("productCode='" + productCode + "'") + .toString(); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if ( ! (o instanceof BillingInfo)) return false; + BillingInfo that = (BillingInfo) o; + return Objects.equals(customerId, that.customerId) && + Objects.equals(productCode, that.productCode); + } + + @Override + public int hashCode() { + return Objects.hash(customerId, productCode); + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/CloudTenant.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/CloudTenant.java new file mode 100644 index 00000000000..cf68c8f3bf9 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/CloudTenant.java @@ -0,0 +1,37 @@ +package com.yahoo.vespa.hosted.controller.tenant; + +import com.yahoo.config.provision.TenantName; + +import java.util.Optional; + +/** + * A tenant as vague as its name. + * + * Only a reference to a cloud identity provider, and some billing info, is known for this tenant type. + * + * @author jonmv + */ +public class CloudTenant extends Tenant { + + private final BillingInfo billingInfo; + + /** Public for the serialization layer — do not use! */ + public CloudTenant(TenantName name, BillingInfo info) { + super(name, Optional.empty()); + billingInfo = info; + } + + /** Creates a tenant with the given name, provided it passes validation. */ + public static CloudTenant create(TenantName tenantName, BillingInfo billingInfo) { + return new CloudTenant(requireName(tenantName), billingInfo); + } + + /** Returns the billing info for this tenant. */ + public BillingInfo billingInfo() { return billingInfo; } + + @Override + public Type type() { + return Type.cloud; + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/Tenant.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/Tenant.java index c6ed9f7b559..19b7229515b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/Tenant.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/Tenant.java @@ -1,7 +1,6 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.tenant; - import com.yahoo.config.provision.TenantName; import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; @@ -35,6 +34,8 @@ public abstract class Tenant { return contact; } + public abstract Type type(); + @Override public boolean equals(Object o) { if (this == o) return true; @@ -49,11 +50,26 @@ public abstract class Tenant { } static TenantName requireName(TenantName name) { - if (!name.value().matches("^(?=.{1,20}$)[a-z](-?[a-z0-9]+)*$")) { + if ( ! name.value().matches("^(?=.{1,20}$)[a-z](-?[a-z0-9]+)*$")) { throw new IllegalArgumentException("New tenant or application names must start with a letter, may " + "contain no more than 20 characters, and may only contain lowercase " + "letters, digits or dashes, but no double-dashes."); } return name; } + + + public enum Type { + + /** Tenant authenticated through Athenz. */ + athenz, + + /** Tenant authenticated through Okta, as a user. */ + user, + + /** Tenant authenticated through some cloud identity provider. */ + cloud; + + } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/UserTenant.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/UserTenant.java index 47e5580fbe4..a46d847f6f3 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/UserTenant.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/UserTenant.java @@ -21,6 +21,11 @@ public class UserTenant extends Tenant { super(name, contact); } + @Override + public Type type() { + return Type.user; + } + public UserTenant(TenantName name) { super(name, Optional.empty()); } @@ -64,4 +69,5 @@ public class UserTenant extends Tenant { } return name; } + } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java index 63a9d0ce905..cf5f8fac69d 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java @@ -114,7 +114,7 @@ public class ControllerTest { applications = tester.controller().applications(); - assertNotNull(tester.controller().tenants().tenant(TenantName.from("tenant1"))); + assertNotNull(tester.controller().tenants().get(TenantName.from("tenant1"))); assertNotNull(applications.get(ApplicationId.from(TenantName.from("tenant1"), ApplicationName.from("application1"), InstanceName.from("default")))); @@ -524,6 +524,17 @@ public class ControllerTest { tester.controller().applications().deactivate(app.id(), ZoneId.from(Environment.prod, RegionName.from("us-west-1"))); } + @Test + public void testDeployApplicationPackageWithApplicationDir() { + DeploymentTester tester = new DeploymentTester(); + Application application = tester.createApplication("app1", "tenant1", 1, 1L); + ApplicationPackage applicationPackage = new ApplicationPackageBuilder() + .environment(Environment.prod) + .region("us-west-1") + .build(true); + tester.deployCompletely(application, applicationPackage); + } + private void runUpgrade(DeploymentTester tester, ApplicationId application, ApplicationVersion version) { Version next = Version.fromString("6.2"); tester.upgradeSystem(next); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java index 615fb017363..e573c12af3b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java @@ -249,14 +249,14 @@ public final class ControllerTester { public TenantName createTenant(String tenantName, String domainName, Long propertyId, Optional<Contact> contact) { TenantName name = TenantName.from(tenantName); - Optional<Tenant> existing = controller().tenants().tenant(name); + Optional<Tenant> existing = controller().tenants().get(name); if (existing.isPresent()) return name; AthenzTenant tenant = AthenzTenant.create(name, createDomain(domainName), new Property("Property"+propertyId), Optional.ofNullable(propertyId) .map(Object::toString) .map(PropertyId::new), contact); controller().tenants().create(tenant, new OktaAccessToken("okta-token")); - assertNotNull(controller().tenants().tenant(name)); + assertNotNull(controller().tenants().get(name)); return name; } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/ZipStreamReaderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/ZipStreamReaderTest.java new file mode 100644 index 00000000000..92462a67e48 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/ZipStreamReaderTest.java @@ -0,0 +1,77 @@ +// 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.controller.application; + +import org.junit.Test; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; +import java.util.Map; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +/** + * @author mpolden + */ +public class ZipStreamReaderTest { + + @Test + public void test_size_limit() { + Map<String, String> entries = Map.of("foo.xml", "foobar"); + try { + new ZipStreamReader(new ByteArrayInputStream(zip(entries)), "foo.xml"::equals, 1); + fail("Expected exception"); + } catch (IllegalArgumentException ignored) {} + + entries = Map.of("foo.xml", "foobar", + "foo.zip", "0".repeat(100) // File not extracted and thus not subject to size limit + ); + ZipStreamReader reader = new ZipStreamReader(new ByteArrayInputStream(zip(entries)), "foo.xml"::equals,10); + byte[] extracted = reader.entries().get(0).content(); + assertEquals("foobar", new String(extracted, StandardCharsets.UTF_8)); + } + + @Test + public void test_paths() { + Map<String, Boolean> tests = Map.of( + "../../services.xml", true, + "/../.././services.xml", true, + "./application/././services.xml", true, + "application//services.xml", true, + "services..xml", false, + "application/services.xml", false, + "components/foo-bar-deploy.jar", false, + "services.xml", false + ); + tests.forEach((name, expectException) -> { + try { + new ZipStreamReader(new ByteArrayInputStream(zip(Map.of(name, "foo"))), name::equals, 1024); + assertFalse("Expected exception for '" + name + "'", expectException); + } catch (IllegalArgumentException ignored) { + assertTrue("Unexpected exception for '" + name + "'", expectException); + } + }); + } + + private static byte[] zip(Map<String, String> entries) { + ByteArrayOutputStream zip = new ByteArrayOutputStream(); + try (ZipOutputStream out = new ZipOutputStream(zip)) { + for (Map.Entry<String, String> entry : entries.entrySet()) { + out.putNextEntry(new ZipEntry(entry.getKey())); + out.write(entry.getValue().getBytes(StandardCharsets.UTF_8)); + out.closeEntry(); + } + } catch (IOException e) { + throw new UncheckedIOException(e); + } + return zip.toByteArray(); + } + +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java index dcc4a2071de..29bec7246ee 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java @@ -172,18 +172,26 @@ public class ApplicationPackageBuilder { } public ApplicationPackage build() { + return build(false); + } + + public ApplicationPackage build(boolean useApplicationDir) { + String dir = ""; + if (useApplicationDir) { + dir = "application/"; + } ByteArrayOutputStream zip = new ByteArrayOutputStream(); try (ZipOutputStream out = new ZipOutputStream(zip)) { - out.putNextEntry(new ZipEntry("deployment.xml")); + out.putNextEntry(new ZipEntry(dir + "deployment.xml")); out.write(deploymentSpec()); out.closeEntry(); - out.putNextEntry(new ZipEntry("validation-overrides.xml")); + out.putNextEntry(new ZipEntry(dir + "validation-overrides.xml")); out.write(validationOverrides()); out.closeEntry(); - out.putNextEntry(new ZipEntry("search-definitions/test.sd")); + out.putNextEntry(new ZipEntry(dir + "search-definitions/test.sd")); out.write(searchDefinition()); out.closeEntry(); - out.putNextEntry(new ZipEntry("build-meta.json")); + out.putNextEntry(new ZipEntry(dir + "build-meta.json")); out.write(buildMeta()); out.closeEntry(); } catch (IOException e) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java index 0b680149bff..dd4558dbe2c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java @@ -112,7 +112,7 @@ public class ApplicationSerializerTest { Optional.of(User.from("by-username")), OptionalInt.of(7), new MetricsService.ApplicationMetrics(0.5, 0.9), - Optional.of("---begin---\nKEY\n---end---"), + Optional.of("-----BEGIN PUBLIC KEY-----\n∠( ᐛ 」∠)＿\n-----END PUBLIC KEY-----"), Optional.of(new RotationId("my-rotation")), rotationStatus); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java index 2e0d7715d7d..b78cff88ccf 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java @@ -3,10 +3,14 @@ package com.yahoo.vespa.hosted.controller.persistence;// Copyright 2018 Yahoo Ho import com.yahoo.config.provision.TenantName; import com.yahoo.vespa.athenz.api.AthenzDomain; +import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.api.identifiers.Property; import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; +import com.yahoo.vespa.hosted.controller.tenant.BillingInfo; +import com.yahoo.vespa.hosted.controller.tenant.CloudTenant; +import com.yahoo.vespa.hosted.controller.tenant.Tenant; import com.yahoo.vespa.hosted.controller.tenant.UserTenant; import org.junit.Test; @@ -32,7 +36,7 @@ public class TenantSerializerTest { new AthenzDomain("domain1"), new Property("property1"), Optional.of(new PropertyId("1"))); - AthenzTenant serialized = serializer.athenzTenantFrom(serializer.toSlime(tenant)); + AthenzTenant serialized = (AthenzTenant) serializer.tenantFrom(serializer.toSlime(tenant)); assertEquals(tenant.name(), serialized.name()); assertEquals(tenant.domain(), serialized.domain()); assertEquals(tenant.property(), serialized.property()); @@ -46,7 +50,7 @@ public class TenantSerializerTest { new AthenzDomain("domain1"), new Property("property1"), Optional.empty()); - AthenzTenant serialized = serializer.athenzTenantFrom(serializer.toSlime(tenant)); + AthenzTenant serialized = (AthenzTenant) serializer.tenantFrom(serializer.toSlime(tenant)); assertFalse(serialized.propertyId().isPresent()); assertEquals(tenant.propertyId(), serialized.propertyId()); } @@ -58,18 +62,33 @@ public class TenantSerializerTest { new Property("property1"), Optional.of(new PropertyId("1")), Optional.of(contact())); - AthenzTenant serialized = serializer.athenzTenantFrom(serializer.toSlime(tenant)); + AthenzTenant serialized = (AthenzTenant) serializer.tenantFrom(serializer.toSlime(tenant)); assertEquals(tenant.contact(), serialized.contact()); } @Test public void user_tenant() { UserTenant tenant = UserTenant.create("by-foo", Optional.of(contact())); - UserTenant serialized = serializer.userTenantFrom(serializer.toSlime(tenant)); + UserTenant serialized = (UserTenant) serializer.tenantFrom(serializer.toSlime(tenant)); assertEquals(tenant.name(), serialized.name()); assertEquals(contact(), serialized.contact().get()); } + @Test + public void cloud_tenant() { + CloudTenant tenant = CloudTenant.create(TenantName.from("elderly-lady"), + new BillingInfo("old cat lady", "vespa")); + CloudTenant serialized = (CloudTenant) serializer.tenantFrom(serializer.toSlime(tenant)); + assertEquals(tenant.name(), serialized.name()); + assertEquals(tenant.billingInfo(), serialized.billingInfo()); + } + + @Test + public void legacy_deserialization() { + UserTenant legayUserTenant = (UserTenant) serializer.tenantFrom(SlimeUtils.jsonToSlime("{\"name\":\"by-someone\"}")); + assertTrue(legayUserTenant.is("someone")); + } + private Contact contact() { return new Contact( URI.create("http://contact1.test"), @@ -83,4 +102,5 @@ public class TenantSerializerTest { Optional.empty() ); } + } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index c0c2d4043d9..705fc8adbac 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -22,6 +22,7 @@ import com.yahoo.vespa.athenz.api.OktaAccessToken; import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.ApplicationController; +import com.yahoo.vespa.hosted.controller.LockedTenant; import com.yahoo.vespa.hosted.controller.api.application.v4.EnvironmentResource; import com.yahoo.vespa.hosted.controller.api.identifiers.Property; import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; @@ -1541,7 +1542,9 @@ public class ApplicationApiTest extends ControllerContainerTest { private void updateContactInformation() { Contact contact = new Contact(URI.create("www.contacts.tld/1234"), URI.create("www.properties.tld/1234"), URI.create("www.issues.tld/1234"), List.of(List.of("alice"), List.of("bob")), "queue", Optional.empty()); - tester.controller().tenants().lockIfPresent(TenantName.from("tenant2"), lockedTenant -> tester.controller().tenants().store(lockedTenant.with(contact))); + tester.controller().tenants().lockIfPresent(TenantName.from("tenant2"), + LockedTenant.Athenz.class, + lockedTenant -> tester.controller().tenants().store(lockedTenant.with(contact))); } private void registerContact(long propertyId) { diff --git a/document/src/vespa/document/datatype/tensor_data_type.cpp b/document/src/vespa/document/datatype/tensor_data_type.cpp index 8aad39c68b7..2608f1227da 100644 --- a/document/src/vespa/document/datatype/tensor_data_type.cpp +++ b/document/src/vespa/document/datatype/tensor_data_type.cpp @@ -50,11 +50,17 @@ TensorDataType::fromSpec(const vespalib::string &spec) } bool -TensorDataType::isAssignableType(const vespalib::eval::ValueType &rhs) const +TensorDataType::isAssignableType(const ValueType &tensorType) const { - const auto &dimensions = _tensorType.dimensions(); - const auto &rhsDimensions = rhs.dimensions(); - if (!rhs.is_tensor() || dimensions.size() != rhsDimensions.size()) { + return isAssignableType(_tensorType, tensorType); +} + +bool +TensorDataType::isAssignableType(const ValueType &fieldTensorType, const ValueType &tensorType) +{ + const auto &dimensions = fieldTensorType.dimensions(); + const auto &rhsDimensions = tensorType.dimensions(); + if (!tensorType.is_tensor() || dimensions.size() != rhsDimensions.size()) { return false; } for (size_t i = 0; i < dimensions.size(); ++i) { @@ -68,7 +74,6 @@ TensorDataType::isAssignableType(const vespalib::eval::ValueType &rhs) const } } return true; - } } // document diff --git a/document/src/vespa/document/datatype/tensor_data_type.h b/document/src/vespa/document/datatype/tensor_data_type.h index 43f14c3c3ea..d6a022ac3e5 100644 --- a/document/src/vespa/document/datatype/tensor_data_type.h +++ b/document/src/vespa/document/datatype/tensor_data_type.h @@ -25,6 +25,7 @@ public: const vespalib::eval::ValueType &getTensorType() const { return _tensorType; } bool isAssignableType(const vespalib::eval::ValueType &tensorType) const; + static bool isAssignableType(const vespalib::eval::ValueType &fieldTensorType, const vespalib::eval::ValueType &tensorType); }; } diff --git a/fastos/src/tests/backtracetest.cpp b/fastos/src/tests/backtracetest.cpp index 6c68765c1ee..8fa3b42bfc9 100644 --- a/fastos/src/tests/backtracetest.cpp +++ b/fastos/src/tests/backtracetest.cpp @@ -6,7 +6,7 @@ #include "tests.h" -#if defined __x86_64__ +#if defined(__x86_64__) && defined(__linux__) class Tracker { private: diff --git a/fastos/src/tests/filetest.cpp b/fastos/src/tests/filetest.cpp index 43051dd1667..15f8ccb526b 100644 --- a/fastos/src/tests/filetest.cpp +++ b/fastos/src/tests/filetest.cpp @@ -813,7 +813,9 @@ public: ScanDirectoryTest(); ReadBufTest(); MemoryMapTest(0); +#ifdef __linux__ MemoryMapTest(MAP_HUGETLB); +#endif PrintSeparator(); printf("END OF TEST (%s)\n", _argv[0]); diff --git a/fastos/src/tests/timetest.cpp b/fastos/src/tests/timetest.cpp index 0618e28bcd0..c0c49884fb1 100644 --- a/fastos/src/tests/timetest.cpp +++ b/fastos/src/tests/timetest.cpp @@ -260,8 +260,8 @@ public: using fastos::TimeStamp; Progress(TimeStamp(97).ns() == 97l, "TimeStamp(int)"); Progress(TimeStamp(97u).ns() == 97l, "TimeStamp(unsigned int)"); - Progress(TimeStamp(97l).ns() == 97l, "TimeStamp(long)"); - Progress(TimeStamp(97ul).ns() == 97l, "TimeStamp(unsigned long)"); + Progress(TimeStamp(INT64_C(97)).ns() == 97l, "TimeStamp(int64_t)"); + Progress(TimeStamp(UINT64_C(97)).ns() == 97l, "TimeStamp(uint64_t)"); Progress(TimeStamp(TimeStamp::Seconds(97.3)).ns() == 97300000000l, "TimeStamp(double)"); PrintSeparator(); } diff --git a/fastos/src/vespa/fastos/app.cpp b/fastos/src/vespa/fastos/app.cpp index aecdc683649..b5c15aea831 100644 --- a/fastos/src/vespa/fastos/app.cpp +++ b/fastos/src/vespa/fastos/app.cpp @@ -34,6 +34,7 @@ FastOS_ApplicationInterface::FastOS_ApplicationInterface() : _argv(nullptr) { FastOS_ProcessInterface::_app = this; +#ifdef __linux__ char * fadvise = getenv("VESPA_FADVISE_OPTIONS"); if (fadvise != nullptr) { int fadviseOptions(0); @@ -44,6 +45,7 @@ FastOS_ApplicationInterface::FastOS_ApplicationInterface() : if (strstr(fadvise, "NOREUSE")) { fadviseOptions |= POSIX_FADV_NOREUSE; } FastOS_FileInterface::setDefaultFAdviseOptions(fadviseOptions); } +#endif } FastOS_ApplicationInterface::~FastOS_ApplicationInterface () diff --git a/fastos/src/vespa/fastos/file.cpp b/fastos/src/vespa/fastos/file.cpp index aff4d1c54b5..56f6addd743 100644 --- a/fastos/src/vespa/fastos/file.cpp +++ b/fastos/src/vespa/fastos/file.cpp @@ -28,7 +28,11 @@ DirectIOException::DirectIOException(const char * fileName, const void * buffer, DirectIOException::~DirectIOException() {} FastOS_FileInterface::FailedHandler FastOS_FileInterface::_failedHandler = nullptr; +#ifdef __linux__ int FastOS_FileInterface::_defaultFAdviseOptions = POSIX_FADV_NORMAL; +#else +int FastOS_FileInterface::_defaultFAdviseOptions = 0; +#endif static const size_t MAX_WRITE_CHUNK_SIZE = 0x4000000; // 64 MB diff --git a/fastos/src/vespa/fastos/file.h b/fastos/src/vespa/fastos/file.h index 8424967f655..15c4dfa33cd 100644 --- a/fastos/src/vespa/fastos/file.h +++ b/fastos/src/vespa/fastos/file.h @@ -804,6 +804,11 @@ public: virtual bool IsValidScan() const = 0; }; +#ifdef __linux__ #include <vespa/fastos/linux_file.h> typedef FastOS_Linux_File FASTOS_PREFIX(File); +#else +#include <vespa/fastos/unix_file.h> +typedef FastOS_UNIX_File FASTOS_PREFIX(File); +#endif typedef FastOS_UNIX_DirectoryScan FASTOS_PREFIX(DirectoryScan); diff --git a/fastos/src/vespa/fastos/linux_file.cpp b/fastos/src/vespa/fastos/linux_file.cpp index 6af3bd207a0..b4acaaa6073 100644 --- a/fastos/src/vespa/fastos/linux_file.cpp +++ b/fastos/src/vespa/fastos/linux_file.cpp @@ -7,6 +7,7 @@ * Implementation of FastOS_Linux_File methods. *****************************************************************************/ +#ifdef __linux__ #include "file.h" #include <sstream> #include <unistd.h> @@ -434,3 +435,4 @@ void forceStaticLinkOf_backtrace() void * dummy[2]; FastOS_backtrace(dummy, 2); } +#endif diff --git a/fastos/src/vespa/fastos/unix_file.cpp b/fastos/src/vespa/fastos/unix_file.cpp index 0b144c1f423..17a11cd68f3 100644 --- a/fastos/src/vespa/fastos/unix_file.cpp +++ b/fastos/src/vespa/fastos/unix_file.cpp @@ -16,7 +16,26 @@ #include <dirent.h> #include <sys/stat.h> #include <sys/mman.h> +#ifdef __linux__ #include <sys/vfs.h> +#else +#include <sys/mount.h> +#endif + +ssize_t +FastOS_UNIX_File::Read(void *buffer, size_t len) +{ + ssize_t nRead = read(_filedes, buffer, len); + return nRead; +} + + +ssize_t +FastOS_UNIX_File::Write2(const void *buffer, size_t len) +{ + ssize_t writeRes = write(_filedes, buffer, len); + return writeRes; +} bool FastOS_UNIX_File::SetPosition(int64_t desiredPosition) @@ -33,6 +52,22 @@ FastOS_UNIX_File::GetPosition(void) return lseek(_filedes, 0, SEEK_CUR); } +void FastOS_UNIX_File::ReadBuf(void *buffer, size_t length, + int64_t readOffset) +{ + ssize_t readResult; + + readResult = pread(_filedes, buffer, length, readOffset); + if (static_cast<size_t>(readResult) != length) { + std::string errorString = readResult != -1 ? + std::string("short read") : + FastOS_FileInterface::getLastErrorString(); + std::ostringstream os; + os << "Fatal: Reading " << length << " bytes, got " << readResult << " from '" + << GetFileName() << "' failed: " << errorString; + throw std::runtime_error(os.str()); + } +} bool FastOS_UNIX_File::Stat(const char *filename, FastOS_StatInfo *statInfo) @@ -51,9 +86,18 @@ FastOS_UNIX_File::Stat(const char *filename, FastOS_StatInfo *statInfo) statInfo->_isDirectory = S_ISDIR(stbuf.st_mode); statInfo->_size = static_cast<int64_t>(stbuf.st_size); statInfo->_modifiedTime = stbuf.st_mtime; +#ifdef __linux__ statInfo->_modifiedTimeNS = stbuf.st_mtim.tv_sec; statInfo->_modifiedTimeNS *= 1000000000; statInfo->_modifiedTimeNS += stbuf.st_mtim.tv_nsec; +#elif defined(__APPLE__) + statInfo->_modifiedTimeNS = stbuf.st_mtimespec.tv_sec; + statInfo->_modifiedTimeNS *= 1000000000; + statInfo->_modifiedTimeNS += stbuf.st_mtimespec.tv_nsec; +#else + statInfo->_modifiedTimeNS = stbuf.st_mtime; + statInfo->_modifiedTimeNS *= 1000000000; +#endif rc = true; } else { if (errno == ENOENT) { @@ -150,9 +194,11 @@ FastOS_UNIX_File::CalcAccessFlags(unsigned int openFlags) accessFlags |= O_FSYNC; #endif +#ifdef __linux__ if ((openFlags & FASTOS_FILE_OPEN_DIRECTIO) != 0) { accessFlags |= O_DIRECT; } +#endif if ((openFlags & FASTOS_FILE_OPEN_TRUNCATE) != 0) { // Truncate file on open @@ -161,7 +207,11 @@ FastOS_UNIX_File::CalcAccessFlags(unsigned int openFlags) return accessFlags; } +#ifdef __linux__ constexpr int ALWAYS_SUPPORTED_MMAP_FLAGS = ~MAP_HUGETLB; +#else +constexpr int ALWAYS_SUPPORTED_MMAP_FLAGS = ~0; +#endif bool FastOS_UNIX_File::Open(unsigned int openFlags, const char *filename) @@ -190,7 +240,11 @@ FastOS_UNIX_File::Open(unsigned int openFlags, const char *filename) abort(); } +#ifdef __linux__ _filedes = file->_fileno; +#else + _filedes = fileno(file); +#endif _openFlags = openFlags; rc = true; } else { @@ -214,6 +268,7 @@ FastOS_UNIX_File::Open(unsigned int openFlags, const char *filename) mbase = mmap(nullptr, mlen, PROT_READ, MAP_SHARED | (_mmapFlags & ALWAYS_SUPPORTED_MMAP_FLAGS), _filedes, 0); } if (mbase != MAP_FAILED) { +#ifdef __linux__ int fadviseOptions = getFAdviseOptions(); int eCode(0); if (POSIX_FADV_RANDOM == fadviseOptions) { @@ -224,6 +279,7 @@ FastOS_UNIX_File::Open(unsigned int openFlags, const char *filename) if (eCode != 0) { fprintf(stderr, "Failed: posix_madvise(%p, %ld, %d) = %d\n", mbase, mlen, fadviseOptions, eCode); } +#endif _mmapbase = mbase; _mmaplen = mlen; } else { @@ -245,7 +301,9 @@ FastOS_UNIX_File::Open(unsigned int openFlags, const char *filename) void FastOS_UNIX_File::dropFromCache() const { +#ifdef __linux__ posix_fadvise(_filedes, 0, 0, POSIX_FADV_DONTNEED); +#endif } diff --git a/fastos/src/vespa/fastos/unix_file.h b/fastos/src/vespa/fastos/unix_file.h index 5702f511ca5..3bca340cb90 100644 --- a/fastos/src/vespa/fastos/unix_file.h +++ b/fastos/src/vespa/fastos/unix_file.h @@ -56,6 +56,9 @@ public: _mmapEnabled(false) { } + void ReadBuf(void *buffer, size_t length, int64_t readOffset) override; + ssize_t Read(void *buffer, size_t len) override; + ssize_t Write2(const void *buffer, size_t len) override; bool Open(unsigned int openFlags, const char *filename) override; bool Close() override; bool IsOpened() const override { return _filedes >= 0; } diff --git a/fastos/src/vespa/fastos/unix_process.cpp b/fastos/src/vespa/fastos/unix_process.cpp index c695c8d79a0..f1c726a4f51 100644 --- a/fastos/src/vespa/fastos/unix_process.cpp +++ b/fastos/src/vespa/fastos/unix_process.cpp @@ -9,6 +9,9 @@ #include <fcntl.h> #include <sys/socket.h> #include <sys/wait.h> +#ifndef __linux__ +#include <signal.h> +#endif #ifndef AF_LOCAL #define AF_LOCAL AF_UNIX diff --git a/fastos/src/vespa/fastos/unix_thread.cpp b/fastos/src/vespa/fastos/unix_thread.cpp index 8ff1913b246..5218bde2630 100644 --- a/fastos/src/vespa/fastos/unix_thread.cpp +++ b/fastos/src/vespa/fastos/unix_thread.cpp @@ -40,6 +40,7 @@ bool FastOS_UNIX_Thread::Initialize (int stackSize, int stackGuardSize) pthread_attr_setscope(&attr, PTHREAD_SCOPE_SYSTEM); pthread_attr_setstacksize(&attr, stackSize); +#ifdef __linux__ if (_G_maxNumCpus > 0) { int cpuid = _G_nextCpuId.fetch_add(1)%_G_maxNumCpus; cpu_set_t cpuset; @@ -50,6 +51,7 @@ bool FastOS_UNIX_Thread::Initialize (int stackSize, int stackGuardSize) fprintf(stderr, "Pinning FAILURE retval = %d, errno=%d sizeof(cpuset_t)=%ld cpuid(%d)\n", retval, errno, sizeof(cpuset), cpuid); } } +#endif if (stackGuardSize != 0) { pthread_attr_setguardsize(&attr, stackGuardSize); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java index 577b4c5eef0..620a0caed8d 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java @@ -122,6 +122,12 @@ public class StorageMaintainer { "-m", "150", "-a", "config-zkbackupage") .withTags(tags)); + + String appName = nodeTypeToRole(context.nodeType()) + "-logd"; + Path logdCheckPath = context.pathInNodeUnderVespaHome("libexec/yms/convert-state-metrics-2-yamas.py"); + configs.add(new SecretAgentCheckConfig(appName, 60, logdCheckPath, + appName, "http://localhost:19089/state/v1/metrics") + .withTags(tags)); } if (context.nodeType() == NodeType.proxy) { diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java index 85562a92af9..01e41cfe1a3 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java @@ -119,7 +119,7 @@ public class StorageMaintainerTest { public void configserver() { Path path = executeAs(NodeType.config); - assertChecks(path, "athenz-certificate-expiry", "configserver", "host-life", + assertChecks(path, "athenz-certificate-expiry", "configserver", "configserver-logd", "host-life", "system-coredumps-processing", "zkbackupage"); assertCheckEnds(path.resolve("configserver.yaml"), @@ -134,7 +134,7 @@ public class StorageMaintainerTest { public void controller() { Path path = executeAs(NodeType.controller); - assertChecks(path, "athenz-certificate-expiry", "controller", "host-life", + assertChecks(path, "athenz-certificate-expiry", "controller", "controller-logd", "host-life", "system-coredumps-processing", "vespa", "vespa-health", "zkbackupage"); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java index 861e1062d66..5a3f0380fa2 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java @@ -16,6 +16,7 @@ import com.yahoo.vespa.hosted.provision.persistence.NameResolver; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.EnumSet; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -121,7 +122,7 @@ class NodePrioritizer { * already have nodes allocated to this tenant */ void addNewDockerNodes(Mutex allocationLock, boolean exclusively) { - NodeList candidates = allNodes; + NodeList candidates; if (exclusively) { Set<String> candidateHostnames = allNodes.asList().stream() @@ -130,7 +131,12 @@ class NodePrioritizer { .flatMap(node -> node.parentHostname().stream()) .collect(Collectors.toSet()); - candidates = candidates.filter(node -> candidateHostnames.contains(node.hostname())); + candidates = allNodes + .filter(node -> candidateHostnames.contains(node.hostname())) + .filter(node -> EnumSet.of(Node.State.provisioned, Node.State.ready, Node.State.active) + .contains(node.state())); + } else { + candidates = allNodes.state(Node.State.active); } addNewDockerNodesOn(allocationLock, candidates); @@ -142,7 +148,6 @@ class NodePrioritizer { for (Node node : candidates) { if (node.type() != NodeType.host) continue; - if (node.state() != Node.State.active) continue; if (node.status().wantToRetire()) continue; boolean hostHasCapacityForWantedFlavor = capacity.hasCapacity(node, wantedResourceCapacity); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilter.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilter.java index 1b0bbffe507..28ead318cc0 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilter.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilter.java @@ -3,11 +3,9 @@ package com.yahoo.vespa.hosted.provision.restapi.v2.filter; import com.google.inject.Inject; import com.yahoo.config.provision.Zone; -import com.yahoo.config.provisioning.NodeRepositoryConfig; import com.yahoo.jdisc.handler.ResponseHandler; import com.yahoo.jdisc.http.filter.DiscFilterRequest; import com.yahoo.jdisc.http.filter.SecurityRequestFilter; -import com.yahoo.net.HostName; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.restapi.v2.ErrorResponse; import com.yahoo.yolean.chain.After; @@ -17,8 +15,6 @@ import java.util.Optional; import java.util.function.BiConsumer; import java.util.function.BiPredicate; import java.util.logging.Logger; -import java.util.stream.Collectors; -import java.util.stream.Stream; /** * Authorization filter for all paths in config server. It assumes that {@link NodeIdentifierFilter} is part of filter chain. @@ -35,23 +31,9 @@ public class AuthorizationFilter implements SecurityRequestFilter { private final BiConsumer<ErrorResponse, ResponseHandler> rejectAction; @Inject - public AuthorizationFilter(Zone zone, NodeRepository nodeRepository, NodeRepositoryConfig nodeRepositoryConfig) { - this( - new Authorizer( - zone.system(), - nodeRepository, - Stream.concat( - Stream.of(HostName.getLocalhost()), - Stream.of(nodeRepositoryConfig.hostnameWhitelist().split(",")) - ).filter(hostname -> !hostname.isEmpty()).collect(Collectors.toSet())), - AuthorizationFilter::logAndReject - ); - } - - AuthorizationFilter(BiPredicate<NodePrincipal, URI> authorizer, - BiConsumer<ErrorResponse, ResponseHandler> rejectAction) { - this.authorizer = authorizer; - this.rejectAction = rejectAction; + public AuthorizationFilter(Zone zone, NodeRepository nodeRepository) { + this.authorizer = new Authorizer(zone.system(), nodeRepository); + this.rejectAction = AuthorizationFilter::logAndReject; } @Override diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/Authorizer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/Authorizer.java index 6225a8a4fc4..afcde0949e3 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/Authorizer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/Authorizer.java @@ -31,7 +31,6 @@ import java.util.stream.Collectors; */ public class Authorizer implements BiPredicate<NodePrincipal, URI> { private final NodeRepository nodeRepository; - private final Set<String> whitelistedHostnames; private final AthenzIdentity controllerIdentity; private final AthenzIdentity configServerIdentity = new AthenzService("vespa.vespa", "configserver"); private final AthenzIdentity proxyIdentity = new AthenzService("vespa.vespa", "proxy"); @@ -39,10 +38,8 @@ public class Authorizer implements BiPredicate<NodePrincipal, URI> { private final Set<AthenzIdentity> trustedIdentities; private final Set<AthenzIdentity> hostAdminIdentities; - // TODO Remove whitelisted hostnames as these nodes should be included through 'trustedIdentities' - public Authorizer(SystemName system, NodeRepository nodeRepository, Set<String> whitelistedHostnames) { + public Authorizer(SystemName system, NodeRepository nodeRepository) { this.nodeRepository = nodeRepository; - this.whitelistedHostnames = whitelistedHostnames; controllerIdentity = system == SystemName.main ? new AthenzService("vespa.vespa", "hosting") : new AthenzService("vespa.vespa.cd", "hosting"); @@ -85,11 +82,6 @@ public class Authorizer implements BiPredicate<NodePrincipal, URI> { if (canAccessAny(nodeTypesFor(uri), principal, this::isNodeType)) { return true; } - - // The host itself can access all resources - if (whitelistedHostnames.contains(hostname)) { - return true; - } } return false; } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilterTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilterTest.java index b5d2861e5a0..2969b608d3a 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilterTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilterTest.java @@ -6,7 +6,6 @@ import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.Zone; -import com.yahoo.config.provisioning.NodeRepositoryConfig; import com.yahoo.vespa.curator.mock.MockCurator; import com.yahoo.vespa.hosted.provision.restapi.v2.filter.FilterTester.Request; import com.yahoo.vespa.hosted.provision.testutils.MockNodeFlavors; @@ -48,8 +47,7 @@ public class AuthorizationFilterTest { Zone zone = new Zone(system, Environment.prod, RegionName.defaultName()); return new FilterTester(new AuthorizationFilter( zone, - new MockNodeRepository(new MockCurator(), new MockNodeFlavors()), - new NodeRepositoryConfig(new NodeRepositoryConfig.Builder()))); + new MockNodeRepository(new MockCurator(), new MockNodeFlavors()))); } } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizerTest.java index 5e643bd09ab..d696328cd7f 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizerTest.java @@ -13,10 +13,6 @@ import org.junit.Before; import org.junit.Test; import java.net.URI; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.Optional; import java.util.Set; @@ -37,30 +33,27 @@ public class AuthorizerTest { public void before() { NodeFlavors flavors = new MockNodeFlavors(); nodeRepository = new MockNodeRepository(new MockCurator(), flavors); - authorizer = new Authorizer(SystemName.main, nodeRepository, new HashSet<>(Arrays.asList("cfg1", "cfghost1"))); - { // Populate with nodes used in this test. Note that only nodes requiring node repository lookup are added here - Set<String> ipAddresses = new HashSet<>(Arrays.asList("127.0.0.1", "::1")); - Flavor flavor = flavors.getFlavorOrThrow("default"); - List<Node> nodes = new ArrayList<>(); - nodes.add(nodeRepository.createNode("host1", "host1", ipAddresses, - Optional.empty(), flavor, NodeType.host)); - nodes.add(nodeRepository.createNode("child1-1", "child1-1", ipAddresses, - Optional.of("host1"), flavor, NodeType.tenant)); - nodes.add(nodeRepository.createNode("child1-2", "child1-2", ipAddresses, - Optional.of("host1"), flavor, NodeType.tenant)); - - nodes.add(nodeRepository.createNode("host2", "host2", ipAddresses, - Optional.empty(), flavor, NodeType.host)); - nodes.add(nodeRepository.createNode("child2-1", "child2-1", ipAddresses, - Optional.of("host1.tld"), flavor, NodeType.tenant)); - - nodes.add(nodeRepository.createNode("proxy1", "proxy1", ipAddresses, Optional.empty(), - flavor, NodeType.proxy)); - - nodes.add(nodeRepository.createNode("proxy1-host1", "proxy1-host", ipAddresses, - Optional.empty(), flavor, NodeType.proxyhost)); - nodeRepository.addNodes(nodes); - } + authorizer = new Authorizer(SystemName.main, nodeRepository); + + Set<String> ipAddresses = Set.of("127.0.0.1", "::1"); + Flavor flavor = flavors.getFlavorOrThrow("default"); + List<Node> nodes = List.of( + nodeRepository.createNode( + "host1", "host1", ipAddresses, Optional.empty(), flavor, NodeType.host), + nodeRepository.createNode( + "child1-1", "child1-1", ipAddresses, Optional.of("host1"), flavor, NodeType.tenant), + nodeRepository.createNode( + "child1-2", "child1-2", ipAddresses, Optional.of("host1"), flavor, NodeType.tenant), + nodeRepository.createNode( + "host2", "host2", ipAddresses, Optional.empty(), flavor, NodeType.host), + nodeRepository.createNode( + "child2-1", "child2-1", ipAddresses, Optional.of("host1.tld"), flavor, NodeType.tenant), + nodeRepository.createNode( + "proxy1", "proxy1", ipAddresses, Optional.of("proxyhost1"), flavor, NodeType.proxy), + nodeRepository.createNode( + "proxyhost1", "proxyhost1", ipAddresses, Optional.empty(), flavor, NodeType.proxyhost) + ); + nodeRepository.addNodes(nodes); } @Test @@ -106,7 +99,7 @@ public class AuthorizerTest { // Trusted services can access everything in their own system assertFalse(authorizedController("vespa.vespa.cd.hosting", "/")); // Wrong system - assertTrue(new Authorizer(SystemName.cd, nodeRepository, Collections.emptySet()).test(NodePrincipal.withAthenzIdentity("vespa.vespa.cd.hosting", emptyList()), uri("/"))); + assertTrue(new Authorizer(SystemName.cd, nodeRepository).test(NodePrincipal.withAthenzIdentity("vespa.vespa.cd.hosting", emptyList()), uri("/"))); assertTrue(authorizedController("vespa.vespa.hosting", "/")); assertTrue(authorizedController("vespa.vespa.configserver", "/")); assertTrue(authorizedController("vespa.vespa.hosting", "/nodes/v2/node/")); @@ -167,14 +160,7 @@ public class AuthorizerTest { // Node of proxy or proxyhost type can access routing resource assertFalse(authorizedTenantNode("node1", "/routing/v1/status")); assertTrue(authorizedTenantNode("proxy1", "/routing/v1/status")); - assertTrue(authorizedTenantNode("proxy1-host", "/routing/v1/status")); - } - - @Test - public void host_authorization() { - assertTrue(authorizedLegacyNode("cfg1", "/")); - assertTrue(authorizedLegacyNode("cfg1", "/application/v2")); - assertTrue(authorizedLegacyNode("cfghost1", "/application/v2")); + assertTrue(authorizedTenantNode("proxyhost1", "/routing/v1/status")); } @Test diff --git a/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp b/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp index b390edfda88..7182bd1ab16 100644 --- a/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp +++ b/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp @@ -771,7 +771,7 @@ TEST_F("require that imported attributes are exposed via attribute context toget TEST_F("require that attribute vector of wrong type is dropped", BaseFixture) { AVConfig generic_tensor(BasicType::TENSOR); - generic_tensor.setTensorType(ValueType::tensor_type({})); + generic_tensor.setTensorType(ValueType::from_spec("tensor(x{})")); AVConfig dense_tensor(BasicType::TENSOR); dense_tensor.setTensorType(ValueType::from_spec("tensor(x[10])")); AVConfig predicate(BasicType::PREDICATE); diff --git a/searchcore/src/tests/proton/attribute/attribute_populator/attribute_populator_test.cpp b/searchcore/src/tests/proton/attribute/attribute_populator/attribute_populator_test.cpp index ed71477d853..01dc2306ef6 100644 --- a/searchcore/src/tests/proton/attribute/attribute_populator/attribute_populator_test.cpp +++ b/searchcore/src/tests/proton/attribute/attribute_populator/attribute_populator_test.cpp @@ -3,17 +3,19 @@ LOG_SETUP("attribute_populator_test"); #include <vespa/vespalib/testkit/testapp.h> -#include <vespa/searchcommon/common/schema.h> +#include <vespa/document/repo/configbuilder.h> +#include <vespa/document/fieldvalue/intfieldvalue.h> #include <vespa/searchcore/proton/attribute/attribute_populator.h> #include <vespa/searchcore/proton/attribute/attributemanager.h> #include <vespa/searchcore/proton/common/hw_info.h> #include <vespa/searchcore/proton/test/test.h> #include <vespa/searchlib/common/foregroundtaskexecutor.h> -#include <vespa/searchlib/index/docbuilder.h> #include <vespa/searchlib/index/dummyfileheadercontext.h> #include <vespa/searchlib/test/directory_handler.h> #include <vespa/vespalib/util/stringfmt.h> +using document::config_builder::DocumenttypesConfigBuilderHelper; +using document::config_builder::Struct; using namespace document; using namespace proton; using namespace search; @@ -27,29 +29,30 @@ using AVConfig = search::attribute::Config; const vespalib::string TEST_DIR = "testdir"; const uint64_t CREATE_SERIAL_NUM = 8u; -Schema -createSchema() +std::unique_ptr<const DocumentTypeRepo> +makeDocTypeRepo() { - Schema schema; - schema.addAttributeField(Schema::AttributeField("a1", Schema::DataType::INT32)); - return schema; + DocumenttypesConfigBuilderHelper builder; + builder.document(-645763131, "searchdocument", + Struct("searchdocument.header"), + Struct("searchdocument.body"). + addField("a1", DataType::T_INT)); + return std::unique_ptr<const DocumentTypeRepo>(new DocumentTypeRepo(builder.config())); } struct DocContext { - Schema _schema; - DocBuilder _builder; + std::unique_ptr<const DocumentTypeRepo> _repo; DocContext() - : _schema(createSchema()), - _builder(_schema) + : _repo(makeDocTypeRepo()) { } std::shared_ptr<Document> create(uint32_t id, int64_t fieldValue) { vespalib::string docId = vespalib::make_string("id:searchdocument:searchdocument::%u", id); - return _builder.startDocument(docId). - startAttributeField("a1").addInt(fieldValue).endField(). - endDocument(); + auto doc = std::make_shared<Document>(*_repo->getDocumentType("searchdocument"), DocumentId(docId)); + doc->setValue("a1", IntFieldValue(fieldValue)); + return doc; } }; diff --git a/searchcore/src/tests/proton/matching/match_phase_limiter/match_phase_limiter_test.cpp b/searchcore/src/tests/proton/matching/match_phase_limiter/match_phase_limiter_test.cpp index d1e7adfedb8..be4bc159b1b 100644 --- a/searchcore/src/tests/proton/matching/match_phase_limiter/match_phase_limiter_test.cpp +++ b/searchcore/src/tests/proton/matching/match_phase_limiter/match_phase_limiter_test.cpp @@ -4,8 +4,11 @@ #include <vespa/searchlib/queryeval/termasstring.h> #include <vespa/searchlib/queryeval/andsearchstrict.h> #include <vespa/searchlib/queryeval/fake_requestcontext.h> +#include <vespa/searchlib/engine/trace.h> +#include <vespa/vespalib/data/slime/slime.h> using namespace proton::matching; +using namespace search::engine; using search::queryeval::SearchIterator; using search::queryeval::Searchable; using search::queryeval::Blueprint; @@ -197,7 +200,7 @@ TEST("require that no limiter has no behavior") { MaybeMatchPhaseLimiter &limiter = no_limiter; EXPECT_FALSE(limiter.is_enabled()); EXPECT_EQUAL(0u, limiter.sample_hits_per_thread(1)); - SearchIterator::UP search = limiter.maybe_limit(prepare(new MockSearch("search")), 1.0, 100000000); + SearchIterator::UP search = limiter.maybe_limit(prepare(new MockSearch("search")), 1.0, 100000000, nullptr); limiter.updateDocIdSpaceEstimate(1000, 9000); EXPECT_EQUAL(std::numeric_limits<size_t>::max(), limiter.getDocIdSpaceEstimate()); MockSearch *ms = dynamic_cast<MockSearch*>(search.get()); @@ -215,8 +218,7 @@ TEST("require that the match phase limiter may chose not to limit the query") { MaybeMatchPhaseLimiter &limiter = yes_limiter; EXPECT_TRUE(limiter.is_enabled()); EXPECT_EQUAL(20u, limiter.sample_hits_per_thread(10)); - SearchIterator::UP search = limiter.maybe_limit(prepare(new MockSearch("search")), - 0.005, 100000); + SearchIterator::UP search = limiter.maybe_limit(prepare(new MockSearch("search")), 0.005, 100000, nullptr); limiter.updateDocIdSpaceEstimate(1000, 9000); EXPECT_EQUAL(10000u, limiter.getDocIdSpaceEstimate()); MockSearch *ms = dynamic_cast<MockSearch*>(search.get()); @@ -244,7 +246,7 @@ struct MaxFilterCoverageLimiterFixture { TEST_F("require that the match phase limiter may chose not to limit the query when considering max-filter-coverage", MaxFilterCoverageLimiterFixture) { MatchPhaseLimiter::UP limiterUP = f.getMaxFilterCoverageLimiter(); MaybeMatchPhaseLimiter & limiter = *limiterUP; - SearchIterator::UP search = limiter.maybe_limit(prepare(new MockSearch("search")), 0.10, 1900000); + SearchIterator::UP search = limiter.maybe_limit(prepare(new MockSearch("search")), 0.10, 1900000, nullptr); limiter.updateDocIdSpaceEstimate(1000, 1899000); EXPECT_EQUAL(1900000u, limiter.getDocIdSpaceEstimate()); MockSearch *ms = dynamic_cast<MockSearch *>(search.get()); @@ -256,7 +258,7 @@ TEST_F("require that the match phase limiter may chose not to limit the query wh TEST_F("require that the match phase limiter may chose to limit the query even when considering max-filter-coverage", MaxFilterCoverageLimiterFixture) { MatchPhaseLimiter::UP limiterUP = f.getMaxFilterCoverageLimiter(); MaybeMatchPhaseLimiter & limiter = *limiterUP; - SearchIterator::UP search = limiter.maybe_limit(prepare(new MockSearch("search")), 0.10, 2100000); + SearchIterator::UP search = limiter.maybe_limit(prepare(new MockSearch("search")), 0.10, 2100000, nullptr); limiter.updateDocIdSpaceEstimate(1000, 2099000); EXPECT_EQUAL(159684u, limiter.getDocIdSpaceEstimate()); LimitedSearch *strict_and = dynamic_cast<LimitedSearch*>(search.get()); @@ -272,6 +274,12 @@ TEST_F("require that the match phase limiter may chose to limit the query even w EXPECT_TRUE(limiter.was_limited()); } +void verify(vespalib::stringref expected, const vespalib::Slime & slime) { + vespalib::Slime expectedSlime; + vespalib::slime::JsonFormat::decode(expected, expectedSlime); + EXPECT_EQUAL(expectedSlime, slime); +} + TEST("require that the match phase limiter is able to pre-limit the query") { FakeRequestContext requestContext; MockSearchable searchable; @@ -281,8 +289,9 @@ TEST("require that the match phase limiter is able to pre-limit the query") { MaybeMatchPhaseLimiter &limiter = yes_limiter; EXPECT_TRUE(limiter.is_enabled()); EXPECT_EQUAL(12u, limiter.sample_hits_per_thread(10)); - SearchIterator::UP search = limiter.maybe_limit(prepare(new MockSearch("search")), - 0.1, 100000); + RelativeTime clock(std::make_unique<CountingClock>(fastos::TimeStamp::fromSec(1500000000), 1700000L)); + Trace trace(clock, 7); + SearchIterator::UP search = limiter.maybe_limit(prepare(new MockSearch("search")), 0.1, 100000, trace.maybeCreateCursor(7, "limit")); limiter.updateDocIdSpaceEstimate(1000, 9000); EXPECT_EQUAL(1680u, limiter.getDocIdSpaceEstimate()); LimitedSearch *strict_and = dynamic_cast<LimitedSearch*>(search.get()); @@ -302,6 +311,27 @@ TEST("require that the match phase limiter is able to pre-limit the query") { EXPECT_EQUAL(0u, ms1->last_unpack); // will not unpack limiting term EXPECT_EQUAL(100u, ms2->last_unpack); EXPECT_TRUE(limiter.was_limited()); + trace.done(); + verify( + "{" + " start_time_utc: '2017-07-14 02:40:00.000 UTC'," + " traces: [" + " {" + " timestamp_ms: 1.7," + " tag: 'limit'," + " hit_rate: 0.1," + " num_docs: 100000," + " max_filter_docs: 100000," + " wanted_docs: 5000," + " action: 'Will limit with prefix filter'," + " max_group_size: 5000," + " current_docid: 0," + " end_docid: 2147483647," + " estimated_total_hits: 10000" + " }" + " ]," + " duration_ms: 3.4" + "}", trace.getSlime()); } TEST("require that the match phase limiter is able to post-limit the query") { @@ -313,7 +343,7 @@ TEST("require that the match phase limiter is able to post-limit the query") { MaybeMatchPhaseLimiter &limiter = yes_limiter; EXPECT_TRUE(limiter.is_enabled()); EXPECT_EQUAL(30u, limiter.sample_hits_per_thread(10)); - SearchIterator::UP search = limiter.maybe_limit(prepare(new MockSearch("search")), 0.1, 100000); + SearchIterator::UP search = limiter.maybe_limit(prepare(new MockSearch("search")), 0.1, 100000, nullptr); limiter.updateDocIdSpaceEstimate(1000, 9000); EXPECT_EQUAL(1680u, limiter.getDocIdSpaceEstimate()); LimitedSearch *strict_and = dynamic_cast<LimitedSearch*>(search.get()); @@ -343,7 +373,7 @@ void verifyDiversity(AttributeLimiter::DiversityCutoffStrategy strategy) DegradationParams("limiter_attribute", 500, true, 1.0, 0.2, 1.0), DiversityParams("category", 10, 13.1, strategy)); MaybeMatchPhaseLimiter &limiter = yes_limiter; - SearchIterator::UP search = limiter.maybe_limit(prepare(new MockSearch("search")), 0.1, 100000); + SearchIterator::UP search = limiter.maybe_limit(prepare(new MockSearch("search")), 0.1, 100000, nullptr); limiter.updateDocIdSpaceEstimate(1000, 9000); EXPECT_EQUAL(1680u, limiter.getDocIdSpaceEstimate()); LimitedSearch *strict_and = dynamic_cast<LimitedSearch*>(search.get()); diff --git a/searchcore/src/vespa/searchcore/proton/matchengine/matchengine.cpp b/searchcore/src/vespa/searchcore/proton/matchengine/matchengine.cpp index 3a28eeb4dfd..c7016d711cb 100644 --- a/searchcore/src/vespa/searchcore/proton/matchengine/matchengine.cpp +++ b/searchcore/src/vespa/searchcore/proton/matchengine/matchengine.cpp @@ -140,8 +140,9 @@ MatchEngine::performSearch(search::engine::SearchRequest::Source req, } ret->request = req.release(); ret->setDistributionKey(_distributionKey); - if (ret->request->getTraceLevel() > 0) { + if (ret->request->trace().getLevel() > 0) { ret->request->trace().getRoot().setLong("distribution-key", _distributionKey); + ret->request->trace().done(); search::fef::Properties & trace = ret->propertiesMap.lookupCreate("trace"); vespalib::SmartBuffer output(4096); vespalib::slime::BinaryFormat::encode(ret->request->trace().getSlime(), output); diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_master.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_master.cpp index 4d49e9b5d1b..7dd06038707 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_master.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/match_master.cpp @@ -5,8 +5,12 @@ #include "match_loop_communicator.h" #include "match_thread.h" #include <vespa/searchlib/attribute/attribute_operation.h> +#include <vespa/searchlib/engine/trace.h> #include <vespa/searchlib/common/featureset.h> #include <vespa/vespalib/util/thread_bundle.h> +#include <vespa/vespalib/data/slime/inserter.h> +#include <vespa/vespalib/data/slime/inject.h> +#include <vespa/vespalib/data/slime/cursor.h> #include <vespa/log/log.h> LOG_SETUP(".proton.matching.match_master"); @@ -54,7 +58,8 @@ createScheduler(uint32_t numThreads, uint32_t numSearchPartitions, uint32_t numD } // namespace proton::matching::<unnamed> ResultProcessor::Result::UP -MatchMaster::match(const MatchParams ¶ms, +MatchMaster::match(search::engine::Trace & trace, + const MatchParams ¶ms, vespalib::ThreadBundle &threadBundle, const MatchToolsFactory &mtf, ResultProcessor &resultProcessor, @@ -75,7 +80,8 @@ MatchMaster::match(const MatchParams ¶ms, ? static_cast<IMatchLoopCommunicator&>(timedCommunicator) : static_cast<IMatchLoopCommunicator&>(communicator); threadState.emplace_back(std::make_unique<MatchThread>(i, threadBundle.size(), params, mtf, com, *scheduler, - resultProcessor, mergeDirector, distributionKey)); + resultProcessor, mergeDirector, distributionKey, + trace.getRelativeTime(), trace.getLevel())); targets.push_back(threadState.back().get()); } resultProcessor.prepareThreadContextCreation(threadBundle.size()); @@ -85,9 +91,17 @@ MatchMaster::match(const MatchParams ¶ms, double query_time_s = query_latency_time.elapsed().sec(); double rerank_time_s = timedCommunicator.rerank_time.elapsed().sec(); double match_time_s = 0.0; + std::unique_ptr<vespalib::slime::Inserter> inserter; + if (trace.shouldTrace(4)) { + inserter = std::make_unique<vespalib::slime::ArrayInserter>(trace.createCursor("match_threads").setArray("threads")); + } for (size_t i = 0; i < threadState.size(); ++i) { - match_time_s = std::max(match_time_s, threadState[i]->get_match_time()); - _stats.merge_partition(threadState[i]->get_thread_stats(), i); + const MatchThread & matchThread = *threadState[i]; + match_time_s = std::max(match_time_s, matchThread.get_match_time()); + _stats.merge_partition(matchThread.get_thread_stats(), i); + if (inserter) { + vespalib::slime::inject(matchThread.getTrace().getRoot(), *inserter); + } } _stats.queryLatency(query_time_s); _stats.matchTime(match_time_s - rerank_time_s); diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_master.h b/searchcore/src/vespa/searchcore/proton/matching/match_master.h index 5de7fc144ce..c9a9a24945a 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_master.h +++ b/searchcore/src/vespa/searchcore/proton/matching/match_master.h @@ -7,6 +7,7 @@ namespace vespalib { struct ThreadBundle; } namespace search { class FeatureSet; } +namespace search::engine { class Trace; } namespace proton::matching { @@ -23,7 +24,8 @@ private: public: const MatchingStats & getStats() const { return _stats; } - ResultProcessor::Result::UP match(const MatchParams ¶ms, + ResultProcessor::Result::UP match(search::engine::Trace & trace, + const MatchParams ¶ms, vespalib::ThreadBundle &threadBundle, const MatchToolsFactory &mtf, ResultProcessor &resultProcessor, diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_phase_limiter.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_phase_limiter.cpp index 5e965084a2d..9a0590da0f2 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_phase_limiter.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/match_phase_limiter.cpp @@ -2,6 +2,7 @@ #include "match_phase_limiter.h" #include <vespa/searchlib/queryeval/andsearchstrict.h> +#include <vespa/vespalib/data/slime/cursor.h> #include <vespa/log/log.h> LOG_SETUP(".proton.matching.match_phase_limiter"); @@ -89,7 +90,7 @@ do_limit(AttributeLimiter &limiter_factory, SearchIterator::UP search, SearchIterator::UP limiter = limiter_factory.create_search(wanted_num_docs, max_group_size, PRE_FILTER); limiter = search->andWith(std::move(limiter), wanted_num_docs); if (limiter) { - search.reset(new LimitedSearchT<PRE_FILTER>(std::move(limiter), std::move(search))); + search = std::make_unique<LimitedSearchT<PRE_FILTER>>(std::move(limiter), std::move(search)); } search->initRange(current_id + 1, end_id); return search; @@ -98,12 +99,21 @@ do_limit(AttributeLimiter &limiter_factory, SearchIterator::UP search, } // namespace proton::matching::<unnamed> SearchIterator::UP -MatchPhaseLimiter::maybe_limit(SearchIterator::UP search, double match_freq, size_t num_docs) +MatchPhaseLimiter::maybe_limit(SearchIterator::UP search, double match_freq, size_t num_docs, Cursor * trace) { size_t wanted_num_docs = _calculator.wanted_num_docs(match_freq); size_t max_filter_docs = static_cast<size_t>(num_docs * _maxFilterCoverage); size_t upper_limited_corpus_size = std::min(num_docs, max_filter_docs); + if (trace) { + trace->setDouble("hit_rate", match_freq); + trace->setLong("num_docs", num_docs); + trace->setLong("max_filter_docs", max_filter_docs); + trace->setLong("wanted_docs", wanted_num_docs); + } if (upper_limited_corpus_size <= wanted_num_docs) { + if (trace) { + trace->setString("action", "Will not limit !"); + } LOG(debug, "Will not limit ! maybe_limit(hit_rate=%g, num_docs=%ld, max_filter_docs=%ld) = wanted_num_docs=%ld", match_freq, num_docs, max_filter_docs, wanted_num_docs); return search; @@ -113,6 +123,13 @@ MatchPhaseLimiter::maybe_limit(SearchIterator::UP search, double match_freq, siz size_t total_query_hits = _calculator.estimated_hits(match_freq, num_docs); size_t max_group_size = _calculator.max_group_size(wanted_num_docs); bool use_pre_filter = (wanted_num_docs < (total_query_hits * _postFilterMultiplier)); + if (trace) { + trace->setString("action", use_pre_filter ? "Will limit with prefix filter" : "Will limit with postfix filter"); + trace->setLong("max_group_size", max_group_size); + trace->setLong("current_docid", current_id); + trace->setLong("end_docid", end_id); + trace->setLong("estimated_total_hits", total_query_hits); + } LOG(debug, "Will do %s filter : maybe_limit(hit_rate=%g, num_docs=%zu, max_filter_docs=%ld) = wanted_num_docs=%zu," " max_group_size=%zu, current_docid=%u, end_docid=%u, total_query_hits=%ld", use_pre_filter ? "pre" : "post", match_freq, num_docs, max_filter_docs, wanted_num_docs, diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_phase_limiter.h b/searchcore/src/vespa/searchcore/proton/matching/match_phase_limiter.h index b39b6695b7f..46973092a05 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_phase_limiter.h +++ b/searchcore/src/vespa/searchcore/proton/matching/match_phase_limiter.h @@ -43,12 +43,13 @@ private: * limit the number of matches. **/ struct MaybeMatchPhaseLimiter { + using Cursor = vespalib::slime::Cursor; typedef search::queryeval::SearchIterator SearchIterator; typedef std::unique_ptr<MaybeMatchPhaseLimiter> UP; virtual bool is_enabled() const = 0; virtual bool was_limited() const = 0; virtual size_t sample_hits_per_thread(size_t num_threads) const = 0; - virtual SearchIterator::UP maybe_limit(SearchIterator::UP search, double match_freq, size_t num_docs) = 0; + virtual SearchIterator::UP maybe_limit(SearchIterator::UP search, double match_freq, size_t num_docs, Cursor * trace) = 0; virtual void updateDocIdSpaceEstimate(size_t searchedDocIdSpace, size_t remainingDocIdSpace) = 0; virtual size_t getDocIdSpaceEstimate() const = 0; virtual ~MaybeMatchPhaseLimiter() {} @@ -61,7 +62,7 @@ struct NoMatchPhaseLimiter : MaybeMatchPhaseLimiter { bool is_enabled() const override { return false; } bool was_limited() const override { return false; } size_t sample_hits_per_thread(size_t) const override { return 0; } - SearchIterator::UP maybe_limit(SearchIterator::UP search, double, size_t) override { + SearchIterator::UP maybe_limit(SearchIterator::UP search, double, size_t, Cursor *) override { return search; } void updateDocIdSpaceEstimate(size_t, size_t) override { } @@ -144,7 +145,7 @@ public: size_t sample_hits_per_thread(size_t num_threads) const override { return _calculator.sample_hits_per_thread(num_threads); } - SearchIterator::UP maybe_limit(SearchIterator::UP search, double match_freq, size_t num_docs) override; + SearchIterator::UP maybe_limit(SearchIterator::UP search, double match_freq, size_t num_docs, Cursor * trace) override; void updateDocIdSpaceEstimate(size_t searchedDocIdSpace, size_t remainingDocIdSpace) override; size_t getDocIdSpaceEstimate() const override; }; diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_thread.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_thread.cpp index 2e92f2d63c3..e91ca65a7e6 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_thread.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/match_thread.cpp @@ -2,6 +2,7 @@ #include "match_thread.h" #include "document_scorer.h" +#include <vespa/searchlib/engine/trace.h> #include <vespa/searchlib/attribute/attribute_operation.h> #include <vespa/searchcommon/attribute/i_attribute_functor.h> #include <vespa/searchcore/grouping/groupingmanager.h> @@ -13,6 +14,8 @@ #include <vespa/searchlib/queryeval/andnotsearch.h> #include <vespa/vespalib/util/closure.h> #include <vespa/vespalib/util/thread_bundle.h> +#include <vespa/vespalib/data/slime/cursor.h> +#include <vespa/vespalib/data/slime/inserter.h> #include <vespa/log/log.h> LOG_SETUP(".proton.matching.match_thread"); @@ -117,14 +120,19 @@ MatchThread::maybe_limit(MatchTools &tools, uint32_t matches, uint32_t docId, ui const size_t searchedSoFar = (scheduler.total_size(thread_id) - local_todo); double match_freq = estimate_match_frequency(matches, searchedSoFar); const size_t global_todo = scheduler.unassigned_size(); + vespalib::slime::Cursor * traceCursor = trace->maybeCreateCursor(5, "maybe_limit"); { auto search = tools.borrow_search(); - search = tools.match_limiter().maybe_limit(std::move(search), match_freq, matchParams.numDocs); + search = tools.match_limiter().maybe_limit(std::move(search), match_freq, matchParams.numDocs, traceCursor); tools.give_back_search(std::move(search)); if (tools.match_limiter().was_limited()) { tools.tag_search_as_changed(); } } + if (isFirstThread() && trace->shouldTrace(6) && tools.match_limiter().was_limited()) { + vespalib::slime::ObjectInserter inserter(trace->createCursor("limited"), "query"); + tools.search().asSlime(inserter); + } size_t left = local_todo + (global_todo / num_threads); tools.match_limiter().updateDocIdSpaceEstimate(searchedSoFar, left); LOG(debug, "Limit=%d has been reached at docid=%d which is after %zu docs.", @@ -267,17 +275,24 @@ MatchThread::findMatches(MatchTools &tools) tools.give_back_search(search::queryeval::MultiBitVectorIteratorBase::optimize(tools.borrow_search())); if (isFirstThread()) { LOG(debug, "SearchIterator after MultiBitVectorIteratorBase::optimize(): %s", tools.search().asString().c_str()); + if (trace->shouldTrace(7)) { + vespalib::slime::ObjectInserter inserter(trace->createCursor("iterator"), "optimized"); + tools.search().asSlime(inserter); + } } HitCollector hits(matchParams.numDocs, matchParams.arraySize); + trace->addEvent(4, "Start match and first phase rank"); match_loop_helper(tools, hits); if (tools.has_second_phase_rank()) { { // 2nd phase ranking + trace->addEvent(4, "Start second phase rerank"); tools.setup_second_phase(); DocidRange docid_range = scheduler.total_span(thread_id); tools.search().initRange(docid_range.begin, docid_range.end); auto sorted_hit_seq = matchToolsFactory.should_diversify() ? hits.getSortedHitSequence(matchParams.arraySize) : hits.getSortedHitSequence(matchParams.heapSize); + trace->addEvent(5, "Synchronize before second phase rerank"); WaitTimer select_best_timer(wait_time_s); auto kept_hits = communicator.selectBest(sorted_hit_seq); select_best_timer.done(); @@ -292,6 +307,7 @@ MatchThread::findMatches(MatchTools &tools) thread_stats.docsReRanked(reRanked); } { // rank scaling + trace->addEvent(5, "Synchronize before rank scaling"); auto my_ranges = hits.getRanges(); WaitTimer range_cover_timer(wait_time_s); auto ranges = communicator.rangeCover(my_ranges); @@ -299,6 +315,7 @@ MatchThread::findMatches(MatchTools &tools) hits.setRanges(ranges); } } + trace->addEvent(4, "Create result set"); return hits.getResultSet(fallback_rank_value()); } @@ -373,7 +390,9 @@ MatchThread::MatchThread(size_t thread_id_in, DocidRangeScheduler &sched, ResultProcessor &rp, vespalib::DualMergeDirector &md, - uint32_t distributionKey) : + uint32_t distributionKey, + const RelativeTime & relativeTime, + uint32_t traceLevel) : thread_id(thread_id_in), num_threads(num_threads_in), matchParams(mp), @@ -389,7 +408,8 @@ MatchThread::MatchThread(size_t thread_id_in, total_time_s(0.0), match_time_s(0.0), wait_time_s(0.0), - match_with_ranking(mtf.has_first_phase_rank() && mp.save_rank_scores()) + match_with_ranking(mtf.has_first_phase_rank() && mp.save_rank_scores()), + trace(std::make_unique<Trace>(relativeTime, traceLevel)) { } @@ -400,12 +420,14 @@ MatchThread::run() fastos::StopWatch match_time; total_time.start(); match_time.start(); + trace->addEvent(4, "Start MatchThread::run"); MatchTools::UP matchTools = matchToolsFactory.createMatchTools(); search::ResultSet::UP result = findMatches(*matchTools); match_time.stop(); match_time_s = match_time.elapsed().sec(); resultContext = resultProcessor.createThreadContext(matchTools->getHardDoom(), thread_id, _distributionKey); { + trace->addEvent(5, "Wait for result processing token"); WaitTimer get_token_timer(wait_time_s); QueryLimiter::Token::UP processToken( matchTools->getQueryLimiter().getToken(matchTools->getHardDoom(), @@ -414,12 +436,15 @@ MatchThread::run() resultContext->sort->hasSortData(), resultContext->grouping.get() != 0)); get_token_timer.done(); + trace->addEvent(5, "Start result processing"); processResult(matchTools->getHardDoom(), std::move(result), *resultContext); } total_time.stop(); total_time_s = total_time.elapsed().sec(); thread_stats.active_time(total_time_s - wait_time_s).wait_time(wait_time_s); + trace->addEvent(4, "Start thread merge"); mergeDirector.dualMerge(thread_id, *resultContext->result, resultContext->groupingSource); + trace->addEvent(4, "MatchThread::run Done"); } } diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_thread.h b/searchcore/src/vespa/searchcore/proton/matching/match_thread.h index 06728dc006a..25f83be84a3 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_thread.h +++ b/searchcore/src/vespa/searchcore/proton/matching/match_thread.h @@ -15,6 +15,11 @@ #include <vespa/searchlib/common/sortresults.h> #include <vespa/searchlib/queryeval/hitcollector.h> +namespace search::engine { + class Trace; + class RelativeTime; +} + namespace proton::matching { /** @@ -30,6 +35,8 @@ public: using RankProgram = search::fef::RankProgram; using LazyValue = search::fef::LazyValue; using Doom = vespalib::Doom; + using Trace = search::engine::Trace; + using RelativeTime = search::engine::RelativeTime; private: size_t thread_id; @@ -48,6 +55,7 @@ private: double match_time_s; double wait_time_s; bool match_with_ranking; + std::unique_ptr<Trace> trace; class Context { public: @@ -103,11 +111,14 @@ public: DocidRangeScheduler &sched, ResultProcessor &rp, vespalib::DualMergeDirector &md, - uint32_t distributionKey); + uint32_t distributionKey, + const RelativeTime & relativeTime, + uint32_t traceLevel); void run() override; const MatchingStats::Partition &get_thread_stats() const { return thread_stats; } double get_match_time() const { return match_time_s; } PartialResult::UP extract_result() { return std::move(resultContext->result); } + const Trace & getTrace() const { return *trace; } }; } diff --git a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp index 4ebf74c373f..1692abed011 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp @@ -100,10 +100,11 @@ Matcher::getFeatureSet(const DocsumRequest & req, ISearchContext & searchCtx, IA SessionManager & sessionMgr, bool summaryFeatures) { SessionId sessionId(&req.sessionId[0], req.sessionId.size()); + bool expectedSessionCached(false); if (!sessionId.empty()) { const Properties &cache_props = req.propertiesMap.cacheProperties(); - bool searchSessionCached = cache_props.lookup("query").found(); - if (searchSessionCached) { + expectedSessionCached = cache_props.lookup("query").found(); + if (expectedSessionCached) { SearchSession::SP session(sessionMgr.pickSearch(sessionId)); if (session) { MatchToolsFactory &mtf = session->getMatchToolsFactory(); @@ -118,8 +119,9 @@ Matcher::getFeatureSet(const DocsumRequest & req, ISearchContext & searchCtx, IA MatchToolsFactory::UP mtf = create_match_tools_factory(req, searchCtx, attrCtx, metaStore, req.propertiesMap.featureOverrides()); if (!mtf->valid()) { - LOG(warning, "getFeatureSet(%s): query execution failed (invalid query). Returning empty feature set", - (summaryFeatures ? "summary features" : "rank features")); + LOG(warning, "getFeatureSet(%s): query execution failed (%s). Returning empty feature set", + (summaryFeatures ? "summary features" : "rank features"), + (expectedSessionCached) ? "session has expired" : "invalid query"); return std::make_shared<FeatureSet>(); } return findFeatureSet(req, *mtf, summaryFeatures); @@ -205,10 +207,10 @@ Matcher::computeNumThreadsPerSearch(Blueprint::HitEstimate hits, const Propertie } namespace { - void traceQuery(const SearchRequest &request, const Query & query) { - if (request.getTraceLevel() > 3) { + void traceQuery(uint32_t traceLevel, Trace & trace, const Query & query) { + if (traceLevel <= trace.getLevel()) { if (query.peekRoot()) { - vespalib::slime::ObjectInserter inserter(request.trace().createCursor("blueprint"), "optimized"); + vespalib::slime::ObjectInserter inserter(trace.createCursor("blueprint"), "optimized"); query.peekRoot()->asSlime(inserter); } } @@ -249,10 +251,10 @@ Matcher::match(const SearchRequest &request, vespalib::ThreadBundle &threadBundl } MatchToolsFactory::UP mtf = create_match_tools_factory(request, searchContext, attrContext, metaStore, *feature_overrides); + traceQuery(6, request.trace(), mtf->query()); if (!mtf->valid()) { reply->errorCode = ECODE_QUERY_PARSE_ERROR; reply->errorMessage = "query execution failed (invalid query)"; - traceQuery(request, mtf->query()); return reply; } @@ -269,10 +271,9 @@ Matcher::match(const SearchRequest &request, vespalib::ThreadBundle &threadBundl size_t numThreadsPerSearch = computeNumThreadsPerSearch(mtf->estimate(), rankProperties); LimitedThreadBundleWrapper limitedThreadBundle(threadBundle, numThreadsPerSearch); MatchMaster master; - uint32_t numSearchPartitions = NumSearchPartitions::lookup(rankProperties, - _rankSetup->getNumSearchPartitions()); - ResultProcessor::Result::UP result = master.match(params, limitedThreadBundle, *mtf, rp, - _distributionKey, numSearchPartitions); + uint32_t numParts = NumSearchPartitions::lookup(rankProperties, _rankSetup->getNumSearchPartitions()); + ResultProcessor::Result::UP result = master.match(request.trace(), params, limitedThreadBundle, *mtf, rp, + _distributionKey, numParts); my_stats = MatchMaster::getStats(std::move(master)); bool wasLimited = mtf->match_limiter().was_limited(); @@ -281,13 +282,12 @@ Matcher::match(const SearchRequest &request, vespalib::ThreadBundle &threadBundl : mtf->match_limiter().getDocIdSpaceEstimate(); uint32_t estHits = mtf->estimate().estHits; if (shouldCacheSearchSession && ((result->_numFs4Hits != 0) || shouldCacheGroupingSession)) { - SearchSession::SP session = std::make_shared<SearchSession>(sessionId, request.getTimeOfDoom(), - std::move(mtf), std::move(owned_objects)); + auto session = std::make_shared<SearchSession>(sessionId, request.getTimeOfDoom(), + std::move(mtf), std::move(owned_objects)); session->releaseEnumGuards(); sessionMgr.insert(std::move(session)); } reply = std::move(result->_reply); - traceQuery(request, mtf->query()); uint32_t numActiveLids = metaStore.getNumActiveLids(); // note: this is actually totalSpace+1, since 0 is reserved diff --git a/searchcore/src/vespa/searchcore/proton/test/attribute_utils.h b/searchcore/src/vespa/searchcore/proton/test/attribute_utils.h index f1d661a3ad0..266c439c6fd 100644 --- a/searchcore/src/vespa/searchcore/proton/test/attribute_utils.h +++ b/searchcore/src/vespa/searchcore/proton/test/attribute_utils.h @@ -46,7 +46,7 @@ struct AttributeUtils return search::attribute::Config(search::attribute::BasicType::PREDICATE); } static search::attribute::Config getTensorConfig() { - return search::attribute::Config(search::attribute::BasicType::TENSOR); + return search::attribute::Config(search::attribute::BasicType::TENSOR).setTensorType(vespalib::eval::ValueType::from_spec("tensor(x{},y{})")); } }; diff --git a/searchlib/src/tests/attribute/tensorattribute/tensorattribute_test.cpp b/searchlib/src/tests/attribute/tensorattribute/tensorattribute_test.cpp index d041dde52a5..2e339a069b6 100644 --- a/searchlib/src/tests/attribute/tensorattribute/tensorattribute_test.cpp +++ b/searchlib/src/tests/attribute/tensorattribute/tensorattribute_test.cpp @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include <vespa/vespalib/testkit/test_kit.h> +#include <vespa/document/base/exceptions.h> #include <vespa/searchlib/tensor/tensor_attribute.h> #include <vespa/searchlib/tensor/generic_tensor_attribute.h> #include <vespa/searchlib/tensor/dense_tensor_attribute.h> @@ -13,6 +14,7 @@ #include <vespa/log/log.h> LOG_SETUP("tensorattribute_test"); +using document::WrongTensorTypeException; using search::tensor::TensorAttribute; using search::tensor::DenseTensorAttribute; using search::tensor::GenericTensorAttribute; @@ -258,10 +260,14 @@ Fixture::testSetTensorValue() EXPECT_EQUAL(5u, _attr->getNumDocs()); EXPECT_EQUAL(5u, _attr->getCommittedDocIdLimit()); TEST_DO(assertGetNoTensor(4)); - setTensor(4, *createTensor({}, {})); + EXPECT_EXCEPTION(setTensor(4, *createTensor({}, {})), + WrongTensorTypeException, + "but other tensor type is 'double'"); + TEST_DO(assertGetNoTensor(4)); + setTensor(4, *_tensorAttr->getEmptyTensor()); if (_denseTensors) { TEST_DO(assertGetTensor(*expEmptyDenseTensor(), 4)); - setTensor(3, *createTensor({ {{{"y","1"}}, 11} }, { "x", "y"})); + setTensor(3, *expDenseTensor3()); TEST_DO(assertGetTensor(*expDenseTensor3(), 3)); } else { TEST_DO(assertGetTensor({}, {"x", "y"}, 4)); @@ -277,8 +283,12 @@ void Fixture::testSaveLoad() { ensureSpace(4); - setTensor(4, *createTensor({}, {})); - setTensor(3, *createTensor({ {{{"y","1"}}, 11} }, { "x", "y"})); + setTensor(4, *_tensorAttr->getEmptyTensor()); + if (_denseTensors) { + setTensor(3, *expDenseTensor3()); + } else { + setTensor(3, *createTensor({ {{{"y","1"}}, 11} }, { "x", "y"})); + } TEST_DO(save()); TEST_DO(load()); EXPECT_EQUAL(5u, _attr->getNumDocs()); @@ -298,10 +308,15 @@ void Fixture::testCompaction() { ensureSpace(4); - Tensor::UP emptytensor = createTensor({}, {}); + Tensor::UP emptytensor = _tensorAttr->getEmptyTensor(); Tensor::UP emptyxytensor = createTensor({}, {"x", "y"}); Tensor::UP simpletensor = createTensor({ {{{"y","1"}}, 11} }, { "x", "y"}); Tensor::UP filltensor = createTensor({ {{}, 5} }, { "x", "y"}); + if (_denseTensors) { + emptyxytensor = expEmptyDenseTensor(); + simpletensor = expDenseTensor3(); + filltensor = expDenseFillTensor(); + } setTensor(4, *emptytensor); setTensor(3, *simpletensor); setTensor(2, *filltensor); @@ -325,11 +340,6 @@ Fixture::testCompaction() "iter = %" PRIu64 ", memory usage %" PRIu64 ", -> %" PRIu64, iter, oldStatus.getUsed(), newStatus.getUsed()); TEST_DO(assertGetNoTensor(1)); - if (_denseTensors) { - emptyxytensor = expEmptyDenseTensor(); - simpletensor = expDenseTensor3(); - filltensor = expDenseFillTensor(); - } TEST_DO(assertGetTensor(*filltensor, 2)); TEST_DO(assertGetTensor(*simpletensor, 3)); TEST_DO(assertGetTensor(*emptyxytensor, 4)); @@ -371,12 +381,6 @@ Fixture::testEmptyTensor() } -TEST_F("Test empty sparse tensor attribute", Fixture("tensor()")) -{ - f.testEmptyAttribute(); -} - - template <class MakeFixture> void testAll(MakeFixture &&f) { diff --git a/searchlib/src/tests/engine/searchapi/searchapi_test.cpp b/searchlib/src/tests/engine/searchapi/searchapi_test.cpp index 297a45f5b22..2f2cf4f22e5 100644 --- a/searchlib/src/tests/engine/searchapi/searchapi_test.cpp +++ b/searchlib/src/tests/engine/searchapi/searchapi_test.cpp @@ -235,12 +235,12 @@ void verify(vespalib::stringref expected, const vespalib::Slime & slime) { } TEST("verify trace") { - RelativeTime clock(std::make_unique<CountingClock>(7)); + RelativeTime clock(std::make_unique<CountingClock>(fastos::TimeStamp::fromSec(1500000000), 1700000L)); Trace t(clock); verify("{" " traces: [" " ]," - " creation_time: 7" + " start_time_utc: '2017-07-14 02:40:00.000 UTC'" "}", t.getSlime()); t.createCursor("tag_a"); @@ -248,27 +248,29 @@ TEST("verify trace") { " traces: [" " {" " tag: 'tag_a'," - " time: 1" + " timestamp_ms: 1.7" " }" " ]," - " creation_time: 7" + " start_time_utc: '2017-07-14 02:40:00.000 UTC'" "}", t.getSlime()); Trace::Cursor & tagB = t.createCursor("tag_b"); tagB.setLong("long", 19); + t.done(); verify("{" " traces: [" " {" " tag: 'tag_a'," - " time: 1" + " timestamp_ms: 1.7" " }," " {" " tag: 'tag_b'," - " time: 2," + " timestamp_ms: 3.4," " long: 19" " }" " ]," - " creation_time: 7" + " start_time_utc: '2017-07-14 02:40:00.000 UTC'," + " duration_ms: 5.1" "}", t.getSlime()); } diff --git a/searchlib/src/tests/queryeval/queryeval.cpp b/searchlib/src/tests/queryeval/queryeval.cpp index bd8252521f8..db7f8d1cda1 100644 --- a/searchlib/src/tests/queryeval/queryeval.cpp +++ b/searchlib/src/tests/queryeval/queryeval.cpp @@ -20,6 +20,7 @@ #include <vespa/vespalib/test/insertion_operators.h> #include <vespa/searchlib/queryeval/isourceselector.h> #include <vespa/searchlib/fef/fef.h> +#include <vespa/vespalib/data/slime/slime.h> #include <vespa/log/log.h> LOG_SETUP("query_eval_test"); @@ -448,6 +449,131 @@ TEST("testRank") { } } +vespalib::string +getExpectedSlime() { + return +"{" +" '[type]': 'search::queryeval::AndSearchStrict<search::queryeval::(anonymous namespace)::FullUnpack>'," +" children: {" +" '[type]': 'std::vector'," +" [0]: {" +" '[type]': 'search::queryeval::(anonymous namespace)::AndNotSearchStrict'," +" children: {" +" '[type]': 'std::vector'," +" [0]: {" +" '[type]': 'search::queryeval::SimpleSearch'," +" tag: '+'" +" }," +" [1]: {" +" '[type]': 'search::queryeval::SimpleSearch'," +" tag: '-'" +" }" +" }" +" }," +" [1]: {" +" '[type]': 'search::queryeval::AndSearchStrict<search::queryeval::(anonymous namespace)::FullUnpack>'," +" children: {" +" '[type]': 'std::vector'," +" [0]: {" +" '[type]': 'search::queryeval::SimpleSearch'," +" tag: 'and_a'" +" }," +" [1]: {" +" '[type]': 'search::queryeval::SimpleSearch'," +" tag: 'and_b'" +" }" +" }" +" }," +" [2]: {" +" '[type]': 'search::queryeval::BooleanMatchIteratorWrapper'," +" 'search': {" +" '[type]': 'search::queryeval::SimpleSearch'," +" tag: 'wrapped'" +" }" +" }," +" [3]: {" +" '[type]': 'search::queryeval::NearSearch'," +" children: {" +" '[type]': 'std::vector'," +" [0]: {" +" '[type]': 'search::queryeval::SimpleSearch'," +" tag: 'near_a'" +" }," +" [1]: {" +" '[type]': 'search::queryeval::SimpleSearch'," +" tag: 'near_b'" +" }" +" }," +" data_size: 0," +" window: 5," +" strict: true" +" }," +" [4]: {" +" '[type]': 'search::queryeval::ONearSearch'," +" children: {" +" '[type]': 'std::vector'," +" [0]: {" +" '[type]': 'search::queryeval::SimpleSearch'," +" tag: 'onear_a'" +" }," +" [1]: {" +" '[type]': 'search::queryeval::SimpleSearch'," +" tag: 'onear_b'" +" }" +" }," +" data_size: 0," +" window: 10," +" strict: true" +" }," +" [5]: {" +" '[type]': 'search::queryeval::OrLikeSearch<false, search::queryeval::(anonymous namespace)::FullUnpack>'," +" children: {" +" '[type]': 'std::vector'," +" [0]: {" +" '[type]': 'search::queryeval::SimpleSearch'," +" tag: 'or_a'" +" }," +" [1]: {" +" '[type]': 'search::queryeval::SimpleSearch'," +" tag: 'or_b'" +" }" +" }," +" strict: false" +" }," +" [6]: {" +" '[type]': 'search::queryeval::RankSearch'," +" children: {" +" '[type]': 'std::vector'," +" [0]: {" +" '[type]': 'search::queryeval::SimpleSearch'," +" tag: 'rank_a'" +" }," +" [1]: {" +" '[type]': 'search::queryeval::SimpleSearch'," +" tag: 'rank_b'" +" }" +" }" +" }," +" [7]: {" +" '[type]': 'search::queryeval::SourceBlenderSearchStrict'," +" children: {" +" '[type]': 'std::vector'," +" [0]: 2," +" [1]: 4" +" }," +" 'Source \u0002': {" +" '[type]': 'search::queryeval::SimpleSearch'," +" tag: 'blend_a'" +" }," +" 'Source \u0004': {" +" '[type]': 'search::queryeval::SimpleSearch'," +" tag: 'blend_b'" +" }" +" }" +" }" +"}"; +} + TEST("testDump") { typedef SourceBlenderSearch::Child Source; SearchIterator::UP search( @@ -469,6 +595,12 @@ TEST("testDump") { , true)); vespalib::string sas = search->asString(); EXPECT_TRUE(sas.size() > 50); + vespalib::Slime slime; + search->asSlime(vespalib::slime::SlimeInserter(slime)); + auto s = slime.toString(); + vespalib::Slime expectedSlime; + vespalib::slime::JsonFormat::decode(getExpectedSlime(), expectedSlime); + EXPECT_EQUAL(expectedSlime, slime); // fprintf(stderr, "%s", search->asString().c_str()); } diff --git a/searchlib/src/vespa/searchlib/engine/packetconverter.cpp b/searchlib/src/vespa/searchlib/engine/packetconverter.cpp index d87d9888aae..c35ce9ded05 100644 --- a/searchlib/src/vespa/searchlib/engine/packetconverter.cpp +++ b/searchlib/src/vespa/searchlib/engine/packetconverter.cpp @@ -19,9 +19,7 @@ struct FS4PropertiesBuilder : public search::fef::IPropertiesVisitor { uint32_t idx; search::fs4transport::FS4Properties &props; FS4PropertiesBuilder(search::fs4transport::FS4Properties &p) : idx(0), props(p) {} - void visitProperty(const Property::Value &key, - const Property &values) override - { + void visitProperty(const Property::Value &key, const Property &values) override { for (uint32_t i = 0; i < values.size(); ++i) { props.setKey(idx, key.data(), key.size()); props.setValue(idx, values.getAt(i).data(), values.getAt(i).size()); diff --git a/searchlib/src/vespa/searchlib/engine/request.cpp b/searchlib/src/vespa/searchlib/engine/request.cpp index 87f5d372609..956653d5269 100644 --- a/searchlib/src/vespa/searchlib/engine/request.cpp +++ b/searchlib/src/vespa/searchlib/engine/request.cpp @@ -8,14 +8,13 @@ namespace search::engine { Request::Request(RelativeTime relativeTime) : _relativeTime(std::move(relativeTime)), _timeOfDoom(fastos::TimeStamp(fastos::TimeStamp::FUTURE)), - _traceLevel(0), queryFlags(0), ranking(), location(), propertiesMap(), stackItems(0), stackDump(), - _trace(_relativeTime) + _trace(_relativeTime, 0) { } diff --git a/searchlib/src/vespa/searchlib/engine/request.h b/searchlib/src/vespa/searchlib/engine/request.h index 9f8b0c9c1ae..733043b0e4e 100644 --- a/searchlib/src/vespa/searchlib/engine/request.h +++ b/searchlib/src/vespa/searchlib/engine/request.h @@ -21,7 +21,7 @@ public: fastos::TimeStamp getTimeout() const { return _timeOfDoom - getStartTime(); } fastos::TimeStamp getTimeUsed() const; fastos::TimeStamp getTimeLeft() const; - const RelativeTime & getRelativeTime() { return _relativeTime; } + const RelativeTime & getRelativeTime() const { return _relativeTime; } bool expired() const { return getTimeLeft() <= 0l; } const vespalib::stringref getStackRef() const { @@ -30,14 +30,12 @@ public: bool should_drop_sort_data() const; - uint32_t getTraceLevel() const { return _traceLevel; } - Request & setTraceLevel(uint32_t traceLevel) { _traceLevel = traceLevel; return *this; } + Request & setTraceLevel(uint32_t level) { _trace.setLevel(level); return *this; } Trace & trace() const { return _trace; } private: RelativeTime _relativeTime; fastos::TimeStamp _timeOfDoom; - uint32_t _traceLevel; public: /// Everything here should move up to private section and have accessors uint32_t queryFlags; diff --git a/searchlib/src/vespa/searchlib/engine/trace.cpp b/searchlib/src/vespa/searchlib/engine/trace.cpp index b11a44f3586..ba92902d2fd 100644 --- a/searchlib/src/vespa/searchlib/engine/trace.cpp +++ b/searchlib/src/vespa/searchlib/engine/trace.cpp @@ -10,13 +10,23 @@ RelativeTime::RelativeTime(std::unique_ptr<Clock> clock) _clock(std::move(clock)) {} -Trace::Trace(const RelativeTime & relativeTime) +namespace { + +Trace::Cursor & +createRoot(vespalib::Slime & slime, const RelativeTime & relativeTime) { + Trace::Cursor & root = slime.setObject(); + root.setString("start_time_utc", relativeTime.timeOfDawn().toString()); + return root; +} + +} +Trace::Trace(const RelativeTime & relativeTime, uint32_t level) : _trace(std::make_unique<vespalib::Slime>()), - _root(_trace->setObject()), + _root(createRoot(*_trace, relativeTime)), _traces(_root.setArray("traces")), - _relativeTime(relativeTime) + _relativeTime(relativeTime), + _level(level) { - _root.setLong("creation_time", _relativeTime.timeOfDawn()); } Trace::~Trace() = default; @@ -24,11 +34,34 @@ Trace::~Trace() = default; Trace::Cursor & Trace::createCursor(vespalib::stringref name) { Cursor & trace = _traces.addObject(); + addTimeStamp(trace); trace.setString("tag", name); - trace.setLong("time", _relativeTime.timeSinceDawn()); return trace; } +Trace::Cursor * +Trace::maybeCreateCursor(uint32_t level, vespalib::stringref name) { + return shouldTrace(level) ? & createCursor(name) : nullptr; +} + +void +Trace::addEvent(uint32_t level, vespalib::stringref event) { + if (!shouldTrace(level)) { return; } + + Cursor & trace = _traces.addObject(); + addTimeStamp(trace); + trace.setString("event", event); +} + +void +Trace::addTimeStamp(Cursor & trace) { + trace.setDouble("timestamp_ms", _relativeTime.timeSinceDawn()/1000000.0); +} + +void Trace::done() { + _root.setDouble("duration_ms", _relativeTime.timeSinceDawn()/1000000.0); +} + vespalib::string Trace::toString() const { return _trace->toString(); diff --git a/searchlib/src/vespa/searchlib/engine/trace.h b/searchlib/src/vespa/searchlib/engine/trace.h index c9a6441c23d..41f2c608615 100644 --- a/searchlib/src/vespa/searchlib/engine/trace.h +++ b/searchlib/src/vespa/searchlib/engine/trace.h @@ -14,21 +14,23 @@ class Clock { public: virtual ~Clock() = default; virtual fastos::TimeStamp now() const = 0; - virtual Clock * clone() const = 0; }; class FastosClock : public Clock { public: fastos::TimeStamp now() const override { return fastos::ClockSystem::now(); } - FastosClock * clone() const override { return new FastosClock(*this); } }; class CountingClock : public Clock { public: - CountingClock(int64_t start) : _nextTime(start) { } - fastos::TimeStamp now() const override { return _nextTime++; } - CountingClock * clone() const override { return new CountingClock(*this); } + CountingClock(int64_t start, int64_t increment) : _increment(increment), _nextTime(start) { } + fastos::TimeStamp now() const override { + int64_t prev = _nextTime; + _nextTime += _increment; + return prev; + } private: + const int64_t _increment; mutable int64_t _nextTime; }; @@ -51,7 +53,7 @@ class Trace { public: using Cursor = vespalib::slime::Cursor; - Trace(const RelativeTime & relativeTime); + Trace(const RelativeTime & relativeTime, uint32_t traceLevel=0); ~Trace(); /** @@ -60,14 +62,33 @@ public: * @return a Cursor to use for further tracing. */ Cursor & createCursor(vespalib::stringref name); + Cursor * maybeCreateCursor(uint32_t level, vespalib::stringref name); + /** + * Will add a simple 'event' string. It will also add a timestamp relative to the creation of the trace. + * @param level require for actually add the trace. + * @param event + */ + void addEvent(uint32_t level, vespalib::stringref event); + + /** + * Will compute and and a final duration timing. + */ + void done(); + vespalib::string toString() const; Cursor & getRoot() const { return _root; } vespalib::Slime & getSlime() const { return *_trace; } + bool shouldTrace(uint32_t level) const { return level <= _level; } + uint32_t getLevel() const { return _level; } + Trace & setLevel(uint32_t level) { _level = level; return *this; } + const RelativeTime & getRelativeTime() const { return _relativeTime; } private: + void addTimeStamp(Cursor & trace); std::unique_ptr<vespalib::Slime> _trace; Cursor & _root; Cursor & _traces; const RelativeTime & _relativeTime; + uint32_t _level; }; } diff --git a/searchlib/src/vespa/searchlib/queryeval/searchiterator.cpp b/searchlib/src/vespa/searchlib/queryeval/searchiterator.cpp index 3384e0fc8c8..c4a308a17f6 100644 --- a/searchlib/src/vespa/searchlib/queryeval/searchiterator.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/searchiterator.cpp @@ -3,12 +3,13 @@ #include "searchiterator.h" #include <vespa/searchlib/index/docidandfeatures.h> #include <vespa/vespalib/objects/objectdumper.h> +#include <vespa/vespalib/objects/object2slime.h> #include <vespa/vespalib/objects/visit.h> #include <vespa/vespalib/util/classname.h> #include <vespa/searchlib/common/bitvector.h> +#include <vespa/vespalib/data/slime/inserter.h> -namespace search { -namespace queryeval { +namespace search::queryeval { SearchIterator::SearchIterator() : _docid(0), @@ -106,6 +107,15 @@ SearchIterator::asString() const return dumper.toString(); } +vespalib::slime::Cursor & +SearchIterator::asSlime(const vespalib::slime::Inserter & inserter) const +{ + vespalib::slime::Cursor & cursor = inserter.insertObject(); + vespalib::Object2Slime dumper(cursor); + visit(dumper, "", this); + return cursor; +} + vespalib::string SearchIterator::getClassName() const { @@ -125,8 +135,7 @@ SearchIterator::getAttributeSearchContext() const return nullptr; } -} // namespace queryeval -} // namespace search +} //----------------------------------------------------------------------------- diff --git a/searchlib/src/vespa/searchlib/queryeval/searchiterator.h b/searchlib/src/vespa/searchlib/queryeval/searchiterator.h index dfa342b018a..ab662dd2592 100644 --- a/searchlib/src/vespa/searchlib/queryeval/searchiterator.h +++ b/searchlib/src/vespa/searchlib/queryeval/searchiterator.h @@ -10,6 +10,10 @@ #include <vector> namespace vespalib { class ObjectVisitor; } +namespace vespalib::slime { + class Cursor; + class Inserter; +} namespace search { class BitVector; } namespace search::attribute { class ISearchContext; } @@ -284,6 +288,15 @@ public: vespalib::string asString() const; /** + * Create a slime representation of this object. This + * method will use object visitation internally to capture the + * full structure of this object. + * + * @return structured slime representation of this object + **/ + vespalib::slime::Cursor & asSlime(const vespalib::slime::Inserter & cursor) const; + + /** * Obtain the fully qualified name of the concrete class for this * object. The default implementation will perform automatic name * resolving. There is only a need to override this function if @@ -310,7 +323,7 @@ public: /** * Empty, just defined to make it virtual. **/ - virtual ~SearchIterator() { } + virtual ~SearchIterator() = default; /** * @return true if it is a bitvector diff --git a/searchlib/src/vespa/searchlib/tensor/dense_tensor_attribute.cpp b/searchlib/src/vespa/searchlib/tensor/dense_tensor_attribute.cpp index 13091241809..ba4c64f1744 100644 --- a/searchlib/src/vespa/searchlib/tensor/dense_tensor_attribute.cpp +++ b/searchlib/src/vespa/searchlib/tensor/dense_tensor_attribute.cpp @@ -14,7 +14,6 @@ LOG_SETUP(".searchlib.tensor.dense_tensor_attribute"); using vespalib::eval::ValueType; using vespalib::tensor::MutableDenseTensorView; using vespalib::tensor::Tensor; -using vespalib::tensor::TensorMapper; namespace search::tensor { @@ -100,8 +99,8 @@ DenseTensorAttribute::~DenseTensorAttribute() void DenseTensorAttribute::setTensor(DocId docId, const Tensor &tensor) { - EntryRef ref = _denseTensorStore.setTensor( - (_tensorMapper ? *_tensorMapper->map(tensor) : tensor)); + checkTensorType(tensor); + EntryRef ref = _denseTensorStore.setTensor(tensor); setTensorRef(docId, ref); } diff --git a/searchlib/src/vespa/searchlib/tensor/generic_tensor_attribute.cpp b/searchlib/src/vespa/searchlib/tensor/generic_tensor_attribute.cpp index 3ea40433a82..7b5a22f6966 100644 --- a/searchlib/src/vespa/searchlib/tensor/generic_tensor_attribute.cpp +++ b/searchlib/src/vespa/searchlib/tensor/generic_tensor_attribute.cpp @@ -11,7 +11,6 @@ using vespalib::eval::ValueType; using vespalib::tensor::Tensor; -using vespalib::tensor::TensorMapper; namespace search::tensor { @@ -49,8 +48,8 @@ GenericTensorAttribute::~GenericTensorAttribute() void GenericTensorAttribute::setTensor(DocId docId, const Tensor &tensor) { - EntryRef ref = _genericTensorStore.setTensor( - (_tensorMapper ? *_tensorMapper->map(tensor) : tensor)); + checkTensorType(tensor); + EntryRef ref = _genericTensorStore.setTensor(tensor); setTensorRef(docId, ref); } diff --git a/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp b/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp index af8c820a3c6..39225e867f8 100644 --- a/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp +++ b/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp @@ -1,12 +1,22 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "tensor_attribute.h" -#include <vespa/eval/tensor/default_tensor.h> +#include <vespa/document/base/exceptions.h> +#include <vespa/document/datatype/tensor_data_type.h> +#include <vespa/eval/eval/simple_tensor.h> +#include <vespa/eval/tensor/dense/dense_tensor.h> +#include <vespa/eval/tensor/sparse/sparse_tensor.h> +#include <vespa/eval/tensor/wrapped_simple_tensor.h> #include <vespa/searchlib/common/rcuvector.hpp> +using vespalib::eval::SimpleTensor; using vespalib::eval::ValueType; using vespalib::tensor::Tensor; -using vespalib::tensor::TensorMapper; +using vespalib::tensor::DenseTensor; +using vespalib::tensor::SparseTensor; +using vespalib::tensor::WrappedSimpleTensor; +using document::TensorDataType; +using document::WrongTensorTypeException; namespace search { @@ -20,20 +30,41 @@ constexpr uint32_t TENSOR_ATTRIBUTE_VERSION = 0; constexpr size_t DEAD_SLACK = 0x10000u; -Tensor::UP -createEmptyTensor(const TensorMapper *mapper) +ValueType +createEmptyTensorType(const ValueType &type) { - vespalib::tensor::DefaultTensor::builder builder; - if (mapper != nullptr) { - return mapper->map(*builder.build()); + std::vector<ValueType::Dimension> list; + for (const auto &dim : type.dimensions()) { + if (dim.is_indexed() && !dim.is_bound()) { + list.emplace_back(dim.name, 1); + } else { + list.emplace_back(dim); + } } - return builder.build(); + return ValueType::tensor_type(std::move(list)); } -bool -shouldCreateMapper(const ValueType &tensorType) +Tensor::UP +createEmptyTensor(const ValueType &type) +{ + if (type.is_sparse()) { + return std::make_unique<SparseTensor>(type, SparseTensor::Cells()); + } else if (type.is_dense()) { + size_t size = 1; + for (const auto &dimension : type.dimensions()) { + size *= dimension.size; + } + return std::make_unique<DenseTensor>(type, DenseTensor::Cells(size)); + } else { + return std::make_unique<WrappedSimpleTensor>(std::make_unique<SimpleTensor>(type, SimpleTensor::Cells())); + } +} + +vespalib::string makeWrongTensorTypeMsg(const ValueType &fieldTensorType, const ValueType &tensorType) { - return tensorType.is_tensor() && !tensorType.dimensions().empty(); + return vespalib::make_string("Field tensor type is '%s' but other tensor type is '%s'", + fieldTensorType.to_spec().c_str(), + tensorType.to_spec().c_str()); } } @@ -45,12 +76,9 @@ TensorAttribute::TensorAttribute(vespalib::stringref name, const Config &cfg, Te cfg.getGrowStrategy().getDocsGrowDelta(), getGenerationHolder()), _tensorStore(tensorStore), - _tensorMapper(), + _emptyTensor(createEmptyTensor(createEmptyTensorType(cfg.tensorType()))), _compactGeneration(0) { - if (shouldCreateMapper(cfg.tensorType())) { - _tensorMapper = std::make_unique<TensorMapper>(cfg.tensorType()); - } } @@ -140,6 +168,15 @@ TensorAttribute::addDoc(DocId &docId) return true; } +void +TensorAttribute::checkTensorType(const Tensor &tensor) +{ + const ValueType &fieldTensorType = getConfig().tensorType(); + const ValueType &tensorType = tensor.type(); + if (!TensorDataType::isAssignableType(fieldTensorType, tensorType)) { + throw WrongTensorTypeException(makeWrongTensorTypeMsg(fieldTensorType, tensorType), VESPA_STRLOC); + } +} void TensorAttribute::setTensorRef(DocId docId, EntryRef ref) @@ -159,7 +196,7 @@ TensorAttribute::setTensorRef(DocId docId, EntryRef ref) Tensor::UP TensorAttribute::getEmptyTensor() const { - return createEmptyTensor(_tensorMapper.get()); + return _emptyTensor->clone(); } vespalib::eval::ValueType diff --git a/searchlib/src/vespa/searchlib/tensor/tensor_attribute.h b/searchlib/src/vespa/searchlib/tensor/tensor_attribute.h index 06758da8063..4241a899018 100644 --- a/searchlib/src/vespa/searchlib/tensor/tensor_attribute.h +++ b/searchlib/src/vespa/searchlib/tensor/tensor_attribute.h @@ -6,7 +6,6 @@ #include <vespa/searchlib/attribute/not_implemented_attribute.h> #include "tensor_store.h" #include <vespa/searchlib/common/rcuvector.h> -#include <vespa/eval/tensor/tensor_mapper.h> namespace search::tensor { @@ -21,11 +20,12 @@ protected: RefVector _refVector; // docId -> ref in data store for serialized tensor TensorStore &_tensorStore; // data store for serialized tensors - std::unique_ptr<vespalib::tensor::TensorMapper> _tensorMapper; // mapper to our tensor type + std::unique_ptr<Tensor> _emptyTensor; uint64_t _compactGeneration; // Generation when last compact occurred template <typename RefType> void doCompactWorst(); + void checkTensorType(const Tensor &tensor); void setTensorRef(DocId docId, EntryRef ref); public: DECLARE_IDENTIFIABLE_ABSTRACT(TensorAttribute); diff --git a/vespalib/src/vespa/vespalib/testkit/test_hook.cpp b/vespalib/src/vespa/vespalib/testkit/test_hook.cpp index e9be9268272..2babd22842b 100644 --- a/vespalib/src/vespa/vespalib/testkit/test_hook.cpp +++ b/vespalib/src/vespa/vespalib/testkit/test_hook.cpp @@ -2,7 +2,7 @@ #include "test_hook.h" #include <vespa/vespalib/util/stringfmt.h> -#include <vespa/vespalib/util/regexp.h> +#include <regex> #include <vespa/fastos/thread.h> namespace vespalib { @@ -98,13 +98,13 @@ TestHook::runAll() FastOSTestThreadFactory threadFactory; TestThreadFactory::factory = &threadFactory; std::string name = TestMaster::master.getName(); - Regexp pattern(lookup_subset_pattern(name)); + std::basic_regex pattern(lookup_subset_pattern(name), std::regex::extended); size_t testsPassed = 0; size_t testsFailed = 0; size_t testsIgnored = 0; size_t testsSkipped = 0; for (TestHook *test = _head; test != 0; test = test->_next) { - if (pattern.match(test->_tag)) { + if (std::regex_search(test->_tag, pattern)) { bool ignored = test->_ignore; bool failed = !test->run(); if (ignored) { |