diff options
66 files changed, 458 insertions, 517 deletions
diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java b/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java index c60389fc55e..b8beeb5c1d1 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java @@ -231,7 +231,10 @@ public class MetricsPacketsHandler extends AbstractRequestHandler { GaugeMetric gauge = (GaugeMetric) value; metrics.put(name + ".average", sanitizeDouble(gauge.getAverage())) .put(name + ".last", sanitizeDouble(gauge.getLast())) - .put(name + ".max", sanitizeDouble(gauge.getMax())); + .put(name + ".max", sanitizeDouble(gauge.getMax())) + .put(name + ".min", sanitizeDouble(gauge.getMin())) + .put(name + ".sum", sanitizeDouble(gauge.getSum())) + .put(name + ".count", gauge.getCount()); if (gauge.getPercentiles().isPresent()) { for (Tuple2<String, Double> prefixAndValue : gauge.getPercentiles().get()) { metrics.put(name + "." + prefixAndValue.first + "percentile", prefixAndValue.second.doubleValue()); @@ -255,6 +258,9 @@ public class MetricsPacketsHandler extends AbstractRequestHandler { metrics.put(name + ".average", sanitizeDouble(gauge.getAverage())); metrics.put(name + ".last", sanitizeDouble(gauge.getLast())); metrics.put(name + ".max", sanitizeDouble(gauge.getMax())); + metrics.put(name + ".min", sanitizeDouble(gauge.getMin())); + metrics.put(name + ".sum", sanitizeDouble(gauge.getSum())); + metrics.put(name + ".count", gauge.getCount()); if (gauge.getPercentiles().isPresent()) { for (Tuple2<String, Double> prefixAndValue : gauge.getPercentiles().get()) { metrics.put(name + "." + prefixAndValue.first + "percentile", prefixAndValue.second.doubleValue()); diff --git a/container-core/src/test/java/com/yahoo/container/jdisc/state/MetricsPacketsHandlerTest.java b/container-core/src/test/java/com/yahoo/container/jdisc/state/MetricsPacketsHandlerTest.java index 38c1072c759..1aa4ee93ab6 100644 --- a/container-core/src/test/java/com/yahoo/container/jdisc/state/MetricsPacketsHandlerTest.java +++ b/container-core/src/test/java/com/yahoo/container/jdisc/state/MetricsPacketsHandlerTest.java @@ -189,10 +189,16 @@ public class MetricsPacketsHandlerTest extends StateHandlerTestBase { "gauge.metric.average" : 0.2, "gauge.metric.last" : 0.2, "gauge.metric.max" : 0.2, + "gauge.metric.min" : 0.2, + "gauge.metric.sum" : 0.2, + "gauge.metric.count" : 1, "configserver.requests.count" : 120, "lockAttempt.lockedLoad.average" : 500.0, "lockAttempt.lockedLoad.last" : 500.0, "lockAttempt.lockedLoad.max" : 500.0, + "lockAttempt.lockedLoad.min" : 500.0, + "lockAttempt.lockedLoad.sum" : 500.0, + "lockAttempt.lockedLoad.count" : 1, "counter.metric.count" : 5 } } @@ -210,9 +216,9 @@ public class MetricsPacketsHandlerTest extends StateHandlerTestBase { "host" : "some-hostname" }, "metrics" : { - "lockAttempt.lockedLoad.max" : 500.0, "configserver.requests.count" : 120, - "lockAttempt.lockedLoad.average" : 500.0 + "lockAttempt.lockedLoad.average" : 500.0, + "lockAttempt.lockedLoad.max" : 500.0 } } """; diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java index 9426952f57e..577769baf1e 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java @@ -182,7 +182,9 @@ public class SystemFlagsDataArchive { if (rawData.isBlank()) { flagData = new FlagData(directoryDeducedFlagId); } else { - Set<ZoneId> zones = force ? zoneRegistry.zones().all().zones().stream().map(ZoneApi::getVirtualId).collect(Collectors.toSet()) + Set<ZoneId> zones = force ? Stream.concat(Stream.of(ZoneId.ofVirtualControllerZone()), + zoneRegistry.zones().all().zones().stream().map(ZoneApi::getVirtualId)) + .collect(Collectors.toSet()) : Set.of(); String normalizedRawData = normalizeJson(rawData, zones); flagData = FlagData.deserialize(normalizedRawData); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java index 43c8e7c9469..c526b335c90 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java @@ -23,7 +23,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RevisionId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; import com.yahoo.vespa.hosted.controller.application.Change; -import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.deployment.ConvergenceSummary; import com.yahoo.vespa.hosted.controller.deployment.DeploymentStatus; @@ -53,7 +52,6 @@ import java.util.Locale; import java.util.Map; import java.util.NavigableMap; import java.util.Optional; -import java.util.SortedMap; import java.util.stream.Stream; import static com.yahoo.config.application.api.DeploymentSpec.UpgradePolicy.canary; @@ -109,10 +107,10 @@ class JobControllerApiHandlerHelper { int limit = limitStr.map(Integer::parseInt).orElse(Integer.MAX_VALUE); toSlime(cursor.setArray("runs"), runs.values(), application, limit, baseUriForJobType); - controller.applications().decideCloudAccountOf(new DeploymentId(id.application(), - runs.lastEntry().getValue().id().job().type().zone()), // Urgh, must use a job with actual zone. - application.deploymentSpec()) - .ifPresent(cloudAccount -> cursor.setObject("enclave").setString("cloudAccount", cloudAccount.value())); + Optional.ofNullable(runs.lastEntry()) + .map(entry -> new DeploymentId(id.application(), entry.getValue().id().job().type().zone())) // Urgh, must use a job with actual zone. + .flatMap(deployment -> controller.applications().decideCloudAccountOf(deployment, application.deploymentSpec())) + .ifPresent(cloudAccount -> cursor.setObject("enclave").setString("cloudAccount", cloudAccount.value())); return new SlimeJsonResponse(slime); } diff --git a/metrics/src/main/java/ai/vespa/metrics/ControllerMetrics.java b/metrics/src/main/java/ai/vespa/metrics/ControllerMetrics.java index 4770fe51830..0f200308862 100644 --- a/metrics/src/main/java/ai/vespa/metrics/ControllerMetrics.java +++ b/metrics/src/main/java/ai/vespa/metrics/ControllerMetrics.java @@ -37,6 +37,7 @@ public enum ControllerMetrics implements VespaMetrics { DNS_QUEUED_REQUESTS("dns.queuedRequests", Unit.REQUEST, "Queued DNS requests"), ZMS_QUOTA_USAGE("zms.quota.usage", Unit.FRACTION, "ZMS Quota usage per resource type"), COREDUMP_PROCESSED("coredump.processed", Unit.FAILURE,"Controller: Core dumps processed"), + AUTH0_EXCEPTIONS("auth0.exceptions", Unit.FAILURE, "Controller: Auth0 exceptions"), // Metrics per API, metrics names generated in ControllerMaintainer/MetricsReporter OPERATION_APPLICATION("operation.application", Unit.REQUEST, "Controller: Requests for /application API"), diff --git a/metrics/src/main/java/ai/vespa/metrics/set/InfrastructureMetricSet.java b/metrics/src/main/java/ai/vespa/metrics/set/InfrastructureMetricSet.java index 1281fd40b5a..38533f40950 100644 --- a/metrics/src/main/java/ai/vespa/metrics/set/InfrastructureMetricSet.java +++ b/metrics/src/main/java/ai/vespa/metrics/set/InfrastructureMetricSet.java @@ -173,6 +173,7 @@ public class InfrastructureMetricSet { addMetric(metrics, ControllerMetrics.DNS_QUEUED_REQUESTS, EnumSet.of(max, last)); // TODO: Vespa 9: Remove last addMetric(metrics, ControllerMetrics.ZMS_QUOTA_USAGE, EnumSet.of(max, last)); // TODO: Vespa 9: Remove last addMetric(metrics, ControllerMetrics.COREDUMP_PROCESSED.count()); + addMetric(metrics, ControllerMetrics.AUTH0_EXCEPTIONS.count()); addMetric(metrics, ControllerMetrics.METERING_AGE_SECONDS, EnumSet.of(min, last)); // TODO: Vespa 9: Remove last addMetric(metrics, ControllerMetrics.METERING_LAST_REPORTED, EnumSet.of(max, last)); // TODO: Vespa 9: Remove last diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperations.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperations.java index 264035b86a1..fa933e9622a 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperations.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperations.java @@ -36,7 +36,7 @@ public class ContainerOperations { public ContainerOperations(ContainerEngine containerEngine, Cgroup cgroup, FileSystem fileSystem, Timer timer) { this.containerEngine = Objects.requireNonNull(containerEngine); - this.imageDownloader = new ContainerImageDownloader(containerEngine); + this.imageDownloader = new ContainerImageDownloader(containerEngine, timer); this.imagePruner = new ContainerImagePruner(containerEngine, timer); this.containerStatsCollector = new ContainerStatsCollector(containerEngine, cgroup, fileSystem); } @@ -62,8 +62,8 @@ public class ContainerOperations { } /** Pull image asynchronously. Returns true if image is still downloading and false if download is complete */ - public boolean pullImageAsyncIfNeeded(TaskContext context, DockerImage dockerImage, RegistryCredentials registryCredentials) { - return !imageDownloader.get(context, dockerImage, registryCredentials); + public boolean pullImageAsyncIfNeeded(TaskContext context, DockerImage dockerImage, RegistryCredentialsProvider credentialsProvider) { + return !imageDownloader.get(context, dockerImage, credentialsProvider); } /** Executes a command inside container identified by given context. Does NOT throw on non-zero exit code */ diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImageDownloader.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImageDownloader.java index 1e37e080528..d3327bf5148 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImageDownloader.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImageDownloader.java @@ -3,10 +3,13 @@ package com.yahoo.vespa.hosted.node.admin.container.image; import com.yahoo.concurrent.DaemonThreadFactory; import com.yahoo.config.provision.DockerImage; +import com.yahoo.jdisc.Timer; import com.yahoo.vespa.hosted.node.admin.component.TaskContext; import com.yahoo.vespa.hosted.node.admin.container.ContainerEngine; -import com.yahoo.vespa.hosted.node.admin.container.RegistryCredentials; +import com.yahoo.vespa.hosted.node.admin.container.RegistryCredentialsProvider; +import java.time.Duration; +import java.time.Instant; import java.util.Collections; import java.util.HashSet; import java.util.Objects; @@ -26,13 +29,15 @@ public class ContainerImageDownloader { private static final Logger LOG = Logger.getLogger(ContainerImageDownloader.class.getName()); private final ContainerEngine containerEngine; + private final Timer timer; private final ExecutorService executorService = Executors.newSingleThreadExecutor( new DaemonThreadFactory("container-image-downloader")); // Download one image at a time private final Set<DockerImage> pendingDownloads = Collections.synchronizedSet(new HashSet<>()); - public ContainerImageDownloader(ContainerEngine containerEngine) { + public ContainerImageDownloader(ContainerEngine containerEngine, Timer timer) { this.containerEngine = Objects.requireNonNull(containerEngine); + this.timer = Objects.requireNonNull(timer); } /** @@ -40,12 +45,14 @@ public class ContainerImageDownloader { * * @return true if the image download has completed. */ - public boolean get(TaskContext context, DockerImage image, RegistryCredentials registryCredentials) { + public boolean get(TaskContext context, DockerImage image, RegistryCredentialsProvider credentialsProvider) { if (pendingDownloads.contains(image)) return false; if (containerEngine.hasImage(context, image)) return true; executorService.submit(() -> { try { - containerEngine.pullImage(context, image, registryCredentials); + Instant start = timer.currentTime(); + containerEngine.pullImage(context, image, credentialsProvider.get()); + LOG.log(Level.INFO, "Downloaded container image " + image + " in " + Duration.between(start, timer.currentTime())); } catch (RuntimeException e) { LOG.log(Level.SEVERE, "Failed to download container image " + image, e); } finally { 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 4c17bfbe039..7fc248024c3 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 @@ -21,7 +21,6 @@ import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.OrchestratorE import com.yahoo.vespa.hosted.node.admin.container.Container; import com.yahoo.vespa.hosted.node.admin.container.ContainerOperations; import com.yahoo.vespa.hosted.node.admin.container.ContainerResources; -import com.yahoo.vespa.hosted.node.admin.container.RegistryCredentials; import com.yahoo.vespa.hosted.node.admin.container.RegistryCredentialsProvider; import com.yahoo.vespa.hosted.node.admin.maintenance.ContainerWireguardTask; import com.yahoo.vespa.hosted.node.admin.maintenance.StorageMaintainer; @@ -431,9 +430,8 @@ public class NodeAgentImpl implements NodeAgent { NodeSpec node = context.node(); if (node.wantedDockerImage().equals(container.map(c -> c.image()))) return false; - RegistryCredentials credentials = registryCredentialsProvider.get(); return node.wantedDockerImage() - .map(image -> containerOperations.pullImageAsyncIfNeeded(context, image, credentials)) + .map(image -> containerOperations.pullImageAsyncIfNeeded(context, image, registryCredentialsProvider)) .orElse(false); } @@ -487,17 +485,14 @@ public class NodeAgentImpl implements NodeAgent { } switch (node.state()) { - case ready: - case reserved: - case failed: - case inactive: - case parked: + case ready, reserved, failed, inactive, parked -> { storageMaintainer.syncLogs(context, true); + if (node.state() == NodeState.reserved) downloadImageIfNeeded(context, container); removeContainerIfNeededUpdateContainerState(context, container); updateNodeRepoWithCurrentAttributes(context, Optional.empty()); stopServicesIfNeeded(context); - break; - case active: + } + case active -> { storageMaintainer.syncLogs(context, true); storageMaintainer.cleanDiskIfFull(context); storageMaintainer.handleCoreDumpsForContainer(context, container, false); @@ -550,11 +545,8 @@ public class NodeAgentImpl implements NodeAgent { orchestrator.resume(context.hostname().value()); suspendedInOrchestrator = false; } - break; - case provisioned: - nodeRepository.setNodeState(context.hostname().value(), NodeState.ready); - break; - case dirty: + } + case dirty -> { removeContainerIfNeededUpdateContainerState(context, container); context.log(logger, "State is " + node.state() + ", will delete application storage and mark node as ready"); credentialsMaintainers.forEach(maintainer -> maintainer.clearCredentials(context)); @@ -562,9 +554,8 @@ public class NodeAgentImpl implements NodeAgent { storageMaintainer.archiveNodeStorage(context); updateNodeRepoWithCurrentAttributes(context, Optional.empty()); nodeRepository.setNodeState(context.hostname().value(), NodeState.ready); - break; - default: - throw ConvergenceException.ofError("UNKNOWN STATE " + node.state().name()); + } + default -> throw ConvergenceException.ofError("Unexpected state " + node.state().name()); } } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImageDownloaderTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImageDownloaderTest.java index 9fd14e7e665..7f002eee315 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImageDownloaderTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImageDownloaderTest.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.node.admin.container.image; import com.yahoo.config.provision.DockerImage; +import com.yahoo.jdisc.test.TestTimer; import com.yahoo.vespa.hosted.node.admin.component.TaskContext; import com.yahoo.vespa.hosted.node.admin.component.TestTaskContext; import com.yahoo.vespa.hosted.node.admin.container.ContainerEngineMock; @@ -21,15 +22,15 @@ public class ContainerImageDownloaderTest { @Timeout(5_000) void test_download() { ContainerEngineMock podman = new ContainerEngineMock().asyncImageDownload(true); - ContainerImageDownloader downloader = new ContainerImageDownloader(podman); + ContainerImageDownloader downloader = new ContainerImageDownloader(podman, new TestTimer()); TaskContext context = new TestTaskContext(); DockerImage image = DockerImage.fromString("registry.example.com/repo/vespa:7.42"); - assertFalse(downloader.get(context, image, RegistryCredentials.none), "Download started"); - assertFalse(downloader.get(context, image, RegistryCredentials.none), "Download pending"); + assertFalse(downloader.get(context, image, () -> RegistryCredentials.none), "Download started"); + assertFalse(downloader.get(context, image, () -> RegistryCredentials.none), "Download pending"); podman.completeDownloadOf(image); boolean downloadCompleted; - while (!(downloadCompleted = downloader.get(context, image, RegistryCredentials.none))) ; + while (!(downloadCompleted = downloader.get(context, image, () -> RegistryCredentials.none))) ; assertTrue(downloadCompleted, "Download completed"); } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java index 0913e1d040a..ef4d6d849f6 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java @@ -487,20 +487,6 @@ public class NodeAgentImplTest { } @Test - void provisionedNodeIsMarkedAsReady() { - final NodeSpec node = nodeBuilder(NodeState.provisioned) - .wantedDockerImage(dockerImage) - .build(); - - NodeAgentContext context = createContext(node); - NodeAgentImpl nodeAgent = makeNodeAgent(null, false); - when(nodeRepository.getOptionalNode(hostName)).thenReturn(Optional.of(node)); - - nodeAgent.doConverge(context); - verify(nodeRepository, times(1)).setNodeState(eq(hostName), eq(NodeState.ready)); - } - - @Test void testRestartDeadContainerAfterNodeAdminRestart() { final NodeSpec node = nodeBuilder(NodeState.active) .currentDockerImage(dockerImage).wantedDockerImage(dockerImage) diff --git a/persistence/src/vespa/persistence/spi/clusterstate.cpp b/persistence/src/vespa/persistence/spi/clusterstate.cpp index ad5039fade1..e6708192d47 100644 --- a/persistence/src/vespa/persistence/spi/clusterstate.cpp +++ b/persistence/src/vespa/persistence/spi/clusterstate.cpp @@ -97,7 +97,7 @@ void ClusterState::serialize(vespalib::nbostream& o) const { assert(_distribution); assert(_state); vespalib::asciistream tmp; - _state->serialize(tmp, false); + _state->serialize(tmp); o << tmp.str() << _nodeIndex; o << _distribution->serialize(); } diff --git a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp index aedfde2521c..4a4a021d6d5 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp @@ -25,6 +25,7 @@ LOG_SETUP(".proton.matching.matcher"); using search::fef::Properties; using namespace search::fef::indexproperties::matching; +using namespace search::fef::indexproperties; using namespace search::engine; using namespace search::grouping; using search::DocumentMetaData; @@ -122,7 +123,7 @@ Matcher::Matcher(const search::index::Schema &schema, Properties props, const ve _rankSetup(), _viewResolver(ViewResolver::createFromSchema(schema)), _statsLock(), - _stats(), + _stats(softtimeout::Factor::lookup(_indexEnv.getProperties())), _startTime(my_clock::now()), _clock(clock), _queryLimiter(queryLimiter), @@ -149,9 +150,6 @@ Matcher::getStats() return stats; } -using search::fef::indexproperties::softtimeout::Enabled; -using search::fef::indexproperties::softtimeout::Factor; - std::unique_ptr<MatchToolsFactory> Matcher::create_match_tools_factory(const search::engine::Request &request, ISearchContext &searchContext, IAttributeContext &attrContext, const search::IDocumentMetaStore &metaStore, @@ -160,11 +158,11 @@ Matcher::create_match_tools_factory(const search::engine::Request &request, ISea bool is_search) const { const Properties & rankProperties = request.propertiesMap.rankProperties(); - bool softTimeoutEnabled = Enabled::lookup(rankProperties, _rankSetup->getSoftTimeoutEnabled()); - bool hasFactorOverride = Factor::isPresent(rankProperties); + bool softTimeoutEnabled = softtimeout::Enabled::lookup(rankProperties, _rankSetup->getSoftTimeoutEnabled()); + bool hasFactorOverride = softtimeout::Factor::isPresent(rankProperties); double factor = softTimeoutEnabled ? ( hasFactorOverride - ? Factor::lookup(rankProperties, _stats.softDoomFactor()) + ? softtimeout::Factor::lookup(rankProperties, _stats.softDoomFactor()) : _stats.softDoomFactor()) : 0.95; vespalib::duration safeLeft = std::chrono::duration_cast<vespalib::duration>(request.getTimeLeft() * factor); diff --git a/searchcore/src/vespa/searchcore/proton/matching/matching_stats.cpp b/searchcore/src/vespa/searchcore/proton/matching/matching_stats.cpp index 86fb3cf8107..47c0fbc8c55 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/matching_stats.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/matching_stats.cpp @@ -19,7 +19,7 @@ constexpr double MAX_CHANGE_FACTOR = 5; } // namespace proton::matching::<unnamed> -MatchingStats::MatchingStats(double prev_soft_doom_factor) +MatchingStats::MatchingStats(double prev_soft_doom_factor) noexcept : _queries(0), _limited_queries(0), _docidSpaceCovered(0), @@ -57,7 +57,7 @@ MatchingStats::merge_partition(const Partition &partition, size_t id) } MatchingStats & -MatchingStats::add(const MatchingStats &rhs) +MatchingStats::add(const MatchingStats &rhs) noexcept { _queries += rhs._queries; _limited_queries += rhs._limited_queries; diff --git a/searchcore/src/vespa/searchcore/proton/matching/matching_stats.h b/searchcore/src/vespa/searchcore/proton/matching/matching_stats.h index 4139bfbaf66..a9f7d3258d9 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/matching_stats.h +++ b/searchcore/src/vespa/searchcore/proton/matching/matching_stats.h @@ -24,21 +24,21 @@ private: double _min; double _max; public: - Avg() : _value(0.0), _count(0), _min(0.0), _max(0.0) {} - Avg & set(double value) { + Avg() noexcept : _value(0.0), _count(0), _min(0.0), _max(0.0) {} + Avg & set(double value) noexcept { _value = value; _count = 1; _min = value; _max = value; return *this; } - double avg() const { + double avg() const noexcept { return (_count > 0) ? (_value / _count) : 0; } - size_t count() const { return _count; } - double min() const { return _min; } - double max() const { return _max; } - void add(const Avg &other) { + size_t count() const noexcept { return _count; } + double min() const noexcept { return _min; } + double max() const noexcept { return _max; } + void add(const Avg &other) noexcept { if (_count == 0) { _min = other._min; _max = other._max; @@ -78,31 +78,31 @@ public: _active_time(), _wait_time() { } - Partition &docsCovered(size_t value) { _docsCovered = value; return *this; } - size_t docsCovered() const { return _docsCovered; } - Partition &docsMatched(size_t value) { _docsMatched = value; return *this; } - size_t docsMatched() const { return _docsMatched; } - Partition &docsRanked(size_t value) { _docsRanked = value; return *this; } - size_t docsRanked() const { return _docsRanked; } - Partition &docsReRanked(size_t value) { _docsReRanked = value; return *this; } - size_t docsReRanked() const { return _docsReRanked; } - Partition &softDoomed(bool v) { _softDoomed += v ? 1 : 0; return *this; } - size_t softDoomed() const { return _softDoomed; } - Partition & doomOvertime(vespalib::duration overtime) { _doomOvertime.set(vespalib::to_s(overtime)); return *this; } - vespalib::duration doomOvertime() const { return vespalib::from_s(_doomOvertime.max()); } - - Partition &active_time(double time_s) { _active_time.set(time_s); return *this; } - double active_time_avg() const { return _active_time.avg(); } - size_t active_time_count() const { return _active_time.count(); } - double active_time_min() const { return _active_time.min(); } - double active_time_max() const { return _active_time.max(); } - Partition &wait_time(double time_s) { _wait_time.set(time_s); return *this; } - double wait_time_avg() const { return _wait_time.avg(); } - size_t wait_time_count() const { return _wait_time.count(); } - double wait_time_min() const { return _wait_time.min(); } - double wait_time_max() const { return _wait_time.max(); } - - Partition &add(const Partition &rhs) { + Partition &docsCovered(size_t value) noexcept { _docsCovered = value; return *this; } + size_t docsCovered() const noexcept { return _docsCovered; } + Partition &docsMatched(size_t value) noexcept { _docsMatched = value; return *this; } + size_t docsMatched() const noexcept { return _docsMatched; } + Partition &docsRanked(size_t value) noexcept { _docsRanked = value; return *this; } + size_t docsRanked() const noexcept { return _docsRanked; } + Partition &docsReRanked(size_t value) noexcept { _docsReRanked = value; return *this; } + size_t docsReRanked() const noexcept { return _docsReRanked; } + Partition &softDoomed(bool v) noexcept { _softDoomed += v ? 1 : 0; return *this; } + size_t softDoomed() const noexcept { return _softDoomed; } + Partition & doomOvertime(vespalib::duration overtime) noexcept { _doomOvertime.set(vespalib::to_s(overtime)); return *this; } + vespalib::duration doomOvertime() const noexcept { return vespalib::from_s(_doomOvertime.max()); } + + Partition &active_time(double time_s) noexcept { _active_time.set(time_s); return *this; } + double active_time_avg() const noexcept { return _active_time.avg(); } + size_t active_time_count() const noexcept { return _active_time.count(); } + double active_time_min() const noexcept { return _active_time.min(); } + double active_time_max() const noexcept { return _active_time.max(); } + Partition &wait_time(double time_s) noexcept { _wait_time.set(time_s); return *this; } + double wait_time_avg() const noexcept { return _wait_time.avg(); } + size_t wait_time_count() const noexcept { return _wait_time.count(); } + double wait_time_min() const noexcept { return _wait_time.min(); } + double wait_time_max() const noexcept { return _wait_time.max(); } + + Partition &add(const Partition &rhs) noexcept { _docsCovered += rhs.docsCovered(); _docsMatched += rhs._docsMatched; _docsRanked += rhs._docsRanked; @@ -138,9 +138,10 @@ public: static constexpr double INITIAL_SOFT_DOOM_FACTOR = 0.5; MatchingStats(const MatchingStats &) = delete; MatchingStats & operator = (const MatchingStats &) = delete; - MatchingStats(MatchingStats &&) = default; - MatchingStats & operator = (MatchingStats &&) = default; - MatchingStats(double prev_soft_doom_factor = INITIAL_SOFT_DOOM_FACTOR); + MatchingStats(MatchingStats &&) noexcept = default; + MatchingStats & operator = (MatchingStats &&) noexcept = default; + MatchingStats() noexcept : MatchingStats(INITIAL_SOFT_DOOM_FACTOR) {} + MatchingStats(double prev_soft_doom_factor) noexcept; ~MatchingStats(); MatchingStats &queries(size_t value) { _queries = value; return *this; } @@ -206,7 +207,7 @@ public: const Partition &getPartition(size_t index) const { return _partitions[index]; } // used to aggregate accross searches (and configurations) - MatchingStats &add(const MatchingStats &rhs); + MatchingStats &add(const MatchingStats &rhs) noexcept; }; } diff --git a/searchlib/src/vespa/searchlib/fef/ranksetup.cpp b/searchlib/src/vespa/searchlib/fef/ranksetup.cpp index 4c676d5ba5c..823e39199df 100644 --- a/searchlib/src/vespa/searchlib/fef/ranksetup.cpp +++ b/searchlib/src/vespa/searchlib/fef/ranksetup.cpp @@ -66,7 +66,6 @@ RankSetup::RankSetup(const BlueprintFactory &factory, const IIndexEnvironment &i _diversityCutoffStrategy("loose"), _softTimeoutEnabled(false), _softTimeoutTailCost(0.1), - _softTimeoutFactor(0.5), _global_filter_lower_limit(0.0), _global_filter_upper_limit(1.0), _mutateOnMatch(), @@ -120,7 +119,6 @@ RankSetup::configure() setRankScoreDropLimit(hitcollector::RankScoreDropLimit::lookup(_indexEnv.getProperties())); setSoftTimeoutEnabled(softtimeout::Enabled::lookup(_indexEnv.getProperties())); setSoftTimeoutTailCost(softtimeout::TailCost::lookup(_indexEnv.getProperties())); - setSoftTimeoutFactor(softtimeout::Factor::lookup(_indexEnv.getProperties())); set_global_filter_lower_limit(matching::GlobalFilterLowerLimit::lookup(_indexEnv.getProperties())); set_global_filter_upper_limit(matching::GlobalFilterUpperLimit::lookup(_indexEnv.getProperties())); _mutateOnMatch._attribute = mutate::on_match::Attribute::lookup(_indexEnv.getProperties()); diff --git a/searchlib/src/vespa/searchlib/fef/ranksetup.h b/searchlib/src/vespa/searchlib/fef/ranksetup.h index 783c1506ff0..832b86d042a 100644 --- a/searchlib/src/vespa/searchlib/fef/ranksetup.h +++ b/searchlib/src/vespa/searchlib/fef/ranksetup.h @@ -74,7 +74,6 @@ private: vespalib::string _diversityCutoffStrategy; bool _softTimeoutEnabled; double _softTimeoutTailCost; - double _softTimeoutFactor; double _global_filter_lower_limit; double _global_filter_upper_limit; MutateOperation _mutateOnMatch; @@ -211,11 +210,6 @@ public: **/ uint32_t getArraySize() const { return _arraySize; } - /** whether match phase should do graceful degradation */ - bool hasMatchPhaseDegradation() const { - return (_degradationAttribute.size() > 0); - } - /** get name of attribute to use for graceful degradation in match phase */ vespalib::string getDegradationAttribute() const { return _degradationAttribute; @@ -390,20 +384,10 @@ public: **/ void setIgnoreDefaultRankFeatures(bool flag) { _ignoreDefaultRankFeatures = flag; } - /** - * Get the flag indicating whether we should ignore the default - * rank features (the ones specified by the plugins themselves) - * - * @return true means ignore default rank features - **/ - bool getIgnoreDefaultRankFeatures() { return _ignoreDefaultRankFeatures; } - void setSoftTimeoutEnabled(bool v) { _softTimeoutEnabled = v; } bool getSoftTimeoutEnabled() const { return _softTimeoutEnabled; } void setSoftTimeoutTailCost(double v) { _softTimeoutTailCost = v; } double getSoftTimeoutTailCost() const { return _softTimeoutTailCost; } - void setSoftTimeoutFactor(double v) { _softTimeoutFactor = v; } - double getSoftTimeoutFactor() const { return _softTimeoutFactor; } void set_global_filter_lower_limit(double v) { _global_filter_lower_limit = v; } double get_global_filter_lower_limit() const { return _global_filter_lower_limit; } diff --git a/storage/src/tests/distributor/bucketdbmetricupdatertest.cpp b/storage/src/tests/distributor/bucketdbmetricupdatertest.cpp index c4536c6fa2c..4d04e3ca51a 100644 --- a/storage/src/tests/distributor/bucketdbmetricupdatertest.cpp +++ b/storage/src/tests/distributor/bucketdbmetricupdatertest.cpp @@ -7,7 +7,6 @@ #include <vespa/vespalib/util/memoryusage.h> #include <vespa/vespalib/gtest/gtest.h> #include <string> -#include <sstream> namespace storage::distributor { @@ -16,19 +15,16 @@ using namespace ::testing; struct BucketDBMetricUpdaterTest : Test { void visitBucketWith2Copies1Trusted(BucketDBMetricUpdater& metricUpdater); - void visitBucketWith2CopiesBothTrusted( - BucketDBMetricUpdater& metricUpdater); + void visitBucketWith2CopiesBothTrusted(BucketDBMetricUpdater& metricUpdater); void visitBucketWith1Copy(BucketDBMetricUpdater& metricUpdater); - using NodeToReplicasMap = std::unordered_map<uint16_t, uint32_t>; + using NodeToReplicasMap = MinReplicaMap; NodeToReplicasMap replicaStatsOf(BucketDBMetricUpdater& metricUpdater); BucketDBMetricUpdaterTest(); }; -BucketDBMetricUpdaterTest::BucketDBMetricUpdaterTest() -{ -} +BucketDBMetricUpdaterTest::BucketDBMetricUpdaterTest() = default; namespace { @@ -38,8 +34,6 @@ void addNode(BucketInfo& info, uint16_t node, uint32_t crc) { info.addNode(BucketCopy(1234, node, apiInfo), order); } -using Trusted = bool; - BucketInfo makeInfo(uint32_t copy0Crc) { @@ -271,8 +265,7 @@ TEST_F(BucketDBMetricUpdaterTest, complete_round_clears_working_state) { // Replicas on nodes 0 and 1. void -BucketDBMetricUpdaterTest::visitBucketWith2Copies1Trusted( - BucketDBMetricUpdater& metricUpdater) +BucketDBMetricUpdaterTest::visitBucketWith2Copies1Trusted(BucketDBMetricUpdater& metricUpdater) { BucketInfo info; addNode(info, 0, 100); @@ -283,8 +276,7 @@ BucketDBMetricUpdaterTest::visitBucketWith2Copies1Trusted( // Replicas on nodes 0 and 2. void -BucketDBMetricUpdaterTest::visitBucketWith2CopiesBothTrusted( - BucketDBMetricUpdater& metricUpdater) +BucketDBMetricUpdaterTest::visitBucketWith2CopiesBothTrusted(BucketDBMetricUpdater& metricUpdater) { BucketInfo info; addNode(info, 0, 200); diff --git a/storage/src/tests/distributor/distributor_host_info_reporter_test.cpp b/storage/src/tests/distributor/distributor_host_info_reporter_test.cpp index 6dfab5abc21..a72dfec2d94 100644 --- a/storage/src/tests/distributor/distributor_host_info_reporter_test.cpp +++ b/storage/src/tests/distributor/distributor_host_info_reporter_test.cpp @@ -14,7 +14,7 @@ namespace storage::distributor { using End = vespalib::JsonStream::End; using File = vespalib::File; -using MinReplicaStats = std::unordered_map<uint16_t, uint32_t>; +using MinReplicaStats = MinReplicaMap; using Object = vespalib::JsonStream::Object; using PerNodeBucketSpacesStats = BucketSpacesStatsProvider::PerNodeBucketSpacesStats; using BucketSpacesStats = BucketSpacesStatsProvider::BucketSpacesStats; @@ -36,7 +36,7 @@ struct MockedMinReplicaProvider : MinReplicaProvider MinReplicaStats minReplica; ~MockedMinReplicaProvider() override; - std::unordered_map<uint16_t, uint32_t> getMinReplica() const override { + MinReplicaMap getMinReplica() const override { return minReplica; } }; diff --git a/storage/src/tests/distributor/mock_tickable_stripe.h b/storage/src/tests/distributor/mock_tickable_stripe.h index ec2f978c029..77a6f537d28 100644 --- a/storage/src/tests/distributor/mock_tickable_stripe.h +++ b/storage/src/tests/distributor/mock_tickable_stripe.h @@ -23,7 +23,7 @@ struct MockTickableStripe : TickableStripe { const lib::Distribution&, const lib::ClusterState&, const char*, - const std::unordered_set<uint16_t>&, + const OutdatedNodes &, const std::vector<dbtransition::Entry>&) override { abort(); diff --git a/storage/src/tests/distributor/multi_thread_stripe_access_guard_test.cpp b/storage/src/tests/distributor/multi_thread_stripe_access_guard_test.cpp index 6bc98ef022e..db89b30efb2 100644 --- a/storage/src/tests/distributor/multi_thread_stripe_access_guard_test.cpp +++ b/storage/src/tests/distributor/multi_thread_stripe_access_guard_test.cpp @@ -25,7 +25,7 @@ struct AggregationTestingMockTickableStripe : MockTickableStripe { } void merge_entries_into_db(document::BucketSpace, api::Timestamp, const lib::Distribution&, - const lib::ClusterState&, const char*, const std::unordered_set<uint16_t>&, + const lib::ClusterState&, const char*, const OutdatedNodes &, const std::vector<dbtransition::Entry>& entries_in) override { entries = entries_in; } diff --git a/storage/src/tests/distributor/simplemaintenancescannertest.cpp b/storage/src/tests/distributor/simplemaintenancescannertest.cpp index 723b0baa6cd..b5dc72d995b 100644 --- a/storage/src/tests/distributor/simplemaintenancescannertest.cpp +++ b/storage/src/tests/distributor/simplemaintenancescannertest.cpp @@ -165,7 +165,7 @@ TEST_F(SimpleMaintenanceScannerTest, pending_maintenance_operation_statistics) { "split bucket: 0, join bucket: 0, " "set bucket state: 0, garbage collection: 0"); { - auto stats(_scanner->getPendingMaintenanceStats()); + const auto & stats = _scanner->getPendingMaintenanceStats(); EXPECT_EQ(expectedEmpty, stringifyGlobalPendingStats(stats)); } @@ -173,7 +173,7 @@ TEST_F(SimpleMaintenanceScannerTest, pending_maintenance_operation_statistics) { // All mock operations generated have the merge type. { - auto stats(_scanner->getPendingMaintenanceStats()); + const auto & stats = _scanner->getPendingMaintenanceStats(); std::string expected("delete bucket: 0, merge bucket: 2, " "split bucket: 0, join bucket: 0, " "set bucket state: 0, garbage collection: 0"); @@ -182,7 +182,7 @@ TEST_F(SimpleMaintenanceScannerTest, pending_maintenance_operation_statistics) { _scanner->reset(); { - auto stats(_scanner->getPendingMaintenanceStats()); + const auto & stats = _scanner->getPendingMaintenanceStats(); EXPECT_EQ(expectedEmpty, stringifyGlobalPendingStats(stats)); } } @@ -191,14 +191,14 @@ TEST_F(SimpleMaintenanceScannerTest, per_node_maintenance_stats_are_tracked) { addBucketToDb(1); addBucketToDb(3); { - auto stats(_scanner->getPendingMaintenanceStats()); + const auto & stats = _scanner->getPendingMaintenanceStats(); NodeMaintenanceStats emptyStats; EXPECT_EQ(emptyStats, stats.perNodeStats.forNode(0, makeBucketSpace())); } ASSERT_TRUE(scanEntireDatabase(2)); // Mock is currently hardwired to increment movingOut for node 1 and // copyingIn for node 2 per bucket iterated (we've got 2). - auto stats(_scanner->getPendingMaintenanceStats()); + const auto & stats = _scanner->getPendingMaintenanceStats(); { NodeMaintenanceStats wantedNode1Stats; wantedNode1Stats.movingOut = 2; diff --git a/storage/src/tests/distributor/top_level_distributor_test.cpp b/storage/src/tests/distributor/top_level_distributor_test.cpp index dad6f477d83..94f8821f9c8 100644 --- a/storage/src/tests/distributor/top_level_distributor_test.cpp +++ b/storage/src/tests/distributor/top_level_distributor_test.cpp @@ -92,7 +92,7 @@ struct TopLevelDistributorTest : Test, TopLevelDistributorTestUtil { return _distributor->getBucketSpacesStats(); } - std::unordered_map<uint16_t, uint32_t> distributor_min_replica_stats() { + MinReplicaMap distributor_min_replica_stats() { return _distributor->getMinReplica(); } @@ -504,7 +504,7 @@ void assert_invalid_bucket_stats_for_all_spaces( ASSERT_FALSE(space_iter->second.valid()); } -void assert_min_replica_stats_zeroed(const std::unordered_map<uint16_t, uint32_t>& stats, uint16_t node_index) { +void assert_min_replica_stats_zeroed(const MinReplicaMap & stats, uint16_t node_index) { auto iter = stats.find(node_index); ASSERT_TRUE(iter != stats.cend()); EXPECT_EQ(iter->second, 0); diff --git a/storage/src/vespa/storage/bucketdb/bucketinfo.h b/storage/src/vespa/storage/bucketdb/bucketinfo.h index 1870d4c91d4..57ebf505a50 100644 --- a/storage/src/vespa/storage/bucketdb/bucketinfo.h +++ b/storage/src/vespa/storage/bucketdb/bucketinfo.h @@ -102,7 +102,7 @@ public: array. This operation has undefined behaviour if the index given is not within the node count. */ - const BucketCopy& getNodeRef(uint16_t idx) const { + const BucketCopy& getNodeRef(uint16_t idx) const noexcept { return _nodes[idx]; } diff --git a/storage/src/vespa/storage/distributor/activecopy.cpp b/storage/src/vespa/storage/distributor/activecopy.cpp index a4ee4a51135..c46e9868cc8 100644 --- a/storage/src/vespa/storage/distributor/activecopy.cpp +++ b/storage/src/vespa/storage/distributor/activecopy.cpp @@ -24,9 +24,9 @@ namespace std { namespace storage::distributor { -ActiveCopy::ActiveCopy(uint16_t node, const BucketDatabase::Entry& e, const std::vector<uint16_t>& idealState) : - _nodeIndex(node), - _ideal(0xffff) +ActiveCopy::ActiveCopy(uint16_t node, const BucketDatabase::Entry& e, const std::vector<uint16_t>& idealState) + : _nodeIndex(node), + _ideal(0xffff) { const BucketCopy* copy = e->getNode(node); assert(copy != nullptr); diff --git a/storage/src/vespa/storage/distributor/bucketdb/bucketdbmetricupdater.cpp b/storage/src/vespa/storage/distributor/bucketdb/bucketdbmetricupdater.cpp index fc6c957b737..dfcbbf63946 100644 --- a/storage/src/vespa/storage/distributor/bucketdb/bucketdbmetricupdater.cpp +++ b/storage/src/vespa/storage/distributor/bucketdb/bucketdbmetricupdater.cpp @@ -6,19 +6,26 @@ namespace storage::distributor { -BucketDBMetricUpdater::Stats::Stats() +BucketDBMetricUpdater::Stats::Stats() noexcept : _docCount(0), _byteCount(0), _tooFewCopies(0), _tooManyCopies(0), _noTrusted(0), - _totalBuckets(0) + _totalBuckets(0), + _mutable_db_mem_usage(), + _read_only_db_mem_usage(), + _minBucketReplica() { } BucketDBMetricUpdater::Stats::Stats(const Stats &rhs) = default; +BucketDBMetricUpdater::Stats & BucketDBMetricUpdater::Stats::operator=(const Stats &rhs) = default; +BucketDBMetricUpdater::Stats::Stats(Stats &&rhs) noexcept = default; +BucketDBMetricUpdater::Stats & BucketDBMetricUpdater::Stats::operator=(Stats &&rhs) noexcept = default; +BucketDBMetricUpdater::Stats::~Stats() = default; -BucketDBMetricUpdater::BucketDBMetricUpdater() +BucketDBMetricUpdater::BucketDBMetricUpdater() noexcept : _workingStats(), _lastCompleteStats(), _replicaCountingMode(ReplicaCountingMode::TRUSTED), @@ -35,8 +42,7 @@ BucketDBMetricUpdater::resetStats() } void -BucketDBMetricUpdater::visit(const BucketDatabase::Entry& entry, - uint32_t redundancy) +BucketDBMetricUpdater::visit(const BucketDatabase::Entry& entry, uint32_t redundancy) { if (entry->getNodeCount() == 0) { // We used to have an assert on >0 but that caused some crashes, see @@ -90,9 +96,7 @@ BucketDBMetricUpdater::visit(const BucketDatabase::Entry& entry, } void -BucketDBMetricUpdater::updateMinReplicationStats( - const BucketDatabase::Entry& entry, - uint32_t trustedCopies) +BucketDBMetricUpdater::updateMinReplicationStats(const BucketDatabase::Entry& entry, uint32_t trustedCopies) { auto& minBucketReplica = _workingStats._minBucketReplica; for (uint32_t i = 0; i < entry->getNodeCount(); i++) { @@ -103,9 +107,9 @@ BucketDBMetricUpdater::updateMinReplicationStats( // sync across each other. // Regardless of counting mode we still have to take the minimum // replica count across all buckets present on any given node. - const uint32_t countedReplicas( - (_replicaCountingMode == ReplicaCountingMode::TRUSTED) - ? trustedCopies : entry->getNodeCount()); + const uint32_t countedReplicas = (_replicaCountingMode == ReplicaCountingMode::TRUSTED) + ? trustedCopies + : entry->getNodeCount(); auto it = minBucketReplica.find(node); if (it == minBucketReplica.end()) { minBucketReplica[node] = countedReplicas; @@ -118,17 +122,18 @@ BucketDBMetricUpdater::updateMinReplicationStats( void BucketDBMetricUpdater::completeRound(bool resetWorkingStats) { - _lastCompleteStats = _workingStats; + _hasCompleteStats = true; if (resetWorkingStats) { + _lastCompleteStats = std::move(_workingStats); resetStats(); + } else { + _lastCompleteStats = _workingStats; } } void -BucketDBMetricUpdater::Stats::propagateMetrics( - IdealStateMetricSet& idealStateMetrics, - DistributorMetricSet& distributorMetrics) +BucketDBMetricUpdater::Stats::propagateMetrics(IdealStateMetricSet& idealStateMetrics, DistributorMetricSet& distributorMetrics) const { distributorMetrics.docsStored.set(_docCount); distributorMetrics.bytesStored.set(_byteCount); diff --git a/storage/src/vespa/storage/distributor/bucketdb/bucketdbmetricupdater.h b/storage/src/vespa/storage/distributor/bucketdb/bucketdbmetricupdater.h index 2edb86cbaa2..366c2f2dc41 100644 --- a/storage/src/vespa/storage/distributor/bucketdb/bucketdbmetricupdater.h +++ b/storage/src/vespa/storage/distributor/bucketdb/bucketdbmetricupdater.h @@ -2,10 +2,11 @@ #pragma once +#include <vespa/storage/distributor/min_replica_provider.h> #include <vespa/storage/bucketdb/bucketdatabase.h> #include <vespa/storage/config/config-stor-distributormanager.h> #include <vespa/vespalib/util/memoryusage.h> -#include <unordered_map> +#include <vespa/vespalib/stllike/hash_map.h> namespace storage::distributor { @@ -25,11 +26,12 @@ public: vespalib::MemoryUsage _mutable_db_mem_usage; vespalib::MemoryUsage _read_only_db_mem_usage; - Stats(); + Stats() noexcept; + Stats(Stats &&rhs) noexcept; + Stats & operator=(Stats &&rhs) noexcept; Stats(const Stats &rhs); - ~Stats() = default; - - Stats &operator=(const Stats &rhs) = default; + Stats & operator=(const Stats &rhs); + ~Stats(); /** * For each node N, look at all the buckets that have or should have a @@ -47,24 +49,24 @@ public: * Note: If no buckets have been found for a node, that node is not in * this map. */ - std::unordered_map<uint16_t, uint32_t> _minBucketReplica; + MinReplicaMap _minBucketReplica; /** * Propagate state values to the appropriate metric values. */ - void propagateMetrics(IdealStateMetricSet&, DistributorMetricSet&); + void propagateMetrics(IdealStateMetricSet&, DistributorMetricSet&) const; }; using ReplicaCountingMode = vespa::config::content::core::StorDistributormanagerConfig::MinimumReplicaCountingMode; private: - Stats _workingStats; - Stats _lastCompleteStats; + Stats _workingStats; + Stats _lastCompleteStats; ReplicaCountingMode _replicaCountingMode; - bool _hasCompleteStats; + bool _hasCompleteStats; public: - BucketDBMetricUpdater(); + BucketDBMetricUpdater() noexcept; ~BucketDBMetricUpdater(); void setMinimumReplicaCountingMode(ReplicaCountingMode mode) noexcept { @@ -91,11 +93,11 @@ public: /** * Returns true iff completeRound() has been called at least once. */ - bool hasCompletedRound() const { + bool hasCompletedRound() const noexcept { return _hasCompleteStats; } - Stats getLastCompleteStats() const { + const Stats & getLastCompleteStats() const noexcept { return _lastCompleteStats; } diff --git a/storage/src/vespa/storage/distributor/distributor_host_info_reporter.cpp b/storage/src/vespa/storage/distributor/distributor_host_info_reporter.cpp index bb7e573c980..46c9a526a8d 100644 --- a/storage/src/vespa/storage/distributor/distributor_host_info_reporter.cpp +++ b/storage/src/vespa/storage/distributor/distributor_host_info_reporter.cpp @@ -3,15 +3,9 @@ #include "bucket_spaces_stats_provider.h" #include "distributor_host_info_reporter.h" #include "min_replica_provider.h" -#include "pendingmessagetracker.h" - #include <set> -using std::set; -using std::unordered_map; - -namespace storage { -namespace distributor { +namespace storage::distributor { using BucketSpacesStats = BucketSpacesStatsProvider::BucketSpacesStats; using PerNodeBucketSpacesStats = BucketSpacesStatsProvider::PerNodeBucketSpacesStats; @@ -48,10 +42,10 @@ writeBucketSpacesStats(vespalib::JsonStream& stream, void outputStorageNodes(vespalib::JsonStream& output, - const unordered_map<uint16_t, uint32_t>& minReplica, + const MinReplicaMap & minReplica, const PerNodeBucketSpacesStats& bucketSpacesStats) { - set<uint16_t> nodes; + std::set<uint16_t> nodes; for (const auto& element : minReplica) { nodes.insert(element.first); } @@ -104,6 +98,5 @@ DistributorHostInfoReporter::report(vespalib::JsonStream& output) output << End(); } -} // distributor -} // storage +} diff --git a/storage/src/vespa/storage/distributor/distributor_stripe.cpp b/storage/src/vespa/storage/distributor/distributor_stripe.cpp index 616fd77fdd7..ede85c036b3 100644 --- a/storage/src/vespa/storage/distributor/distributor_stripe.cpp +++ b/storage/src/vespa/storage/distributor/distributor_stripe.cpp @@ -120,8 +120,7 @@ DistributorStripe::sendReply(const std::shared_ptr<api::StorageReply>& reply) } void DistributorStripe::send_shutdown_abort_reply(const std::shared_ptr<api::StorageMessage>& msg) { - api::StorageReply::UP reply( - std::dynamic_pointer_cast<api::StorageCommand>(msg)->makeReply()); + auto reply = std::dynamic_pointer_cast<api::StorageCommand>(msg)->makeReply(); reply->setResult(api::ReturnCode(api::ReturnCode::ABORTED, "Distributor is shutting down")); send_up_with_tracking(std::shared_ptr<api::StorageMessage>(reply.release())); } @@ -179,8 +178,7 @@ DistributorStripe::handle_or_enqueue_message(const std::shared_ptr<api::StorageM } void -DistributorStripe::handleCompletedMerge( - const std::shared_ptr<api::MergeBucketReply>& reply) +DistributorStripe::handleCompletedMerge(const std::shared_ptr<api::MergeBucketReply>& reply) { _maintenanceOperationOwner.handleReply(reply); } @@ -236,9 +234,7 @@ DistributorStripe::handleReply(const std::shared_ptr<api::StorageReply>& reply) } bool -DistributorStripe::generateOperation( - const std::shared_ptr<api::StorageMessage>& msg, - Operation::SP& operation) +DistributorStripe::generateOperation(const std::shared_ptr<api::StorageMessage>& msg, Operation::SP& operation) { return _externalOperationHandler.handleMessage(msg, operation); } @@ -277,7 +273,6 @@ DistributorStripe::getClusterStateBundle() const void DistributorStripe::enableClusterStateBundle(const lib::ClusterStateBundle& state) { - lib::Node my_node(lib::NodeType::DISTRIBUTOR, getDistributorIndex()); lib::ClusterStateBundle oldState = _clusterStateBundle; _clusterStateBundle = state; propagateClusterStates(); @@ -415,7 +410,6 @@ public: bool check(uint32_t msgType, uint16_t node, uint8_t pri) override { (void) node; - (void) pri; if (msgType == api::MessageType::SPLITBUCKET_ID && pri <= maxPri) { found = true; return false; @@ -428,9 +422,7 @@ public: } void -DistributorStripe::checkBucketForSplit(document::BucketSpace bucketSpace, - const BucketDatabase::Entry& e, - uint8_t priority) +DistributorStripe::checkBucketForSplit(document::BucketSpace bucketSpace, const BucketDatabase::Entry& e, uint8_t priority) { if (!getConfig().doInlineSplit()) { return; @@ -440,16 +432,13 @@ DistributorStripe::checkBucketForSplit(document::BucketSpace bucketSpace, // appropriate priority. SplitChecker checker(priority); for (uint32_t i = 0; i < e->getNodeCount(); ++i) { - _pendingMessageTracker.checkPendingMessages(e->getNodeRef(i).getNode(), - document::Bucket(bucketSpace, e.getBucketId()), - checker); + _pendingMessageTracker.checkPendingMessages(e->getNodeRef(i).getNode(), document::Bucket(bucketSpace, e.getBucketId()), checker); if (checker.found) { return; } } - Operation::SP operation = - _idealStateManager.generateInterceptingSplit(bucketSpace, e, priority); + Operation::SP operation = _idealStateManager.generateInterceptingSplit(bucketSpace, e, priority); if (operation.get()) { _maintenanceOperationOwner.start(operation, priority); @@ -458,8 +447,7 @@ DistributorStripe::checkBucketForSplit(document::BucketSpace bucketSpace, // TODO STRIPE must be invoked by top-level bucket db updater probably void -DistributorStripe::propagateDefaultDistribution( - std::shared_ptr<const lib::Distribution> distribution) +DistributorStripe::propagateDefaultDistribution(std::shared_ptr<const lib::Distribution> distribution) { auto global_distr = GlobalBucketSpaceDistributionConverter::convert_to_global(*distribution); for (auto* repo : {_bucketSpaceRepo.get(), _readOnlyBucketSpaceRepo.get()}) { @@ -562,7 +550,7 @@ void DistributorStripe::startExternalOperations() { _fetchedMessages.clear(); } -std::unordered_map<uint16_t, uint32_t> +MinReplicaMap DistributorStripe::getMinReplica() const { std::lock_guard guard(_metricLock); @@ -699,9 +687,7 @@ DistributorStripe::scanNextBucket() _scanner->reset(); } else { const auto &distribution(_bucketSpaceRepo->get(scanResult.getBucketSpace()).getDistribution()); - _bucketDBMetricUpdater.visit( - scanResult.getEntry(), - distribution.getRedundancy()); + _bucketDBMetricUpdater.visit(scanResult.getEntry(), distribution.getRedundancy()); } return scanResult; } @@ -823,12 +809,6 @@ DistributorStripe::getActiveIdealStateOperations() const return _maintenanceOperationOwner.toString(); } -std::string -DistributorStripe::getActiveOperations() const -{ - return _operationOwner.toString(); -} - StripeAccessGuard::PendingOperationStats DistributorStripe::pending_operation_stats() const { @@ -881,7 +861,7 @@ DistributorStripe::merge_entries_into_db(document::BucketSpace bucket_space, const lib::Distribution& distribution, const lib::ClusterState& new_state, const char* storage_up_states, - const std::unordered_set<uint16_t>& outdated_nodes, + const OutdatedNodes& outdated_nodes, const std::vector<dbtransition::Entry>& entries) { bucket_db_updater().merge_entries_into_db(bucket_space, gathered_at_timestamp, distribution, diff --git a/storage/src/vespa/storage/distributor/distributor_stripe.h b/storage/src/vespa/storage/distributor/distributor_stripe.h index 801efa0ff73..338a6c72125 100644 --- a/storage/src/vespa/storage/distributor/distributor_stripe.h +++ b/storage/src/vespa/storage/distributor/distributor_stripe.h @@ -122,7 +122,6 @@ public: StripeAccessGuard::PendingOperationStats pending_operation_stats() const override; std::string getActiveIdealStateOperations() const; - std::string getActiveOperations() const; framework::ThreadWaitInfo doNonCriticalTick(framework::ThreadIndex); @@ -219,7 +218,7 @@ private: /** * Return a copy of the latest min replica data, see MinReplicaProvider. */ - std::unordered_map<uint16_t, uint32_t> getMinReplica() const override; + MinReplicaMap getMinReplica() const override; PerNodeBucketSpacesStats getBucketSpacesStats() const override; @@ -286,7 +285,7 @@ private: const lib::Distribution& distribution, const lib::ClusterState& new_state, const char* storage_up_states, - const std::unordered_set<uint16_t>& outdated_nodes, + const OutdatedNodes & outdated_nodes, const std::vector<dbtransition::Entry>& entries) override; void update_read_snapshot_before_db_pruning() override; void update_read_snapshot_after_db_pruning(const lib::ClusterStateBundle& new_state) override; diff --git a/storage/src/vespa/storage/distributor/idealstatemetricsset.cpp b/storage/src/vespa/storage/distributor/idealstatemetricsset.cpp index d79bf2c4810..d50b2004bf2 100644 --- a/storage/src/vespa/storage/distributor/idealstatemetricsset.cpp +++ b/storage/src/vespa/storage/distributor/idealstatemetricsset.cpp @@ -134,7 +134,7 @@ IdealStateMetricSet::IdealStateMetricSet() IdealStateMetricSet::~IdealStateMetricSet() = default; -void IdealStateMetricSet::setPendingOperations(const std::vector<uint64_t>& newMetrics) { +void IdealStateMetricSet::setPendingOperations(vespalib::ConstArrayRef<uint64_t> newMetrics) { for (uint32_t i = 0; i < IdealStateOperation::OPERATION_COUNT; i++) { operations[i]->pending.set(newMetrics[i]); } diff --git a/storage/src/vespa/storage/distributor/idealstatemetricsset.h b/storage/src/vespa/storage/distributor/idealstatemetricsset.h index 6528ad7dc72..0bbc13d061a 100644 --- a/storage/src/vespa/storage/distributor/idealstatemetricsset.h +++ b/storage/src/vespa/storage/distributor/idealstatemetricsset.h @@ -5,6 +5,7 @@ #include <vespa/metrics/valuemetric.h> #include <vespa/metrics/countmetric.h> #include <vespa/storage/distributor/operations/idealstate/idealstateoperation.h> +#include <vespa/vespalib/util/arrayref.h> namespace storage::distributor { @@ -61,7 +62,7 @@ public: IdealStateMetricSet(); ~IdealStateMetricSet() override; - void setPendingOperations(const std::vector<uint64_t>& newMetrics); + void setPendingOperations(vespalib::ConstArrayRef<uint64_t> newMetrics); }; } // storage::distributor diff --git a/storage/src/vespa/storage/distributor/maintenance/maintenancescanner.h b/storage/src/vespa/storage/distributor/maintenance/maintenancescanner.h index 4fec2e57cbc..e4ccb6d88ad 100644 --- a/storage/src/vespa/storage/distributor/maintenance/maintenancescanner.h +++ b/storage/src/vespa/storage/distributor/maintenance/maintenancescanner.h @@ -32,8 +32,6 @@ public: }; virtual ScanResult scanNext() = 0; - - virtual void reset() = 0; }; } diff --git a/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.cpp b/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.cpp index 47b8aec4aa6..7ac99f5712f 100644 --- a/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.cpp +++ b/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.cpp @@ -7,16 +7,6 @@ namespace storage::distributor { const NodeMaintenanceStats NodeMaintenanceStatsTracker::_emptyNodeMaintenanceStats; -void -NodeMaintenanceStats::merge(const NodeMaintenanceStats& rhs) -{ - movingOut += rhs.movingOut; - syncing += rhs.syncing; - copyingIn += rhs.copyingIn; - copyingOut += rhs.copyingOut; - total += rhs.total; -} - namespace { void diff --git a/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.h b/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.h index deeef118685..3818dd4bacb 100644 --- a/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.h +++ b/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.h @@ -37,7 +37,13 @@ struct NodeMaintenanceStats return !(*this == other); } - void merge(const NodeMaintenanceStats& rhs); + void merge(const NodeMaintenanceStats& rhs) noexcept { + movingOut += rhs.movingOut; + syncing += rhs.syncing; + copyingIn += rhs.copyingIn; + copyingOut += rhs.copyingOut; + total += rhs.total; + } }; std::ostream& operator<<(std::ostream&, const NodeMaintenanceStats&); diff --git a/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.cpp b/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.cpp index cb60e3eb0fc..afcbef32584 100644 --- a/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.cpp +++ b/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.cpp @@ -13,22 +13,22 @@ SimpleMaintenanceScanner::SimpleMaintenanceScanner(BucketPriorityDatabase& bucke _priorityGenerator(priorityGenerator), _bucketSpaceRepo(bucketSpaceRepo), _bucketSpaceItr(_bucketSpaceRepo.begin()), - _bucketCursor() + _bucketCursor(), + _pendingMaintenance() { } SimpleMaintenanceScanner::~SimpleMaintenanceScanner() = default; bool -SimpleMaintenanceScanner::GlobalMaintenanceStats::operator==(const GlobalMaintenanceStats& rhs) const +SimpleMaintenanceScanner::GlobalMaintenanceStats::operator==(const GlobalMaintenanceStats& rhs) const noexcept { return pending == rhs.pending; } void -SimpleMaintenanceScanner::GlobalMaintenanceStats::merge(const GlobalMaintenanceStats& rhs) +SimpleMaintenanceScanner::GlobalMaintenanceStats::merge(const GlobalMaintenanceStats& rhs) noexcept { - assert(pending.size() == rhs.pending.size()); for (size_t i = 0; i < pending.size(); ++i) { pending[i] += rhs.pending[i]; } @@ -99,8 +99,7 @@ SimpleMaintenanceScanner::prioritizeBucket(const document::Bucket &bucket) } std::ostream& -operator<<(std::ostream& os, - const SimpleMaintenanceScanner::GlobalMaintenanceStats& stats) +operator<<(std::ostream& os, const SimpleMaintenanceScanner::GlobalMaintenanceStats& stats) { using MO = MaintenanceOperation; os << "delete bucket: " << stats.pending[MO::DELETE_BUCKET] diff --git a/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.h b/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.h index a867d4a5267..7af61815c31 100644 --- a/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.h +++ b/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.h @@ -13,14 +13,14 @@ class SimpleMaintenanceScanner : public MaintenanceScanner { public: struct GlobalMaintenanceStats { - std::vector<uint64_t> pending; + std::array<uint64_t, MaintenanceOperation::OPERATION_COUNT> pending; - GlobalMaintenanceStats() - : pending(MaintenanceOperation::OPERATION_COUNT) + GlobalMaintenanceStats() noexcept + : pending() { } - bool operator==(const GlobalMaintenanceStats& rhs) const; - void merge(const GlobalMaintenanceStats& rhs); + bool operator==(const GlobalMaintenanceStats& rhs) const noexcept; + void merge(const GlobalMaintenanceStats& rhs) noexcept; }; struct PendingMaintenanceStats { PendingMaintenanceStats(); @@ -47,10 +47,10 @@ public: const DistributorBucketSpaceRepo& bucketSpaceRepo); SimpleMaintenanceScanner(const SimpleMaintenanceScanner&) = delete; SimpleMaintenanceScanner& operator=(const SimpleMaintenanceScanner&) = delete; - ~SimpleMaintenanceScanner(); + ~SimpleMaintenanceScanner() override; ScanResult scanNext() override; - void reset() override; + void reset(); // TODO: move out into own interface! void prioritizeBucket(const document::Bucket &id); diff --git a/storage/src/vespa/storage/distributor/min_replica_provider.cpp b/storage/src/vespa/storage/distributor/min_replica_provider.cpp index f503672af39..52780b99948 100644 --- a/storage/src/vespa/storage/distributor/min_replica_provider.cpp +++ b/storage/src/vespa/storage/distributor/min_replica_provider.cpp @@ -5,8 +5,7 @@ namespace storage::distributor { void -merge_min_replica_stats(std::unordered_map<uint16_t, uint32_t>& dest, - const std::unordered_map<uint16_t, uint32_t>& src) +merge_min_replica_stats(MinReplicaMap & dest, const MinReplicaMap & src) { for (const auto& entry : src) { auto node_index = entry.first; diff --git a/storage/src/vespa/storage/distributor/min_replica_provider.h b/storage/src/vespa/storage/distributor/min_replica_provider.h index a4374b906fe..75d3a150d21 100644 --- a/storage/src/vespa/storage/distributor/min_replica_provider.h +++ b/storage/src/vespa/storage/distributor/min_replica_provider.h @@ -1,11 +1,12 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once -#include <stdint.h> -#include <unordered_map> +#include <vespa/vespalib/stllike/hash_map.h> namespace storage::distributor { +using MinReplicaMap = vespalib::hash_map<uint16_t, uint32_t>; + class MinReplicaProvider { public: @@ -17,11 +18,10 @@ public: * Can be called at any time after registration from another thread context * and the call must thus be thread safe and data race free. */ - virtual std::unordered_map<uint16_t, uint32_t> getMinReplica() const = 0; + virtual MinReplicaMap getMinReplica() const = 0; }; -void merge_min_replica_stats(std::unordered_map<uint16_t, uint32_t>& dest, - const std::unordered_map<uint16_t, uint32_t>& src); +void merge_min_replica_stats(MinReplicaMap & dest, const MinReplicaMap & src); } diff --git a/storage/src/vespa/storage/distributor/multi_threaded_stripe_access_guard.cpp b/storage/src/vespa/storage/distributor/multi_threaded_stripe_access_guard.cpp index b00e4ce3cba..01c2875671b 100644 --- a/storage/src/vespa/storage/distributor/multi_threaded_stripe_access_guard.cpp +++ b/storage/src/vespa/storage/distributor/multi_threaded_stripe_access_guard.cpp @@ -83,7 +83,7 @@ MultiThreadedStripeAccessGuard::merge_entries_into_db(document::BucketSpace buck const lib::Distribution& distribution, const lib::ClusterState& new_state, const char* storage_up_states, - const std::unordered_set<uint16_t>& outdated_nodes, + const OutdatedNodes & outdated_nodes, const std::vector<dbtransition::Entry>& entries) { if (entries.empty()) { diff --git a/storage/src/vespa/storage/distributor/multi_threaded_stripe_access_guard.h b/storage/src/vespa/storage/distributor/multi_threaded_stripe_access_guard.h index c52a01fdded..7a58a784eda 100644 --- a/storage/src/vespa/storage/distributor/multi_threaded_stripe_access_guard.h +++ b/storage/src/vespa/storage/distributor/multi_threaded_stripe_access_guard.h @@ -46,7 +46,7 @@ public: const lib::Distribution& distribution, const lib::ClusterState& new_state, const char* storage_up_states, - const std::unordered_set<uint16_t>& outdated_nodes, + const OutdatedNodes & outdated_nodes, const std::vector<dbtransition::Entry>& entries) override; void update_read_snapshot_before_db_pruning() override; diff --git a/storage/src/vespa/storage/distributor/outdated_nodes.h b/storage/src/vespa/storage/distributor/outdated_nodes.h index cef799ee4aa..d014a3074a4 100644 --- a/storage/src/vespa/storage/distributor/outdated_nodes.h +++ b/storage/src/vespa/storage/distributor/outdated_nodes.h @@ -2,10 +2,10 @@ #pragma once -#include <unordered_set> +#include <vespa/vespalib/stllike/hash_set.h> namespace storage::distributor::dbtransition { -using OutdatedNodes = std::unordered_set<uint16_t>; +using OutdatedNodes = vespalib::hash_set<uint16_t>; } diff --git a/storage/src/vespa/storage/distributor/pending_bucket_space_db_transition.cpp b/storage/src/vespa/storage/distributor/pending_bucket_space_db_transition.cpp index 62de3b50b51..19cc7bc522f 100644 --- a/storage/src/vespa/storage/distributor/pending_bucket_space_db_transition.cpp +++ b/storage/src/vespa/storage/distributor/pending_bucket_space_db_transition.cpp @@ -113,10 +113,7 @@ PendingBucketSpaceDbTransition::DbMerger::removeCopiesFromNodesThatWereRequested && (info.getTimestamp() < _creation_timestamp) && e->removeNode(entryNode, TrustedUpdate::DEFER)) { - LOG(spam, - "Removed bucket %s from node %d", - bucketId.toString().c_str(), - entryNode); + LOG(spam, "Removed bucket %s from node %d", bucketId.toString().c_str(), entryNode); updated = true; // After removing current node, getNodeRef(i) will point to the _next_ node, so don't increment `i`. } else { @@ -391,8 +388,7 @@ PendingBucketSpaceDbTransition::markAllAvailableNodesAsRequiringRequest() } void -PendingBucketSpaceDbTransition::addAdditionalNodesToOutdatedSet( - const std::unordered_set<uint16_t>& nodes) +PendingBucketSpaceDbTransition::addAdditionalNodesToOutdatedSet(const OutdatedNodes & nodes) { const uint16_t nodeCount(newStateStorageNodeCount()); for (uint16_t node : nodes) { diff --git a/storage/src/vespa/storage/distributor/pending_bucket_space_db_transition.h b/storage/src/vespa/storage/distributor/pending_bucket_space_db_transition.h index bce0c9bdc93..9fb6e4ed315 100644 --- a/storage/src/vespa/storage/distributor/pending_bucket_space_db_transition.h +++ b/storage/src/vespa/storage/distributor/pending_bucket_space_db_transition.h @@ -76,7 +76,7 @@ public: const lib::Distribution& _distribution; const lib::ClusterState& _new_state; const char* _storage_up_states; - const std::unordered_set<uint16_t>& _outdated_nodes; // TODO hash_set + const OutdatedNodes & _outdated_nodes; // TODO hash_set const std::vector<dbtransition::Entry>& _entries; uint32_t _iter; public: @@ -84,7 +84,7 @@ public: const lib::Distribution& distribution, const lib::ClusterState& new_state, const char* storage_up_states, - const std::unordered_set<uint16_t>& outdated_nodes, + const OutdatedNodes & outdated_nodes, const std::vector<dbtransition::Entry>& entries) : _creation_timestamp(creation_timestamp), _distribution(distribution), diff --git a/storage/src/vespa/storage/distributor/statechecker.cpp b/storage/src/vespa/storage/distributor/statechecker.cpp index eaff1f0b780..27a60b73716 100644 --- a/storage/src/vespa/storage/distributor/statechecker.cpp +++ b/storage/src/vespa/storage/distributor/statechecker.cpp @@ -4,6 +4,7 @@ #include "distributor_stripe_component.h" #include <vespa/vdslib/distribution/distribution.h> #include <vespa/vdslib/state/clusterstate.h> +#include <vespa/vespalib/stllike/hash_set_insert.hpp> #include <vespa/log/log.h> LOG_SETUP(".distributor.statechecker"); diff --git a/storage/src/vespa/storage/distributor/statechecker.h b/storage/src/vespa/storage/distributor/statechecker.h index 348d90bc712..830e05676be 100644 --- a/storage/src/vespa/storage/distributor/statechecker.h +++ b/storage/src/vespa/storage/distributor/statechecker.h @@ -6,7 +6,7 @@ #include <vespa/storage/distributor/operations/idealstate/idealstateoperation.h> #include <vespa/storage/common/storagecomponent.h> #include <vespa/storage/bucketdb/bucketdatabase.h> -#include <unordered_set> +#include <vespa/vespalib/stllike/hash_set.h> #include <map> #include <set> @@ -74,7 +74,7 @@ public: // well as have the ability to quickly check if a node is in an ideal // location. std::vector<uint16_t> idealState; - std::unordered_set<uint16_t> unorderedIdealState; + vespalib::hash_set<uint16_t> unorderedIdealState; const DistributorNodeContext& node_ctx; const DistributorStripeOperationContext& op_ctx; diff --git a/storage/src/vespa/storage/distributor/statecheckers.cpp b/storage/src/vespa/storage/distributor/statecheckers.cpp index f9c26bf113e..fe1f4422c45 100644 --- a/storage/src/vespa/storage/distributor/statecheckers.cpp +++ b/storage/src/vespa/storage/distributor/statecheckers.cpp @@ -856,9 +856,8 @@ SynchronizeAndMoveStateChecker::check(Context& c) const result += checkIfBucketsAreOutOfSyncAndNeedMerging(c); if (result.shouldMerge()) { - IdealStateOperation::UP op( - new MergeOperation(BucketAndNodes(c.getBucket(), result.nodes()), - c.distributorConfig.getMaxNodesPerMerge())); + auto op = std::make_unique<MergeOperation>(BucketAndNodes(c.getBucket(), result.nodes()), + c.distributorConfig.getMaxNodesPerMerge()); op->setDetailedReason(result.reason()); MaintenancePriority::Priority schedPri; if ((c.getBucketSpace() == document::FixedBucketSpaces::default_space()) diff --git a/storage/src/vespa/storage/distributor/stripe_access_guard.h b/storage/src/vespa/storage/distributor/stripe_access_guard.h index 2ed40cfcf2e..d2d3615b776 100644 --- a/storage/src/vespa/storage/distributor/stripe_access_guard.h +++ b/storage/src/vespa/storage/distributor/stripe_access_guard.h @@ -4,6 +4,7 @@ #include "bucket_space_distribution_configs.h" #include "pending_bucket_space_db_transition_entry.h" #include "potential_data_loss_report.h" +#include "outdated_nodes.h" #include <vespa/document/bucket/bucketspace.h> #include <vespa/storageapi/defs.h> #include <unordered_set> // TODO use hash_set instead @@ -30,6 +31,7 @@ class NodeSupportedFeaturesRepo; */ class StripeAccessGuard { public: + using OutdatedNodes = dbtransition::OutdatedNodes; virtual ~StripeAccessGuard() = default; virtual void flush_and_close() = 0; @@ -51,7 +53,7 @@ public: const lib::Distribution& distribution, const lib::ClusterState& new_state, const char* storage_up_states, - const std::unordered_set<uint16_t>& outdated_nodes, + const OutdatedNodes& outdated_nodes, const std::vector<dbtransition::Entry>& entries) = 0; virtual void update_read_snapshot_before_db_pruning() = 0; diff --git a/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.cpp b/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.cpp index 8fce8c3137a..09e7d370a98 100644 --- a/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.cpp +++ b/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.cpp @@ -257,7 +257,7 @@ StripeBucketDBUpdater::merge_entries_into_db(document::BucketSpace bucket_space, const lib::Distribution& distribution, const lib::ClusterState& new_state, const char* storage_up_states, - const std::unordered_set<uint16_t>& outdated_nodes, + const OutdatedNodes & outdated_nodes, const std::vector<dbtransition::Entry>& entries) { auto& s = _op_ctx.bucket_space_repo().get(bucket_space); diff --git a/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.h b/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.h index 04efe91e9e7..b8b729edbeb 100644 --- a/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.h +++ b/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.h @@ -33,6 +33,7 @@ class StripeBucketDBUpdater final public api::MessageHandler { public: + using OutdatedNodes = dbtransition::OutdatedNodes; StripeBucketDBUpdater(const DistributorNodeContext& node_ctx, DistributorStripeOperationContext& op_ctx, DistributorStripeInterface& owner, @@ -178,7 +179,7 @@ private: const lib::Distribution& distribution, const lib::ClusterState& new_state, const char* storage_up_states, - const std::unordered_set<uint16_t>& outdated_nodes, + const OutdatedNodes & outdated_nodes, const std::vector<dbtransition::Entry>& entries); void enqueueRecheckUntilPendingStateEnabled(uint16_t node, const document::Bucket&); diff --git a/storage/src/vespa/storage/distributor/tickable_stripe.h b/storage/src/vespa/storage/distributor/tickable_stripe.h index e458043ac64..499cb41ee34 100644 --- a/storage/src/vespa/storage/distributor/tickable_stripe.h +++ b/storage/src/vespa/storage/distributor/tickable_stripe.h @@ -24,6 +24,7 @@ class NodeSupportedFeaturesRepo; */ class TickableStripe { public: + using OutdatedNodes = dbtransition::OutdatedNodes; virtual ~TickableStripe() = default; // Perform a single operation tick of the stripe logic. @@ -53,7 +54,7 @@ public: const lib::Distribution& distribution, const lib::ClusterState& new_state, const char* storage_up_states, - const std::unordered_set<uint16_t>& outdated_nodes, + const OutdatedNodes & outdated_nodes, const std::vector<dbtransition::Entry>& entries) = 0; virtual void update_read_snapshot_before_db_pruning() = 0; diff --git a/storage/src/vespa/storage/distributor/top_level_distributor.cpp b/storage/src/vespa/storage/distributor/top_level_distributor.cpp index 80c096135fa..f957af5362e 100644 --- a/storage/src/vespa/storage/distributor/top_level_distributor.cpp +++ b/storage/src/vespa/storage/distributor/top_level_distributor.cpp @@ -340,10 +340,10 @@ TopLevelDistributor::propagate_default_distribution_thread_unsafe( } } -std::unordered_map<uint16_t, uint32_t> +MinReplicaMap TopLevelDistributor::getMinReplica() const { - std::unordered_map<uint16_t, uint32_t> result; + MinReplicaMap result; for (const auto& stripe : _stripes) { merge_min_replica_stats(result, stripe->getMinReplica()); } @@ -360,15 +360,6 @@ TopLevelDistributor::getBucketSpacesStats() const return result; } -SimpleMaintenanceScanner::PendingMaintenanceStats -TopLevelDistributor::pending_maintenance_stats() const { - SimpleMaintenanceScanner::PendingMaintenanceStats result; - for (const auto& stripe : _stripes) { - result.merge(stripe->pending_maintenance_stats()); - } - return result; -} - void TopLevelDistributor::propagateInternalScanMetricsToExternal() { diff --git a/storage/src/vespa/storage/distributor/top_level_distributor.h b/storage/src/vespa/storage/distributor/top_level_distributor.h index aa3a7b3655d..278a68f72c6 100644 --- a/storage/src/vespa/storage/distributor/top_level_distributor.h +++ b/storage/src/vespa/storage/distributor/top_level_distributor.h @@ -142,10 +142,9 @@ private: /** * Return a copy of the latest min replica data, see MinReplicaProvider. */ - std::unordered_map<uint16_t, uint32_t> getMinReplica() const override; + MinReplicaMap getMinReplica() const override; PerNodeBucketSpacesStats getBucketSpacesStats() const override; - SimpleMaintenanceScanner::PendingMaintenanceStats pending_maintenance_stats() const; /** * Atomically publish internal metrics to external ideal state metrics. diff --git a/storage/src/vespa/storage/storageserver/rpc/slime_cluster_state_bundle_codec.cpp b/storage/src/vespa/storage/storageserver/rpc/slime_cluster_state_bundle_codec.cpp index 8c994991b9b..ea049493348 100644 --- a/storage/src/vespa/storage/storageserver/rpc/slime_cluster_state_bundle_codec.cpp +++ b/storage/src/vespa/storage/storageserver/rpc/slime_cluster_state_bundle_codec.cpp @@ -44,7 +44,7 @@ OutputBuf::~OutputBuf() = default; vespalib::string serialize_state(const lib::ClusterState& state) { vespalib::asciistream as; - state.serialize(as, false); + state.serialize(as); return as.str(); } diff --git a/vdslib/src/tests/state/clusterstatetest.cpp b/vdslib/src/tests/state/clusterstatetest.cpp index a08ec007f55..0b278177453 100644 --- a/vdslib/src/tests/state/clusterstatetest.cpp +++ b/vdslib/src/tests/state/clusterstatetest.cpp @@ -13,10 +13,10 @@ using ::testing::ContainsRegex; namespace storage::lib { -#define VERIFY3(source, result, type, typestr) { \ +#define VERIFY3(source, result, typestr) { \ vespalib::asciistream ost; \ try { \ - state->serialize(ost, type); \ + state->serialize(ost); \ } catch (std::exception& e) { \ FAIL() << ("Failed to serialize system state " \ + state->toString(true) + " in " + std::string(typestr) \ @@ -26,24 +26,18 @@ namespace storage::lib { vespalib::string(typestr) + " \"" + ost.str() + "\"") << state->toString(true); \ } -#define VERIFY2(serialized, result, testOld, testNew) { \ +#define VERIFY2(serialized, result) { \ std::unique_ptr<ClusterState> state; \ try { \ state.reset(new ClusterState(serialized)); \ } catch (std::exception& e) { \ - FAIL() << ("Failed to parse '" + std::string(serialized) \ - + "': " + e.what()); \ + FAIL() << ("Failed to parse '" + std::string(serialized) + "': " + e.what()); \ } \ - if (testOld) VERIFY3(serialized, result, true, "Old") \ - if (testNew) VERIFY3(serialized, result, false, "New") \ + VERIFY3(serialized, result, "New") \ } -#define VERIFYSAMEOLD(serialized) VERIFY2(serialized, serialized, true, false) -#define VERIFYOLD(serialized, result) VERIFY2(serialized, result, true, false) -#define VERIFYSAMENEW(serialized) VERIFY2(serialized, serialized, false, true) -#define VERIFYNEW(serialized, result) VERIFY2(serialized, result, false, true) -#define VERIFYSAME(serialized) VERIFY2(serialized, serialized, true, true) -#define VERIFY(serialized, result) VERIFY2(serialized, result, true, true) +#define VERIFYSAMENEW(serialized) VERIFY2(serialized, serialized) +#define VERIFYNEW(serialized, result) VERIFY2(serialized, result) #define VERIFY_FAIL(serialized, error) { \ try{ \ diff --git a/vdslib/src/vespa/vdslib/distribution/distribution.cpp b/vdslib/src/vespa/vdslib/distribution/distribution.cpp index e9113d7dd23..637a5089822 100644 --- a/vdslib/src/vespa/vdslib/distribution/distribution.cpp +++ b/vdslib/src/vespa/vdslib/distribution/distribution.cpp @@ -494,8 +494,7 @@ Distribution::splitNodesIntoLeafGroups(IndexList nodeList) const for (auto node : nodeList) { const Group* group((node < _node2Group.size()) ? _node2Group[node] : nullptr); if (group == nullptr) { - LOGBP(warning, "Node %u is not assigned to a group. " - "Should not happen?", node); + LOGBP(warning, "Node %u is not assigned to a group. Should not happen?", node); } else { assert(group->isLeafGroup()); nodes[group->getIndex()].push_back(node); diff --git a/vdslib/src/vespa/vdslib/distribution/group.cpp b/vdslib/src/vespa/vdslib/distribution/group.cpp index 537b4635e75..254a20e1052 100644 --- a/vdslib/src/vespa/vdslib/distribution/group.cpp +++ b/vdslib/src/vespa/vdslib/distribution/group.cpp @@ -11,7 +11,7 @@ namespace storage::lib { -Group::Group(uint16_t index, vespalib::stringref name) +Group::Group(uint16_t index, vespalib::stringref name) noexcept : _name(name), _index(index), _distributionHash(0), @@ -46,7 +46,7 @@ Group::~Group() } bool -Group::operator==(const Group& other) const +Group::operator==(const Group& other) const noexcept { return (_name == other._name && _index == other._index && diff --git a/vdslib/src/vespa/vdslib/distribution/group.h b/vdslib/src/vespa/vdslib/distribution/group.h index 5767f55d20a..3f468bee995 100644 --- a/vdslib/src/vespa/vdslib/distribution/group.h +++ b/vdslib/src/vespa/vdslib/distribution/group.h @@ -49,28 +49,25 @@ private: public: // Create leaf node - Group(uint16_t index, vespalib::stringref name); + Group(uint16_t index, vespalib::stringref name) noexcept; // Create branch node Group(uint16_t index, vespalib::stringref name, const Distribution&, uint16_t redundancy); virtual ~Group(); - bool isLeafGroup() const { return _nodes.size() > 0; } - bool operator==(const Group& other) const; + bool isLeafGroup() const noexcept { return ! _nodes.empty(); } + bool operator==(const Group& other) const noexcept; void print(std::ostream& out, bool verbose, const std::string& indent) const override; - vespalib::Double getCapacity() const { return _capacity; } - const vespalib::string & getName() const { return _name; } - uint16_t getIndex() const { return _index; } + vespalib::Double getCapacity() const noexcept { return _capacity; } + const vespalib::string & getName() const noexcept { return _name; } + uint16_t getIndex() const noexcept { return _index; } std::map<uint16_t, Group*>& getSubGroups() { return _subGroups; } - const std::map<uint16_t, Group*>& getSubGroups() const - { return _subGroups; } - const std::vector<uint16_t>& getNodes() const { return _nodes; }; - const Distribution& getDistributionSpec() const - { return _distributionSpec; } - const Distribution& getDistribution(uint16_t redundancy) const - { return _preCalculated[redundancy]; } - uint32_t getDistributionHash() const { return _distributionHash; } + const std::map<uint16_t, Group*>& getSubGroups() const noexcept { return _subGroups; } + const std::vector<uint16_t>& getNodes() const noexcept { return _nodes; }; + const Distribution& getDistributionSpec() const noexcept { return _distributionSpec; } + const Distribution& getDistribution(uint16_t redundancy) const noexcept { return _preCalculated[redundancy]; } + uint32_t getDistributionHash() const noexcept { return _distributionHash; } void addSubGroup(Group::UP); void setCapacity(vespalib::Double capacity); diff --git a/vdslib/src/vespa/vdslib/state/clusterstate.cpp b/vdslib/src/vespa/vdslib/state/clusterstate.cpp index e9159eef631..f4314a6624b 100644 --- a/vdslib/src/vespa/vdslib/state/clusterstate.cpp +++ b/vdslib/src/vespa/vdslib/state/clusterstate.cpp @@ -7,7 +7,10 @@ #include <vespa/vdslib/distribution/distribution.h> #include <vespa/vespalib/util/exceptions.h> #include <vespa/vespalib/stllike/asciistream.h> +#include <vespa/vespalib/stllike/hash_map.hpp> +#include <vespa/vespalib/stllike/hash_map_equal.hpp> #include <sstream> +#include <cassert> #include <vespa/log/log.h> LOG_SETUP(".vdslib.state.cluster"); @@ -24,9 +27,9 @@ namespace storage::lib { ClusterState::ClusterState() : Printable(), _version(0), + _nodeCount(), _clusterState(&State::DOWN), _nodeStates(), - _nodeCount(2), _description(), _distributionBits(16) { } @@ -41,14 +44,10 @@ struct NodeData { NodeData() : empty(true), node(NodeType::STORAGE, 0), ost() {} - void addTo(std::map<Node, NodeState>& nodeStates, - std::vector<uint16_t>& nodeCount) - { + void addTo(ClusterState::NodeMap & nodeStates, ClusterState::NodeCounts & nodeCount) { if (!empty) { NodeState state(ost.str(), &node.getType()); - if (state != NodeState(node.getType(), State::UP) - || state.getDescription().size() > 0) - { + if ((state != NodeState(node.getType(), State::UP)) || (state.getDescription().size() > 0)) { nodeStates.insert(std::make_pair(node, state)); } if (nodeCount[node.getType()] <= node.getIndex()) { @@ -63,9 +62,9 @@ struct NodeData { ClusterState::ClusterState(const vespalib::string& serialized) : Printable(), _version(0), + _nodeCount(), _clusterState(&State::UP), _nodeStates(), - _nodeCount(2), _description(), _distributionBits(16) { @@ -74,13 +73,13 @@ ClusterState::ClusterState(const vespalib::string& serialized) NodeData nodeData; vespalib::string lastAbsolutePath; - for (vespalib::StringTokenizer::Iterator it = st.begin(); it != st.end(); ++it) { - vespalib::string::size_type index = it->find(':'); + for (const auto & token : st) { + vespalib::string::size_type index = token.find(':'); if (index == vespalib::string::npos) { - throw IllegalArgumentException("Token " + *it + " does not contain ':': " + serialized, VESPA_STRLOC); + throw IllegalArgumentException("Token " + token + " does not contain ':': " + serialized, VESPA_STRLOC); } - vespalib::string key = it->substr(0, index); - vespalib::stringref value = it->substr(index + 1); + vespalib::string key = token.substr(0, index); + vespalib::stringref value = token.substr(index + 1); if (key.size() > 0 && key[0] == '.') { if (lastAbsolutePath == "") { throw IllegalArgumentException("The first path in system state string needs to be absolute", VESPA_STRLOC); @@ -111,7 +110,9 @@ ClusterState::parse(vespalib::stringref key, vespalib::stringref value, NodeData break; case 'b': if (key == "bits") { - _distributionBits = atoi(value.data()); + uint32_t numBits = atoi(value.data()); + assert(numBits <= 64); + _distributionBits = numBits; return true; } break; @@ -138,7 +139,7 @@ ClusterState::parse(vespalib::stringref key, vespalib::stringref value, NodeData bool ClusterState::parseSorD(vespalib::stringref key, vespalib::stringref value, NodeData & nodeData) { - const NodeType* nodeType(0); + const NodeType* nodeType = nullptr; vespalib::string::size_type dot = key.find('.'); vespalib::stringref type(dot == vespalib::string::npos ? key : key.substr(0, dot)); @@ -147,10 +148,9 @@ ClusterState::parseSorD(vespalib::stringref key, vespalib::stringref value, Node } else if (type == "distributor") { nodeType = &NodeType::DISTRIBUTOR; } - if (nodeType == 0) return false; + if (nodeType == nullptr) return false; if (dot == vespalib::string::npos) { // Entry that set node counts - uint16_t nodeCount = 0; - nodeCount = atoi(value.data()); + uint16_t nodeCount = atoi(value.data()); if (nodeCount > _nodeCount[*nodeType] ) { _nodeCount[*nodeType] = nodeCount; @@ -158,12 +158,9 @@ ClusterState::parseSorD(vespalib::stringref key, vespalib::stringref value, Node return true; } vespalib::string::size_type dot2 = key.find('.', dot + 1); - Node node; - if (dot2 == vespalib::string::npos) { - node = Node(*nodeType, atoi(key.substr(dot + 1).data())); - } else { - node = Node(*nodeType, atoi(key.substr(dot + 1, dot2 - dot - 1).data())); - } + Node node(*nodeType, (dot2 == vespalib::string::npos) + ? atoi(key.substr(dot + 1).data()) + : atoi(key.substr(dot + 1, dot2 - dot - 1).data())); if (node.getIndex() >= _nodeCount[*nodeType]) { vespalib::asciistream ost; @@ -183,74 +180,73 @@ ClusterState::parseSorD(vespalib::stringref key, vespalib::stringref value, Node return true; } +struct SeparatorPrinter { + bool first; + SeparatorPrinter() : first(true) {} + const char * toString() { + if (first) { + first = false; + return ""; + } + return " "; + } +}; + namespace { - struct SeparatorPrinter { - bool first; - SeparatorPrinter() : first(true) {} - const char * toString() { - if (first) { - first = false; - return ""; + +void +serialize_node(vespalib::asciistream & out, const Node & node, const NodeState & state) { + vespalib::asciistream prefix; + prefix << "." << node.getIndex() << "."; + vespalib::asciistream ost; + state.serialize(ost, prefix.str(), false); + vespalib::stringref content = ost.str(); + if ( !content.empty()) { + out << " " << content; + } +} + +} + +void +ClusterState::serialize_nodes(vespalib::asciistream & out, SeparatorPrinter & sep, const NodeType & nodeType, + const std::vector<NodeStatePair> & nodeStates) const +{ + uint16_t nodeCount = getNodeCount(nodeType); + if (nodeCount > 0) { + out << sep.toString() << nodeType.serialize() << ":" << nodeCount; + for (const auto & entry : nodeStates) { + if (entry.first.getType() == nodeType) { + serialize_node(out, entry.first, entry.second); } - return " "; } - }; + } } void -ClusterState::serialize(vespalib::asciistream & out, bool ignoreNewFeatures) const +ClusterState::serialize(vespalib::asciistream & out) const { SeparatorPrinter sep; - if (!ignoreNewFeatures && _version != 0) { + if (_version != 0) { out << sep.toString() << "version:" << _version; } - if (!ignoreNewFeatures && *_clusterState != State::UP) { + if (*_clusterState != State::UP) { out << sep.toString() << "cluster:" << _clusterState->serialize(); } - if (!ignoreNewFeatures && _distributionBits != 16) { + if (_distributionBits != 16) { out << sep.toString() << "bits:" << _distributionBits; } - uint16_t distCount = getNodeCount(NodeType::DISTRIBUTOR); - if (ignoreNewFeatures || distCount > 0) { - out << sep.toString() << "distributor:" << distCount; - for (std::map<Node, NodeState>::const_iterator it = - _nodeStates.begin(); - it != _nodeStates.end(); ++it) - { - if (it->first.getType() != NodeType::DISTRIBUTOR) continue; - vespalib::asciistream prefix; - prefix << "." << it->first.getIndex() << "."; - vespalib::asciistream ost; - it->second.serialize(ost, prefix.str(), false); - vespalib::stringref content = ost.str(); - if (content.size() > 0) { - out << " " << content; - } - } - } - uint16_t storCount = getNodeCount(NodeType::STORAGE); - if (ignoreNewFeatures || storCount > 0) { - out << sep.toString() << "storage:" << storCount; - for (std::map<Node, NodeState>::const_iterator it = - _nodeStates.begin(); - it != _nodeStates.end(); ++it) - { - if (it->first.getType() != NodeType::STORAGE) continue; - vespalib::asciistream prefix; - prefix << "." << it->first.getIndex() << "."; - vespalib::asciistream ost; - it->second.serialize(ost, prefix.str(), false); - vespalib::stringref content = ost.str(); - if ( !content.empty()) { - out << " " << content; - } - } - } + if ((getNodeCount(NodeType::DISTRIBUTOR) + getNodeCount(NodeType::STORAGE)) == 0u) return; + + std::vector<NodeStatePair> nodeStates(_nodeStates.cbegin(), _nodeStates.cend()); + std::sort(nodeStates.begin(), nodeStates.end(), [](const NodeStatePair &a, const NodeStatePair &b) { return a.first < b.first; }); + serialize_nodes(out, sep, NodeType::DISTRIBUTOR, nodeStates); + serialize_nodes(out, sep, NodeType::STORAGE, nodeStates); } bool -ClusterState::operator==(const ClusterState& other) const +ClusterState::operator==(const ClusterState& other) const noexcept { return (_version == other._version && *_clusterState == *other._clusterState && @@ -260,17 +256,11 @@ ClusterState::operator==(const ClusterState& other) const } bool -ClusterState::operator!=(const ClusterState& other) const +ClusterState::operator!=(const ClusterState& other) const noexcept { return !(*this == other); } -uint16_t -ClusterState::getNodeCount(const NodeType& type) const -{ - return _nodeCount[type]; -} - namespace { [[noreturn]] void throwUnknownType(const Node & node) __attribute__((noinline)); void throwUnknownType(const Node & node) { @@ -282,7 +272,7 @@ const NodeState& ClusterState::getNodeState(const Node& node) const { // If it actually has an entry in map, return that - std::map<Node, NodeState>::const_iterator it = _nodeStates.find(node); + const auto it = _nodeStates.find(node); if (it != _nodeStates.end()) return it->second; // If beyond node count, the node is down. @@ -307,9 +297,7 @@ void ClusterState::setClusterState(const State& state) { if (!state.validClusterState()) { - throw vespalib::IllegalStateException( - state.toString(true) + " is not a legal cluster state", - VESPA_STRLOC); + throw vespalib::IllegalStateException(state.toString(true) + " is not a legal cluster state", VESPA_STRLOC); } _clusterState = &state; } @@ -319,17 +307,12 @@ ClusterState::setNodeState(const Node& node, const NodeState& state) { state.verifySupportForNodeType(node.getType()); if (node.getIndex() >= _nodeCount[node.getType()]) { - for (uint32_t i = _nodeCount[node.getType()]; i < node.getIndex(); ++i) - { - _nodeStates.insert(std::make_pair( - Node(node.getType(), i), - NodeState(node.getType(), State::DOWN))); + for (uint32_t i = _nodeCount[node.getType()]; i < node.getIndex(); ++i) { + _nodeStates.insert(std::make_pair(Node(node.getType(), i), NodeState(node.getType(), State::DOWN))); } _nodeCount[node.getType()] = node.getIndex() + 1; } - if (state == NodeState(node.getType(), State::UP) - && state.getDescription().size() == 0) - { + if ((state == NodeState(node.getType(), State::UP)) && state.getDescription().empty()) { _nodeStates.erase(node); } else { _nodeStates.insert(std::make_pair(node, state)); @@ -339,32 +322,34 @@ ClusterState::setNodeState(const Node& node, const NodeState& state) } void -ClusterState::print(std::ostream& out, bool verbose, - const std::string&) const +ClusterState::print(std::ostream& out, bool verbose, const std::string&) const { (void) verbose; vespalib::asciistream tmp; - serialize(tmp, false); + serialize(tmp); out << tmp.str(); } void ClusterState::removeExtraElements() { + removeExtraElements(NodeType::STORAGE); + removeExtraElements(NodeType::DISTRIBUTOR); +} + +void +ClusterState::removeExtraElements(const NodeType & type) +{ // Simplify the system state by removing the last indexes if the nodes // are down. - for (uint32_t i=0; i<2; ++i) { - const NodeType& type(i == 0 ? NodeType::STORAGE - : NodeType::DISTRIBUTOR); - for (int32_t index = _nodeCount[type]; index >= 0; --index) { - Node node(type, index - 1); - std::map<Node, NodeState>::iterator it(_nodeStates.find(node)); - if (it == _nodeStates.end()) break; - if (it->second.getState() != State::DOWN) break; - if (it->second.getDescription() != "") break; - _nodeStates.erase(it); - --_nodeCount[type]; - } + for (int32_t index = _nodeCount[type]; index >= 0; --index) { + Node node(type, index - 1); + const auto it = _nodeStates.find(node); + if (it == _nodeStates.end()) break; + if (it->second.getState() != State::DOWN) break; + if (it->second.getDescription() != "") break; + _nodeStates.erase(it); + --_nodeCount[type]; } } @@ -413,90 +398,89 @@ void ClusterState::printStateGroupwise(std::ostream& out, const Distribution& dist, bool verbose, const std::string& indent) const { - out << "ClusterState(Version: " << _version << ", Cluster state: " - << _clusterState->toString(true) << ", Distribution bits: " - << _distributionBits << ") {"; + out << "ClusterState(Version: " << _version << ", Cluster state: " << _clusterState->toString(true) + << ", Distribution bits: " << _distributionBits << ") {"; printStateGroupwise(out, dist.getNodeGraph(), verbose, indent + " ", true); out << "\n" << indent << "}"; } namespace { - template<typename T> - std::string getNumberSpec(const std::vector<T>& numbers) { - std::ostringstream ost; - bool first = true; - uint32_t firstInRange = numbers.size() == 0 ? 0 : numbers[0];; - uint32_t lastInRange = firstInRange; - for (uint32_t i=1; i<=numbers.size(); ++i) { - if (i < numbers.size() && numbers[i] == lastInRange + 1) { - ++lastInRange; + +template<typename T> +std::string getNumberSpec(const std::vector<T>& numbers) { + std::ostringstream ost; + bool first = true; + uint32_t firstInRange = numbers.size() == 0 ? 0 : numbers[0];; + uint32_t lastInRange = firstInRange; + for (uint32_t i=1; i<=numbers.size(); ++i) { + if (i < numbers.size() && numbers[i] == lastInRange + 1) { + ++lastInRange; + } else { + if (first) { + first = false; } else { - if (first) { - first = false; - } else { - ost << ","; - } - if (firstInRange == lastInRange) { - ost << firstInRange; - } else { - ost << firstInRange << "-" << lastInRange; - } - if (i < numbers.size()) { - firstInRange = lastInRange = numbers[i]; - } + ost << ","; + } + if (firstInRange == lastInRange) { + ost << firstInRange; + } else { + ost << firstInRange << "-" << lastInRange; + } + if (i < numbers.size()) { + firstInRange = lastInRange = numbers[i]; } } - return ost.str(); } + return ost.str(); +} + +} + +size_t +ClusterState::printStateGroupwise(std::ostream& out, const Group& group, bool verbose, + const std::string& indent, const NodeType& nodeType) const +{ + NodeState defState(nodeType, State::UP); + size_t printed = 0; + for (uint16_t nodeId : group.getNodes()) { + Node node(nodeType, nodeId); + const NodeState& state(getNodeState(node)); + if (state != defState) { + out << "\n" << indent << " " << node << ": "; + state.print(out, verbose, indent + " "); + printed++; + } + } + return printed; } void -ClusterState::printStateGroupwise(std::ostream& out, const Group& group, - bool verbose, const std::string& indent, - bool rootGroup) const +ClusterState::printStateGroupwise(std::ostream& out, const Group& group, bool verbose, + const std::string& indent, bool rootGroup) const { if (rootGroup) { out << "\n" << indent << "Top group"; } else { - out << "\n" << indent << "Group " << group.getIndex() << ": " - << group.getName(); + out << "\n" << indent << "Group " << group.getIndex() << ": " << group.getName(); if (group.getCapacity() != 1.0) { out << ", capacity " << group.getCapacity(); } } out << "."; if (group.isLeafGroup()) { - out << " " << group.getNodes().size() << " node" - << (group.getNodes().size() != 1 ? "s" : "") << " [" - << getNumberSpec(group.getNodes()) << "] {"; - bool printedAny = false; - for (uint32_t j=0; j<2; ++j) { - const NodeType& nodeType( - j == 0 ? NodeType::DISTRIBUTOR : NodeType::STORAGE); - NodeState defState(nodeType, State::UP); - for (uint32_t i=0; i<group.getNodes().size(); ++i) { - Node node(nodeType, group.getNodes()[i]); - const NodeState& state(getNodeState(node)); - if (state != defState) { - out << "\n" << indent << " " << node << ": "; - state.print(out, verbose, indent + " "); - printedAny = true; - } - } - } - if (!printedAny) { + out << " " << group.getNodes().size() << " node" << (group.getNodes().size() != 1 ? "s" : "") + << " [" << getNumberSpec(group.getNodes()) << "] {"; + size_t printed = printStateGroupwise(out, group, verbose, indent, NodeType::DISTRIBUTOR) + + printStateGroupwise(out, group, verbose, indent, NodeType::STORAGE); + if (printed == 0) { out << "\n" << indent << " All nodes in group up and available."; } } else { - const std::map<uint16_t, Group*>& children(group.getSubGroups()); - out << " " << children.size() << " branch" - << (children.size() != 1 ? "es" : "") << " with distribution " - << group.getDistributionSpec() << " {"; - for (std::map<uint16_t, Group*>::const_iterator it = children.begin(); - it != children.end(); ++it) - { - printStateGroupwise(out, *it->second, verbose, - indent + " ", false); + const auto & children(group.getSubGroups()); + out << " " << children.size() << " branch" << (children.size() != 1 ? "es" : "") + << " with distribution " << group.getDistributionSpec() << " {"; + for (const auto & child : children) { + printStateGroupwise(out, *child.second, verbose,indent + " ", false); } } out << "\n" << indent << "}"; diff --git a/vdslib/src/vespa/vdslib/state/clusterstate.h b/vdslib/src/vespa/vdslib/state/clusterstate.h index 3af5a45fcac..90ec7c1aa65 100644 --- a/vdslib/src/vespa/vdslib/state/clusterstate.h +++ b/vdslib/src/vespa/vdslib/state/clusterstate.h @@ -10,26 +10,21 @@ #include "node.h" #include "nodestate.h" -#include <map> +#include <vespa/vespalib/stllike/hash_map.h> +#include <array> namespace storage::lib { class Distribution; class Group; struct NodeData; +struct SeparatorPrinter; class ClusterState : public document::Printable { - uint32_t _version; - const State* _clusterState; - std::map<Node, NodeState> _nodeStates; - std::vector<uint16_t> _nodeCount; - vespalib::string _description; - uint16_t _distributionBits; - - void getTextualDifference(std::ostringstream& builder, const NodeType& type, - const ClusterState& other) const; - public: + using NodeStatePair = std::pair<Node, NodeState>; + using NodeMap = vespalib::hash_map<Node, NodeState>; + using NodeCounts = std::array<uint16_t, 2>; using CSP = std::shared_ptr<const ClusterState>; using SP = std::shared_ptr<ClusterState>; using UP = std::unique_ptr<ClusterState>; @@ -43,31 +38,29 @@ public: ~ClusterState(); std::string getTextualDifference(const ClusterState& other) const; - void serialize(vespalib::asciistream & out, bool ignoreNewFeatures) const; + void serialize(vespalib::asciistream & out) const; - bool operator==(const ClusterState& other) const; - bool operator!=(const ClusterState& other) const; + bool operator==(const ClusterState& other) const noexcept; + bool operator!=(const ClusterState& other) const noexcept; - uint32_t getVersion() const { return _version; } + uint32_t getVersion() const noexcept { return _version; } /** * Returns the smallest number above the highest node index found of the * given type that is not down. */ - uint16_t getNodeCount(const NodeType& type) const; - uint16_t getDistributionBitCount() const { return _distributionBits; } - const State& getClusterState() const { return *_clusterState; } + uint16_t getNodeCount(const NodeType& type) const noexcept { return _nodeCount[type]; } + uint16_t getDistributionBitCount() const noexcept { return _distributionBits; } + const State& getClusterState() const noexcept { return *_clusterState; } const NodeState& getNodeState(const Node& node) const; - void setVersion(uint32_t version) { _version = version; } + void setVersion(uint32_t version) noexcept { _version = version; } void setClusterState(const State& state); void setNodeState(const Node& node, const NodeState& state); - void setDistributionBitCount(uint16_t count) { _distributionBits = count; } + void setDistributionBitCount(uint16_t count) noexcept { _distributionBits = count; } void print(std::ostream& out, bool verbose, const std::string& indent) const override; - void printStateGroupwise(std::ostream& out, - const Distribution&, bool verbose = false, - const std::string& indent = "") const; + void printStateGroupwise(std::ostream& out, const Distribution&, bool verbose, const std::string& indent) const; private: // Preconditions: `key` and `value` MUST point into null-terminated strings. @@ -75,9 +68,18 @@ private: // Preconditions: `key` and `value` MUST point into null-terminated strings. bool parseSorD(vespalib::stringref key, vespalib::stringref value, NodeData & nodeData); void removeExtraElements(); - void printStateGroupwise(std::ostream& out, const Group&, bool verbose, - const std::string& indent, bool rootGroup) const; - + void removeExtraElements(const NodeType& type); + void printStateGroupwise(std::ostream& out, const Group&, bool verbose, const std::string& indent, bool rootGroup) const; + void getTextualDifference(std::ostringstream& builder, const NodeType& type, const ClusterState& other) const; + size_t printStateGroupwise(std::ostream& out, const Group&, bool verbose, const std::string& indent, const NodeType& type) const; + void serialize_nodes(vespalib::asciistream & out, SeparatorPrinter & sep, const NodeType & nodeType, + const std::vector<NodeStatePair> & nodeStates) const; + uint32_t _version; + NodeCounts _nodeCount; + const State* _clusterState; + NodeMap _nodeStates; + vespalib::string _description; + uint16_t _distributionBits; }; } diff --git a/vdslib/src/vespa/vdslib/state/node.h b/vdslib/src/vespa/vdslib/state/node.h index 2e33e54c638..49c8f0e641b 100644 --- a/vdslib/src/vespa/vdslib/state/node.h +++ b/vdslib/src/vespa/vdslib/state/node.h @@ -13,24 +13,25 @@ namespace storage::lib { class Node { const NodeType* _type; - uint16_t _index; + uint16_t _index; public: Node() noexcept : _type(&NodeType::STORAGE), _index(0) { } Node(const NodeType& type, uint16_t index) noexcept : _type(&type), _index(index) { } - const NodeType& getType() const { return *_type; } - uint16_t getIndex() const { return _index; } + const NodeType& getType() const noexcept { return *_type; } + uint16_t getIndex() const noexcept { return _index; } + uint32_t hash() const noexcept { return (_index << 1) | *_type; } - bool operator==(const Node& other) const { + bool operator==(const Node& other) const noexcept { return (other._index == _index && *other._type == *_type); } - bool operator!=(const Node& other) const { + bool operator!=(const Node& other) const noexcept { return (other._index != _index || *other._type != *_type); } - bool operator<(const Node& n) const { + bool operator<(const Node& n) const noexcept { if (*_type != *n._type) return (*_type < *n._type); return (_index < n._index); } diff --git a/vespalib/src/tests/arrayref/arrayref_test.cpp b/vespalib/src/tests/arrayref/arrayref_test.cpp index bd8646b2f99..8c41d38b292 100644 --- a/vespalib/src/tests/arrayref/arrayref_test.cpp +++ b/vespalib/src/tests/arrayref/arrayref_test.cpp @@ -53,4 +53,24 @@ TEST("require that references can be unconstified") { EXPECT_EQUAL(data[1], 5); } +TEST("require that std::array references can be constified") { + std::array<int,3> data({1,2,3}); + const ArrayRef<int> array_ref(data); + ConstArrayRef<int> const_ref(array_ref); + EXPECT_EQUAL(const_ref.size(), 3u); + EXPECT_EQUAL(const_ref.end() - const_ref.begin(), 3); + EXPECT_EQUAL(const_ref[2], 3); +} + +TEST("require that references can be unconstified") { + std::array<int, 3> data({1,2,3}); + const ConstArrayRef<int> const_ref(data); + ArrayRef<int> array_ref = unconstify(const_ref); + EXPECT_EQUAL(array_ref.size(), 3u); + EXPECT_EQUAL(array_ref.end() - array_ref.begin(), 3); + EXPECT_EQUAL(array_ref[1], 2); + array_ref[1] = 5; + EXPECT_EQUAL(data[1], 5); +} + TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/vespalib/src/vespa/vespalib/stllike/hash_map.cpp b/vespalib/src/vespa/vespalib/stllike/hash_map.cpp index 9540a47eec3..abb88fe674f 100644 --- a/vespalib/src/vespa/vespalib/stllike/hash_map.cpp +++ b/vespalib/src/vespa/vespalib/stllike/hash_map.cpp @@ -16,6 +16,7 @@ VESPALIB_HASH_MAP_INSTANTIATE(vespalib::string, double); VESPALIB_HASH_MAP_INSTANTIATE(int64_t, int32_t); VESPALIB_HASH_MAP_INSTANTIATE(int64_t, uint32_t); VESPALIB_HASH_MAP_INSTANTIATE(int32_t, uint32_t); +VESPALIB_HASH_MAP_INSTANTIATE(uint16_t, uint32_t); VESPALIB_HASH_MAP_INSTANTIATE(uint32_t, int32_t); VESPALIB_HASH_MAP_INSTANTIATE(uint32_t, uint32_t); VESPALIB_HASH_MAP_INSTANTIATE(uint64_t, uint32_t); diff --git a/vespalib/src/vespa/vespalib/stllike/hash_map.h b/vespalib/src/vespa/vespalib/stllike/hash_map.h index 889093a9550..c4f60f879d7 100644 --- a/vespalib/src/vespa/vespalib/stllike/hash_map.h +++ b/vespalib/src/vespa/vespalib/stllike/hash_map.h @@ -35,6 +35,8 @@ public: constexpr iterator end() noexcept { return _ht.end(); } constexpr const_iterator begin() const noexcept { return _ht.begin(); } constexpr const_iterator end() const noexcept { return _ht.end(); } + constexpr const_iterator cbegin() const noexcept { return _ht.begin(); } + constexpr const_iterator cend() const noexcept { return _ht.end(); } constexpr size_t capacity() const noexcept { return _ht.capacity(); } constexpr size_t size() const noexcept { return _ht.size(); } constexpr bool empty() const noexcept { return _ht.empty(); } diff --git a/vespalib/src/vespa/vespalib/stllike/hash_set.cpp b/vespalib/src/vespa/vespalib/stllike/hash_set.cpp index 8812af426bf..54614329a97 100644 --- a/vespalib/src/vespa/vespalib/stllike/hash_set.cpp +++ b/vespalib/src/vespa/vespalib/stllike/hash_set.cpp @@ -6,6 +6,8 @@ namespace vespalib { } +VESPALIB_HASH_SET_INSTANTIATE(int16_t); +VESPALIB_HASH_SET_INSTANTIATE(uint16_t); VESPALIB_HASH_SET_INSTANTIATE(int32_t); VESPALIB_HASH_SET_INSTANTIATE(uint32_t); VESPALIB_HASH_SET_INSTANTIATE(uint64_t); diff --git a/vespalib/src/vespa/vespalib/util/arrayref.h b/vespalib/src/vespa/vespalib/util/arrayref.h index 319947e4cd9..9057f56fac0 100644 --- a/vespalib/src/vespa/vespalib/util/arrayref.h +++ b/vespalib/src/vespa/vespalib/util/arrayref.h @@ -3,6 +3,7 @@ #include <cstddef> #include <vector> +#include <array> namespace vespalib { @@ -17,6 +18,8 @@ public: constexpr ArrayRef(T * v, size_t sz) noexcept : _v(v), _sz(sz) { } template<typename A=std::allocator<T>> ArrayRef(std::vector<T, A> & v) noexcept : _v(v.data()), _sz(v.size()) { } + template<size_t SZ> + ArrayRef(std::array<T, SZ> & v) noexcept : _v(v.data()), _sz(SZ) { } T & operator [] (size_t i) noexcept { return _v[i]; } const T & operator [] (size_t i) const noexcept { return _v[i]; } T * data() noexcept { return _v; } @@ -36,6 +39,8 @@ public: constexpr ConstArrayRef(const T *v, size_t sz) noexcept : _v(v), _sz(sz) { } template<typename A=std::allocator<T>> ConstArrayRef(const std::vector<T, A> & v) noexcept : _v(v.data()), _sz(v.size()) { } + template<size_t SZ> + ConstArrayRef(const std::array<T, SZ> & v) noexcept : _v(v.data()), _sz(SZ) { } ConstArrayRef(const ArrayRef<T> & v) noexcept : _v(v.data()), _sz(v.size()) { } constexpr ConstArrayRef() noexcept : _v(nullptr), _sz(0) {} const T & operator [] (size_t i) const noexcept { return _v[i]; } |