diff options
63 files changed, 1080 insertions, 356 deletions
diff --git a/configd/src/apps/sentinel/cc-result.h b/configd/src/apps/sentinel/cc-result.h new file mode 100644 index 00000000000..3468cf4324a --- /dev/null +++ b/configd/src/apps/sentinel/cc-result.h @@ -0,0 +1,9 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +namespace config::sentinel { + +enum class CcResult { UNKNOWN, CONN_FAIL, UNREACHABLE_UP, INDIRECT_PING_FAIL, INDIRECT_PING_UNAVAIL, ALL_OK }; + +} diff --git a/configd/src/apps/sentinel/connectivity.cpp b/configd/src/apps/sentinel/connectivity.cpp index 132b57fc884..8d1aa0e9673 100644 --- a/configd/src/apps/sentinel/connectivity.cpp +++ b/configd/src/apps/sentinel/connectivity.cpp @@ -27,25 +27,33 @@ namespace { std::string toString(CcResult value) { switch (value) { case CcResult::UNKNOWN: return "BAD: missing result"; // very very bad - case CcResult::REVERSE_FAIL: return "connect OK, but reverse check FAILED"; // very bad + case CcResult::INDIRECT_PING_FAIL: return "connect OK, but reverse check FAILED"; // very bad + case CcResult::UNREACHABLE_UP: return "unreachable from me, but up"; // very bad case CcResult::CONN_FAIL: return "failed to connect"; // bad - case CcResult::REVERSE_UNAVAIL: return "connect OK (but reverse check unavailable)"; // unfortunate + case CcResult::INDIRECT_PING_UNAVAIL: return "connect OK (but reverse check unavailable)"; // unfortunate case CcResult::ALL_OK: return "OK: both ways connectivity verified"; // good } LOG(error, "Unknown CcResult enum value: %d", (int)value); LOG_ABORT("Unknown CcResult enum value"); } -std::map<std::string, std::string> specsFrom(const ModelConfig &model) { - std::map<std::string, std::string> checkSpecs; +using ConnectivityMap = std::map<std::string, OutwardCheck>; +using HostAndPort = Connectivity::HostAndPort; +using SpecMap = Connectivity::SpecMap; + +std::string spec(const SpecMap::value_type &host_and_port) { + return fmt("tcp/%s:%d", host_and_port.first.c_str(), host_and_port.second); +} + +SpecMap specsFrom(const ModelConfig &model) { + SpecMap checkSpecs; for (const auto & h : model.hosts) { bool foundSentinelPort = false; for (const auto & s : h.services) { if (s.name == "config-sentinel") { for (const auto & p : s.ports) { if (p.tags.find("rpc") != p.tags.npos) { - auto spec = fmt("tcp/%s:%d", h.name.c_str(), p.number); - checkSpecs[h.name] = spec; + checkSpecs[h.name] = p.number; foundSentinelPort = true; } } @@ -59,8 +67,57 @@ std::map<std::string, std::string> specsFrom(const ModelConfig &model) { return checkSpecs; } +void classifyConnFails(ConnectivityMap &connectivityMap, + const SpecMap &specMap, + RpcServer &rpcServer) +{ + std::vector<HostAndPort> failedConnSpecs; + std::vector<HostAndPort> goodNeighborSpecs; + std::string myHostname = vespa::Defaults::vespaHostname(); + for (auto & [hostname, check] : connectivityMap) { + if (hostname == myHostname) { + if (check.result() == CcResult::CONN_FAIL) { + check.classifyResult(CcResult::UNREACHABLE_UP); + } + } else { + auto iter = specMap.find(hostname); + LOG_ASSERT(iter != specMap.end()); + if (check.result() == CcResult::ALL_OK) { + goodNeighborSpecs.push_back(*iter); + } + if (check.result() == CcResult::CONN_FAIL) { + failedConnSpecs.push_back(*iter); + } + } + } + if ((failedConnSpecs.size() == 0) || (goodNeighborSpecs.size() == 0)) { + return; + } + for (const auto & [ nameToCheck, portToCheck ] : failedConnSpecs) { + auto cmIter = connectivityMap.find(nameToCheck); + LOG_ASSERT(cmIter != connectivityMap.end()); + OutwardCheckContext checkContext(goodNeighborSpecs.size(), nameToCheck, portToCheck, rpcServer.orb()); + ConnectivityMap cornerProbes; + for (const auto & hp : goodNeighborSpecs) { + cornerProbes.try_emplace(hp.first, spec(hp), checkContext); + } + checkContext.latch.await(); + size_t numReportsUp = 0; + size_t numReportsDown = 0; + for (const auto & [hostname, probe] : cornerProbes) { + if (probe.result() == CcResult::INDIRECT_PING_FAIL) ++numReportsDown; + if (probe.result() == CcResult::ALL_OK) ++numReportsUp; + } + if (numReportsUp > 0) { + LOG(debug, "Unreachable: %s is up according to %zd hosts (down according to me + %zd others)", + nameToCheck.c_str(), numReportsUp, numReportsDown); + cmIter->second.classifyResult(CcResult::UNREACHABLE_UP); + } + } } +} // namespace <unnamed> + void Connectivity::configure(const SentinelConfig::Connectivity &config) { _config = config; LOG(config, "connectivity.maxBadReverseCount = %d", _config.maxBadReverseCount); @@ -77,18 +134,18 @@ Connectivity::checkConnectivity(RpcServer &rpcServer) { LOG(warning, "could not get model config, skipping connectivity checks"); return true; } + std::string myHostname = vespa::Defaults::vespaHostname(); OutwardCheckContext checkContext(clusterSize, - vespa::Defaults::vespaHostname(), + myHostname, rpcServer.getPort(), rpcServer.orb()); - std::map<std::string, OutwardCheck> connectivityMap; - for (const auto & [ hn, spec ] : _checkSpecs) { - connectivityMap.try_emplace(hn, spec, checkContext); + ConnectivityMap connectivityMap; + for (const auto &host_and_port : _checkSpecs) { + connectivityMap.try_emplace(host_and_port.first, spec(host_and_port), checkContext); } checkContext.latch.await(); - size_t numFailedConns = 0; - size_t numFailedReverse = 0; - bool allChecksOk = true; + classifyConnFails(connectivityMap, _checkSpecs, rpcServer); + Accumulated accumulated; for (const auto & [hostname, check] : connectivityMap) { std::string detail = toString(check.result()); std::string prev = _detailsPerHost[hostname]; @@ -97,26 +154,47 @@ Connectivity::checkConnectivity(RpcServer &rpcServer) { } _detailsPerHost[hostname] = detail; LOG_ASSERT(check.result() != CcResult::UNKNOWN); - if (check.result() == CcResult::CONN_FAIL) ++numFailedConns; - if (check.result() == CcResult::REVERSE_FAIL) ++numFailedReverse; + accumulate(accumulated, check.result()); + } + return enoughOk(accumulated, clusterSize); +} + +void Connectivity::accumulate(Accumulated &target, CcResult value) { + switch (value) { + case CcResult::UNKNOWN: + case CcResult::UNREACHABLE_UP: + case CcResult::INDIRECT_PING_FAIL: + ++target.numSeriousIssues; + ++target.numIssues; + break; + case CcResult::CONN_FAIL: + ++target.numIssues; + break; + case CcResult::INDIRECT_PING_UNAVAIL: + case CcResult::ALL_OK: + break; } - if (numFailedReverse > size_t(_config.maxBadReverseCount)) { - LOG(warning, "%zu of %zu nodes report problems connecting to me (max is %d)", - numFailedReverse, clusterSize, _config.maxBadReverseCount); - allChecksOk = false; +} + +bool Connectivity::enoughOk(const Accumulated &results, size_t clusterSize) { + bool enough = true; + if (results.numSeriousIssues > size_t(_config.maxBadReverseCount)) { + LOG(warning, "%zu of %zu nodes up but with network connectivity problems (max is %d)", + results.numSeriousIssues, clusterSize, _config.maxBadReverseCount); + enough = false; } - if (numFailedConns * 100.0 > _config.maxBadOutPercent * clusterSize) { - double pct = numFailedConns * 100.0 / clusterSize; - LOG(warning, "Problems connecting to %zu of %zu nodes, %.2f %% (max is %d)", - numFailedConns, clusterSize, pct, _config.maxBadOutPercent); - allChecksOk = false; + if (results.numIssues * 100.0 > _config.maxBadOutPercent * clusterSize) { + double pct = results.numIssues * 100.0 / clusterSize; + LOG(warning, "Problems with connection to %zu of %zu nodes, %.1f%% (max is %d%%)", + results.numIssues, clusterSize, pct, _config.maxBadOutPercent); + enough = false; } - if (allChecksOk && (numFailedConns == 0) && (numFailedReverse == 0)) { + if (results.numIssues == 0) { LOG(info, "All connectivity checks OK, proceeding with service startup"); - } else if (allChecksOk) { + } else if (enough) { LOG(info, "Enough connectivity checks OK, proceeding with service startup"); } - return allChecksOk; + return enough; } } diff --git a/configd/src/apps/sentinel/connectivity.h b/configd/src/apps/sentinel/connectivity.h index 1c7ee8ddc57..a1e454a255a 100644 --- a/configd/src/apps/sentinel/connectivity.h +++ b/configd/src/apps/sentinel/connectivity.h @@ -3,6 +3,7 @@ #pragma once #include "rpcserver.h" +#include "cc-result.h" #include <vespa/config-sentinel.h> #include <vespa/config-model.h> #include <string> @@ -18,13 +19,22 @@ namespace config::sentinel { **/ class Connectivity { public: + using SpecMap = std::map<std::string, int>; + using HostAndPort = SpecMap::value_type; + Connectivity(); ~Connectivity(); void configure(const SentinelConfig::Connectivity &config); bool checkConnectivity(RpcServer &rpcServer); private: + struct Accumulated { + size_t numIssues = 0; + size_t numSeriousIssues = 0; + }; + void accumulate(Accumulated &target, CcResult value); + bool enoughOk(const Accumulated &results, size_t clusterSize); SentinelConfig::Connectivity _config; - std::map<std::string, std::string> _checkSpecs; + SpecMap _checkSpecs; std::map<std::string, std::string> _detailsPerHost; }; diff --git a/configd/src/apps/sentinel/outward-check.cpp b/configd/src/apps/sentinel/outward-check.cpp index 5fed69d0b6e..c9c94443274 100644 --- a/configd/src/apps/sentinel/outward-check.cpp +++ b/configd/src/apps/sentinel/outward-check.cpp @@ -7,6 +7,8 @@ LOG_SETUP(".outward-check"); namespace config::sentinel { +OutwardCheckContext::~OutwardCheckContext() = default; + OutwardCheck::OutwardCheck(const std::string &spec, OutwardCheckContext &context) : _spec(spec), _context(context) @@ -14,8 +16,8 @@ OutwardCheck::OutwardCheck(const std::string &spec, OutwardCheckContext &context _target = context.orb.GetTarget(spec.c_str()); _req = context.orb.AllocRPCRequest(); _req->SetMethodName("sentinel.check.connectivity"); - _req->GetParams()->AddString(context.myHostname); - _req->GetParams()->AddInt32(context.myPortnum); + _req->GetParams()->AddString(context.targetHostname.c_str()); + _req->GetParams()->AddInt32(context.targetPortnum); _req->GetParams()->AddInt32(500); _target->InvokeAsync(_req, 1.500, this); } @@ -29,17 +31,21 @@ void OutwardCheck::RequestDone(FRT_RPCRequest *req) { if (answer == "ok") { LOG(debug, "ping to %s with reverse connectivity OK", _spec.c_str()); _result = CcResult::ALL_OK; - } else { + } else if (answer == "bad") { LOG(debug, "connected to %s, but reverse connectivity fails: %s", _spec.c_str(), answer.c_str()); - _result = CcResult::REVERSE_FAIL; + _result = CcResult::INDIRECT_PING_FAIL; + } else { + LOG(warning, "connected to %s, but strange reverse connectivity: %s", + _spec.c_str(), answer.c_str()); + _result = CcResult::INDIRECT_PING_UNAVAIL; } } else if (req->GetErrorCode() == FRTE_RPC_NO_SUCH_METHOD || req->GetErrorCode() == FRTE_RPC_WRONG_PARAMS || req->GetErrorCode() == FRTE_RPC_WRONG_RETURN) { LOG(debug, "Connected OK to %s but no reverse connectivity check available", _spec.c_str()); - _result = CcResult::REVERSE_UNAVAIL; + _result = CcResult::INDIRECT_PING_UNAVAIL; } else { LOG(debug, "error on request to %s : %s (%d)", _spec.c_str(), req->GetErrorMessage(), req->GetErrorCode()); @@ -52,4 +58,9 @@ void OutwardCheck::RequestDone(FRT_RPCRequest *req) { _context.latch.countDown(); } +void OutwardCheck::classifyResult(CcResult value) { + LOG_ASSERT(_result == CcResult::CONN_FAIL); + _result = value; +} + } diff --git a/configd/src/apps/sentinel/outward-check.h b/configd/src/apps/sentinel/outward-check.h index 01a298aee18..0e53b9010dc 100644 --- a/configd/src/apps/sentinel/outward-check.h +++ b/configd/src/apps/sentinel/outward-check.h @@ -1,5 +1,8 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#pragma once + +#include "cc-result.h" #include <string> #include <vespa/vespalib/util/count_down_latch.h> #include <vespa/fnet/frt/supervisor.h> @@ -12,22 +15,21 @@ namespace config::sentinel { struct OutwardCheckContext { vespalib::CountDownLatch latch; - const char * myHostname; - int myPortnum; + std::string targetHostname; + int targetPortnum; FRT_Supervisor &orb; OutwardCheckContext(size_t count, - const char * hostname, + const std::string &hostname, int portnumber, FRT_Supervisor &supervisor) : latch(count), - myHostname(hostname), - myPortnum(portnumber), + targetHostname(hostname), + targetPortnum(portnumber), orb(supervisor) {} + ~OutwardCheckContext(); }; -enum class CcResult { UNKNOWN, CONN_FAIL, REVERSE_FAIL, REVERSE_UNAVAIL, ALL_OK }; - class OutwardCheck : public FRT_IRequestWait { private: CcResult _result = CcResult::UNKNOWN; @@ -41,6 +43,7 @@ public: void RequestDone(FRT_RPCRequest *req) override; bool ok() const { return _result == CcResult::ALL_OK; } CcResult result() const { return _result; } + void classifyResult(CcResult value); }; } diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactoryTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactoryTest.java index 93261a2401f..bb736122867 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactoryTest.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactoryTest.java @@ -8,6 +8,8 @@ import com.yahoo.jdisc.http.ssl.impl.ConfiguredSslContextFactoryProvider; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.handler.AbstractHandler; +import org.junit.After; +import org.junit.Before; import org.junit.Test; import javax.servlet.http.HttpServletRequest; @@ -16,46 +18,73 @@ import java.io.IOException; import java.util.Map; import static org.hamcrest.CoreMatchers.equalTo; +import static org.junit.Assert.assertEquals; /** * @author Einar M R Rosenvinge + * @author bjorncs */ public class ConnectorFactoryTest { - @Test - public void requireThatServerCanBindChannel() throws Exception { - Server server = new Server(); + private Server server; + + @Before + public void createServer() { + server = new Server(); + } + + @After + public void stopServer() { try { - ConnectorConfig config = new ConnectorConfig(new ConnectorConfig.Builder()); - ConnectorFactory factory = createConnectorFactory(config); - JettyConnectionLogger connectionLogger = new JettyConnectionLogger( - new ServerConfig.ConnectionLog.Builder().enabled(false).build(), - new VoidConnectionLog()); - DummyMetric metric = new DummyMetric(); - var connectionMetricAggregator = new ConnectionMetricAggregator(new ServerConfig(new ServerConfig.Builder()), metric); - JDiscServerConnector connector = - (JDiscServerConnector)factory.createConnector(metric, server, connectionLogger, connectionMetricAggregator); - server.addConnector(connector); - server.setHandler(new HelloWorldHandler()); - server.start(); - - SimpleHttpClient client = new SimpleHttpClient(null, connector.getLocalPort(), false); - SimpleHttpClient.RequestExecutor ex = client.newGet("/blaasdfnb"); - SimpleHttpClient.ResponseValidator val = ex.execute(); - val.expectContent(equalTo("Hello world")); - } finally { - try { - server.stop(); - } catch (Exception e) { - //ignore - } + server.stop(); + server = null; + } catch (Exception e) { + //ignore } } + @Test + public void requireThatServerCanBindChannel() throws Exception { + ConnectorConfig config = new ConnectorConfig(new ConnectorConfig.Builder()); + ConnectorFactory factory = createConnectorFactory(config); + JDiscServerConnector connector = createConnectorFromFactory(factory); + server.addConnector(connector); + server.setHandler(new HelloWorldHandler()); + server.start(); + + SimpleHttpClient client = new SimpleHttpClient(null, connector.getLocalPort(), false); + SimpleHttpClient.RequestExecutor ex = client.newGet("/blaasdfnb"); + SimpleHttpClient.ResponseValidator val = ex.execute(); + val.expectContent(equalTo("Hello world")); + } + + @Test + public void constructed_connector_is_based_on_jdisc_connector_config() { + ConnectorConfig config = new ConnectorConfig.Builder() + .idleTimeout(25) + .name("my-server-name") + .listenPort(12345) + .build(); + ConnectorFactory factory = createConnectorFactory(config); + JDiscServerConnector connector = createConnectorFromFactory(factory); + assertEquals(25000, connector.getIdleTimeout()); + assertEquals(12345, connector.listenPort()); + assertEquals("my-server-name", connector.getName()); + } + private static ConnectorFactory createConnectorFactory(ConnectorConfig config) { return new ConnectorFactory(config, new ConfiguredSslContextFactoryProvider(config)); } + private JDiscServerConnector createConnectorFromFactory(ConnectorFactory factory) { + JettyConnectionLogger connectionLogger = new JettyConnectionLogger( + new ServerConfig.ConnectionLog.Builder().enabled(false).build(), + new VoidConnectionLog()); + DummyMetric metric = new DummyMetric(); + var connectionMetricAggregator = new ConnectionMetricAggregator(new ServerConfig(new ServerConfig.Builder()), metric); + return (JDiscServerConnector)factory.createConnector(metric, server, connectionLogger, connectionMetricAggregator); + } + private static class HelloWorldHandler extends AbstractHandler { @Override public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException { diff --git a/container-search/src/main/java/com/yahoo/prelude/searcher/FillSearcher.java b/container-search/src/main/java/com/yahoo/prelude/searcher/FillSearcher.java index 45034482bb6..5390f202ef0 100644 --- a/container-search/src/main/java/com/yahoo/prelude/searcher/FillSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/searcher/FillSearcher.java @@ -1,7 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.prelude.searcher; -import com.yahoo.component.ComponentId; import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.Searcher; diff --git a/container-search/src/main/java/com/yahoo/search/Searcher.java b/container-search/src/main/java/com/yahoo/search/Searcher.java index 5fefe9d2468..cd6b7167f08 100644 --- a/container-search/src/main/java/com/yahoo/search/Searcher.java +++ b/container-search/src/main/java/com/yahoo/search/Searcher.java @@ -73,6 +73,7 @@ public abstract class Searcher extends Processor { // Note to developers: If you think you should add something here you are probably wrong // Create a subclass containing the new method instead. + private final Logger logger = Logger.getLogger(getClass().getName()); public Searcher() {} diff --git a/container-search/src/main/java/com/yahoo/search/query/Presentation.java b/container-search/src/main/java/com/yahoo/search/query/Presentation.java index b10e8442a5f..db2fbf525e0 100644 --- a/container-search/src/main/java/com/yahoo/search/query/Presentation.java +++ b/container-search/src/main/java/com/yahoo/search/query/Presentation.java @@ -23,7 +23,7 @@ import java.util.Set; public class Presentation implements Cloneable { /** The type representing the property arguments consumed by this */ - private static QueryProfileType argumentType; + private static final QueryProfileType argumentType; public static final String PRESENTATION = "presentation"; public static final String BOLDING = "bolding"; @@ -48,7 +48,7 @@ public class Presentation implements Cloneable { public static QueryProfileType getArgumentType() { return argumentType; } /** How the result should be highlighted */ - private Highlight highlight= null; + private Highlight highlight = null; /** The terms to highlight in the result (only used by BoldingSearcher, may be removed later). */ private List<IndexedItem> boldingData = null; diff --git a/container-search/src/main/java/com/yahoo/search/searchchain/Execution.java b/container-search/src/main/java/com/yahoo/search/searchchain/Execution.java index 0574fc660c3..fac0d35d509 100644 --- a/container-search/src/main/java/com/yahoo/search/searchchain/Execution.java +++ b/container-search/src/main/java/com/yahoo/search/searchchain/Execution.java @@ -523,7 +523,6 @@ public class Execution extends com.yahoo.processing.execution.Execution { * * @param result the result to fill */ - @SuppressWarnings("deprecation") public void fillAttributes(Result result) { fill(result, ATTRIBUTEPREFETCH); } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/horizon/HorizonClient.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/horizon/HorizonClient.java index 5624fc7e4e6..bd6c2607fc2 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/horizon/HorizonClient.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/horizon/HorizonClient.java @@ -9,7 +9,7 @@ import java.io.InputStream; */ public interface HorizonClient { - InputStream getMetrics(Slime query); + InputStream getMetrics(byte[] query); InputStream getUser(); diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/horizon/MockHorizonClient.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/horizon/MockHorizonClient.java index 8afd613fb11..7d7af046810 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/horizon/MockHorizonClient.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/horizon/MockHorizonClient.java @@ -1,8 +1,6 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.api.integration.horizon; -import com.yahoo.slime.Slime; - import java.io.InputStream; /** @@ -11,7 +9,7 @@ import java.io.InputStream; public class MockHorizonClient implements HorizonClient { @Override - public InputStream getMetrics(Slime query) { + public InputStream getMetrics(byte[] query) { return null; } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/NodeHistory.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/NodeHistory.java index 624e4c61662..cf40ac00d64 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/NodeHistory.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/NodeHistory.java @@ -17,7 +17,7 @@ public class NodeHistory { @JsonProperty("at") public Long at; @JsonProperty("agent") - public Agent agent; + public String agent; @JsonProperty("event") public String event; @@ -25,7 +25,7 @@ public class NodeHistory { return at; } - public Agent getAgent() { + public String getAgent() { return agent; } @@ -33,24 +33,4 @@ public class NodeHistory { return event; } - public enum Agent { - operator, - application, - system, - DirtyExpirer, - DynamicProvisioningMaintainer, - FailedExpirer, - InactiveExpirer, - NodeFailer, - NodeHealthTracker, - ProvisionedExpirer, - Rebalancer, - ReservationExpirer, - RetiringUpgrader, - RebuildingOsUpgrader, - SpareCapacityMaintainer, - SwitchRebalancer, - HostEncrypter, - } - } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java index 3b6c86222ac..327175c19ed 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java @@ -228,7 +228,7 @@ enum PathGroup { secretStore(Matcher.tenant, "/application/v4/tenant/{tenant}/secret-store/{*}"), /** Paths used to proxy Horizon metric requests */ - horizonProxy("/horizion/v1/{*}"); + horizonProxy("/horizon/v1/{*}"); final List<String> pathSpecs; final List<Matcher> matchers; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java index 1b1df28c201..990549b6d8c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java @@ -72,7 +72,6 @@ public class RoutingController { private final RoutingPolicies routingPolicies; private final RotationRepository rotationRepository; private final BooleanFlag hideSharedRoutingEndpoint; - private final BooleanFlag vespaAppDomainInCertificate; public RoutingController(Controller controller, RotationsConfig rotationsConfig) { this.controller = Objects.requireNonNull(controller, "controller must be non-null"); @@ -80,7 +79,6 @@ public class RoutingController { this.rotationRepository = new RotationRepository(rotationsConfig, controller.applications(), controller.curator()); this.hideSharedRoutingEndpoint = Flags.HIDE_SHARED_ROUTING_ENDPOINT.bindTo(controller.flagSource()); - this.vespaAppDomainInCertificate = Flags.VESPA_APP_DOMAIN_IN_CERTIFICATE.bindTo(controller.flagSource()); } public RoutingPolicies policies() { @@ -180,7 +178,7 @@ public class RoutingController { builder = builder.routingMethod(RoutingMethod.exclusive) .on(Port.tls()); Endpoint endpoint = builder.in(controller.system()); - if (controller.system().isPublic() && vespaAppDomainInCertificate.with(FetchVector.Dimension.APPLICATION_ID, deployment.applicationId().serializedForm()).value()) { + if (controller.system().isPublic()) { Endpoint legacyEndpoint = builder.legacy().in(controller.system()); endpointDnsNames.add(legacyEndpoint.dnsName()); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/horizon/HorizonApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/horizon/HorizonApiHandler.java index 83efccbf1e5..422b8f22000 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/horizon/HorizonApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/horizon/HorizonApiHandler.java @@ -1,10 +1,23 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.restapi.horizon; import com.google.inject.Inject; +import com.yahoo.config.provision.SystemName; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.container.jdisc.LoggingRequestHandler; -import com.yahoo.restapi.MessageResponse; +import com.yahoo.restapi.ErrorResponse; +import com.yahoo.restapi.Path; +import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.api.integration.horizon.HorizonClient; +import com.yahoo.vespa.hosted.controller.api.role.SecurityContext; +import com.yahoo.yolean.Exceptions; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.util.Optional; +import java.util.logging.Level; /** * Proxies metrics requests from Horizon UI @@ -13,13 +26,93 @@ import com.yahoo.restapi.MessageResponse; */ public class HorizonApiHandler extends LoggingRequestHandler { + private final SystemName systemName; + private final HorizonClient client; + @Inject - public HorizonApiHandler(LoggingRequestHandler.Context parentCtx) { + public HorizonApiHandler(LoggingRequestHandler.Context parentCtx, Controller controller) { super(parentCtx); + this.systemName = controller.system(); + this.client = controller.serviceRegistry().horizonClient(); } @Override public HttpResponse handle(HttpRequest request) { - return new MessageResponse("OK"); + try { + switch (request.getMethod()) { + case GET: return get(request); + case POST: return post(request); + case PUT: return put(request); + default: return ErrorResponse.methodNotAllowed("Method '" + request.getMethod() + "' is not supported"); + } + } + catch (IllegalArgumentException e) { + return ErrorResponse.badRequest(Exceptions.toMessageString(e)); + } + catch (RuntimeException e) { + log.log(Level.WARNING, "Unexpected error handling '" + request.getUri() + "'", e); + return ErrorResponse.internalServerError(Exceptions.toMessageString(e)); + } + } + + private HttpResponse get(HttpRequest request) { + Path path = new Path(request.getUri()); + if (path.matches("/horizon/v1/config/dashboard/topFolders")) return new JsonInputStreamResponse(client.getTopFolders()); + if (path.matches("/horizon/v1/config/dashboard/file/{id}")) return new JsonInputStreamResponse(client.getDashboard(path.get("id"))); + if (path.matches("/horizon/v1/config/dashboard/favorite")) return new JsonInputStreamResponse(client.getFavorite(request.getProperty("user"))); + if (path.matches("/horizon/v1/config/dashboard/recent")) return new JsonInputStreamResponse(client.getRecent(request.getProperty("user"))); + return ErrorResponse.notFoundError("Nothing at " + path); + } + + private HttpResponse post(HttpRequest request) { + Path path = new Path(request.getUri()); + if (path.matches("/horizon/v1/tsdb/api/query/graph")) return tsdbQuery(request); + if (path.matches("/horizon/v1/meta/search/timeseries")) assert true; // TODO + return ErrorResponse.notFoundError("Nothing at " + path); + } + + private HttpResponse put(HttpRequest request) { + Path path = new Path(request.getUri()); + if (path.matches("/horizon/v1/config/user")) return new JsonInputStreamResponse(client.getUser()); + return ErrorResponse.notFoundError("Nothing at " + path); + } + + private HttpResponse tsdbQuery(HttpRequest request) { + SecurityContext securityContext = getAttribute(request, SecurityContext.ATTRIBUTE_NAME, SecurityContext.class); + try { + byte[] data = TsdbQueryRewriter.rewrite(request.getData().readAllBytes(), securityContext.roles(), systemName); + return new JsonInputStreamResponse(client.getMetrics(data)); + } catch (TsdbQueryRewriter.UnauthorizedException e) { + return ErrorResponse.forbidden("Access denied"); + } catch (IOException e) { + return ErrorResponse.badRequest("Failed to parse request body: " + e.getMessage()); + } + } + + private static <T> T getAttribute(HttpRequest request, String attributeName, Class<T> clazz) { + return Optional.ofNullable(request.getJDiscRequest().context().get(attributeName)) + .filter(clazz::isInstance) + .map(clazz::cast) + .orElseThrow(() -> new IllegalArgumentException("Attribute '" + attributeName + "' was not set on request")); + } + + private static class JsonInputStreamResponse extends HttpResponse { + + private final InputStream jsonInputStream; + + public JsonInputStreamResponse(InputStream jsonInputStream) { + super(200); + this.jsonInputStream = jsonInputStream; + } + + @Override + public String getContentType() { + return "application/json"; + } + + @Override + public void render(OutputStream outputStream) throws IOException { + jsonInputStream.transferTo(outputStream); + } } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/horizon/TsdbQueryRewriter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/horizon/TsdbQueryRewriter.java new file mode 100644 index 00000000000..40bc9145ce2 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/horizon/TsdbQueryRewriter.java @@ -0,0 +1,102 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.restapi.horizon; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ArrayNode; +import com.fasterxml.jackson.databind.node.ObjectNode; +import com.yahoo.config.provision.SystemName; +import com.yahoo.config.provision.TenantName; +import com.yahoo.vespa.hosted.controller.api.role.Role; +import com.yahoo.vespa.hosted.controller.api.role.RoleDefinition; +import com.yahoo.vespa.hosted.controller.api.role.TenantRole; + +import java.io.IOException; +import java.util.EnumSet; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; + +/** + * @author valerijf + */ +public class TsdbQueryRewriter { + + private static final ObjectMapper mapper = new ObjectMapper(); + private static final EnumSet<RoleDefinition> operatorRoleDefinitions = + EnumSet.of(RoleDefinition.hostedOperator, RoleDefinition.hostedSupporter); + + public static byte[] rewrite(byte[] data, Set<Role> roles, SystemName systemName) throws IOException { + boolean operator = roles.stream().map(Role::definition).anyMatch(operatorRoleDefinitions::contains); + + // Anyone with any tenant relation can view metrics for apps within those tenants + Set<TenantName> authorizedTenants = roles.stream() + .filter(TenantRole.class::isInstance) + .map(role -> ((TenantRole) role).tenant()) + .collect(Collectors.toUnmodifiableSet()); + if (!operator && authorizedTenants.isEmpty()) + throw new UnauthorizedException(); + + JsonNode root = mapper.readTree(data); + getField(root, "executionGraph", ArrayNode.class) + .ifPresent(graph -> rewriteExecutionGraph(graph, authorizedTenants, operator, systemName)); + getField(root, "filters", ArrayNode.class) + .ifPresent(filters -> rewriteFilters(filters, authorizedTenants, operator, systemName)); + + return mapper.writeValueAsBytes(root); + } + + private static void rewriteExecutionGraph(ArrayNode executionGraph, Set<TenantName> tenantNames, boolean operator, SystemName systemName) { + for (int i = 0; i < executionGraph.size(); i++) { + JsonNode execution = executionGraph.get(i); + + // Will be handled by rewriteFilters() + if (execution.has("filterId")) continue; + + rewriteFilter((ObjectNode) execution, tenantNames, operator, systemName); + } + } + + private static void rewriteFilters(ArrayNode filters, Set<TenantName> tenantNames, boolean operator, SystemName systemName) { + for (int i = 0; i < filters.size(); i++) + rewriteFilter((ObjectNode) filters.get(i), tenantNames, operator, systemName); + } + + private static void rewriteFilter(ObjectNode parent, Set<TenantName> tenantNames, boolean operator, SystemName systemName) { + ObjectNode prev = ((ObjectNode) parent.get("filter")); + ArrayNode filters; + // If we dont already have a filter object, or the object that we have is not an AND filter + if (prev == null || !"Chain".equals(prev.get("type").asText()) || !"AND".equals(prev.get("op").asText())) { + // Create new filter object + filters = parent.putObject("filter") + .put("type", "Chain") + .put("op", "AND") + .putArray("filters"); + + // Add the previous filter to the AND expression + if (prev != null) filters.add(prev); + } else filters = (ArrayNode) prev.get("filters"); + + // Make sure we only show metrics in the relevant system + ObjectNode systemFilter = filters.addObject(); + systemFilter.put("type", "TagValueLiteralOr"); + systemFilter.put("filter", systemName.name()); + systemFilter.put("tagKey", "system"); + + // Make sure non-operators cannot see metrics outside of their tenants + if (!operator) { + ObjectNode appFilter = filters.addObject(); + appFilter.put("type", "TagValueRegex"); + appFilter.put("filter", + tenantNames.stream().map(TenantName::value).sorted().collect(Collectors.joining("|", "(", ")\\..*"))); + appFilter.put("tagKey", "applicationId"); + } + } + + private static <T extends JsonNode> Optional<T> getField(JsonNode object, String fieldName, Class<T> clazz) { + return Optional.ofNullable(object.get(fieldName)).filter(clazz::isInstance).map(clazz::cast); + } + + static class UnauthorizedException extends RuntimeException { } + +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesTest.java index d8544ff3947..a3580a9fda3 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesTest.java @@ -13,8 +13,6 @@ import com.yahoo.security.SignatureAlgorithm; import com.yahoo.security.X509CertificateBuilder; import com.yahoo.security.X509CertificateUtils; import com.yahoo.test.ManualClock; -import com.yahoo.vespa.flags.Flags; -import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.controller.ControllerTester; import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMetadata; @@ -133,7 +131,6 @@ public class EndpointCertificatesTest { @Test public void provisions_new_certificate_in_public_prod() { ControllerTester tester = new ControllerTester(SystemName.Public); - ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.VESPA_APP_DOMAIN_IN_CERTIFICATE.id(), true); EndpointCertificateValidatorImpl endpointCertificateValidator = new EndpointCertificateValidatorImpl(secretStore, clock); EndpointCertificates endpointCertificates = new EndpointCertificates(tester.controller(), endpointCertificateMock, endpointCertificateValidator); List<String> expectedSans = List.of( diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirerTest.java index 232521c9609..ffc82f90ad4 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirerTest.java @@ -70,7 +70,7 @@ public class DeploymentExpirerTest { assertEquals(1, permanentDeployments(prodApp.instance())); // Dev application expires when enough time has passed since most recent attempt - tester.clock().advance(Duration.ofDays(12)); + tester.clock().advance(Duration.ofDays(12).plus(Duration.ofSeconds(1))); expirer.maintain(); assertEquals(0, permanentDeployments(devApp.instance())); assertEquals(1, permanentDeployments(prodApp.instance())); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/horizon/TsdbQueryRewriterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/horizon/TsdbQueryRewriterTest.java new file mode 100644 index 00000000000..937b0e95440 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/horizon/TsdbQueryRewriterTest.java @@ -0,0 +1,55 @@ +package com.yahoo.vespa.hosted.controller.restapi.horizon; + +import com.yahoo.config.provision.SystemName; +import com.yahoo.config.provision.TenantName; +import com.yahoo.slime.JsonFormat; +import com.yahoo.slime.SlimeUtils; +import com.yahoo.vespa.hosted.controller.api.role.Role; +import org.junit.Test; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.util.Set; + +import static org.junit.Assert.assertEquals; + +/** + * @author valerijf + */ +public class TsdbQueryRewriterTest { + + @Test + public void rewrites_query() throws IOException { + assertRewrite("filters-complex.json", "filters-complex.expected.json", Role.reader(TenantName.from("tenant2"))); + + assertRewrite("filter-in-execution-graph.json", + "filter-in-execution-graph.expected.json", + Role.reader(TenantName.from("tenant2")), Role.athenzTenantAdmin(TenantName.from("tenant3"))); + + assertRewrite("filter-in-execution-graph.json", + "filter-in-execution-graph.expected.operator.json", + Role.reader(TenantName.from("tenant2")), Role.athenzTenantAdmin(TenantName.from("tenant3")), Role.hostedOperator()); + + assertRewrite("no-filters.json", + "no-filters.expected.json", + Role.reader(TenantName.from("tenant2")), Role.athenzTenantAdmin(TenantName.from("tenant3"))); + } + + @Test(expected = TsdbQueryRewriter.UnauthorizedException.class) + public void throws_if_no_roles() throws IOException { + assertRewrite("filters-complex.json", "filters-complex.expected.json"); + } + + private static void assertRewrite(String initialFilename, String expectedFilename, Role... roles) throws IOException { + byte[] data = Files.readAllBytes(Paths.get("src/test/resources/horizon", initialFilename)); + data = TsdbQueryRewriter.rewrite(data, Set.of(roles), SystemName.Public); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + new JsonFormat(false).encode(baos, SlimeUtils.jsonToSlime(data)); + String expectedJson = Files.readString(Paths.get("src/test/resources/horizon", expectedFilename)); + + assertEquals(expectedJson, baos.toString()); + } +}
\ No newline at end of file diff --git a/controller-server/src/test/resources/horizon/filter-in-execution-graph.expected.json b/controller-server/src/test/resources/horizon/filter-in-execution-graph.expected.json new file mode 100644 index 00000000000..7c279442f1d --- /dev/null +++ b/controller-server/src/test/resources/horizon/filter-in-execution-graph.expected.json @@ -0,0 +1,37 @@ +{ + "start": 1619301600000, + "end": 1623161217471, + "executionGraph": [ + { + "id": "q1_m1", + "type": "TimeSeriesDataSource", + "metric": { + "type": "MetricLiteral", + "metric": "Vespa.vespa.distributor.vds.distributor.docsstored.average" + }, + "sourceId": null, + "fetchLast": false, + "filter": { + "type": "Chain", + "op": "AND", + "filters": [ + { + "type": "TagValueLiteralOr", + "filter": "tenant1.application1.instance1", + "tagKey": "applicationId" + }, + { + "type": "TagValueLiteralOr", + "filter": "Public", + "tagKey": "system" + }, + { + "type": "TagValueRegex", + "filter": "(tenant2|tenant3)\\..*", + "tagKey": "applicationId" + } + ] + } + } + ] +} diff --git a/controller-server/src/test/resources/horizon/filter-in-execution-graph.expected.operator.json b/controller-server/src/test/resources/horizon/filter-in-execution-graph.expected.operator.json new file mode 100644 index 00000000000..5eb5b403b2f --- /dev/null +++ b/controller-server/src/test/resources/horizon/filter-in-execution-graph.expected.operator.json @@ -0,0 +1,32 @@ +{ + "start": 1619301600000, + "end": 1623161217471, + "executionGraph": [ + { + "id": "q1_m1", + "type": "TimeSeriesDataSource", + "metric": { + "type": "MetricLiteral", + "metric": "Vespa.vespa.distributor.vds.distributor.docsstored.average" + }, + "sourceId": null, + "fetchLast": false, + "filter": { + "type": "Chain", + "op": "AND", + "filters": [ + { + "type": "TagValueLiteralOr", + "filter": "tenant1.application1.instance1", + "tagKey": "applicationId" + }, + { + "type": "TagValueLiteralOr", + "filter": "Public", + "tagKey": "system" + } + ] + } + } + ] +} diff --git a/controller-server/src/test/resources/horizon/filter-in-execution-graph.json b/controller-server/src/test/resources/horizon/filter-in-execution-graph.json new file mode 100644 index 00000000000..6a2512c3642 --- /dev/null +++ b/controller-server/src/test/resources/horizon/filter-in-execution-graph.json @@ -0,0 +1,21 @@ +{ + "start": 1619301600000, + "end": 1623161217471, + "executionGraph": [ + { + "id": "q1_m1", + "type": "TimeSeriesDataSource", + "metric": { + "type": "MetricLiteral", + "metric": "Vespa.vespa.distributor.vds.distributor.docsstored.average" + }, + "sourceId": null, + "fetchLast": false, + "filter": { + "type": "TagValueLiteralOr", + "filter": "tenant1.application1.instance1", + "tagKey": "applicationId" + } + } + ] +}
\ No newline at end of file diff --git a/controller-server/src/test/resources/horizon/filters-complex.expected.json b/controller-server/src/test/resources/horizon/filters-complex.expected.json new file mode 100644 index 00000000000..333e79150e4 --- /dev/null +++ b/controller-server/src/test/resources/horizon/filters-complex.expected.json @@ -0,0 +1,56 @@ +{ + "start": 1623080040000, + "end": 1623166440000, + "executionGraph": [ + { + "id": "q1_m1", + "type": "TimeSeriesDataSource", + "metric": { + "type": "MetricLiteral", + "metric": "Vespa.vespa.qrserver.documents_covered.count" + }, + "sourceId": null, + "fetchLast": false, + "filterId": "filter-ni8" + } + ], + "filters": [ + { + "filter": { + "type": "Chain", + "op": "AND", + "filters": [ + { + "type": "NOT", + "filter": { + "type": "TagValueLiteralOr", + "filter": "tenant1.app1.instance1", + "tagKey": "applicationId" + } + }, + { + "type": "TagValueLiteralOr", + "filter": "Public", + "tagKey": "system" + }, + { + "type": "TagValueRegex", + "filter": "(tenant2)\\..*", + "tagKey": "applicationId" + } + ] + }, + "id": "filter-ni8" + } + ], + "serdesConfigs": [ + { + "id": "JsonV3QuerySerdes", + "filter": [ + "summarizer" + ] + } + ], + "logLevel": "ERROR", + "cacheMode": null +} diff --git a/controller-server/src/test/resources/horizon/filters-complex.json b/controller-server/src/test/resources/horizon/filters-complex.json new file mode 100644 index 00000000000..3acc7fe5044 --- /dev/null +++ b/controller-server/src/test/resources/horizon/filters-complex.json @@ -0,0 +1,46 @@ +{ + "start": 1623080040000, + "end": 1623166440000, + "executionGraph": [ + { + "id": "q1_m1", + "type": "TimeSeriesDataSource", + "metric": { + "type": "MetricLiteral", + "metric": "Vespa.vespa.qrserver.documents_covered.count" + }, + "sourceId": null, + "fetchLast": false, + "filterId": "filter-ni8" + } + ], + "filters": [ + { + "filter": { + "type": "Chain", + "op": "AND", + "filters": [ + { + "type": "NOT", + "filter": { + "type": "TagValueLiteralOr", + "filter": "tenant1.app1.instance1", + "tagKey": "applicationId" + } + } + ] + }, + "id": "filter-ni8" + } + ], + "serdesConfigs": [ + { + "id": "JsonV3QuerySerdes", + "filter": [ + "summarizer" + ] + } + ], + "logLevel": "ERROR", + "cacheMode": null +} diff --git a/controller-server/src/test/resources/horizon/no-filters.expected.json b/controller-server/src/test/resources/horizon/no-filters.expected.json new file mode 100644 index 00000000000..d2d6407e6b4 --- /dev/null +++ b/controller-server/src/test/resources/horizon/no-filters.expected.json @@ -0,0 +1,32 @@ +{ + "start": 1619301600000, + "end": 1623161217471, + "executionGraph": [ + { + "id": "q1_m1", + "type": "TimeSeriesDataSource", + "metric": { + "type": "MetricLiteral", + "metric": "Vespa.vespa.distributor.vds.distributor.docsstored.average" + }, + "sourceId": null, + "fetchLast": false, + "filter": { + "type": "Chain", + "op": "AND", + "filters": [ + { + "type": "TagValueLiteralOr", + "filter": "Public", + "tagKey": "system" + }, + { + "type": "TagValueRegex", + "filter": "(tenant2|tenant3)\\..*", + "tagKey": "applicationId" + } + ] + } + } + ] +} diff --git a/controller-server/src/test/resources/horizon/no-filters.json b/controller-server/src/test/resources/horizon/no-filters.json new file mode 100644 index 00000000000..3ff80feba02 --- /dev/null +++ b/controller-server/src/test/resources/horizon/no-filters.json @@ -0,0 +1,16 @@ +{ + "start": 1619301600000, + "end": 1623161217471, + "executionGraph": [ + { + "id": "q1_m1", + "type": "TimeSeriesDataSource", + "metric": { + "type": "MetricLiteral", + "metric": "Vespa.vespa.distributor.vds.distributor.docsstored.average" + }, + "sourceId": null, + "fetchLast": false + } + ] +}
\ No newline at end of file diff --git a/eval/CMakeLists.txt b/eval/CMakeLists.txt index 302b6768cea..16c9c72d8a5 100644 --- a/eval/CMakeLists.txt +++ b/eval/CMakeLists.txt @@ -5,6 +5,7 @@ vespa_define_module( staging_vespalib APPS + src/apps/analyze_onnx_model src/apps/eval_expr src/apps/make_tensor_binary_format_test_spec src/apps/tensor_conformance diff --git a/eval/src/apps/analyze_onnx_model/.gitignore b/eval/src/apps/analyze_onnx_model/.gitignore new file mode 100644 index 00000000000..12ce20b03ba --- /dev/null +++ b/eval/src/apps/analyze_onnx_model/.gitignore @@ -0,0 +1 @@ +/vespa-analyze-onnx-model diff --git a/eval/src/apps/analyze_onnx_model/CMakeLists.txt b/eval/src/apps/analyze_onnx_model/CMakeLists.txt new file mode 100644 index 00000000000..47cbb6504f4 --- /dev/null +++ b/eval/src/apps/analyze_onnx_model/CMakeLists.txt @@ -0,0 +1,8 @@ +# Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +vespa_add_executable(vespa-analyze-onnx-model + SOURCES + analyze_onnx_model.cpp + INSTALL bin + DEPENDS + vespaeval +) diff --git a/eval/src/apps/analyze_onnx_model/analyze_onnx_model.cpp b/eval/src/apps/analyze_onnx_model/analyze_onnx_model.cpp new file mode 100644 index 00000000000..f1cc3b28751 --- /dev/null +++ b/eval/src/apps/analyze_onnx_model/analyze_onnx_model.cpp @@ -0,0 +1,59 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include <vespa/eval/onnx/onnx_wrapper.h> +#include <vespa/vespalib/util/guard.h> + +using vespalib::FilePointer; +using namespace vespalib::eval; + +bool read_line(FilePointer &file, vespalib::string &line) { + char line_buffer[1024]; + char *res = fgets(line_buffer, sizeof(line_buffer), file.fp()); + if (res == nullptr) { + line.clear(); + return false; + } + line = line_buffer; + while (!line.empty() && isspace(line[line.size() - 1])) { + line.pop_back(); + } + return true; +} + +void extract(const vespalib::string &str, const vespalib::string &prefix, vespalib::string &dst) { + if (starts_with(str, prefix)) { + size_t pos = prefix.size(); + while ((str.size() > pos) && isspace(str[pos])) { + ++pos; + } + dst = str.substr(pos); + } +} + +void report_memory_usage(const vespalib::string &desc) { + vespalib::string vm_size = "unknown"; + vespalib::string vm_rss = "unknown"; + vespalib::string line; + FilePointer file(fopen("/proc/self/status", "r")); + while (read_line(file, line)) { + extract(line, "VmSize:", vm_size); + extract(line, "VmRSS:", vm_rss); + } + fprintf(stderr, "vm_size: %s, vm_rss: %s (%s)\n", vm_size.c_str(), vm_rss.c_str(), desc.c_str()); +} + +int usage(const char *self) { + fprintf(stderr, "usage: %s <onnx-model>\n", self); + fprintf(stderr, " load onnx model and report memory usage\n"); + return 1; +} + +int main(int argc, char **argv) { + if (argc != 2) { + return usage(argv[0]); + } + report_memory_usage("before loading model"); + Onnx onnx(argv[1], Onnx::Optimize::ENABLE); + report_memory_usage("after loading model"); + return 0; +} diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 54410175849..e0f28357872 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -253,13 +253,6 @@ public class Flags { "Takes effect immediately", APPLICATION_ID); - public static final UnboundBooleanFlag VESPA_APP_DOMAIN_IN_CERTIFICATE = defineFeatureFlag( - "new-domain-in-certificate", false, - List.of("mpolden"), "2021-05-25", "2021-09-01", - "Whether to include the vespa-app.cloud names in certificate requests", - "Takes effect on next deployment through controller", - APPLICATION_ID); - public static final UnboundIntFlag MAX_ENCRYPTING_HOSTS = defineIntFlag( "max-encrypting-hosts", 0, List.of("mpolden", "hakonhall"), "2021-05-27", "2021-10-01", diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java index ef9520969af..5d7ab48753f 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java @@ -15,11 +15,13 @@ import java.time.Clock; import java.time.Duration; import java.time.Instant; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; import java.util.stream.Collectors; +import java.util.stream.Stream; /** * Administers a host (for now only docker hosts) and its nodes (docker containers nodes). @@ -130,7 +132,7 @@ public class NodeAdminImpl implements NodeAdmin { } // Use filter with count instead of allMatch() because allMatch() will short circuit on first non-match - boolean allNodeAgentsConverged = nodeAgentWithSchedulerByHostname.values().parallelStream() + boolean allNodeAgentsConverged = parallelStreamOfNodeAgentWithScheduler() .filter(nodeAgentScheduler -> !nodeAgentScheduler.setFrozen(wantFrozen, freezeTimeout)) .count() == 0; @@ -158,9 +160,7 @@ public class NodeAdminImpl implements NodeAdmin { @Override public void stopNodeAgentServices() { // Each container may spend 1-1:30 minutes stopping - nodeAgentWithSchedulerByHostname.values() - .parallelStream() - .forEach(NodeAgentWithScheduler::stopForHostSuspension); + parallelStreamOfNodeAgentWithScheduler().forEach(NodeAgentWithScheduler::stopForHostSuspension); } @Override @@ -171,7 +171,18 @@ public class NodeAdminImpl implements NodeAdmin { @Override public void stop() { // Stop all node-agents in parallel, will block until the last NodeAgent is stopped - nodeAgentWithSchedulerByHostname.values().parallelStream().forEach(NodeAgentWithScheduler::stopForRemoval); + parallelStreamOfNodeAgentWithScheduler().forEach(NodeAgentWithScheduler::stopForRemoval); + } + + /** + * Returns a parallel stream of NodeAgentWithScheduler. + * + * <p>Why not just call nodeAgentWithSchedulerByHostname.values().parallelStream()? Experiments + * with Java 11 have shown that with 10 nodes and forEach(), there are a maximum of 3 concurrent + * threads. With HashMap it produces 5. With List it produces 10 concurrent threads.</p> + */ + private Stream<NodeAgentWithScheduler> parallelStreamOfNodeAgentWithScheduler() { + return List.copyOf(nodeAgentWithSchedulerByHostname.values()).parallelStream(); } // Set-difference. Returns minuend minus subtrahend. diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java index df3f075e8d9..05c765c9d78 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.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 Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.nodeagent; import com.yahoo.config.provision.ApplicationId; @@ -357,7 +357,7 @@ public class NodeAgentImpl implements NodeAgent { } try { - if (context.node().state() != NodeState.dirty) { + if (context.node().state() == NodeState.active) { suspend(context); } stopServices(context); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporter.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporter.java index 3990c5099eb..4537d99d107 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporter.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporter.java @@ -200,6 +200,9 @@ public class MetricsReporter extends NodeRepositoryMaintainer { metric.set("wantToDeprovision", node.status().wantToDeprovision() ? 1 : 0, context); metric.set("failReport", NodeFailer.reasonsToFailParentHost(node).isEmpty() ? 0 : 1, context); + metric.set("wantToEncrypt", node.reports().getReport("wantToEncrypt").isPresent() ? 1 : 0, context); + metric.set("diskEncrypted", node.reports().getReport("diskEncrypted").isPresent() ? 1 : 0, context); + HostName hostname = new HostName(node.hostname()); serviceModel.getApplication(hostname) diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java index 968427f0781..4251d1276f8 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java @@ -112,6 +112,10 @@ public class MetricsReporterTest { expectedMetrics.put("wantToRetire", 0); expectedMetrics.put("wantToDeprovision", 0); expectedMetrics.put("failReport", 0); + + expectedMetrics.put("wantToEncrypt", 0); + expectedMetrics.put("diskEncrypted", 0); + expectedMetrics.put("allowedToBeDown", 1); expectedMetrics.put("suspended", 1); expectedMetrics.put("suspendedSeconds", 123L); diff --git a/searchlib/src/tests/predicate/predicate_bounds_posting_list_test.cpp b/searchlib/src/tests/predicate/predicate_bounds_posting_list_test.cpp index 191c7495271..31aebf95ea2 100644 --- a/searchlib/src/tests/predicate/predicate_bounds_posting_list_test.cpp +++ b/searchlib/src/tests/predicate/predicate_bounds_posting_list_test.cpp @@ -29,7 +29,7 @@ SimpleIndexConfig config; const uint64_t hash = 0x123; TEST("require that empty bounds posting list starts at 0.") { - PredicateIndex index(generation_handler, generation_holder, limit_provider, config, 8); + PredicateIndex index(generation_holder, limit_provider, config, 8); vespalib::datastore::EntryRef ref; PredicateBoundsPostingList<PredicateIndex::BTreeIterator> posting_list(index.getIntervalStore(), @@ -54,7 +54,7 @@ void checkNext(PredicateBoundsPostingList<PredicateIndex::BTreeIterator> &postin } TEST("require that bounds posting list checks bounds.") { - PredicateIndex index(generation_handler, generation_holder, limit_provider, config, 8); + PredicateIndex index(generation_holder, limit_provider, config, 8); const auto &bounds_index = index.getBoundsIndex(); for (uint32_t id = 1; id < 100; ++id) { PredicateTreeAnnotations annotations(id); diff --git a/searchlib/src/tests/predicate/predicate_index_test.cpp b/searchlib/src/tests/predicate/predicate_index_test.cpp index 669f70dd544..facf0054c4a 100644 --- a/searchlib/src/tests/predicate/predicate_index_test.cpp +++ b/searchlib/src/tests/predicate/predicate_index_test.cpp @@ -33,7 +33,7 @@ DummyDocIdLimitProvider dummy_provider; SimpleIndexConfig simple_index_config; TEST("require that PredicateIndex can index empty documents") { - PredicateIndex index(generation_handler, generation_holder, dummy_provider, simple_index_config, 10); + PredicateIndex index(generation_holder, dummy_provider, simple_index_config, 10); EXPECT_EQUAL(0u, index.getZeroConstraintDocs().size()); index.indexEmptyDocument(2); index.commit(); @@ -41,7 +41,7 @@ TEST("require that PredicateIndex can index empty documents") { } TEST("require that indexDocument don't index empty documents") { - PredicateIndex index(generation_handler, generation_holder, dummy_provider, simple_index_config, 10); + PredicateIndex index(generation_holder, dummy_provider, simple_index_config, 10); EXPECT_EQUAL(0u, index.getZeroConstraintDocs().size()); PredicateTreeAnnotations annotations; index.indexDocument(3, annotations); @@ -50,7 +50,7 @@ TEST("require that indexDocument don't index empty documents") { } TEST("require that PredicateIndex can remove empty documents") { - PredicateIndex index(generation_handler, generation_holder, dummy_provider, simple_index_config, 10); + PredicateIndex index(generation_holder, dummy_provider, simple_index_config, 10); EXPECT_EQUAL(0u, index.getZeroConstraintDocs().size()); index.indexEmptyDocument(2); index.commit(); @@ -61,7 +61,7 @@ TEST("require that PredicateIndex can remove empty documents") { } TEST("require that indexing the same empty document multiple times is ok") { - PredicateIndex index(generation_handler, generation_holder, dummy_provider, simple_index_config, 10); + PredicateIndex index(generation_holder, dummy_provider, simple_index_config, 10); EXPECT_EQUAL(0u, index.getZeroConstraintDocs().size()); index.indexEmptyDocument(2); index.commit(); @@ -109,7 +109,7 @@ const IntervalWithBounds bounds = {0x0001ffff, 0x03}; Interval single_buf; TEST("require that PredicateIndex can index document") { - PredicateIndex index(generation_handler, generation_holder, dummy_provider, simple_index_config, 10); + PredicateIndex index(generation_holder, dummy_provider, simple_index_config, 10); EXPECT_FALSE(index.getIntervalIndex().lookup(hash).valid()); indexFeature(index, doc_id, min_feature, {{hash, interval}}, {}); index.commit(); @@ -124,7 +124,7 @@ TEST("require that PredicateIndex can index document") { } TEST("require that PredicateIndex can index document with bounds") { - PredicateIndex index(generation_handler, generation_holder, dummy_provider, simple_index_config, 10); + PredicateIndex index(generation_holder, dummy_provider, simple_index_config, 10); EXPECT_FALSE(index.getIntervalIndex().lookup(hash).valid()); indexFeature(index, doc_id, min_feature, {}, {{hash, bounds}}); index.commit(); @@ -149,7 +149,7 @@ TEST("require that PredicateIndex can index document with bounds") { TEST("require that PredicateIndex can index multiple documents " "with the same feature") { - PredicateIndex index(generation_handler, generation_holder, dummy_provider, simple_index_config, 10); + PredicateIndex index(generation_holder, dummy_provider, simple_index_config, 10); EXPECT_FALSE(index.getIntervalIndex().lookup(hash).valid()); for (uint32_t id = 1; id < 100; ++id) { indexFeature(index, id, min_feature, {{hash, interval}}, {}); @@ -171,7 +171,7 @@ TEST("require that PredicateIndex can index multiple documents " } TEST("require that PredicateIndex can remove indexed documents") { - PredicateIndex index(generation_handler, generation_holder, dummy_provider, simple_index_config, 10); + PredicateIndex index(generation_holder, dummy_provider, simple_index_config, 10); EXPECT_FALSE(index.getIntervalIndex().lookup(hash).valid()); indexFeature(index, doc_id, min_feature, {{hash, interval}}, {{hash2, bounds}}); @@ -187,7 +187,7 @@ TEST("require that PredicateIndex can remove indexed documents") { } TEST("require that PredicateIndex can remove multiple documents") { - PredicateIndex index(generation_handler, generation_holder, dummy_provider, simple_index_config, 10); + PredicateIndex index(generation_holder, dummy_provider, simple_index_config, 10); const auto &interval_index = index.getIntervalIndex(); EXPECT_FALSE(interval_index.lookup(hash).valid()); for (uint32_t id = 1; id < 100; ++id) { @@ -214,7 +214,7 @@ TEST("require that PredicateIndex can remove multiple documents with " intervals.push_back(make_pair(hash + i, interval)); bounds_intervals.push_back(make_pair(hash2 + i, bounds)); } - PredicateIndex index(generation_handler, generation_holder, dummy_provider, simple_index_config, 10); + PredicateIndex index(generation_holder, dummy_provider, simple_index_config, 10); const auto &interval_index = index.getIntervalIndex(); EXPECT_FALSE(interval_index.lookup(hash).valid()); for (uint32_t id = 1; id < 100; ++id) { @@ -272,7 +272,7 @@ TEST("require that PredicateIndex can be (de)serialized") { intervals.push_back(make_pair(hash + i, interval)); bounds_intervals.push_back(make_pair(hash2 + i, bounds)); } - PredicateIndex index(generation_handler, generation_holder, dummy_provider, simple_index_config, 8); + PredicateIndex index(generation_holder, dummy_provider, simple_index_config, 8); EXPECT_FALSE(index.getIntervalIndex().lookup(hash).valid()); for (uint32_t id = 1; id < 100; ++id) { indexFeature(index, id, id, intervals, bounds_intervals); @@ -284,7 +284,7 @@ TEST("require that PredicateIndex can be (de)serialized") { index.serialize(buffer); uint32_t doc_id_limit; DocIdLimitFinder finder(doc_id_limit); - PredicateIndex index2(generation_handler, generation_holder, dummy_provider, simple_index_config, + PredicateIndex index2(generation_holder, dummy_provider, simple_index_config, buffer, finder, PredicateAttribute::PREDICATE_ATTRIBUTE_VERSION); const PredicateIntervalStore &interval_store = index2.getIntervalStore(); EXPECT_EQUAL(199u, doc_id_limit); @@ -322,7 +322,7 @@ TEST("require that PredicateIndex can be (de)serialized") { } TEST("require that DocumentFeaturesStore is restored on deserialization") { - PredicateIndex index(generation_handler, generation_holder, dummy_provider, simple_index_config, 10); + PredicateIndex index(generation_holder, dummy_provider, simple_index_config, 10); EXPECT_FALSE(index.getIntervalIndex().lookup(hash).valid()); indexFeature(index, doc_id, min_feature, {{hash, interval}}, {{hash2, bounds}}); @@ -330,7 +330,7 @@ TEST("require that DocumentFeaturesStore is restored on deserialization") { index.serialize(buffer); uint32_t doc_id_limit; DocIdLimitFinder finder(doc_id_limit); - PredicateIndex index2(generation_handler, generation_holder, dummy_provider, simple_index_config, + PredicateIndex index2(generation_holder, dummy_provider, simple_index_config, buffer, finder, PredicateAttribute::PREDICATE_ATTRIBUTE_VERSION); const auto &interval_index = index2.getIntervalIndex(); const auto &bounds_index = index2.getBoundsIndex(); @@ -351,7 +351,7 @@ TEST("require that DocumentFeaturesStore is restored on deserialization") { } TEST("require that hold lists are attempted emptied on destruction") { - PredicateIndex index(generation_handler, generation_holder, dummy_provider, simple_index_config, 10); + PredicateIndex index(generation_holder, dummy_provider, simple_index_config, 10); indexFeature(index, doc_id, min_feature, {{hash, interval}}, {{hash2, bounds}}); { diff --git a/searchlib/src/tests/predicate/predicate_interval_posting_list_test.cpp b/searchlib/src/tests/predicate/predicate_interval_posting_list_test.cpp index a77542f364e..660d8556b5c 100644 --- a/searchlib/src/tests/predicate/predicate_interval_posting_list_test.cpp +++ b/searchlib/src/tests/predicate/predicate_interval_posting_list_test.cpp @@ -28,7 +28,7 @@ SimpleIndexConfig config; const uint64_t hash = 0x123; TEST("require that empty posting list starts at 0.") { - PredicateIndex index(generation_handler, generation_holder, limit_provider, config, 8); + PredicateIndex index(generation_holder, limit_provider, config, 8); vespalib::datastore::EntryRef ref; PredicateIntervalPostingList<PredicateIndex::BTreeIterator> posting_list(index.getIntervalStore(), index.getIntervalIndex().getBTreePostingList(ref)); @@ -38,7 +38,7 @@ TEST("require that empty posting list starts at 0.") { } TEST("require that posting list can iterate.") { - PredicateIndex index(generation_handler, generation_holder, limit_provider, config, 8); + PredicateIndex index(generation_holder, limit_provider, config, 8); const auto &interval_index = index.getIntervalIndex(); for (uint32_t id = 1; id < 100; ++id) { PredicateTreeAnnotations annotations(id); diff --git a/searchlib/src/tests/predicate/predicate_zero_constraint_posting_list_test.cpp b/searchlib/src/tests/predicate/predicate_zero_constraint_posting_list_test.cpp index e427c99c007..12de48b5d31 100644 --- a/searchlib/src/tests/predicate/predicate_zero_constraint_posting_list_test.cpp +++ b/searchlib/src/tests/predicate/predicate_zero_constraint_posting_list_test.cpp @@ -25,7 +25,7 @@ DummyDocIdLimitProvider limit_provider; SimpleIndexConfig config; TEST("require that empty posting list starts at 0.") { - PredicateIndex index(generation_handler, generation_holder, limit_provider, config, 8); + PredicateIndex index(generation_holder, limit_provider, config, 8); PredicateZeroConstraintPostingList posting_list(index.getZeroConstraintDocs().begin()); EXPECT_EQUAL(0u, posting_list.getDocId()); EXPECT_EQUAL(0x00010001u, posting_list.getInterval()); @@ -33,7 +33,7 @@ TEST("require that empty posting list starts at 0.") { } TEST("require that posting list can iterate.") { - PredicateIndex index(generation_handler, generation_holder, limit_provider, config, 8); + PredicateIndex index(generation_holder, limit_provider, config, 8); for (uint32_t id = 1; id < 100; ++id) { index.indexEmptyDocument(id); } diff --git a/searchlib/src/tests/predicate/predicate_zstar_compressed_posting_list_test.cpp b/searchlib/src/tests/predicate/predicate_zstar_compressed_posting_list_test.cpp index 4e86e996704..6d00b45a283 100644 --- a/searchlib/src/tests/predicate/predicate_zstar_compressed_posting_list_test.cpp +++ b/searchlib/src/tests/predicate/predicate_zstar_compressed_posting_list_test.cpp @@ -29,7 +29,7 @@ SimpleIndexConfig config; const uint64_t hash = 0x123; TEST("require that empty posting list starts at 0.") { - PredicateIndex index(generation_handler, generation_holder, limit_provider, config, 8); + PredicateIndex index(generation_holder, limit_provider, config, 8); vespalib::datastore::EntryRef ref; PredicateZstarCompressedPostingList<PredicateIndex::BTreeIterator> posting_list(index.getIntervalStore(), index.getIntervalIndex().getBTreePostingList(ref)); @@ -39,7 +39,7 @@ TEST("require that empty posting list starts at 0.") { } TEST("require that posting list can iterate.") { - PredicateIndex index(generation_handler, generation_holder, limit_provider, config, 8); + PredicateIndex index(generation_holder, limit_provider, config, 8); const auto &interval_index = index.getIntervalIndex(); vector<vector<Interval>> intervals = {{{0x00010000}}, diff --git a/searchlib/src/vespa/searchlib/attribute/predicate_attribute.cpp b/searchlib/src/vespa/searchlib/attribute/predicate_attribute.cpp index 7913e617d70..c4a0e036a01 100644 --- a/searchlib/src/vespa/searchlib/attribute/predicate_attribute.cpp +++ b/searchlib/src/vespa/searchlib/attribute/predicate_attribute.cpp @@ -26,7 +26,8 @@ constexpr uint8_t MAX_MIN_FEATURE = 255; constexpr uint16_t MAX_INTERVAL_RANGE = static_cast<uint16_t>(predicate::MAX_INTERVAL); -int64_t adjustBound(int32_t arity, int64_t bound) { +int64_t +adjustBound(int32_t arity, int64_t bound) { int64_t adjusted = arity; int64_t value = bound; int64_t max = LLONG_MAX / arity; @@ -39,7 +40,8 @@ int64_t adjustBound(int32_t arity, int64_t bound) { return adjusted - 1; } -int64_t adjustLowerBound(int32_t arity, int64_t lower_bound) { +int64_t +adjustLowerBound(int32_t arity, int64_t lower_bound) { if (lower_bound == LLONG_MIN) { return lower_bound; } else if (lower_bound > 0) { @@ -49,7 +51,8 @@ int64_t adjustLowerBound(int32_t arity, int64_t lower_bound) { } } -int64_t adjustUpperBound(int32_t arity, int64_t upper_bound) { +int64_t +adjustUpperBound(int32_t arity, int64_t upper_bound) { if (upper_bound == LLONG_MAX) { return upper_bound; } else if (upper_bound < 0) { @@ -66,13 +69,11 @@ SimpleIndexConfig createSimpleIndexConfig(const search::attribute::Config &confi } // namespace -PredicateAttribute::PredicateAttribute(const vespalib::string &base_file_name, - const Config &config) +PredicateAttribute::PredicateAttribute(const vespalib::string &base_file_name, const Config &config) : NotImplementedAttribute(base_file_name, config), - _base_file_name(base_file_name), _limit_provider(*this), - _index(new PredicateIndex(getGenerationHandler(), getGenerationHolder(), - _limit_provider, createSimpleIndexConfig(config), config.predicateParams().arity())), + _index(std::make_unique<PredicateIndex>(getGenerationHolder(), _limit_provider, + createSimpleIndexConfig(config), config.predicateParams().arity())), _lower_bound(adjustLowerBound(config.predicateParams().arity(), config.predicateParams().lower_bound())), _upper_bound(adjustUpperBound(config.predicateParams().arity(), config.predicateParams().upper_bound())), _min_feature(config.getGrowStrategy().to_generic_strategy(), getGenerationHolder()), @@ -183,7 +184,8 @@ struct DummyObserver : SimpleIndexDeserializeObserver<> { } -bool PredicateAttribute::onLoad() +bool +PredicateAttribute::onLoad() { auto loaded_buffer = attribute::LoadUtils::loadDAT(*this); char *rawBuffer = const_cast<char *>(static_cast<const char *>(loaded_buffer->buffer())); @@ -202,12 +204,12 @@ bool PredicateAttribute::onLoad() DocId highest_doc_id; if (version == 0) { DocIdLimitFinderAndMinFeatureFiller<MinFeatureVector> observer(_min_feature, *_index); - _index = std::make_unique<PredicateIndex>(getGenerationHandler(), getGenerationHolder(), _limit_provider, + _index = std::make_unique<PredicateIndex>(getGenerationHolder(), _limit_provider, createSimpleIndexConfig(getConfig()), buffer, observer, 0); highest_doc_id = observer._highest_doc_id; } else { DummyObserver observer; - _index = std::make_unique<PredicateIndex>(getGenerationHandler(), getGenerationHolder(), _limit_provider, + _index = std::make_unique<PredicateIndex>(getGenerationHolder(), _limit_provider, createSimpleIndexConfig(getConfig()), buffer, observer, version); highest_doc_id = buffer.readInt32(); // Deserialize min feature vector @@ -240,6 +242,7 @@ PredicateAttribute::addDoc(DocId &doc_id) _min_feature.ensure_size(doc_id + 1); return true; } + uint32_t PredicateAttribute::clearDoc(DocId doc_id) { diff --git a/searchlib/src/vespa/searchlib/attribute/predicate_attribute.h b/searchlib/src/vespa/searchlib/attribute/predicate_attribute.h index 6e3f0c4399f..4d7fd3c235b 100644 --- a/searchlib/src/vespa/searchlib/attribute/predicate_attribute.h +++ b/searchlib/src/vespa/searchlib/attribute/predicate_attribute.h @@ -80,7 +80,6 @@ public: void populateIfNeeded(); private: - vespalib::string _base_file_name; const AttributeVectorDocIdLimitProvider _limit_provider; std::unique_ptr<predicate::PredicateIndex> _index; int64_t _lower_bound; diff --git a/searchlib/src/vespa/searchlib/common/bitvectorcache.cpp b/searchlib/src/vespa/searchlib/common/bitvectorcache.cpp index 81f01de0c33..8cfab1f64cf 100644 --- a/searchlib/src/vespa/searchlib/common/bitvectorcache.cpp +++ b/searchlib/src/vespa/searchlib/common/bitvectorcache.cpp @@ -87,10 +87,10 @@ BitVectorCache::lookupCachedSet(const KeyAndCountSet & keys) BitVectorCache::SortedKeyMeta BitVectorCache::getSorted(Key2Index & keys) { - std::vector<std::pair<Key, KeyMeta *>> sorted; + SortedKeyMeta sorted; sorted.reserve(keys.size()); for (auto & e : keys) { - sorted.push_back({e.first, &e.second}); + sorted.emplace_back(e.first, &e.second); } std::sort(sorted.begin(), sorted.end(), [&] (const auto & a, const auto & b) { diff --git a/searchlib/src/vespa/searchlib/common/bitvectorcache.h b/searchlib/src/vespa/searchlib/common/bitvectorcache.h index c1415d9130f..f81fd0163d8 100644 --- a/searchlib/src/vespa/searchlib/common/bitvectorcache.h +++ b/searchlib/src/vespa/searchlib/common/bitvectorcache.h @@ -3,6 +3,7 @@ #include "condensedbitvectors.h" #include <vespa/vespalib/stllike/hash_set.h> +#include <vespa/vespalib/stllike/hash_map.h> #include <vespa/fastos/dynamiclibrary.h> #include <mutex> @@ -76,12 +77,12 @@ private: VESPA_DLL_LOCAL static void populate(Key2Index & newKeys, CondensedBitVector & chunk, const PopulateInterface & lookup); VESPA_DLL_LOCAL bool hasCostChanged(const std::lock_guard<std::mutex> &); - uint64_t _lookupCount; - bool _needPopulation; + uint64_t _lookupCount; + bool _needPopulation; mutable std::mutex _lock; - Key2Index _keys; - ChunkV _chunks; - GenerationHolder &_genHolder; + Key2Index _keys; + ChunkV _chunks; + GenerationHolder &_genHolder; }; } diff --git a/searchlib/src/vespa/searchlib/common/condensedbitvectors.cpp b/searchlib/src/vespa/searchlib/common/condensedbitvectors.cpp index d6efc4fddc2..50b971f499f 100644 --- a/searchlib/src/vespa/searchlib/common/condensedbitvectors.cpp +++ b/searchlib/src/vespa/searchlib/common/condensedbitvectors.cpp @@ -129,9 +129,7 @@ void throwIllegalKey(size_t numKeys, size_t key) } -CondensedBitVector::~CondensedBitVector() -{ -} +CondensedBitVector::~CondensedBitVector() = default; void CondensedBitVector::addKey(Key key) const @@ -144,7 +142,7 @@ CondensedBitVector::addKey(Key key) const CondensedBitVector::UP CondensedBitVector::create(size_t size, GenerationHolder &genHolder) { - return UP(new CondensedBitVectorT<uint32_t>(size, genHolder)); + return std::make_unique<CondensedBitVectorT<uint32_t>>(size, genHolder); } } diff --git a/searchlib/src/vespa/searchlib/common/condensedbitvectors.h b/searchlib/src/vespa/searchlib/common/condensedbitvectors.h index 4bda29894cc..02355a61e40 100644 --- a/searchlib/src/vespa/searchlib/common/condensedbitvectors.h +++ b/searchlib/src/vespa/searchlib/common/condensedbitvectors.h @@ -1,7 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once -#include <vespa/vespalib/stllike/hash_map.h> #include <vespa/vespalib/util/generationholder.h> #include <vespa/vespalib/util/arrayref.h> #include <set> @@ -31,9 +30,6 @@ public: bool hasKey(Key key) const { return key < getKeyCapacity(); } void addKey(Key key) const; static CondensedBitVector::UP create(size_t size, vespalib::GenerationHolder &genHolder); -private: - typedef vespalib::hash_map<Key, uint32_t> Key2Index; - Key2Index _keys; }; } diff --git a/searchlib/src/vespa/searchlib/predicate/document_features_store.cpp b/searchlib/src/vespa/searchlib/predicate/document_features_store.cpp index dcda13cac54..ad7d6fe3456 100644 --- a/searchlib/src/vespa/searchlib/predicate/document_features_store.cpp +++ b/searchlib/src/vespa/searchlib/predicate/document_features_store.cpp @@ -7,8 +7,6 @@ #include <vespa/vespalib/btree/btreeroot.hpp> #include <vespa/vespalib/btree/btreenodeallocator.hpp> -//#include "predicate_index.h" - using vespalib::btree::BTreeNoLeafData; using vespalib::datastore::EntryRef; using vespalib::DataBuffer; @@ -38,10 +36,8 @@ DocumentFeaturesStore::DocumentFeaturesStore(uint32_t arity) namespace { template <typename KeyComp, typename WordIndex> -void deserializeWords(DataBuffer &buffer, - memoryindex::WordStore &word_store, - WordIndex &word_index, - vector<EntryRef> &word_refs) { +void +deserializeWords(DataBuffer &buffer, memoryindex::WordStore &word_store, WordIndex &word_index, vector<EntryRef> &word_refs) { uint32_t word_list_size = buffer.readInt32(); word_refs.reserve(word_list_size); vector<char> word; @@ -57,8 +53,8 @@ void deserializeWords(DataBuffer &buffer, } template <typename RangeFeaturesMap> -void deserializeRanges(DataBuffer &buffer, vector<EntryRef> &word_refs, - RangeFeaturesMap &ranges, size_t &num_ranges) { +void +deserializeRanges(DataBuffer &buffer, vector<EntryRef> &word_refs, RangeFeaturesMap &ranges, size_t &num_ranges) { typedef typename RangeFeaturesMap::mapped_type::value_type Range; uint32_t ranges_size = buffer.readInt32(); for (uint32_t i = 0; i < ranges_size; ++i) { @@ -78,8 +74,8 @@ void deserializeRanges(DataBuffer &buffer, vector<EntryRef> &word_refs, } template <typename DocumentFeaturesMap> -void deserializeDocs(DataBuffer &buffer, DocumentFeaturesMap &docs, - size_t &num_features) { +void +deserializeDocs(DataBuffer &buffer, DocumentFeaturesMap &docs, size_t &num_features) { uint32_t docs_size = buffer.readInt32(); for (uint32_t i = 0; i < docs_size; ++i) { uint32_t doc_id = buffer.readInt32(); @@ -111,7 +107,8 @@ DocumentFeaturesStore::~DocumentFeaturesStore() { _word_index.clear(); } -void DocumentFeaturesStore::insert(uint64_t featureId, uint32_t docId) { +void +DocumentFeaturesStore::insert(uint64_t featureId, uint32_t docId) { assert(docId != 0); if (_currDocId != docId) { auto docsItr = _docs.find(docId); @@ -125,8 +122,8 @@ void DocumentFeaturesStore::insert(uint64_t featureId, uint32_t docId) { ++_numFeatures; } -void DocumentFeaturesStore::insert(const PredicateTreeAnnotations &annotations, - uint32_t doc_id) { +void +DocumentFeaturesStore::insert(const PredicateTreeAnnotations &annotations, uint32_t doc_id) { assert(doc_id != 0); if (!annotations.features.empty()) { auto it = _docs.find(doc_id); @@ -172,15 +169,15 @@ DocumentFeaturesStore::get(uint32_t docId) const { if (rangeItr != _ranges.end()) { for (auto range : rangeItr->second) { const char *label = _word_store.getWord(range.label_ref); - PredicateRangeExpander::expandRange( - label, range.from, range.to, _arity, - std::inserter(features, features.end())); + PredicateRangeExpander::expandRange(label, range.from, range.to, _arity, + std::inserter(features, features.end())); } } return features; } -void DocumentFeaturesStore::remove(uint32_t doc_id) { +void +DocumentFeaturesStore::remove(uint32_t doc_id) { auto itr = _docs.find(doc_id); if (itr != _docs.end()) { _numFeatures = _numFeatures >= itr->second.size() ? @@ -198,7 +195,8 @@ void DocumentFeaturesStore::remove(uint32_t doc_id) { } } -vespalib::MemoryUsage DocumentFeaturesStore::getMemoryUsage() const { +vespalib::MemoryUsage +DocumentFeaturesStore::getMemoryUsage() const { vespalib::MemoryUsage usage; usage.incAllocatedBytes(_docs.getMemoryConsumption()); usage.incUsedBytes(_docs.getMemoryUsed()); @@ -219,9 +217,11 @@ vespalib::MemoryUsage DocumentFeaturesStore::getMemoryUsage() const { namespace { template <typename RangeFeaturesMap> -void findUsedWords(const RangeFeaturesMap &ranges, - unordered_map<uint32_t, uint32_t> &word_map, - vector<EntryRef> &word_list) { +void +findUsedWords(const RangeFeaturesMap &ranges, + unordered_map<uint32_t, uint32_t> &word_map, + vector<EntryRef> &word_list) +{ for (const auto &range_features_entry : ranges) { for (const auto &range : range_features_entry.second) { if (!word_map.count(range.label_ref.ref())) { @@ -232,8 +232,10 @@ void findUsedWords(const RangeFeaturesMap &ranges, } } -void serializeWords(DataBuffer &buffer, const vector<EntryRef> &word_list, - const memoryindex::WordStore &word_store) { +void +serializeWords(DataBuffer &buffer, const vector<EntryRef> &word_list, + const memoryindex::WordStore &word_store) +{ buffer.writeInt32(word_list.size()); for (const auto &word_ref : word_list) { const char *word = word_store.getWord(word_ref); @@ -244,8 +246,10 @@ void serializeWords(DataBuffer &buffer, const vector<EntryRef> &word_list, } template <typename RangeFeaturesMap> -void serializeRanges(DataBuffer &buffer, RangeFeaturesMap &ranges, - unordered_map<uint32_t, uint32_t> &word_map) { +void +serializeRanges(DataBuffer &buffer, RangeFeaturesMap &ranges, + unordered_map<uint32_t, uint32_t> &word_map) +{ buffer.writeInt32(ranges.size()); for (const auto &range_features_entry : ranges) { buffer.writeInt32(range_features_entry.first); // doc id @@ -259,7 +263,8 @@ void serializeRanges(DataBuffer &buffer, RangeFeaturesMap &ranges, } template <typename DocumentFeaturesMap> -void serializeDocs(DataBuffer &buffer, DocumentFeaturesMap &docs) { +void +serializeDocs(DataBuffer &buffer, DocumentFeaturesMap &docs) { buffer.writeInt32(docs.size()); for (const auto &doc_features_entry : docs) { buffer.writeInt32(doc_features_entry.first); // doc id @@ -271,7 +276,8 @@ void serializeDocs(DataBuffer &buffer, DocumentFeaturesMap &docs) { } } // namespace -void DocumentFeaturesStore::serialize(DataBuffer &buffer) const { +void +DocumentFeaturesStore::serialize(DataBuffer &buffer) const { vector<EntryRef> word_list; unordered_map<uint32_t, uint32_t> word_map; diff --git a/searchlib/src/vespa/searchlib/predicate/document_features_store.h b/searchlib/src/vespa/searchlib/predicate/document_features_store.h index a45c7ba043a..442249d619a 100644 --- a/searchlib/src/vespa/searchlib/predicate/document_features_store.h +++ b/searchlib/src/vespa/searchlib/predicate/document_features_store.h @@ -54,14 +54,14 @@ class DocumentFeaturesStore { vespalib::btree::NoAggregated, const KeyComp &> WordIndex; DocumentFeaturesMap _docs; - RangeFeaturesMap _ranges; - WordStore _word_store; - WordIndex _word_index; - uint32_t _currDocId; - FeatureVector *_currFeatures; - size_t _numFeatures; - size_t _numRanges; - uint32_t _arity; + RangeFeaturesMap _ranges; + WordStore _word_store; + WordIndex _word_index; + uint32_t _currDocId; + FeatureVector *_currFeatures; + size_t _numFeatures; + size_t _numRanges; + uint32_t _arity; void setCurrent(uint32_t docId, FeatureVector *features); diff --git a/searchlib/src/vespa/searchlib/predicate/predicate_bounds_posting_list.h b/searchlib/src/vespa/searchlib/predicate/predicate_bounds_posting_list.h index 0ef2d81f094..9d2e90af7a5 100644 --- a/searchlib/src/vespa/searchlib/predicate/predicate_bounds_posting_list.h +++ b/searchlib/src/vespa/searchlib/predicate/predicate_bounds_posting_list.h @@ -53,7 +53,8 @@ namespace { } // namespace template<typename Iterator> -bool PredicateBoundsPostingList<Iterator>::next(uint32_t doc_id) { +bool +PredicateBoundsPostingList<Iterator>::next(uint32_t doc_id) { if (_iterator.valid() && _iterator.getKey() <= doc_id) { _iterator.linearSeek(doc_id + 1); } @@ -74,7 +75,8 @@ bool PredicateBoundsPostingList<Iterator>::next(uint32_t doc_id) { } template<typename Iterator> -bool PredicateBoundsPostingList<Iterator>::nextInterval() { +bool +PredicateBoundsPostingList<Iterator>::nextInterval() { uint32_t next_bounds; do { if (__builtin_expect(_interval_count == 1, true)) { diff --git a/searchlib/src/vespa/searchlib/predicate/predicate_index.cpp b/searchlib/src/vespa/searchlib/predicate/predicate_index.cpp index e9b1a6bd685..6cbe11e2240 100644 --- a/searchlib/src/vespa/searchlib/predicate/predicate_index.cpp +++ b/searchlib/src/vespa/searchlib/predicate/predicate_index.cpp @@ -17,16 +17,19 @@ using std::vector; namespace search::predicate { template <> -void PredicateIndex::addPosting<Interval>(uint64_t feature, uint32_t doc_id, EntryRef ref) { +void +PredicateIndex::addPosting<Interval>(uint64_t feature, uint32_t doc_id, EntryRef ref) { _interval_index.addPosting(feature, doc_id, ref); } template <> -void PredicateIndex::addPosting<IntervalWithBounds>(uint64_t feature, uint32_t doc_id, EntryRef ref) { +void +PredicateIndex::addPosting<IntervalWithBounds>(uint64_t feature, uint32_t doc_id, EntryRef ref) { _bounds_index.addPosting(feature, doc_id, ref); } template <typename IntervalT> -void PredicateIndex::indexDocumentFeatures(uint32_t doc_id, const PredicateIndex::FeatureMap<IntervalT> &interval_map) { +void +PredicateIndex::indexDocumentFeatures(uint32_t doc_id, const PredicateIndex::FeatureMap<IntervalT> &interval_map) { if (interval_map.empty()) { return; } @@ -80,11 +83,10 @@ public: } // namespace -PredicateIndex::PredicateIndex(GenerationHandler &generation_handler, GenerationHolder &genHolder, +PredicateIndex::PredicateIndex(GenerationHolder &genHolder, const DocIdLimitProvider &limit_provider, const SimpleIndexConfig &simple_index_config, uint32_t arity) : _arity(arity), - _generation_handler(generation_handler), _limit_provider(limit_provider), _interval_index(genHolder, limit_provider, simple_index_config), _bounds_index(genHolder, limit_provider, simple_index_config), @@ -95,12 +97,11 @@ PredicateIndex::PredicateIndex(GenerationHandler &generation_handler, Generation { } -PredicateIndex::PredicateIndex(GenerationHandler &generation_handler, GenerationHolder &genHolder, +PredicateIndex::PredicateIndex(GenerationHolder &genHolder, const DocIdLimitProvider &limit_provider, const SimpleIndexConfig &simple_index_config, DataBuffer &buffer, SimpleIndexDeserializeObserver<> & observer, uint32_t version) : _arity(0), - _generation_handler(generation_handler), _limit_provider(limit_provider), _interval_index(genHolder, limit_provider, simple_index_config), _bounds_index(genHolder, limit_provider, simple_index_config), @@ -121,15 +122,15 @@ PredicateIndex::PredicateIndex(GenerationHandler &generation_handler, Generation _zero_constraint_docs.assign(builder); IntervalDeserializer<Interval> interval_deserializer(_interval_store); _interval_index.deserialize(buffer, interval_deserializer, observer, version); - IntervalDeserializer<IntervalWithBounds> - bounds_deserializer(_interval_store); + IntervalDeserializer<IntervalWithBounds> bounds_deserializer(_interval_store); _bounds_index.deserialize(buffer, bounds_deserializer, observer, version); commit(); } PredicateIndex::~PredicateIndex() = default; -void PredicateIndex::serialize(DataBuffer &buffer) const { +void +PredicateIndex::serialize(DataBuffer &buffer) const { _features_store.serialize(buffer); buffer.writeInt16(_arity); buffer.writeInt32(_zero_constraint_docs.size()); @@ -142,25 +143,29 @@ void PredicateIndex::serialize(DataBuffer &buffer) const { _bounds_index.serialize(buffer, bounds_serializer); } -void PredicateIndex::onDeserializationCompleted() { +void +PredicateIndex::onDeserializationCompleted() { _interval_index.promoteOverThresholdVectors(); _bounds_index.promoteOverThresholdVectors(); } -void PredicateIndex::indexDocument(uint32_t doc_id, const PredicateTreeAnnotations &annotations) { +void +PredicateIndex::indexDocument(uint32_t doc_id, const PredicateTreeAnnotations &annotations) { indexDocumentFeatures(doc_id, annotations.interval_map); indexDocumentFeatures(doc_id, annotations.bounds_map); _features_store.insert(annotations, doc_id); } -void PredicateIndex::indexEmptyDocument(uint32_t doc_id) +void +PredicateIndex::indexEmptyDocument(uint32_t doc_id) { _zero_constraint_docs.insert(doc_id, vespalib::btree::BTreeNoLeafData::_instance); } namespace { -void removeFromIndex( - uint64_t feature, uint32_t doc_id, SimpleIndex<vespalib::datastore::EntryRef> &index, PredicateIntervalStore &interval_store) +void +removeFromIndex(uint64_t feature, uint32_t doc_id, SimpleIndex<vespalib::datastore::EntryRef> &index, + PredicateIntervalStore &interval_store) { auto result = index.removeFromPostingList(feature, doc_id); if (result.second) { // Posting was removed @@ -189,7 +194,8 @@ private: } // namespace -void PredicateIndex::removeDocument(uint32_t doc_id) { +void +PredicateIndex::removeDocument(uint32_t doc_id) { _zero_constraint_docs.remove(doc_id); auto features = _features_store.get(doc_id); @@ -203,27 +209,31 @@ void PredicateIndex::removeDocument(uint32_t doc_id) { _features_store.remove(doc_id); } -void PredicateIndex::commit() { +void +PredicateIndex::commit() { _interval_index.commit(); _bounds_index.commit(); _zero_constraint_docs.getAllocator().freeze(); } -void PredicateIndex::trimHoldLists(generation_t used_generation) { +void +PredicateIndex::trimHoldLists(generation_t used_generation) { _interval_index.trimHoldLists(used_generation); _bounds_index.trimHoldLists(used_generation); _interval_store.trimHoldLists(used_generation); _zero_constraint_docs.getAllocator().trimHoldLists(used_generation); } -void PredicateIndex::transferHoldLists(generation_t generation) { +void +PredicateIndex::transferHoldLists(generation_t generation) { _interval_index.transferHoldLists(generation); _bounds_index.transferHoldLists(generation); _interval_store.transferHoldLists(generation); _zero_constraint_docs.getAllocator().transferHoldLists(generation); } -vespalib::MemoryUsage PredicateIndex::getMemoryUsage() const { +vespalib::MemoryUsage +PredicateIndex::getMemoryUsage() const { // TODO Include bit vector cache memory usage vespalib::MemoryUsage combined; combined.merge(_interval_index.getMemoryUsage()); diff --git a/searchlib/src/vespa/searchlib/predicate/predicate_index.h b/searchlib/src/vespa/searchlib/predicate/predicate_index.h index d2ed70694a2..f4c89a2b369 100644 --- a/searchlib/src/vespa/searchlib/predicate/predicate_index.h +++ b/searchlib/src/vespa/searchlib/predicate/predicate_index.h @@ -38,16 +38,15 @@ public: using BTreeIterator = SimpleIndex<vespalib::datastore::EntryRef>::BTreeIterator; using VectorIterator = SimpleIndex<vespalib::datastore::EntryRef>::VectorIterator; private: - uint32_t _arity; - GenerationHandler &_generation_handler; + uint32_t _arity; const DocIdLimitProvider &_limit_provider; - IntervalIndex _interval_index; - BoundsIndex _bounds_index; - PredicateIntervalStore _interval_store; - BTreeSet _zero_constraint_docs; + IntervalIndex _interval_index; + BoundsIndex _bounds_index; + PredicateIntervalStore _interval_store; + BTreeSet _zero_constraint_docs; - DocumentFeaturesStore _features_store; - mutable BitVectorCache _cache; + DocumentFeaturesStore _features_store; + mutable BitVectorCache _cache; template <typename IntervalT> void addPosting(uint64_t feature, uint32_t doc_id, vespalib::datastore::EntryRef ref); @@ -58,12 +57,12 @@ private: PopulateInterface::Iterator::UP lookup(uint64_t key) const override; public: - PredicateIndex(GenerationHandler &generation_handler, GenerationHolder &genHolder, + PredicateIndex(GenerationHolder &genHolder, const DocIdLimitProvider &limit_provider, const SimpleIndexConfig &simple_index_config, uint32_t arity); // deserializes PredicateIndex from buffer. // The observer can be used to gain some insight into what has been added to the index.. - PredicateIndex(GenerationHandler &generation_handler, GenerationHolder &genHolder, + PredicateIndex(GenerationHolder &genHolder, const DocIdLimitProvider &limit_provider, const SimpleIndexConfig &simple_index_config, vespalib::DataBuffer &buffer, SimpleIndexDeserializeObserver<> & observer, uint32_t version); diff --git a/searchlib/src/vespa/searchlib/predicate/predicate_interval.cpp b/searchlib/src/vespa/searchlib/predicate/predicate_interval.cpp index a92c16de462..d98e8a151dc 100644 --- a/searchlib/src/vespa/searchlib/predicate/predicate_interval.cpp +++ b/searchlib/src/vespa/searchlib/predicate/predicate_interval.cpp @@ -5,14 +5,16 @@ namespace search::predicate { -std::ostream &operator<<(std::ostream &out, const Interval &i) { +std::ostream & +operator<<(std::ostream &out, const Interval &i) { std::ios_base::fmtflags flags = out.flags(); out << "0x" << std::hex << i.interval; out.flags(flags); return out; } -std::ostream &operator<<(std::ostream &out, const IntervalWithBounds &i) { +std::ostream & +operator<<(std::ostream &out, const IntervalWithBounds &i) { std::ios_base::fmtflags flags = out.flags(); out << "0x" << std::hex << i.interval << ", 0x" << i.bounds; out.flags(flags); diff --git a/searchlib/src/vespa/searchlib/predicate/predicate_interval_posting_list.h b/searchlib/src/vespa/searchlib/predicate/predicate_interval_posting_list.h index f93d99b550b..33e15b2be33 100644 --- a/searchlib/src/vespa/searchlib/predicate/predicate_interval_posting_list.h +++ b/searchlib/src/vespa/searchlib/predicate/predicate_interval_posting_list.h @@ -14,10 +14,10 @@ namespace search::predicate { template<typename Iterator> class PredicateIntervalPostingList : public PredicatePostingList { const PredicateIntervalStore &_interval_store; - Iterator _iterator; - const Interval *_current_interval; - uint32_t _interval_count; - Interval _single_buf; + Iterator _iterator; + const Interval *_current_interval; + uint32_t _interval_count; + Interval _single_buf; public: PredicateIntervalPostingList(const PredicateIntervalStore &interval_store, Iterator it); @@ -46,7 +46,8 @@ PredicateIntervalPostingList<Iterator>::PredicateIntervalPostingList( } template<typename Iterator> -bool PredicateIntervalPostingList<Iterator>::next(uint32_t doc_id) { +bool +PredicateIntervalPostingList<Iterator>::next(uint32_t doc_id) { if (!_iterator.valid()) { return false; } diff --git a/searchlib/src/vespa/searchlib/predicate/predicate_interval_store.cpp b/searchlib/src/vespa/searchlib/predicate/predicate_interval_store.cpp index 28c82cb7a97..13be0f0127b 100644 --- a/searchlib/src/vespa/searchlib/predicate/predicate_interval_store.cpp +++ b/searchlib/src/vespa/searchlib/predicate/predicate_interval_store.cpp @@ -21,7 +21,8 @@ PredicateIntervalStore::PredicateIntervalStore() : _store(), _size1Type(1, 1024u, RefType::offsetSize()), _store_adapter(_store), - _ref_cache(_store_adapter) { + _ref_cache(_store_adapter) +{ // This order determines type ids. _store.addType(&_size1Type); @@ -46,7 +47,8 @@ PredicateIntervalStore::~PredicateIntervalStore() { // anyway. // template <typename IntervalT> -EntryRef PredicateIntervalStore::insert(const vector<IntervalT> &intervals) { +EntryRef +PredicateIntervalStore::insert(const vector<IntervalT> &intervals) { const uint32_t size = entrySize<IntervalT>() * intervals.size(); if (size == 0) { return EntryRef(); @@ -81,7 +83,8 @@ EntryRef PredicateIntervalStore::insert(const vector<Interval> &); template EntryRef PredicateIntervalStore::insert(const vector<IntervalWithBounds> &); -void PredicateIntervalStore::remove(EntryRef ref) { +void +PredicateIntervalStore::remove(EntryRef ref) { if (ref.valid()) { uint32_t buffer_id = RefType(ref).bufferId(); if (buffer_id == 0) { // single interval optimization. @@ -96,11 +99,13 @@ void PredicateIntervalStore::remove(EntryRef ref) { } } -void PredicateIntervalStore::trimHoldLists(generation_t used_generation) { +void +PredicateIntervalStore::trimHoldLists(generation_t used_generation) { _store.trimHoldLists(used_generation); } -void PredicateIntervalStore::transferHoldLists(generation_t generation) { +void +PredicateIntervalStore::transferHoldLists(generation_t generation) { _store.transferHoldLists(generation); } diff --git a/searchlib/src/vespa/searchlib/predicate/predicate_interval_store.h b/searchlib/src/vespa/searchlib/predicate/predicate_interval_store.h index e4573866eb8..5f55a2d3d5f 100644 --- a/searchlib/src/vespa/searchlib/predicate/predicate_interval_store.h +++ b/searchlib/src/vespa/searchlib/predicate/predicate_interval_store.h @@ -34,7 +34,7 @@ class PredicateIntervalStore { } }; DataStoreAdapter _store_adapter; - RefCacheType _ref_cache; + RefCacheType _ref_cache; // Return type for private allocation functions template <typename T> @@ -89,7 +89,8 @@ public: * single interval optimization. */ template <typename IntervalT> - const IntervalT *get(vespalib::datastore::EntryRef btree_ref, + const IntervalT + *get(vespalib::datastore::EntryRef btree_ref, uint32_t &size_out, IntervalT *single_buf) const { diff --git a/searchlib/src/vespa/searchlib/predicate/predicate_posting_list.h b/searchlib/src/vespa/searchlib/predicate/predicate_posting_list.h index 93e671f603f..50024913dcb 100644 --- a/searchlib/src/vespa/searchlib/predicate/predicate_posting_list.h +++ b/searchlib/src/vespa/searchlib/predicate/predicate_posting_list.h @@ -16,9 +16,9 @@ class PredicatePostingList { protected: PredicatePostingList() - : _docId(0), - _subquery(UINT64_MAX) { - } + : _docId(0), + _subquery(UINT64_MAX) + { } void setDocId(uint32_t docId) { _docId = docId; } diff --git a/searchlib/src/vespa/searchlib/predicate/predicate_zstar_compressed_posting_list.h b/searchlib/src/vespa/searchlib/predicate/predicate_zstar_compressed_posting_list.h index 965c4ad3042..0268d2bdb0c 100644 --- a/searchlib/src/vespa/searchlib/predicate/predicate_zstar_compressed_posting_list.h +++ b/searchlib/src/vespa/searchlib/predicate/predicate_zstar_compressed_posting_list.h @@ -41,7 +41,8 @@ PredicateZstarCompressedPostingList<Iterator>::PredicateZstarCompressedPostingLi } template<typename Iterator> -bool PredicateZstarCompressedPostingList<Iterator>::next(uint32_t doc_id) { +bool +PredicateZstarCompressedPostingList<Iterator>::next(uint32_t doc_id) { if (_iterator.valid() && _iterator.getKey() <= doc_id) { _iterator.linearSeek(doc_id + 1); } @@ -57,7 +58,8 @@ bool PredicateZstarCompressedPostingList<Iterator>::next(uint32_t doc_id) { } template<typename Iterator> -bool PredicateZstarCompressedPostingList<Iterator>::nextInterval() { +bool +PredicateZstarCompressedPostingList<Iterator>::nextInterval() { uint32_t next_interval = UINT32_MAX; if (_interval_count > 1) { next_interval = _current_interval[1].interval; diff --git a/searchlib/src/vespa/searchlib/predicate/simple_index.cpp b/searchlib/src/vespa/searchlib/predicate/simple_index.cpp index 1b0db8f52d4..b0ef11e1c25 100644 --- a/searchlib/src/vespa/searchlib/predicate/simple_index.cpp +++ b/searchlib/src/vespa/searchlib/predicate/simple_index.cpp @@ -6,14 +6,12 @@ #include <vespa/vespalib/btree/btreeiterator.hpp> #include <vespa/vespalib/btree/btreestore.hpp> #include <vespa/vespalib/btree/btreenodeallocator.hpp> -#include <vespa/vespalib/util/array.hpp> #include <vespa/vespalib/datastore/buffer_type.hpp> #include <vespa/log/log.h> LOG_SETUP(".searchlib.predicate.simple_index"); -namespace search::predicate { - namespace simpleindex { +namespace search::predicate::simpleindex { bool log_enabled() { return LOG_WOULD_LOG(debug); @@ -25,6 +23,8 @@ void log_debug(vespalib::string &str) { } // namespace simpleindex +namespace search::predicate { + template class SimpleIndex<vespalib::datastore::EntryRef>; } diff --git a/searchlib/src/vespa/searchlib/predicate/simple_index.h b/searchlib/src/vespa/searchlib/predicate/simple_index.h index cfc288770c8..1398bb0817c 100644 --- a/searchlib/src/vespa/searchlib/predicate/simple_index.h +++ b/searchlib/src/vespa/searchlib/predicate/simple_index.h @@ -141,12 +141,12 @@ private: template <typename T> using optional = std::optional<T>; - Dictionary _dictionary; - BTreeStore _btree_posting_lists; - VectorStore _vector_posting_lists; - GenerationHolder &_generation_holder; - uint32_t _insert_remove_counter = 0; - const SimpleIndexConfig _config; + Dictionary _dictionary; + BTreeStore _btree_posting_lists; + VectorStore _vector_posting_lists; + GenerationHolder &_generation_holder; + uint32_t _insert_remove_counter = 0; + const SimpleIndexConfig _config; const DocIdLimitProvider &_limit_provider; void insertIntoPosting(vespalib::datastore::EntryRef &ref, Key key, DocId doc_id, const Posting &posting); @@ -164,7 +164,7 @@ private: bool shouldRemoveVectorPosting(size_t size, double ratio) const; size_t getVectorPostingSize(const PostingVector &vector) const { return std::min(vector.size(), - static_cast<size_t>(_limit_provider.getCommittedDocIdLimit())); + static_cast<size_t>(_limit_provider.getCommittedDocIdLimit())); } public: @@ -219,8 +219,8 @@ public: template<typename Posting, typename Key, typename DocId> template<typename FunctionType> -void SimpleIndex<Posting, Key, DocId>::foreach_frozen_key( - vespalib::datastore::EntryRef ref, Key key, FunctionType func) const { +void +SimpleIndex<Posting, Key, DocId>::foreach_frozen_key(vespalib::datastore::EntryRef ref, Key key, FunctionType func) const { auto it = _vector_posting_lists.getFrozenView().find(key); double ratio = getDocumentRatio(getDocumentCount(ref), _limit_provider.getDocIdLimit()); if (it.valid() && ratio > _config.foreach_vector_threshold) { diff --git a/searchlib/src/vespa/searchlib/predicate/simple_index.hpp b/searchlib/src/vespa/searchlib/predicate/simple_index.hpp index b49218f1ba6..ada77b9fe38 100644 --- a/searchlib/src/vespa/searchlib/predicate/simple_index.hpp +++ b/searchlib/src/vespa/searchlib/predicate/simple_index.hpp @@ -13,8 +13,8 @@ namespace simpleindex { } template <typename Posting, typename Key, typename DocId> -void SimpleIndex<Posting, Key, DocId>::insertIntoPosting( - vespalib::datastore::EntryRef &ref, Key key, DocId doc_id, const Posting &posting) { +void +SimpleIndex<Posting, Key, DocId>::insertIntoPosting(vespalib::datastore::EntryRef &ref, Key key, DocId doc_id, const Posting &posting) { bool ok = _btree_posting_lists.insert(ref, doc_id, posting); if (!ok) { _btree_posting_lists.remove(ref, doc_id); @@ -26,8 +26,8 @@ void SimpleIndex<Posting, Key, DocId>::insertIntoPosting( } template <typename Posting, typename Key, typename DocId> -void SimpleIndex<Posting, Key, DocId>::insertIntoVectorPosting( - vespalib::datastore::EntryRef ref, Key key, DocId doc_id, const Posting &posting) { +void +SimpleIndex<Posting, Key, DocId>::insertIntoVectorPosting(vespalib::datastore::EntryRef ref, Key key, DocId doc_id, const Posting &posting) { assert(doc_id < _limit_provider.getDocIdLimit()); auto it = _vector_posting_lists.find(key); if (it.valid()) { @@ -69,9 +69,8 @@ SimpleIndex<Posting, Key, DocId>::~SimpleIndex() { } template <typename Posting, typename Key, typename DocId> -void SimpleIndex<Posting, Key, DocId>::serialize( - vespalib::DataBuffer &buffer, - const PostingSerializer<Posting> &serializer) const { +void +SimpleIndex<Posting, Key, DocId>::serialize(vespalib::DataBuffer &buffer, const PostingSerializer<Posting> &serializer) const { assert(sizeof(Key) <= sizeof(uint64_t)); assert(sizeof(DocId) <= sizeof(uint32_t)); buffer.writeInt32(_dictionary.size()); @@ -90,10 +89,10 @@ void SimpleIndex<Posting, Key, DocId>::serialize( } template <typename Posting, typename Key, typename DocId> -void SimpleIndex<Posting, Key, DocId>::deserialize( - vespalib::DataBuffer &buffer, - PostingDeserializer<Posting> &deserializer, - SimpleIndexDeserializeObserver<Key, DocId> &observer, uint32_t version) { +void +SimpleIndex<Posting, Key, DocId>::deserialize(vespalib::DataBuffer &buffer, PostingDeserializer<Posting> &deserializer, + SimpleIndexDeserializeObserver<Key, DocId> &observer, uint32_t version) +{ typename Dictionary::Builder builder(_dictionary.getAllocator()); uint32_t size = buffer.readInt32(); std::vector<vespalib::btree::BTreeKeyData<DocId, Posting>> postings; @@ -128,8 +127,8 @@ void SimpleIndex<Posting, Key, DocId>::deserialize( } template <typename Posting, typename Key, typename DocId> -void SimpleIndex<Posting, Key, DocId>::addPosting(Key key, DocId doc_id, - const Posting &posting) { +void +SimpleIndex<Posting, Key, DocId>::addPosting(Key key, DocId doc_id, const Posting &posting) { auto iter = _dictionary.find(key); vespalib::datastore::EntryRef ref; if (iter.valid()) { @@ -178,8 +177,8 @@ SimpleIndex<Posting, Key, DocId>::removeFromPostingList(Key key, DocId doc_id) { } template <typename Posting, typename Key, typename DocId> -void SimpleIndex<Posting, Key, DocId>::removeFromVectorPostingList( - vespalib::datastore::EntryRef ref, Key key, DocId doc_id) { +void +SimpleIndex<Posting, Key, DocId>::removeFromVectorPostingList(vespalib::datastore::EntryRef ref, Key key, DocId doc_id) { auto it = _vector_posting_lists.find(key); if (it.valid()) { if (!removeVectorIfBelowThreshold(ref, it)) { @@ -189,7 +188,8 @@ void SimpleIndex<Posting, Key, DocId>::removeFromVectorPostingList( }; template <typename Posting, typename Key, typename DocId> -void SimpleIndex<Posting, Key, DocId>::pruneBelowThresholdVectors() { +void +SimpleIndex<Posting, Key, DocId>::pruneBelowThresholdVectors() { // Check if it is time to prune any vector postings if (++_insert_remove_counter % _config.vector_prune_frequency > 0) return; @@ -204,7 +204,8 @@ void SimpleIndex<Posting, Key, DocId>::pruneBelowThresholdVectors() { }; template <typename Posting, typename Key, typename DocId> -void SimpleIndex<Posting, Key, DocId>::promoteOverThresholdVectors() { +void +SimpleIndex<Posting, Key, DocId>::promoteOverThresholdVectors() { for (auto it = _dictionary.begin(); it.valid(); ++it) { Key key = it.getKey(); if (!_vector_posting_lists.find(key).valid()) { @@ -214,8 +215,8 @@ void SimpleIndex<Posting, Key, DocId>::promoteOverThresholdVectors() { } template <typename Posting, typename Key, typename DocId> -void SimpleIndex<Posting, Key, DocId>::logVector( - const char *action, Key key, size_t document_count, double ratio, size_t vector_length) const { +void +SimpleIndex<Posting, Key, DocId>::logVector(const char *action, Key key, size_t document_count, double ratio, size_t vector_length) const { if (!simpleindex::log_enabled()) return; auto msg = vespalib::make_string( "%s vector for key '%016" PRIx64 "' with length %zu. Contains %zu documents " @@ -227,7 +228,8 @@ void SimpleIndex<Posting, Key, DocId>::logVector( } template <typename Posting, typename Key, typename DocId> -void SimpleIndex<Posting, Key, DocId>::createVectorIfOverThreshold(vespalib::datastore::EntryRef ref, Key key) { +void +SimpleIndex<Posting, Key, DocId>::createVectorIfOverThreshold(vespalib::datastore::EntryRef ref, Key key) { uint32_t doc_id_limit = _limit_provider.getDocIdLimit(); size_t size = getDocumentCount(ref); double ratio = getDocumentRatio(size, doc_id_limit); @@ -242,8 +244,8 @@ void SimpleIndex<Posting, Key, DocId>::createVectorIfOverThreshold(vespalib::dat } template <typename Posting, typename Key, typename DocId> -bool SimpleIndex<Posting, Key, DocId>::removeVectorIfBelowThreshold( - vespalib::datastore::EntryRef ref, typename VectorStore::Iterator &it) { +bool +SimpleIndex<Posting, Key, DocId>::removeVectorIfBelowThreshold(vespalib::datastore::EntryRef ref, typename VectorStore::Iterator &it) { size_t size = getDocumentCount(ref); double ratio = getDocumentRatio(size, _limit_provider.getDocIdLimit()); if (shouldRemoveVectorPosting(size, ratio)) { @@ -257,36 +259,41 @@ bool SimpleIndex<Posting, Key, DocId>::removeVectorIfBelowThreshold( } template <typename Posting, typename Key, typename DocId> -double SimpleIndex<Posting, Key, DocId>::getDocumentRatio(size_t document_count, - uint32_t doc_id_limit) const { +double +SimpleIndex<Posting, Key, DocId>::getDocumentRatio(size_t document_count, uint32_t doc_id_limit) const { assert(doc_id_limit > 1); return document_count / static_cast<double>(doc_id_limit - 1); }; template <typename Posting, typename Key, typename DocId> -size_t SimpleIndex<Posting, Key, DocId>::getDocumentCount(vespalib::datastore::EntryRef ref) const { +size_t +SimpleIndex<Posting, Key, DocId>::getDocumentCount(vespalib::datastore::EntryRef ref) const { return _btree_posting_lists.size(ref); }; template <typename Posting, typename Key, typename DocId> -bool SimpleIndex<Posting, Key, DocId>::shouldRemoveVectorPosting(size_t size, double ratio) const { +bool +SimpleIndex<Posting, Key, DocId>::shouldRemoveVectorPosting(size_t size, double ratio) const { return size < _config.lower_vector_size_threshold || ratio < _config.lower_docid_freq_threshold; }; template <typename Posting, typename Key, typename DocId> -bool SimpleIndex<Posting, Key, DocId>::shouldCreateVectorPosting(size_t size, double ratio) const { +bool +SimpleIndex<Posting, Key, DocId>::shouldCreateVectorPosting(size_t size, double ratio) const { return size >= _config.upper_vector_size_threshold && ratio >= _config.upper_docid_freq_threshold; }; template <typename Posting, typename Key, typename DocId> -void SimpleIndex<Posting, Key, DocId>::commit() { +void +SimpleIndex<Posting, Key, DocId>::commit() { _dictionary.getAllocator().freeze(); _btree_posting_lists.freeze(); _vector_posting_lists.getAllocator().freeze(); } template <typename Posting, typename Key, typename DocId> -void SimpleIndex<Posting, Key, DocId>::trimHoldLists(generation_t used_generation) { +void +SimpleIndex<Posting, Key, DocId>::trimHoldLists(generation_t used_generation) { _btree_posting_lists.trimHoldLists(used_generation); _dictionary.getAllocator().trimHoldLists(used_generation); _vector_posting_lists.getAllocator().trimHoldLists(used_generation); @@ -294,14 +301,16 @@ void SimpleIndex<Posting, Key, DocId>::trimHoldLists(generation_t used_generatio } template <typename Posting, typename Key, typename DocId> -void SimpleIndex<Posting, Key, DocId>::transferHoldLists(generation_t generation) { +void +SimpleIndex<Posting, Key, DocId>::transferHoldLists(generation_t generation) { _dictionary.getAllocator().transferHoldLists(generation); _btree_posting_lists.transferHoldLists(generation); _vector_posting_lists.getAllocator().transferHoldLists(generation); } template <typename Posting, typename Key, typename DocId> -vespalib::MemoryUsage SimpleIndex<Posting, Key, DocId>::getMemoryUsage() const { +vespalib::MemoryUsage +SimpleIndex<Posting, Key, DocId>::getMemoryUsage() const { vespalib::MemoryUsage combined; combined.merge(_dictionary.getMemoryUsage()); combined.merge(_btree_posting_lists.getMemoryUsage()); diff --git a/searchlib/src/vespa/searchlib/queryeval/predicate_blueprint.cpp b/searchlib/src/vespa/searchlib/queryeval/predicate_blueprint.cpp index 66acc2f0836..24d731156b3 100644 --- a/searchlib/src/vespa/searchlib/queryeval/predicate_blueprint.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/predicate_blueprint.cpp @@ -8,13 +8,13 @@ #include <vespa/searchlib/predicate/predicate_hash.h> #include <vespa/searchlib/predicate/predicate_index.h> #include <vespa/searchlib/query/tree/termnodes.h> -#include <vespa/vespalib/btree/btree.hpp> #include <vespa/vespalib/btree/btreeroot.hpp> #include <vespa/vespalib/btree/btreeiterator.hpp> #include <vespa/vespalib/btree/btreestore.hpp> #include <vespa/vespalib/btree/btreenodeallocator.hpp> #include <vespa/vespalib/util/memory_allocator.h> #include <algorithm> + #include <vespa/log/log.h> LOG_SETUP(".searchlib.predicate.predicate_blueprint"); #include <vespa/searchlib/predicate/predicate_range_term_expander.h> @@ -54,7 +54,8 @@ struct MyRangeHandler { vector<BoundsEntry> &bounds_entries; uint64_t subquery_bitmap; - void handleRange(const string &label) { + void + handleRange(const string &label) { uint64_t feature = PredicateHash::hash64(label); auto iterator = interval_index.lookup(feature); if (iterator.valid()) { @@ -62,7 +63,8 @@ struct MyRangeHandler { interval_entries.push_back({iterator.getData(), subquery_bitmap, sz, feature}); } } - void handleEdge(const string &label, uint32_t value) { + void + handleEdge(const string &label, uint32_t value) { uint64_t feature = PredicateHash::hash64(label); auto iterator = bounds_index.lookup(feature); if (iterator.valid()) { @@ -73,18 +75,19 @@ struct MyRangeHandler { }; template <typename Entry> -void pushRangeDictionaryEntries( - const Entry &entry, - const PredicateIndex &index, - vector<IntervalEntry> &interval_entries, - vector<BoundsEntry> &bounds_entries) { +void +pushRangeDictionaryEntries(const Entry &entry, const PredicateIndex &index, + vector<IntervalEntry> &interval_entries, + vector<BoundsEntry> &bounds_entries) +{ PredicateRangeTermExpander expander(index.getArity()); MyRangeHandler handler{index.getIntervalIndex(), index.getBoundsIndex(), interval_entries, bounds_entries, entry.getSubQueryBitmap()}; expander.expand(entry.getKey(), entry.getValue(), handler); } -void pushZStarPostingList(const SimpleIndex<vespalib::datastore::EntryRef> &interval_index, +void +pushZStarPostingList(const SimpleIndex<vespalib::datastore::EntryRef> &interval_index, vector<IntervalEntry> &interval_entries) { uint64_t feature = Constants::z_star_hash; auto iterator = interval_index.lookup(feature); @@ -96,7 +99,8 @@ void pushZStarPostingList(const SimpleIndex<vespalib::datastore::EntryRef> &inte } // namespace -void PredicateBlueprint::addPostingToK(uint64_t feature) +void +PredicateBlueprint::addPostingToK(uint64_t feature) { const auto &interval_index = _index.getIntervalIndex(); auto tmp = interval_index.lookup(feature); @@ -115,7 +119,8 @@ void PredicateBlueprint::addPostingToK(uint64_t feature) } } -void PredicateBlueprint::addBoundsPostingToK(uint64_t feature) +void +PredicateBlueprint::addBoundsPostingToK(uint64_t feature) { const auto &bounds_index = _index.getBoundsIndex(); auto tmp = bounds_index.lookup(feature); @@ -134,7 +139,8 @@ void PredicateBlueprint::addBoundsPostingToK(uint64_t feature) } } -void PredicateBlueprint::addZeroConstraintToK() +void +PredicateBlueprint::addZeroConstraintToK() { uint8_t *kVBase = &_kV[0]; size_t kVSize = _kV.size(); @@ -174,15 +180,14 @@ PredicateBlueprint::PredicateBlueprint(const FieldSpecBase &field, pushValueDictionaryEntry(entry, interval_index, _interval_dict_entries); } for (const auto &entry : term.getRangeFeatures()) { - pushRangeDictionaryEntries(entry, _index, _interval_dict_entries, - _bounds_dict_entries); + pushRangeDictionaryEntries(entry, _index, _interval_dict_entries,_bounds_dict_entries); } pushZStarPostingList(interval_index, _interval_dict_entries); BitVectorCache::KeyAndCountSet keys; keys.reserve(_interval_dict_entries.size()); for (const auto & e : _interval_dict_entries) { - keys.push_back({e.feature, e.size}); + keys.emplace_back(e.feature, e.size); } _cachedFeatures = _index.lookupCachedSet(keys); @@ -202,40 +207,43 @@ PredicateBlueprint::PredicateBlueprint(const FieldSpecBase &field, }); - if (zero_constraints_docs.size() == 0 && + if ((zero_constraints_docs.size() == 0) && _interval_dict_entries.empty() && _bounds_dict_entries.empty() && - !_zstar_dict_entry.valid()) { + !_zstar_dict_entry.valid()) + { setEstimate(HitEstimate(0, true)); } else { setEstimate(HitEstimate(static_cast<uint32_t>(zero_constraints_docs.size()), false)); } } -PredicateBlueprint::~PredicateBlueprint() {} +PredicateBlueprint::~PredicateBlueprint() = default; namespace { - template<typename DictEntry, typename VectorIteratorEntry, typename BTreeIteratorEntry> - void lookupPostingLists(const std::vector<DictEntry> &dict_entries, - std::vector<VectorIteratorEntry> &vector_iterators, - std::vector<BTreeIteratorEntry> &btree_iterators, - const SimpleIndex<vespalib::datastore::EntryRef> &index) - { - for (const auto &entry : dict_entries) { - auto vector_iterator = index.getVectorPostingList(entry.feature); - if (vector_iterator) { - vector_iterators.push_back(VectorIteratorEntry{*vector_iterator, entry}); - } else { - auto btree_iterator = index.getBTreePostingList(entry.entry_ref); - btree_iterators.push_back(BTreeIteratorEntry{btree_iterator, entry}); - } +template<typename DictEntry, typename VectorIteratorEntry, typename BTreeIteratorEntry> +void +lookupPostingLists(const std::vector<DictEntry> &dict_entries, + std::vector<VectorIteratorEntry> &vector_iterators, + std::vector<BTreeIteratorEntry> &btree_iterators, + const SimpleIndex<vespalib::datastore::EntryRef> &index) +{ + for (const auto &entry : dict_entries) { + auto vector_iterator = index.getVectorPostingList(entry.feature); + if (vector_iterator) { + vector_iterators.push_back(VectorIteratorEntry{*vector_iterator, entry}); + } else { + auto btree_iterator = index.getBTreePostingList(entry.entry_ref); + btree_iterators.push_back(BTreeIteratorEntry{btree_iterator, entry}); } + } - }; +} } -void PredicateBlueprint::fetchPostings(const ExecuteInfo &) { +void +PredicateBlueprint::fetchPostings(const ExecuteInfo &) { if (!_fetch_postings_done) { const auto &interval_index = _index.getIntervalIndex(); const auto &bounds_index = _index.getBoundsIndex(); @@ -277,29 +285,31 @@ PredicateBlueprint::createLeafSearch(const fef::TermFieldMatchDataArray &tfmda, PredicateAttribute::MinFeatureHandle mfh = attribute.getMinFeatureVector(); auto interval_range_vector = attribute.getIntervalRangeVector(); auto max_interval_range = attribute.getMaxIntervalRange(); - return SearchIterator::UP(new PredicateSearch(mfh.first, interval_range_vector, max_interval_range, _kV, - createPostingLists(), tfmda)); + return std::make_unique<PredicateSearch>(mfh.first, interval_range_vector, max_interval_range, _kV, + createPostingLists(), tfmda); } namespace { - template<typename IteratorEntry, typename PostingListFactory> - void createPredicatePostingLists(const std::vector<IteratorEntry> &iterator_entries, - std::vector<PredicatePostingList::UP> &posting_lists, - PostingListFactory posting_list_factory) - { - for (const auto &entry : iterator_entries) { - if (entry.iterator.valid()) { - auto posting_list = posting_list_factory(entry); - posting_list->setSubquery(entry.entry.subquery); - posting_lists.emplace_back(PredicatePostingList::UP(posting_list)); - } +template<typename IteratorEntry, typename PostingListFactory> +void +createPredicatePostingLists(const std::vector<IteratorEntry> &iterator_entries, + std::vector<PredicatePostingList::UP> &posting_lists, + PostingListFactory posting_list_factory) +{ + for (const auto &entry : iterator_entries) { + if (entry.iterator.valid()) { + auto posting_list = posting_list_factory(entry); + posting_list->setSubquery(entry.entry.subquery); + posting_lists.emplace_back(PredicatePostingList::UP(posting_list)); } } +} } -std::vector<PredicatePostingList::UP> PredicateBlueprint::createPostingLists() const { +std::vector<PredicatePostingList::UP> +PredicateBlueprint::createPostingLists() const { size_t total_size = _interval_btree_iterators.size() + _interval_vector_iterators.size() + _bounds_btree_iterators.size() + _bounds_vector_iterators.size() + 2; std::vector<PredicatePostingList::UP> posting_lists; @@ -333,17 +343,15 @@ std::vector<PredicatePostingList::UP> PredicateBlueprint::createPostingLists() c }); if (_zstar_vector_iterator && _zstar_vector_iterator->valid()) { - auto posting_list = PredicatePostingList::UP( - new PredicateZstarCompressedPostingList<VectorIterator>(interval_store, *_zstar_vector_iterator)); + auto posting_list = std::make_unique<PredicateZstarCompressedPostingList<VectorIterator>>(interval_store, *_zstar_vector_iterator); posting_lists.emplace_back(std::move(posting_list)); } else if (_zstar_btree_iterator && _zstar_btree_iterator->valid()) { - auto posting_list = PredicatePostingList::UP( - new PredicateZstarCompressedPostingList<BTreeIterator>(interval_store, *_zstar_btree_iterator)); + auto posting_list = std::make_unique<PredicateZstarCompressedPostingList<BTreeIterator>>(interval_store, *_zstar_btree_iterator); posting_lists.emplace_back(std::move(posting_list)); } auto iterator = _index.getZeroConstraintDocs().begin(); if (iterator.valid()) { - auto posting_list = PredicatePostingList::UP(new PredicateZeroConstraintPostingList(iterator)); + auto posting_list = std::make_unique<PredicateZeroConstraintPostingList>(iterator); posting_lists.emplace_back(std::move(posting_list)); } return posting_lists; diff --git a/vespalib/src/vespa/vespalib/datastore/datastorebase.h b/vespalib/src/vespa/vespalib/datastore/datastorebase.h index 13c242e85a7..5a50dba6a3c 100644 --- a/vespalib/src/vespa/vespalib/datastore/datastorebase.h +++ b/vespalib/src/vespa/vespalib/datastore/datastorebase.h @@ -27,11 +27,11 @@ public: { public: EntryRef _ref; - size_t _len; // Aligned length + size_t _len; // Aligned length ElemHold1ListElem(EntryRef ref, size_t len) - : _ref(ref), - _len(len) + : _ref(ref), + _len(len) { } }; |