diff options
40 files changed, 932 insertions, 1018 deletions
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentCluster.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentCluster.java index 06d728d5625..01ec8d65c28 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentCluster.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentCluster.java @@ -14,6 +14,7 @@ import com.yahoo.vdslib.state.Node; import com.yahoo.vdslib.state.NodeState; import com.yahoo.vdslib.state.NodeType; import com.yahoo.vdslib.state.State; +import com.yahoo.vespa.clustercontroller.core.status.statuspage.HtmlTable; import com.yahoo.vespa.clustercontroller.core.status.statuspage.VdsClusterHtmlRenderer; import com.yahoo.vespa.clustercontroller.utils.staterestapi.requests.SetUnitStateRequest; @@ -53,6 +54,7 @@ public class ContentCluster { setNodes(configuredNodes); } + // TODO move out, this doesn't belong in a domain model class public void writeHtmlState( final VdsClusterHtmlRenderer vdsClusterHtmlRenderer, final StringBuilder sb, @@ -66,6 +68,12 @@ public class ContentCluster { final VdsClusterHtmlRenderer.Table table = vdsClusterHtmlRenderer.createNewClusterHtmlTable(clusterName, slobrokGenerationCount); + if (state.clusterFeedIsBlocked()) { // Implies FeedBlock != null + table.appendRaw("<h3 style=\"color: red\">Cluster feeding is blocked!</h3>\n"); + table.appendRaw(String.format("<p>Summary: <strong>%s</strong></p>\n", + HtmlTable.escape(state.getFeedBlockOrNull().getDescription()))); + } + final List<Group> groups = LeafGroups.enumerateFrom(distribution.getRootGroup()); for (int j=0; j<groups.size(); ++j) { @@ -87,6 +95,7 @@ public class ContentCluster { statsAggregator, options.minMergeCompletionRatio, options.maxPrematureCrashes, + options.clusterFeedBlockLimit, eventLog, clusterName, localName); diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/statuspage/VdsClusterHtmlRenderer.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/statuspage/VdsClusterHtmlRenderer.java index 6d21aa430a2..ac4cb25a9c1 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/statuspage/VdsClusterHtmlRenderer.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/statuspage/VdsClusterHtmlRenderer.java @@ -50,6 +50,10 @@ public class VdsClusterHtmlRenderer { .append(slobrokGenerationCount).append(".</p>\n"); } + public void appendRaw(String rawHtml) { + contentBuilder.append(rawHtml); + } + public void addTable(final StringBuilder destination, final long stableStateTimePeriode) { destination.append(contentBuilder); @@ -67,6 +71,7 @@ public class VdsClusterHtmlRenderer { final ClusterStatsAggregator statsAggregator, final double minMergeCompletionRatio, final int maxPrematureCrashes, + final Map<String, Double> feedBlockLimits, final EventLog eventLog, final String pathPrefix, final String name) { @@ -80,6 +85,7 @@ public class VdsClusterHtmlRenderer { statsAggregator, minMergeCompletionRatio, maxPrematureCrashes, + feedBlockLimits, eventLog, pathPrefix, dominantVtag, @@ -91,6 +97,7 @@ public class VdsClusterHtmlRenderer { statsAggregator, minMergeCompletionRatio, maxPrematureCrashes, + feedBlockLimits, eventLog, pathPrefix, dominantVtag, @@ -144,6 +151,8 @@ public class VdsClusterHtmlRenderer { .addProperties(new HtmlTable.CellProperties().setColSpan(2).setRowSpan(1))) .addCell(new HtmlTable.Cell(FixedBucketSpaces.globalSpace() + " buckets") .addProperties(new HtmlTable.CellProperties().setColSpan(2).setRowSpan(1))) + .addCell(new HtmlTable.Cell("Resource usage (%)") + .addProperties(new HtmlTable.CellProperties().setColSpan(2).setRowSpan(1))) .addCell(new HtmlTable.Cell("Start Time")) .addCell(new HtmlTable.Cell("RPC Address"))); table.addRow(new HtmlTable.Row().setHeaderRow().addProperties(headerProperties) @@ -153,7 +162,9 @@ public class VdsClusterHtmlRenderer { .addCell(new HtmlTable.Cell("Pending")) .addCell(new HtmlTable.Cell("Total")) .addCell(new HtmlTable.Cell("Pending")) - .addCell(new HtmlTable.Cell("Total"))); + .addCell(new HtmlTable.Cell("Total")) + .addCell(new HtmlTable.Cell("Disk")) + .addCell(new HtmlTable.Cell("Memory"))); } private void renderNodesOneType( @@ -164,6 +175,7 @@ public class VdsClusterHtmlRenderer { final ClusterStatsAggregator statsAggregator, final double minMergeCompletionRatio, final int maxPrematureCrashes, + final Map<String, Double> feedBlockLimits, final EventLog eventLog, final String pathPrefix, final String dominantVtag, @@ -188,6 +200,7 @@ public class VdsClusterHtmlRenderer { addPrematureCrashes(maxPrematureCrashes, nodeInfo, row); addEventsLastWeek(eventLog, currentTime, nodeInfo, row); addBucketSpacesStats(nodeType, statsAggregator, minMergeCompletionRatio, nodeInfo, row); + addResourceUsage(nodeInfo, feedBlockLimits, row); addStartTime(nodeInfo, row); addRpcAddress(nodeInfo, row); @@ -230,6 +243,37 @@ public class VdsClusterHtmlRenderer { } } + private void addResourceUsage(NodeInfo nodeInfo, Map<String, Double> feedBlockLimits, HtmlTable.Row row) { + if (nodeInfo.isDistributor()) { + row.addCell(new HtmlTable.Cell("-").addProperties(CENTERED_PROPERTY)); + row.addCell(new HtmlTable.Cell("-").addProperties(CENTERED_PROPERTY)); + return; + } + addSingleResourceUsageCell(nodeInfo, "disk", feedBlockLimits, row); + addSingleResourceUsageCell(nodeInfo, "memory", feedBlockLimits, row); + } + + private void addSingleResourceUsageCell(NodeInfo nodeInfo, String resourceType, + Map<String, Double> feedBlockLimits, HtmlTable.Row row) + { + var hostInfo = nodeInfo.getHostInfo(); + var usages = hostInfo.getContentNode().getResourceUsage(); + + var usage = usages.get(resourceType); + if (usage != null && usage.getUsage() != null) { + row.addCell(new HtmlTable.Cell(String.format("%.2f", usage.getUsage() * 100.0))); + double limit = feedBlockLimits.getOrDefault(resourceType, 1.0); + // Mark as error if limit exceeded, warn if within 5% of exceeding + if (usage.getUsage() > limit) { + row.getLastCell().addProperties(ERROR_PROPERTY); + } else if (usage.getUsage() > (limit - 0.05)) { + row.getLastCell().addProperties(WARNING_PROPERTY); + } + } else { + row.addCell(new HtmlTable.Cell("-").addProperties(CENTERED_PROPERTY)); + } + } + private void addEventsLastWeek(EventLog eventLog, long currentTime, NodeInfo nodeInfo, HtmlTable.Row row) { int nodeEvents = eventLog.getNodeEventsSince(nodeInfo.getNode(), currentTime - eventLog.getRecentTimePeriod()); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterHtmlRendrerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterHtmlRendrerTest.java index 11b07c037ae..3042e323879 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterHtmlRendrerTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterHtmlRendrerTest.java @@ -12,6 +12,7 @@ import org.junit.Test; import java.io.StringWriter; import java.io.Writer; +import java.util.Collections; import java.util.TreeMap; import static org.hamcrest.core.Is.is; @@ -60,6 +61,7 @@ public class ContentClusterHtmlRendrerTest { statsAggregator, 1.0, 10, + Collections.emptyMap(), eventLog, "pathPrefix", "name"); diff --git a/configdefinitions/src/vespa/stor-filestor.def b/configdefinitions/src/vespa/stor-filestor.def index eb858cfe65c..a8466987560 100644 --- a/configdefinitions/src/vespa/stor-filestor.def +++ b/configdefinitions/src/vespa/stor-filestor.def @@ -64,3 +64,10 @@ enable_multibit_split_optimalization bool default=true restart ## This async message is then handled by the calling thread immediately, ## instead of going via a persistence thread. use_async_message_handling_on_schedule bool default=false restart + +## The noise level used when deciding whether a resource usage sample should be reported to the cluster controller. +## +## If one of the resource categories (e.g. disk or memory) has a usage delta that is larger than the noise level, +## the entire resource usage sample is immediately reported to the cluster controller (via host info). +## This config can be live updated (doesn't require restart). +resource_usage_reporter_noise_level double default=0.001 diff --git a/container-search/src/main/java/com/yahoo/prelude/searcher/BlendingSearcher.java b/container-search/src/main/java/com/yahoo/prelude/searcher/BlendingSearcher.java index b2f5d104890..d79386a88ed 100644 --- a/container-search/src/main/java/com/yahoo/prelude/searcher/BlendingSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/searcher/BlendingSearcher.java @@ -78,8 +78,7 @@ public class BlendingSearcher extends Searcher { * This assumes that all hits are organized into hitgroups. If not, blending will not be performed. */ protected Result blendResults(Result result, Query q, int offset, int hits, Execution execution) { - - //Assert that there are more than one hitgroup and that there are only hitgroups on the lowest level + // Assert that there are more than one hitgroup and that there are only hitgroups on the lowest level boolean foundNonGroup = false; Iterator<Hit> hitIterator = result.hits().iterator(); List<HitGroup> groups = new ArrayList<>(); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java index dedb915a101..75f8419b336 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java @@ -56,7 +56,6 @@ public class ControllerMaintenance extends AbstractComponent { maintainers.add(new CostReportMaintainer(controller, intervals.costReportMaintainer, controller.serviceRegistry().costReportConsumer())); maintainers.add(new ResourceMeterMaintainer(controller, intervals.resourceMeterMaintainer, metric, controller.serviceRegistry().meteringService())); maintainers.add(new CloudEventReporter(controller, intervals.cloudEventReporter, metric)); - maintainers.add(new RotationStatusUpdater(controller, intervals.defaultInterval)); maintainers.add(new ResourceTagMaintainer(controller, intervals.resourceTagMaintainer, controller.serviceRegistry().resourceTagger())); maintainers.add(new SystemRoutingPolicyMaintainer(controller, intervals.systemRoutingPolicyMaintainer)); maintainers.add(new ApplicationMetaDataGarbageCollector(controller, intervals.applicationMetaDataGarbageCollector)); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java index e59875e9588..55a957f0247 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java @@ -12,7 +12,6 @@ import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMetadata; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateProvider; -import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger; @@ -92,20 +91,20 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { private void deployRefreshedCertificates() { var now = clock.instant(); curator.readAllEndpointCertificateMetadata().forEach((applicationId, endpointCertificateMetadata) -> - endpointCertificateMetadata.lastRefreshed().ifPresent(lastRefreshTime -> { - Instant refreshTime = Instant.ofEpochSecond(lastRefreshTime); - if (now.isAfter(refreshTime.plus(7, ChronoUnit.DAYS))) { - - controller().jobController().jobs(applicationId).forEach(job -> - controller().jobController().jobStatus(new JobId(applicationId, JobType.fromJobName(job.jobName()))).lastTriggered().ifPresent(run -> { - if (run.start().isBefore(refreshTime) && job.isProduction() && job.isDeployment()) { - deploymentTrigger.reTrigger(applicationId, job); - log.info("Re-triggering deployment job " + job.jobName() + " for instance " + - applicationId.serializedForm() + " to roll out refreshed endpoint certificate"); - } - })); - } - })); + endpointCertificateMetadata.lastRefreshed().ifPresent(lastRefreshTime -> { + Instant refreshTime = Instant.ofEpochSecond(lastRefreshTime); + if (now.isAfter(refreshTime.plus(7, ChronoUnit.DAYS))) { + controller().applications().getInstance(applicationId) + .ifPresent(instance -> instance.productionDeployments().forEach((zone, deployment) -> { + if (deployment.at().isBefore(refreshTime)) { + JobType job = JobType.from(controller().system(), zone).get(); + deploymentTrigger.reTrigger(applicationId, job); + log.info("Re-triggering deployment job " + job.jobName() + " for instance " + + applicationId.serializedForm() + " to roll out refreshed endpoint certificate"); + } + })); + } + })); } private OptionalInt latestVersionInSecretStore(EndpointCertificateMetadata originalCertificateMetadata) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RotationStatusUpdater.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RotationStatusUpdater.java deleted file mode 100644 index 935bcbec597..00000000000 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RotationStatusUpdater.java +++ /dev/null @@ -1,106 +0,0 @@ -// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.maintenance; - -import com.yahoo.vespa.hosted.controller.ApplicationController; -import com.yahoo.vespa.hosted.controller.Controller; -import com.yahoo.vespa.hosted.controller.Instance; -import com.yahoo.vespa.hosted.controller.api.integration.routing.GlobalRoutingService; -import com.yahoo.vespa.hosted.controller.application.ApplicationList; -import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; -import com.yahoo.vespa.hosted.controller.rotation.RotationId; -import com.yahoo.vespa.hosted.controller.rotation.RotationState; -import com.yahoo.vespa.hosted.controller.rotation.RotationStatus; -import com.yahoo.yolean.Exceptions; - -import java.time.Duration; -import java.util.LinkedHashMap; -import java.util.Map; -import java.util.concurrent.ForkJoinPool; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.AtomicReference; -import java.util.logging.Level; -import java.util.stream.Collectors; - -/** - * Periodically updates the status of assigned global rotations. - * - * @author mpolden - */ -public class RotationStatusUpdater extends ControllerMaintainer { - - private static final int applicationsToUpdateInParallel = 10; - - private final GlobalRoutingService service; - private final ApplicationController applications; - - public RotationStatusUpdater(Controller controller, Duration interval) { - super(controller, interval); - this.service = controller.serviceRegistry().globalRoutingService(); - this.applications = controller.applications(); - } - - @Override - protected boolean maintain() { - var failures = new AtomicInteger(0); - var attempts = new AtomicInteger(0); - var lastException = new AtomicReference<Exception>(null); - var instancesWithRotations = ApplicationList.from(applications.readable()).hasRotation().asList().stream() - .flatMap(application -> application.instances().values().stream()) - .filter(instance -> ! instance.rotations().isEmpty()); - - // Run parallel stream inside a custom ForkJoinPool so that we can control the number of threads used - var pool = new ForkJoinPool(applicationsToUpdateInParallel); - - pool.submit(() -> { - instancesWithRotations.parallel().forEach(instance -> { - attempts.incrementAndGet(); - try { - RotationStatus status = getStatus(instance); - applications.lockApplicationIfPresent(TenantAndApplicationId.from(instance.id()), app -> - applications.store(app.with(instance.name(), locked -> locked.with(status)))); - } catch (Exception e) { - failures.incrementAndGet(); - lastException.set(e); - } - }); - }); - pool.shutdown(); - try { - pool.awaitTermination(30, TimeUnit.SECONDS); - if (lastException.get() != null) { - log.log(Level.WARNING, String.format("Failed to get global routing status of %d/%d applications. Retrying in %s. Last error: %s", - failures.get(), - attempts.get(), - interval(), - Exceptions.toMessageString(lastException.get()))); - } - } catch (InterruptedException e) { - throw new RuntimeException(e); - } - return lastException.get() == null; - } - - private RotationStatus getStatus(Instance instance) { - var statusMap = new LinkedHashMap<RotationId, RotationStatus.Targets>(); - for (var assignedRotation : instance.rotations()) { - var rotation = controller().routing().rotations().getRotation(assignedRotation.rotationId()); - if (rotation.isEmpty()) continue; - var targets = service.getHealthStatus(rotation.get().name()).entrySet().stream() - .collect(Collectors.toMap(Map.Entry::getKey, (kv) -> from(kv.getValue()))); - var lastUpdated = controller().clock().instant(); - statusMap.put(assignedRotation.rotationId(), new RotationStatus.Targets(targets, lastUpdated)); - } - return RotationStatus.from(statusMap); - } - - private static RotationState from(com.yahoo.vespa.hosted.controller.api.integration.routing.RotationStatus status) { - switch (status) { - case IN: return RotationState.in; - case OUT: return RotationState.out; - case UNKNOWN: return RotationState.unknown; - default: throw new IllegalArgumentException("Unknown API value for rotation status: " + status); - } - } - -} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index 9331e5086cc..431a694fcd8 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -1683,20 +1683,11 @@ public class ApplicationApiHandler extends LoggingRequestHandler { controller.jobController().deploy(id, type, version, applicationPackage); RunId runId = controller.jobController().last(id, type).get().id(); - DeploymentId deploymentId = new DeploymentId(id, type.zone(controller.system())); - Slime slime = new Slime(); Cursor rootObject = slime.setObject(); rootObject.setString("message", "Deployment started in " + runId + ". This may take about 15 minutes the first time."); rootObject.setLong("run", runId.number()); - var endpointArray = rootObject.setArray("endpoints"); - EndpointList zoneEndpoints = controller.routing().endpointsOf(deploymentId) - .scope(Endpoint.Scope.zone) - .not().legacy(); - for (var endpoint : controller.routing().directEndpoints(zoneEndpoints, deploymentId.applicationId())) { - toSlime(endpoint, endpointArray.addObject()); - } return new SlimeJsonResponse(slime); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RotationStatusUpdaterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RotationStatusUpdaterTest.java deleted file mode 100644 index 87c9a4996b9..00000000000 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RotationStatusUpdaterTest.java +++ /dev/null @@ -1,92 +0,0 @@ -// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.maintenance; - -import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.zone.ZoneId; -import com.yahoo.vespa.hosted.controller.api.integration.routing.RotationStatus; -import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; -import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; -import com.yahoo.vespa.hosted.controller.rotation.RotationState; -import org.junit.Test; - -import java.time.Duration; - -import static org.junit.Assert.assertEquals; - -/** - * @author mpolden - */ -public class RotationStatusUpdaterTest { - - @Test - public void updates_rotation_status() { - var tester = new DeploymentTester(); - var globalRotationService = tester.controllerTester().serviceRegistry().globalRoutingServiceMock(); - var updater = new RotationStatusUpdater(tester.controller(), Duration.ofDays(1)); - - var context = tester.newDeploymentContext(ApplicationId.from("tenant1", "app1", "default")); - var zone1 = ZoneId.from("prod", "us-west-1"); - var zone2 = ZoneId.from("prod", "us-east-3"); - var zone3 = ZoneId.from("prod", "eu-west-1"); - - // Deploy application with global rotation - var applicationPackage = new ApplicationPackageBuilder() - .globalServiceId("foo") - .region(zone1.region().value()) - .region(zone2.region().value()) - .build(); - context.submit(applicationPackage) - .deploy(); - - // No status gathered yet - var rotation1 = context.instance().rotations().get(0).rotationId(); - assertEquals(RotationState.unknown, context.instance().rotationStatus().of(rotation1, context.deployment(zone1))); - assertEquals(RotationState.unknown, context.instance().rotationStatus().of(rotation1, context.deployment(zone2))); - - // First rotation: One zone out, one in - var rotationName1 = "rotation-fqdn-01"; - globalRotationService.setStatus(rotationName1, zone1, RotationStatus.IN) - .setStatus(rotationName1, zone2, RotationStatus.OUT); - updater.maintain(); - assertEquals(RotationState.in, context.instance().rotationStatus().of(rotation1, context.deployment(zone1))); - assertEquals(RotationState.out, context.instance().rotationStatus().of(rotation1, context.deployment(zone2))); - - // First rotation: All zones in - globalRotationService.setStatus(rotationName1, zone2, RotationStatus.IN); - updater.maintain(); - assertEquals(RotationState.in, context.instance().rotationStatus().of(rotation1, context.deployment(zone1))); - assertEquals(RotationState.in, context.instance().rotationStatus().of(rotation1, context.deployment(zone2))); - - // Another rotation is assigned - applicationPackage = new ApplicationPackageBuilder() - .region(zone1.region().value()) - .region(zone2.region().value()) - .region(zone3.region().value()) - .endpoint("default", "foo", "us-east-3", "us-west-1") - .endpoint("eu", "default", "eu-west-1") - .build(); - context.submit(applicationPackage) - .deploy(); - assertEquals(2, context.instance().rotations().size()); - - // Second rotation: No status gathered yet - var rotation2 = context.instance().rotations().get(1).rotationId(); - updater.maintain(); - assertEquals(RotationState.unknown, context.instance().rotationStatus().of(rotation2, context.deployment(zone3))); - - // Status of third zone is retrieved via second rotation - var rotationName2 = "rotation-fqdn-02"; - globalRotationService.setStatus(rotationName2, zone3, RotationStatus.IN); - updater.maintain(); - assertEquals(RotationState.in, context.instance().rotationStatus().of(rotation2, context.deployment(zone3))); - - // Each rotation only has status for their configured zones - assertEquals("Rotation " + rotation1 + " does not know about " + context.deployment(zone3), RotationState.unknown, - context.instance().rotationStatus().of(rotation1, context.deployment(zone3))); - assertEquals("Rotation " + rotation2 + " does not know about " + context.deployment(zone1), RotationState.unknown, - context.instance().rotationStatus().of(rotation2, context.deployment(zone1))); - assertEquals("Rotation " + rotation2 + " does not know about " + context.deployment(zone2), RotationState.unknown, - context.instance().rotationStatus().of(rotation2, context.deployment(zone2))); - } - -} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index 6626134b69a..673563097f4 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -59,7 +59,6 @@ import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger; import com.yahoo.vespa.hosted.controller.integration.ConfigServerMock; import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock; -import com.yahoo.vespa.hosted.controller.maintenance.RotationStatusUpdater; import com.yahoo.vespa.hosted.controller.metric.ApplicationMetrics; import com.yahoo.vespa.hosted.controller.restapi.ContainerTester; import com.yahoo.vespa.hosted.controller.restapi.ControllerContainerTest; @@ -75,7 +74,6 @@ import org.junit.Test; import java.io.File; import java.net.URI; -import java.time.Duration; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.ArrayList; @@ -242,8 +240,7 @@ public class ApplicationApiTest extends ControllerContainerTest { tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploy/production-us-east-3/", POST) .data(entity) .userIdentity(HOSTED_VESPA_OPERATOR), - "{\"message\":\"Deployment started in run 1 of production-us-east-3 for tenant1.application1.instance1. This may take about 15 minutes the first time.\",\"run\":1," + - "\"endpoints\":[{\"cluster\":\"default\",\"tls\":true,\"url\":\"https://instance1--application1--tenant1.us-east-3.vespa.oath.cloud:4443/\",\"scope\":\"zone\",\"routingMethod\":\"shared\"}]}"); + "{\"message\":\"Deployment started in run 1 of production-us-east-3 for tenant1.application1.instance1. This may take about 15 minutes the first time.\",\"run\":1}"); app1.runJob(JobType.productionUsEast3); tester.controller().applications().deactivate(app1.instanceId(), ZoneId.from("prod", "us-east-3")); @@ -251,8 +248,7 @@ public class ApplicationApiTest extends ControllerContainerTest { tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploy/dev-us-east-1/", POST) .data(entity) .userIdentity(USER_ID), - "{\"message\":\"Deployment started in run 1 of dev-us-east-1 for tenant1.application1.instance1. This may take about 15 minutes the first time.\",\"run\":1," + - "\"endpoints\":[{\"cluster\":\"default\",\"tls\":true,\"url\":\"https://instance1--application1--tenant1.us-east-1.dev.vespa.oath.cloud:4443/\",\"scope\":\"zone\",\"routingMethod\":\"shared\"}]}"); + "{\"message\":\"Deployment started in run 1 of dev-us-east-1 for tenant1.application1.instance1. This may take about 15 minutes the first time.\",\"run\":1}"); app1.runJob(JobType.devUsEast1); // GET dev application package @@ -954,7 +950,7 @@ public class ApplicationApiTest extends ControllerContainerTest { tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/environment/prod/region/us-west-1/global-rotation", GET) .properties(Map.of("endpointId", "default")) .userIdentity(USER_ID), - "{\"bcpStatus\":{\"rotationStatus\":\"IN\"}}", + "{\"bcpStatus\":{\"rotationStatus\":\"UNKNOWN\"}}", 200); // GET global rotation status for us-west-1 in eu endpoint @@ -968,7 +964,7 @@ public class ApplicationApiTest extends ControllerContainerTest { tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/environment/prod/region/eu-west-1/global-rotation", GET) .properties(Map.of("endpointId", "eu")) .userIdentity(USER_ID), - "{\"bcpStatus\":{\"rotationStatus\":\"IN\"}}", + "{\"bcpStatus\":{\"rotationStatus\":\"UNKNOWN\"}}", 200); } @@ -1428,8 +1424,7 @@ public class ApplicationApiTest extends ControllerContainerTest { tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/new-user/deploy/dev-us-east-1", POST) .data(entity) .userIdentity(userId), - "{\"message\":\"Deployment started in run 1 of dev-us-east-1 for tenant1.application1.new-user. This may take about 15 minutes the first time.\",\"run\":1," + - "\"endpoints\":[{\"cluster\":\"default\",\"tls\":true,\"url\":\"https://new-user--application1--tenant1.us-east-1.dev.vespa.oath.cloud:4443/\",\"scope\":\"zone\",\"routingMethod\":\"shared\"}]}"); + "{\"message\":\"Deployment started in run 1 of dev-us-east-1 for tenant1.application1.new-user. This may take about 15 minutes the first time.\",\"run\":1}"); } @Test @@ -1474,8 +1469,7 @@ public class ApplicationApiTest extends ControllerContainerTest { tester.assertResponse(request("/application/v4/tenant/sandbox/application/myapp/instance/default/deploy/dev-us-east-1", POST) .data(entity) .userIdentity(developer), - "{\"message\":\"Deployment started in run 1 of dev-us-east-1 for sandbox.myapp. This may take about 15 minutes the first time.\",\"run\":1," + - "\"endpoints\":[{\"cluster\":\"default\",\"tls\":true,\"url\":\"https://myapp--sandbox.us-east-1.dev.vespa.oath.cloud:4443/\",\"scope\":\"zone\",\"routingMethod\":\"shared\"}]}", + "{\"message\":\"Deployment started in run 1 of dev-us-east-1 for sandbox.myapp. This may take about 15 minutes the first time.\",\"run\":1}", 200); // To add temporary support allowing tenant admins to launch services @@ -1486,8 +1480,7 @@ public class ApplicationApiTest extends ControllerContainerTest { tester.assertResponse(request("/application/v4/tenant/sandbox/application/myapp/instance/default/deploy/dev-us-east-1", POST) .data(entity) .userIdentity(developer2), - "{\"message\":\"Deployment started in run 2 of dev-us-east-1 for sandbox.myapp. This may take about 15 minutes the first time.\",\"run\":2," + - "\"endpoints\":[{\"cluster\":\"default\",\"tls\":true,\"url\":\"https://myapp--sandbox.us-east-1.dev.vespa.oath.cloud:4443/\",\"scope\":\"zone\",\"routingMethod\":\"shared\"}]}", + "{\"message\":\"Deployment started in run 2 of dev-us-east-1 for sandbox.myapp. This may take about 15 minutes the first time.\",\"run\":2}", 200); @@ -1496,8 +1489,7 @@ public class ApplicationApiTest extends ControllerContainerTest { .data(applicationPackageInstance1.zippedContent()) .contentType("application/zip") .userIdentity(developer2), - "{\"message\":\"Deployment started in run 3 of dev-us-east-1 for sandbox.myapp. This may take about 15 minutes the first time.\",\"run\":3," + - "\"endpoints\":[{\"cluster\":\"default\",\"tls\":true,\"url\":\"https://myapp--sandbox.us-east-1.dev.vespa.oath.cloud:4443/\",\"scope\":\"zone\",\"routingMethod\":\"shared\"}]}"); + "{\"message\":\"Deployment started in run 3 of dev-us-east-1 for sandbox.myapp. This may take about 15 minutes the first time.\",\"run\":3}"); // POST (deploy) an application package not as content type application/zip — not multipart — is disallowed tester.assertResponse(request("/application/v4/tenant/sandbox/application/myapp/instance/default/deploy/dev-us-east-1", POST) @@ -1655,7 +1647,7 @@ public class ApplicationApiTest extends ControllerContainerTest { private void setZoneInRotation(String rotationName, ZoneId zone) { tester.serviceRegistry().globalRoutingServiceMock().setStatus(rotationName, zone, com.yahoo.vespa.hosted.controller.api.integration.routing.RotationStatus.IN); - new RotationStatusUpdater(tester.controller(), Duration.ofDays(1)).run(); + //new RotationStatusUpdater(tester.controller(), Duration.ofDays(1)).run(); } private void updateContactInformation() { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-job-accepted-2.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-job-accepted-2.json index c53cee8fd97..8ea3f318d1d 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-job-accepted-2.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-job-accepted-2.json @@ -1,13 +1,4 @@ { "message": "Deployment started in run 1 of dev-us-east-1 for tenant1.application1.myuser. This may take about 15 minutes the first time.", - "run": 1, - "endpoints": [ - { - "cluster": "default", - "tls": true, - "url": "https://myuser--application1--tenant1.us-east-1.dev.vespa.oath.cloud:4443/", - "scope": "zone", - "routingMethod": "shared" - } - ] + "run": 1 }
\ No newline at end of file diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment.json index a7755278a39..946593fca00 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment.json @@ -35,7 +35,7 @@ "endpointId": "default", "rotationId": "rotation-id-1", "clusterId": "foo", - "status": "IN", + "status": "UNKNOWN", "lastUpdated": "(ignore)" } ], diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/global-rotation.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/global-rotation.json index 530e21c6c7a..ea8c63cffb6 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/global-rotation.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/global-rotation.json @@ -1,5 +1,5 @@ { "bcpStatus": { - "rotationStatus": "IN" + "rotationStatus": "UNKNOWN" } -}
\ No newline at end of file +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance.json index 1b2c0b4e237..745d0ad162a 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance.json @@ -62,14 +62,14 @@ }, { "bcpStatus": { - "rotationStatus": "IN" + "rotationStatus": "UNKNOWN" }, "endpointStatus": [ { "endpointId": "default", "rotationId": "rotation-id-1", "clusterId": "foo", - "status": "IN", + "status": "UNKNOWN", "lastUpdated": "(ignore)" } ], diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/prod-us-central-1.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/prod-us-central-1.json index 4d387f626a1..4251ba1ad95 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/prod-us-central-1.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/prod-us-central-1.json @@ -1,6 +1,6 @@ { "bcpStatus": { - "rotationStatus": "IN" + "rotationStatus": "UNKNOWN" }, "tenant": "tenant1", "application": "application1", @@ -38,7 +38,7 @@ "endpointId": "default", "rotationId": "rotation-id-1", "clusterId": "foo", - "status": "IN", + "status": "UNKNOWN", "lastUpdated": "(ignore)" } ], diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json index 6f67b0d8aa8..14fd7abd96c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json @@ -64,9 +64,6 @@ "name": "ResourceTagMaintainer" }, { - "name": "RotationStatusUpdater" - }, - { "name": "SystemRoutingPolicyMaintainer" }, { diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index c66e223040a..27864541a90 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -143,14 +143,6 @@ public class Flags { "Takes effect immediately" ); - public static final UnboundBooleanFlag CONTROLLER_PROVISION_LB = defineFeatureFlag( - "controller-provision-lb", false, - List.of("mpolden"), "2020-12-02", "2021-02-01", - "Provision load balancer for controller cluster", - "Takes effect when controller application is redeployed", - ZONE_ID - ); - public static final UnboundBooleanFlag ONLY_PUBLIC_ACCESS = defineFeatureFlag( "enable-public-only", false, List.of("ogronnesby"), "2020-12-02", "2021-02-01", diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsFetcher.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsFetcher.java index 70ac915f792..ba833519b0c 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsFetcher.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsFetcher.java @@ -6,6 +6,7 @@ import com.yahoo.config.provision.ApplicationId; import java.time.Instant; import java.util.Collection; +import java.util.concurrent.CompletableFuture; /** * Interface to retrieve metrics on (tenant) nodes. @@ -15,11 +16,10 @@ import java.util.Collection; public interface MetricsFetcher { /** - * Fetches metrics for all hosts of an application. This call may be expensive. + * Fetches metrics asynchronously for all hosts of an application. This call may be expensive. * * @param application the application to fetch metrics from - * @return a metric snapshot for each hostname of this application */ - Collection<Pair<String, MetricSnapshot>> fetchMetrics(ApplicationId application); + CompletableFuture<MetricsResponse> fetchMetrics(ApplicationId application); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsResponse.java index 963ab85c5a0..341a2028f05 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsResponse.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsResponse.java @@ -22,26 +22,32 @@ import java.util.Map; import java.util.Optional; /** - * Consumes a response from the metrics/v2 API and populates the fields of this with the resulting values + * A response containing metrics for a collection of nodes. * * @author bratseth */ public class MetricsResponse { - private final Collection<Pair<String, MetricSnapshot>> nodeMetrics = new ArrayList<>(); + private final Collection<Pair<String, MetricSnapshot>> nodeMetrics; + /** Creates this from a metrics/V2 response */ public MetricsResponse(String response, NodeList applicationNodes, NodeRepository nodeRepository) { this(SlimeUtils.jsonToSlime(response), applicationNodes, nodeRepository); } - public Collection<Pair<String, MetricSnapshot>> metrics() { return nodeMetrics; } + public MetricsResponse(Collection<Pair<String, MetricSnapshot>> metrics) { + this.nodeMetrics = metrics; + } private MetricsResponse(Slime response, NodeList applicationNodes, NodeRepository nodeRepository) { + nodeMetrics = new ArrayList<>(); Inspector root = response.get(); Inspector nodes = root.field("nodes"); nodes.traverse((ArrayTraverser)(__, node) -> consumeNode(node, applicationNodes, nodeRepository)); } + public Collection<Pair<String, MetricSnapshot>> metrics() { return nodeMetrics; } + private void consumeNode(Inspector node, NodeList applicationNodes, NodeRepository nodeRepository) { String hostname = node.field("hostname").asString(); consumeNodeMetrics(hostname, node.field("node"), applicationNodes, nodeRepository); @@ -83,6 +89,8 @@ public class MetricsResponse { item.field("values").traverse((ObjectTraverser)(name, value) -> values.put(name, value.asDouble())); } + public static MetricsResponse empty() { return new MetricsResponse(List.of()); } + /** The metrics this can read */ private enum Metric { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsV2MetricsFetcher.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsV2MetricsFetcher.java index 4afc876056a..961c1393550 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsV2MetricsFetcher.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsV2MetricsFetcher.java @@ -1,9 +1,8 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.autoscale; -import ai.vespa.util.http.VespaHttpClientBuilder; +import ai.vespa.util.http.VespaAsyncHttpClientBuilder; import com.google.inject.Inject; -import com.yahoo.collections.Pair; import com.yahoo.component.AbstractComponent; import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.applicationmodel.HostName; @@ -12,15 +11,14 @@ import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.orchestrator.HostNameNotFoundException; import com.yahoo.vespa.orchestrator.Orchestrator; -import org.apache.http.client.methods.HttpGet; -import org.apache.http.impl.client.BasicResponseHandler; -import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.hc.client5.http.async.methods.SimpleHttpRequest; +import org.apache.hc.client5.http.async.methods.SimpleHttpResponse; +import org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient; +import org.apache.hc.core5.concurrent.FutureCallback; import java.io.IOException; -import java.io.UncheckedIOException; -import java.util.Collection; -import java.util.Collections; import java.util.Optional; +import java.util.concurrent.CompletableFuture; import java.util.logging.Level; import java.util.logging.Logger; @@ -37,32 +35,37 @@ public class MetricsV2MetricsFetcher extends AbstractComponent implements Metric private final NodeRepository nodeRepository; private final Orchestrator orchestrator; - private final HttpClient httpClient; + private final AsyncHttpClient httpClient; @Inject @SuppressWarnings("unused") public MetricsV2MetricsFetcher(NodeRepository nodeRepository, Orchestrator orchestrator) { - this(nodeRepository, orchestrator, new ApacheHttpClient()); + this(nodeRepository, orchestrator, new AsyncApacheHttpClient()); } - public MetricsV2MetricsFetcher(NodeRepository nodeRepository, Orchestrator orchestrator, HttpClient httpClient) { + public MetricsV2MetricsFetcher(NodeRepository nodeRepository, Orchestrator orchestrator, AsyncHttpClient httpClient) { this.nodeRepository = nodeRepository; this.orchestrator = orchestrator; this.httpClient = httpClient; } @Override - public Collection<Pair<String, MetricSnapshot>> fetchMetrics(ApplicationId application) { + public CompletableFuture<MetricsResponse> fetchMetrics(ApplicationId application) { NodeList applicationNodes = nodeRepository.list(application).state(Node.State.active); Optional<Node> metricsV2Container = applicationNodes.container() .matching(node -> expectedUp(node)) .stream() .findFirst(); - if (metricsV2Container.isEmpty()) return Collections.emptyList(); - // Consumer 'autoscaling' defined in com.yahoo.vespa.model.admin.monitoring.MetricConsumer - String url = "http://" + metricsV2Container.get().hostname() + ":" + 4080 + apiPath + "?consumer=autoscaling"; - return new MetricsResponse(httpClient.get(url), applicationNodes, nodeRepository).metrics(); + if (metricsV2Container.isEmpty()) { + return CompletableFuture.completedFuture(MetricsResponse.empty()); + } + else { + // Consumer 'autoscaling' defined in com.yahoo.vespa.model.admin.monitoring.MetricConsumer + String url = "http://" + metricsV2Container.get().hostname() + ":" + 4080 + apiPath + "?consumer=autoscaling"; + return httpClient.get(url) + .thenApply(response -> new MetricsResponse(response, applicationNodes, nodeRepository)); + } } @Override @@ -79,27 +82,28 @@ public class MetricsV2MetricsFetcher extends AbstractComponent implements Metric } } - /** The simplest possible http client interface */ - public interface HttpClient { + /** A simple async HTTP client */ + public interface AsyncHttpClient { - String get(String url); + CompletableFuture<String> get(String url); void close(); } - /** Implements the HttpClient interface by delegating to an Apache HTTP client */ - public static class ApacheHttpClient implements HttpClient { + /** Implements the AsyncHttpClient interface by delegating to an Apache HTTP client */ + public static class AsyncApacheHttpClient implements AsyncHttpClient { - private final CloseableHttpClient httpClient = VespaHttpClientBuilder.createWithBasicConnectionManager().build(); + private final CloseableHttpAsyncClient httpClient = VespaAsyncHttpClientBuilder.create().build(); + + public AsyncApacheHttpClient() { + httpClient.start(); + } @Override - public String get(String url) { - try { - return httpClient.execute(new HttpGet(url), new BasicResponseHandler()); - } - catch (IOException e) { - throw new UncheckedIOException("Could not get " + url, e); - } + public CompletableFuture<String> get(String url) { + CompletableFuture<String> callback = new CompletableFuture<>(); + httpClient.execute(new SimpleHttpRequest("GET", url), new CallbackAdaptor(callback)); + return callback; } @Override @@ -112,6 +116,31 @@ public class MetricsV2MetricsFetcher extends AbstractComponent implements Metric } } + private static class CallbackAdaptor implements FutureCallback<SimpleHttpResponse> { + + private final CompletableFuture<String> callback; + + public CallbackAdaptor(CompletableFuture<String> callback) { + this.callback = callback; + } + + @Override + public void completed(SimpleHttpResponse simpleHttpResponse) { + callback.complete(simpleHttpResponse.getBodyText()); + } + + @Override + public void failed(Exception e) { + callback.completeExceptionally(e); + } + + @Override + public void cancelled() { + callback.cancel(true); + } + + } + } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerService.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerService.java index 2538619367f..a5a586e1bda 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerService.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerService.java @@ -31,8 +31,9 @@ public interface LoadBalancerService { /** Returns whether load balancers created by this service can forward traffic to given node and cluster type */ default boolean canForwardTo(NodeType nodeType, ClusterSpec.Type clusterType) { return (nodeType == NodeType.tenant && clusterType.isContainer()) || - (nodeType == NodeType.config && clusterType == ClusterSpec.Type.admin) || - (nodeType == NodeType.controller && clusterType.isContainer()); + (nodeType == NodeType.config && clusterType == ClusterSpec.Type.admin); + // TODO(mpolden): Allow this when controllers support provisioning their own LBs + // (nodeType == NodeType.controller && clusterType.isContainer()); } /** Load balancer protocols */ diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainer.java index 017e1264f1c..b8548c4c3f4 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainer.java @@ -3,12 +3,15 @@ package com.yahoo.vespa.hosted.provision.maintenance; import com.yahoo.config.provision.ApplicationId; import com.yahoo.jdisc.Metric; +import com.yahoo.lang.MutableInteger; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.autoscale.MetricsFetcher; import com.yahoo.vespa.hosted.provision.autoscale.MetricsDb; +import com.yahoo.vespa.hosted.provision.autoscale.MetricsResponse; import com.yahoo.yolean.Exceptions; import java.time.Duration; +import java.util.Set; import java.util.logging.Level; /** @@ -35,23 +38,44 @@ public class NodeMetricsDbMaintainer extends NodeRepositoryMaintainer { @Override protected boolean maintain() { - int warnings = 0; - for (ApplicationId application : activeNodesByApplication().keySet()) { - try { - metricsDb.add(metricsFetcher.fetchMetrics(application)); - } - catch (Exception e) { - // TODO: Don't warn if this only happens occasionally - if (warnings++ < maxWarningsPerInvocation) - log.log(Level.WARNING, "Could not update metrics for " + application + ": " + Exceptions.toMessageString(e), e); + try { + var warnings = new MutableInteger(0); + Set<ApplicationId> applications = activeNodesByApplication().keySet(); + if (applications.isEmpty()) return true; + + long pauseMs = interval().toMillis() / applications.size() - 1; // spread requests over interval + int done = 0; + for (ApplicationId application : applications) { + metricsFetcher.fetchMetrics(application) + .whenComplete((metricsResponse, exception) -> handleResponse(metricsResponse, + exception, + warnings, + application)); + if (++done < applications.size()) + Thread.sleep(pauseMs); } - } - metricsDb.gc(); + metricsDb.gc(); - // Suppress failures for manual zones for now to avoid noise - if (nodeRepository().zone().environment().isManuallyDeployed()) return true; + // Suppress failures for manual zones for now to avoid noise + return nodeRepository().zone().environment().isManuallyDeployed() || warnings.get() == 0; + } + catch (InterruptedException e) { + return false; + } + } - return warnings == 0; + private void handleResponse(MetricsResponse response, + Throwable exception, + MutableInteger warnings, + ApplicationId application) { + if (exception != null) { + if (warnings.get() < maxWarningsPerInvocation) + log.log(Level.WARNING, "Could not update metrics for " + application, exception); + warnings.add(1); + } + else if (response != null) { + metricsDb.add(response.metrics()); + } } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java index b66dab58e55..a048f8bb8d2 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java @@ -11,9 +11,7 @@ import com.yahoo.config.provision.ProvisionLock; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.exception.LoadBalancerServiceException; import com.yahoo.transaction.NestedTransaction; -import com.yahoo.vespa.flags.BooleanFlag; import com.yahoo.vespa.flags.FlagSource; -import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; @@ -53,13 +51,11 @@ public class LoadBalancerProvisioner { private final NodeRepository nodeRepository; private final CuratorDatabaseClient db; private final LoadBalancerService service; - private final BooleanFlag provisionControllerLoadBalancer; public LoadBalancerProvisioner(NodeRepository nodeRepository, LoadBalancerService service, FlagSource flagSource) { this.nodeRepository = nodeRepository; this.db = nodeRepository.database(); this.service = service; - this.provisionControllerLoadBalancer = Flags.CONTROLLER_PROVISION_LB.bindTo(flagSource); // Read and write all load balancers to make sure they are stored in the latest version of the serialization format for (var id : db.readLoadBalancerIds()) { try (var lock = db.lock(id.application())) { @@ -80,7 +76,7 @@ public class LoadBalancerProvisioner { * Calling this for irrelevant node or cluster types is a no-op. */ public void prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes) { - if (!canForwardTo(requestedNodes.type(), cluster)) return; // Nothing to provision for this node and cluster type + if (!service.canForwardTo(requestedNodes.type(), cluster.type())) return; // Nothing to provision for this node and cluster type if (application.instance().isTester()) return; // Do not provision for tester instances try (var lock = db.lock(application)) { ClusterSpec.Id clusterId = effectiveId(cluster); @@ -146,15 +142,6 @@ public class LoadBalancerProvisioner { db.writeLoadBalancers(deactivatedLoadBalancers, transaction); } - // TODO(mpolden): Inline when feature flag is removed - private boolean canForwardTo(NodeType type, ClusterSpec cluster) { - boolean canForwardTo = service.canForwardTo(type, cluster.type()); - if (canForwardTo) { - if (type == NodeType.controller) return provisionControllerLoadBalancer.value(); - } - return canForwardTo; - } - /** Find all load balancer IDs owned by given tenant and application */ private List<LoadBalancerId> findLoadBalancers(TenantName tenant, ApplicationName application) { return db.readLoadBalancerIds().stream() diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockMetricsFetcher.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockMetricsFetcher.java index ed2d7eed9e4..da5fa67425f 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockMetricsFetcher.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockMetricsFetcher.java @@ -1,13 +1,11 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.testutils; -import com.yahoo.collections.Pair; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.vespa.hosted.provision.autoscale.MetricSnapshot; import com.yahoo.vespa.hosted.provision.autoscale.MetricsFetcher; +import com.yahoo.vespa.hosted.provision.autoscale.MetricsResponse; -import java.util.ArrayList; -import java.util.Collection; +import java.util.concurrent.CompletableFuture; /** * @author bratseth @@ -15,8 +13,8 @@ import java.util.Collection; public class MockMetricsFetcher implements MetricsFetcher { @Override - public Collection<Pair<String, MetricSnapshot>> fetchMetrics(ApplicationId application) { - return new ArrayList<>(); + public CompletableFuture<MetricsResponse> fetchMetrics(ApplicationId application) { + return CompletableFuture.completedFuture(MetricsResponse.empty()); } } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingIntegrationTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingIntegrationTest.java index 0d423333ce1..8ef2fd72d08 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingIntegrationTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingIntegrationTest.java @@ -14,6 +14,7 @@ import org.junit.Test; import java.time.Duration; import java.util.Set; +import java.util.concurrent.CompletableFuture; import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; @@ -45,7 +46,7 @@ public class AutoscalingIntegrationTest { for (int i = 0; i < 1000; i++) { tester.clock().advance(Duration.ofSeconds(10)); - tester.nodeMetricsDb().add(fetcher.fetchMetrics(application1)); + fetcher.fetchMetrics(application1).whenComplete((r, e) -> tester.nodeMetricsDb().add(r.metrics())); tester.clock().advance(Duration.ofSeconds(10)); tester.nodeMetricsDb().gc(); } @@ -63,7 +64,7 @@ public class AutoscalingIntegrationTest { assertTrue(scaledResources.isPresent()); } - private static class MockHttpClient implements MetricsV2MetricsFetcher.HttpClient { + private static class MockHttpClient implements MetricsV2MetricsFetcher.AsyncHttpClient { private final ManualClock clock; @@ -116,7 +117,9 @@ public class AutoscalingIntegrationTest { "}\n"; @Override - public String get(String url) { return cannedResponse.replace("[now]", String.valueOf(clock.millis())); } + public CompletableFuture<String> get(String url) { + return CompletableFuture.completedFuture(cannedResponse.replace("[now]", + String.valueOf(clock.millis()))); } @Override public void close() { } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsV2MetricsFetcherTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsV2MetricsFetcherTest.java index cc0450ec2ea..14626a40070 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsV2MetricsFetcherTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsV2MetricsFetcherTest.java @@ -15,6 +15,7 @@ import org.junit.Test; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.CompletableFuture; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -25,7 +26,7 @@ public class MetricsV2MetricsFetcherTest { private static final double delta = 0.00000001; @Test - public void testMetricsFetch() { + public void testMetricsFetch() throws Exception { NodeResources resources = new NodeResources(1, 10, 100, 1); ProvisioningTester tester = new ProvisioningTester.Builder().build(); OrchestratorMock orchestrator = new OrchestratorMock(); @@ -44,7 +45,7 @@ public class MetricsV2MetricsFetcherTest { { httpClient.cannedResponse = cannedResponseForApplication1; - List<Pair<String, MetricSnapshot>> values = new ArrayList<>(fetcher.fetchMetrics(application1)); + List<Pair<String, MetricSnapshot>> values = new ArrayList<>(fetcher.fetchMetrics(application1).get().metrics()); assertEquals("http://host-1.yahoo.com:4080/metrics/v2/values?consumer=autoscaling", httpClient.requestsReceived.get(0)); assertEquals(2, values.size()); @@ -62,7 +63,7 @@ public class MetricsV2MetricsFetcherTest { { httpClient.cannedResponse = cannedResponseForApplication2; - List<Pair<String, MetricSnapshot>> values = new ArrayList<>(fetcher.fetchMetrics(application2)); + List<Pair<String, MetricSnapshot>> values = new ArrayList<>(fetcher.fetchMetrics(application2).get().metrics()); assertEquals("http://host-3.yahoo.com:4080/metrics/v2/values?consumer=autoscaling", httpClient.requestsReceived.get(1)); assertEquals(1, values.size()); @@ -80,21 +81,21 @@ public class MetricsV2MetricsFetcherTest { tester.nodeRepository().write(tester.nodeRepository().getNodes(application2, Node.State.active) .get(0).retire(tester.clock().instant()), lock); } - List<Pair<String, MetricSnapshot>> values = new ArrayList<>(fetcher.fetchMetrics(application2)); + List<Pair<String, MetricSnapshot>> values = new ArrayList<>(fetcher.fetchMetrics(application2).get().metrics()); assertFalse(values.get(0).getSecond().stable()); } } - private static class MockHttpClient implements MetricsV2MetricsFetcher.HttpClient { + private static class MockHttpClient implements MetricsV2MetricsFetcher.AsyncHttpClient { List<String> requestsReceived = new ArrayList<>(); String cannedResponse = null; @Override - public String get(String url) { + public CompletableFuture<String> get(String url) { requestsReceived.add(url); - return cannedResponse; + return CompletableFuture.completedFuture(cannedResponse); } @Override diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainerTest.java index 4f0b0d55742..0a12a255f30 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainerTest.java @@ -16,6 +16,7 @@ import java.time.Duration; import java.time.Instant; import java.util.List; import java.util.Set; +import java.util.concurrent.CompletableFuture; import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; @@ -23,7 +24,6 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; /** - * @author bratseth */ public class NodeMetricsDbMaintainerTest { @@ -56,7 +56,7 @@ public class NodeMetricsDbMaintainerTest { assertTrue(allSnapshots.stream().anyMatch(snapshot -> ! snapshot.inService())); } - private static class MockHttpClient implements MetricsV2MetricsFetcher.HttpClient { + private static class MockHttpClient implements MetricsV2MetricsFetcher.AsyncHttpClient { final String cannedResponse = "{\n" + @@ -107,8 +107,8 @@ public class NodeMetricsDbMaintainerTest { "}\n"; @Override - public String get(String url) { - return cannedResponse; + public CompletableFuture<String> get(String url) { + return CompletableFuture.completedFuture(cannedResponse); } @Override @@ -116,5 +116,4 @@ public class NodeMetricsDbMaintainerTest { } - } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java index 7a636a030ec..eef342b527b 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java @@ -10,7 +10,6 @@ import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; -import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.lb.LoadBalancer; @@ -18,6 +17,7 @@ import com.yahoo.vespa.hosted.provision.lb.LoadBalancerInstance; import com.yahoo.vespa.hosted.provision.lb.Real; import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.node.IP; +import org.junit.Ignore; import org.junit.Test; import java.util.Collection; @@ -222,9 +222,9 @@ public class LoadBalancerProvisionerTest { assertEquals(cluster, lbs.get().get(0).id().cluster()); } + @Ignore // TODO: Re-enable when controller support is implemented @Test public void provision_load_balancer_controller_cluster() { - flagSource.withBooleanFlag(Flags.CONTROLLER_PROVISION_LB.id(), true); ApplicationId controllerApp = ApplicationId.from("hosted-vespa", "controller", "default"); Supplier<List<LoadBalancer>> lbs = () -> tester.nodeRepository().loadBalancers(controllerApp).asList(); var cluster = ClusterSpec.Id.from("zone-config-servers"); diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/CMakeLists.txt b/searchcore/src/tests/proton/documentdb/documentbucketmover/CMakeLists.txt index 2a9215dd605..1aa0b1c585d 100644 --- a/searchcore/src/tests/proton/documentdb/documentbucketmover/CMakeLists.txt +++ b/searchcore/src/tests/proton/documentdb/documentbucketmover/CMakeLists.txt @@ -1,4 +1,4 @@ -# Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +# Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. vespa_add_library(searchcore_bucketmover_test STATIC SOURCES @@ -13,6 +13,7 @@ vespa_add_executable(searchcore_documentbucketmover_test_app TEST searchcore_test searchcore_server searchcore_feedoperation + GTest::GTest ) vespa_add_test(NAME searchcore_documentbucketmover_test_app COMMAND searchcore_documentbucketmover_test_app) @@ -24,6 +25,7 @@ vespa_add_executable(searchcore_scaniterator_test_app TEST searchcore_server searchcore_test searchcore_feedoperation + GTest::GTest ) vespa_add_test(NAME searchcore_scaniterator_test_app COMMAND searchcore_scaniterator_test_app) @@ -35,5 +37,6 @@ vespa_add_executable(searchcore_documentmover_test_app TEST searchcore_server searchcore_test searchcore_feedoperation + GTest::GTest ) vespa_add_test(NAME searchcore_documentmover_test_app COMMAND searchcore_documentmover_test_app) diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.cpp b/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.cpp index 76ff3eb74b6..7c8388b05ed 100644 --- a/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.cpp +++ b/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.cpp @@ -1,4 +1,4 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "bucketmover_common.h" #include <vespa/vespalib/testkit/test_macros.h> diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.h b/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.h index 2702d603078..a7e568dc00f 100644 --- a/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.h +++ b/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.h @@ -1,4 +1,4 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include <vespa/searchcore/proton/bucketdb/bucketdbhandler.h> #include <vespa/searchcore/proton/bucketdb/bucket_create_notifier.h> diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp b/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp index fbdaf38930b..70b173ece75 100644 --- a/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp +++ b/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp @@ -1,7 +1,8 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "bucketmover_common.h" #include <vespa/vespalib/testkit/testapp.h> +#include <vespa/vespalib/gtest/gtest.h> #include <vespa/log/log.h> LOG_SETUP("document_bucket_mover_test"); @@ -64,7 +65,7 @@ struct MyCountJobRunner : public IMaintenanceJobRunner { void run() override { ++runCount; } }; -struct ControllerFixtureBase +struct ControllerFixtureBase : public ::testing::Test { test::UserDocumentsBuilder _builder; test::BucketStateCalculator::SP _calc; @@ -183,587 +184,587 @@ struct OnlyReadyControllerFixture : public ControllerFixtureBase } }; -TEST_F("require that nothing is moved if bucket state says so", ControllerFixture) +TEST_F(ControllerFixture, require_that_nothing_is_moved_if_bucket_state_says_so) { - EXPECT_FALSE(f._bmj.done()); - f.addReady(f._ready.bucket(1)); - f.addReady(f._ready.bucket(2)); - f._bmj.scanAndMove(4, 3); - EXPECT_TRUE(f._bmj.done()); - EXPECT_TRUE(f.docsMoved().empty()); - EXPECT_TRUE(f.bucketsModified().empty()); + EXPECT_FALSE(_bmj.done()); + addReady(_ready.bucket(1)); + addReady(_ready.bucket(2)); + _bmj.scanAndMove(4, 3); + EXPECT_TRUE(_bmj.done()); + EXPECT_TRUE(docsMoved().empty()); + EXPECT_TRUE(bucketsModified().empty()); } -TEST_F("require that not ready bucket is moved to ready if bucket state says so", ControllerFixture) +TEST_F(ControllerFixture, require_that_not_ready_bucket_is_moved_to_ready_if_bucket_state_says_so) { // bucket 4 should be moved - f.addReady(f._ready.bucket(1)); - f.addReady(f._ready.bucket(2)); - f.addReady(f._notReady.bucket(4)); - f._bmj.scanAndMove(4, 3); - EXPECT_TRUE(f._bmj.done()); - EXPECT_EQUAL(3u, f.docsMoved().size()); - assertEqual(f._notReady.bucket(4), f._notReady.docs(4)[0], 2, 1, f.docsMoved()[0]); - assertEqual(f._notReady.bucket(4), f._notReady.docs(4)[1], 2, 1, f.docsMoved()[1]); - assertEqual(f._notReady.bucket(4), f._notReady.docs(4)[2], 2, 1, f.docsMoved()[2]); - EXPECT_EQUAL(1u, f.bucketsModified().size()); - EXPECT_EQUAL(f._notReady.bucket(4), f.bucketsModified()[0]); + addReady(_ready.bucket(1)); + addReady(_ready.bucket(2)); + addReady(_notReady.bucket(4)); + _bmj.scanAndMove(4, 3); + EXPECT_TRUE(_bmj.done()); + EXPECT_EQ(3u, docsMoved().size()); + assertEqual(_notReady.bucket(4), _notReady.docs(4)[0], 2, 1, docsMoved()[0]); + assertEqual(_notReady.bucket(4), _notReady.docs(4)[1], 2, 1, docsMoved()[1]); + assertEqual(_notReady.bucket(4), _notReady.docs(4)[2], 2, 1, docsMoved()[2]); + EXPECT_EQ(1u, bucketsModified().size()); + EXPECT_EQ(_notReady.bucket(4), bucketsModified()[0]); } -TEST_F("require that ready bucket is moved to not ready if bucket state says so", ControllerFixture) +TEST_F(ControllerFixture, require_that_ready_bucket_is_moved_to_not_ready_if_bucket_state_says_so) { // bucket 2 should be moved - f.addReady(f._ready.bucket(1)); - f._bmj.scanAndMove(4, 3); - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(2u, f.docsMoved().size()); - assertEqual(f._ready.bucket(2), f._ready.docs(2)[0], 1, 2, f.docsMoved()[0]); - assertEqual(f._ready.bucket(2), f._ready.docs(2)[1], 1, 2, f.docsMoved()[1]); - EXPECT_EQUAL(1u, f.bucketsModified().size()); - EXPECT_EQUAL(f._ready.bucket(2), f.bucketsModified()[0]); + addReady(_ready.bucket(1)); + _bmj.scanAndMove(4, 3); + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(2u, docsMoved().size()); + assertEqual(_ready.bucket(2), _ready.docs(2)[0], 1, 2, docsMoved()[0]); + assertEqual(_ready.bucket(2), _ready.docs(2)[1], 1, 2, docsMoved()[1]); + EXPECT_EQ(1u, bucketsModified().size()); + EXPECT_EQ(_ready.bucket(2), bucketsModified()[0]); } -TEST_F("require that maxBucketsToScan is taken into consideration between not ready and ready scanning", ControllerFixture) +TEST_F(ControllerFixture, require_that_maxBucketsToScan_is_taken_into_consideration_between_not_ready_and_ready_scanning) { // bucket 4 should moved (last bucket) - f.addReady(f._ready.bucket(1)); - f.addReady(f._ready.bucket(2)); - f.addReady(f._notReady.bucket(4)); + addReady(_ready.bucket(1)); + addReady(_ready.bucket(2)); + addReady(_notReady.bucket(4)); // buckets 1, 2, and 3 considered - f._bmj.scanAndMove(3, 3); - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(0u, f.docsMoved().size()); - EXPECT_EQUAL(0u, f.bucketsModified().size()); + _bmj.scanAndMove(3, 3); + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(0u, docsMoved().size()); + EXPECT_EQ(0u, bucketsModified().size()); // move bucket 4 - f._bmj.scanAndMove(1, 4); - EXPECT_TRUE(f._bmj.done()); - EXPECT_EQUAL(3u, f.docsMoved().size()); - assertEqual(f._notReady.bucket(4), f._notReady.docs(4)[0], 2, 1, f.docsMoved()[0]); - assertEqual(f._notReady.bucket(4), f._notReady.docs(4)[1], 2, 1, f.docsMoved()[1]); - assertEqual(f._notReady.bucket(4), f._notReady.docs(4)[2], 2, 1, f.docsMoved()[2]); - EXPECT_EQUAL(1u, f.bucketsModified().size()); - EXPECT_EQUAL(f._notReady.bucket(4), f.bucketsModified()[0]); + _bmj.scanAndMove(1, 4); + EXPECT_TRUE(_bmj.done()); + EXPECT_EQ(3u, docsMoved().size()); + assertEqual(_notReady.bucket(4), _notReady.docs(4)[0], 2, 1, docsMoved()[0]); + assertEqual(_notReady.bucket(4), _notReady.docs(4)[1], 2, 1, docsMoved()[1]); + assertEqual(_notReady.bucket(4), _notReady.docs(4)[2], 2, 1, docsMoved()[2]); + EXPECT_EQ(1u, bucketsModified().size()); + EXPECT_EQ(_notReady.bucket(4), bucketsModified()[0]); } -TEST_F("require that we move buckets in several steps", ControllerFixture) +TEST_F(ControllerFixture, require_that_we_move_buckets_in_several_steps) { // bucket 2, 3, and 4 should be moved - f.addReady(f._ready.bucket(1)); - f.addReady(f._notReady.bucket(3)); - f.addReady(f._notReady.bucket(4)); + addReady(_ready.bucket(1)); + addReady(_notReady.bucket(3)); + addReady(_notReady.bucket(4)); // consider move bucket 1 - f._bmj.scanAndMove(1, 2); - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(0u, f.docsMoved().size()); - EXPECT_EQUAL(0u, f.bucketsModified().size()); + _bmj.scanAndMove(1, 2); + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(0u, docsMoved().size()); + EXPECT_EQ(0u, bucketsModified().size()); // move bucket 2, docs 1,2 - f._bmj.scanAndMove(1, 2); - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(2u, f.docsMoved().size()); - EXPECT_TRUE(assertEqual(f._ready.bucket(2), f._ready.docs(2)[0], 1, 2, f.docsMoved()[0])); - EXPECT_TRUE(assertEqual(f._ready.bucket(2), f._ready.docs(2)[1], 1, 2, f.docsMoved()[1])); - EXPECT_EQUAL(1u, f.bucketsModified().size()); - EXPECT_EQUAL(f._ready.bucket(2), f.bucketsModified()[0]); + _bmj.scanAndMove(1, 2); + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(2u, docsMoved().size()); + EXPECT_TRUE(assertEqual(_ready.bucket(2), _ready.docs(2)[0], 1, 2, docsMoved()[0])); + EXPECT_TRUE(assertEqual(_ready.bucket(2), _ready.docs(2)[1], 1, 2, docsMoved()[1])); + EXPECT_EQ(1u, bucketsModified().size()); + EXPECT_EQ(_ready.bucket(2), bucketsModified()[0]); // move bucket 3, docs 1,2 - f._bmj.scanAndMove(1, 2); - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(4u, f.docsMoved().size()); - EXPECT_TRUE(assertEqual(f._notReady.bucket(3), f._notReady.docs(3)[0], 2, 1, f.docsMoved()[2])); - EXPECT_TRUE(assertEqual(f._notReady.bucket(3), f._notReady.docs(3)[1], 2, 1, f.docsMoved()[3])); - EXPECT_EQUAL(2u, f.bucketsModified().size()); - EXPECT_EQUAL(f._notReady.bucket(3), f.bucketsModified()[1]); + _bmj.scanAndMove(1, 2); + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(4u, docsMoved().size()); + EXPECT_TRUE(assertEqual(_notReady.bucket(3), _notReady.docs(3)[0], 2, 1, docsMoved()[2])); + EXPECT_TRUE(assertEqual(_notReady.bucket(3), _notReady.docs(3)[1], 2, 1, docsMoved()[3])); + EXPECT_EQ(2u, bucketsModified().size()); + EXPECT_EQ(_notReady.bucket(3), bucketsModified()[1]); // move bucket 4, docs 1,2 - f._bmj.scanAndMove(1, 2); - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(6u, f.docsMoved().size()); - EXPECT_TRUE(assertEqual(f._notReady.bucket(4), f._notReady.docs(4)[0], 2, 1, f.docsMoved()[4])); - EXPECT_TRUE(assertEqual(f._notReady.bucket(4), f._notReady.docs(4)[1], 2, 1, f.docsMoved()[5])); - EXPECT_EQUAL(2u, f.bucketsModified().size()); + _bmj.scanAndMove(1, 2); + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(6u, docsMoved().size()); + EXPECT_TRUE(assertEqual(_notReady.bucket(4), _notReady.docs(4)[0], 2, 1, docsMoved()[4])); + EXPECT_TRUE(assertEqual(_notReady.bucket(4), _notReady.docs(4)[1], 2, 1, docsMoved()[5])); + EXPECT_EQ(2u, bucketsModified().size()); // move bucket 4, docs 3 - f._bmj.scanAndMove(1, 2); - EXPECT_TRUE(f._bmj.done()); - EXPECT_EQUAL(7u, f.docsMoved().size()); - EXPECT_TRUE(assertEqual(f._notReady.bucket(4), f._notReady.docs(4)[2], 2, 1, f.docsMoved()[6])); - EXPECT_EQUAL(3u, f.bucketsModified().size()); - EXPECT_EQUAL(f._notReady.bucket(4), f.bucketsModified()[2]); + _bmj.scanAndMove(1, 2); + EXPECT_TRUE(_bmj.done()); + EXPECT_EQ(7u, docsMoved().size()); + EXPECT_TRUE(assertEqual(_notReady.bucket(4), _notReady.docs(4)[2], 2, 1, docsMoved()[6])); + EXPECT_EQ(3u, bucketsModified().size()); + EXPECT_EQ(_notReady.bucket(4), bucketsModified()[2]); } -TEST_F("require that we can change calculator and continue scanning where we left off", ControllerFixture) +TEST_F(ControllerFixture, require_that_we_can_change_calculator_and_continue_scanning_where_we_left_off) { // no buckets should move // original scan sequence is bucket1, bucket2, bucket3, bucket4 - f.addReady(f._ready.bucket(1)); - f.addReady(f._ready.bucket(2)); + addReady(_ready.bucket(1)); + addReady(_ready.bucket(2)); // start with bucket2 - f._bmj.scanAndMove(1, 0); - f.changeCalc(); - f._bmj.scanAndMove(5, 0); - EXPECT_TRUE(f._bmj.done()); - EXPECT_EQUAL(4u, f.calcAsked().size()); - EXPECT_EQUAL(f._ready.bucket(2), f.calcAsked()[0]); - EXPECT_EQUAL(f._notReady.bucket(3), f.calcAsked()[1]); - EXPECT_EQUAL(f._notReady.bucket(4), f.calcAsked()[2]); - EXPECT_EQUAL(f._ready.bucket(1), f.calcAsked()[3]); + _bmj.scanAndMove(1, 0); + changeCalc(); + _bmj.scanAndMove(5, 0); + EXPECT_TRUE(_bmj.done()); + EXPECT_EQ(4u, calcAsked().size()); + EXPECT_EQ(_ready.bucket(2), calcAsked()[0]); + EXPECT_EQ(_notReady.bucket(3), calcAsked()[1]); + EXPECT_EQ(_notReady.bucket(4), calcAsked()[2]); + EXPECT_EQ(_ready.bucket(1), calcAsked()[3]); // start with bucket3 - f.changeCalc(); - f._bmj.scanAndMove(2, 0); - f.changeCalc(); - f._bmj.scanAndMove(5, 0); - EXPECT_TRUE(f._bmj.done()); - EXPECT_EQUAL(4u, f.calcAsked().size()); - EXPECT_EQUAL(f._notReady.bucket(3), f.calcAsked()[0]); - EXPECT_EQUAL(f._notReady.bucket(4), f.calcAsked()[1]); - EXPECT_EQUAL(f._ready.bucket(1), f.calcAsked()[2]); - EXPECT_EQUAL(f._ready.bucket(2), f.calcAsked()[3]); + changeCalc(); + _bmj.scanAndMove(2, 0); + changeCalc(); + _bmj.scanAndMove(5, 0); + EXPECT_TRUE(_bmj.done()); + EXPECT_EQ(4u, calcAsked().size()); + EXPECT_EQ(_notReady.bucket(3), calcAsked()[0]); + EXPECT_EQ(_notReady.bucket(4), calcAsked()[1]); + EXPECT_EQ(_ready.bucket(1), calcAsked()[2]); + EXPECT_EQ(_ready.bucket(2), calcAsked()[3]); // start with bucket4 - f.changeCalc(); - f._bmj.scanAndMove(3, 0); - f.changeCalc(); - f._bmj.scanAndMove(5, 0); - EXPECT_TRUE(f._bmj.done()); - EXPECT_EQUAL(4u, f.calcAsked().size()); - EXPECT_EQUAL(f._notReady.bucket(4), f.calcAsked()[0]); - EXPECT_EQUAL(f._ready.bucket(1), f.calcAsked()[1]); - EXPECT_EQUAL(f._ready.bucket(2), f.calcAsked()[2]); - EXPECT_EQUAL(f._notReady.bucket(3), f.calcAsked()[3]); + changeCalc(); + _bmj.scanAndMove(3, 0); + changeCalc(); + _bmj.scanAndMove(5, 0); + EXPECT_TRUE(_bmj.done()); + EXPECT_EQ(4u, calcAsked().size()); + EXPECT_EQ(_notReady.bucket(4), calcAsked()[0]); + EXPECT_EQ(_ready.bucket(1), calcAsked()[1]); + EXPECT_EQ(_ready.bucket(2), calcAsked()[2]); + EXPECT_EQ(_notReady.bucket(3), calcAsked()[3]); // start with bucket1 - f.changeCalc(); - f._bmj.scanAndMove(5, 0); - EXPECT_TRUE(f._bmj.done()); - EXPECT_EQUAL(4u, f.calcAsked().size()); - EXPECT_EQUAL(f._ready.bucket(1), f.calcAsked()[0]); - EXPECT_EQUAL(f._ready.bucket(2), f.calcAsked()[1]); - EXPECT_EQUAL(f._notReady.bucket(3), f.calcAsked()[2]); - EXPECT_EQUAL(f._notReady.bucket(4), f.calcAsked()[3]); + changeCalc(); + _bmj.scanAndMove(5, 0); + EXPECT_TRUE(_bmj.done()); + EXPECT_EQ(4u, calcAsked().size()); + EXPECT_EQ(_ready.bucket(1), calcAsked()[0]); + EXPECT_EQ(_ready.bucket(2), calcAsked()[1]); + EXPECT_EQ(_notReady.bucket(3), calcAsked()[2]); + EXPECT_EQ(_notReady.bucket(4), calcAsked()[3]); // change calc in second pass - f.changeCalc(); - f._bmj.scanAndMove(3, 0); - f.changeCalc(); - f._bmj.scanAndMove(2, 0); - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(2u, f.calcAsked().size()); - EXPECT_EQUAL(f._notReady.bucket(4), f.calcAsked()[0]); - EXPECT_EQUAL(f._ready.bucket(1), f.calcAsked()[1]); - f.changeCalc(); - f._bmj.scanAndMove(5, 0); - EXPECT_TRUE(f._bmj.done()); - EXPECT_EQUAL(4u, f.calcAsked().size()); - EXPECT_EQUAL(f._ready.bucket(2), f.calcAsked()[0]); - EXPECT_EQUAL(f._notReady.bucket(3), f.calcAsked()[1]); - EXPECT_EQUAL(f._notReady.bucket(4), f.calcAsked()[2]); - EXPECT_EQUAL(f._ready.bucket(1), f.calcAsked()[3]); + changeCalc(); + _bmj.scanAndMove(3, 0); + changeCalc(); + _bmj.scanAndMove(2, 0); + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(2u, calcAsked().size()); + EXPECT_EQ(_notReady.bucket(4), calcAsked()[0]); + EXPECT_EQ(_ready.bucket(1), calcAsked()[1]); + changeCalc(); + _bmj.scanAndMove(5, 0); + EXPECT_TRUE(_bmj.done()); + EXPECT_EQ(4u, calcAsked().size()); + EXPECT_EQ(_ready.bucket(2), calcAsked()[0]); + EXPECT_EQ(_notReady.bucket(3), calcAsked()[1]); + EXPECT_EQ(_notReady.bucket(4), calcAsked()[2]); + EXPECT_EQ(_ready.bucket(1), calcAsked()[3]); // check 1 bucket at a time, start with bucket2 - f.changeCalc(); - f._bmj.scanAndMove(1, 0); - f.changeCalc(); - f._bmj.scanAndMove(1, 0); - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(1u, f.calcAsked().size()); - EXPECT_EQUAL(f._ready.bucket(2), f.calcAsked()[0]); - f._bmj.scanAndMove(1, 0); - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(2u, f.calcAsked().size()); - EXPECT_EQUAL(f._notReady.bucket(3), f.calcAsked()[1]); - f._bmj.scanAndMove(1, 0); - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(3u, f.calcAsked().size()); - EXPECT_EQUAL(f._notReady.bucket(4), f.calcAsked()[2]); - f._bmj.scanAndMove(1, 0); - EXPECT_TRUE(f._bmj.done()); - EXPECT_EQUAL(4u, f.calcAsked().size()); - EXPECT_EQUAL(f._ready.bucket(1), f.calcAsked()[3]); + changeCalc(); + _bmj.scanAndMove(1, 0); + changeCalc(); + _bmj.scanAndMove(1, 0); + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(1u, calcAsked().size()); + EXPECT_EQ(_ready.bucket(2), calcAsked()[0]); + _bmj.scanAndMove(1, 0); + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(2u, calcAsked().size()); + EXPECT_EQ(_notReady.bucket(3), calcAsked()[1]); + _bmj.scanAndMove(1, 0); + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(3u, calcAsked().size()); + EXPECT_EQ(_notReady.bucket(4), calcAsked()[2]); + _bmj.scanAndMove(1, 0); + EXPECT_TRUE(_bmj.done()); + EXPECT_EQ(4u, calcAsked().size()); + EXPECT_EQ(_ready.bucket(1), calcAsked()[3]); } -TEST_F("require that current bucket moving is cancelled when we change calculator", ControllerFixture) +TEST_F(ControllerFixture, require_that_current_bucket_moving_is_cancelled_when_we_change_calculator) { // bucket 1 should be moved - f.addReady(f._ready.bucket(2)); - f._bmj.scanAndMove(3, 1); - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(1u, f.docsMoved().size()); - EXPECT_EQUAL(1u, f.calcAsked().size()); - f.changeCalc(); // Not cancelled, bucket 1 still moving to notReady - EXPECT_EQUAL(1u, f.calcAsked().size()); - EXPECT_EQUAL(f._ready.bucket(1), f.calcAsked()[0]); - f._calc->resetAsked(); - f._bmj.scanAndMove(2, 1); - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(1u, f.docsMoved().size()); - EXPECT_EQUAL(0u, f.calcAsked().size()); - f.addReady(f._ready.bucket(1)); - f.changeCalc(); // cancelled, bucket 1 no longer moving to notReady - EXPECT_EQUAL(1u, f.calcAsked().size()); - EXPECT_EQUAL(f._ready.bucket(1), f.calcAsked()[0]); - f._calc->resetAsked(); - f.remReady(f._ready.bucket(1)); - f.changeCalc(); // not cancelled. No active bucket move - EXPECT_EQUAL(0u, f.calcAsked().size()); - f._calc->resetAsked(); - f._bmj.scanAndMove(2, 1); - EXPECT_EQUAL(0u, f.docsMoved().size()); - EXPECT_EQUAL(2u, f.calcAsked().size()); - EXPECT_EQUAL(f._ready.bucket(2), f.calcAsked()[0]); - EXPECT_EQUAL(f._notReady.bucket(3), f.calcAsked()[1]); - f._bmj.scanAndMove(2, 3); - EXPECT_TRUE(f._bmj.done()); - EXPECT_EQUAL(3u, f.docsMoved().size()); - EXPECT_EQUAL(4u, f.calcAsked().size()); - EXPECT_EQUAL(f._notReady.bucket(4), f.calcAsked()[2]); - EXPECT_EQUAL(f._ready.bucket(1), f.calcAsked()[3]); + addReady(_ready.bucket(2)); + _bmj.scanAndMove(3, 1); + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(1u, docsMoved().size()); + EXPECT_EQ(1u, calcAsked().size()); + changeCalc(); // Not cancelled, bucket 1 still moving to notReady + EXPECT_EQ(1u, calcAsked().size()); + EXPECT_EQ(_ready.bucket(1), calcAsked()[0]); + _calc->resetAsked(); + _bmj.scanAndMove(2, 1); + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(1u, docsMoved().size()); + EXPECT_EQ(0u, calcAsked().size()); + addReady(_ready.bucket(1)); + changeCalc(); // cancelled, bucket 1 no longer moving to notReady + EXPECT_EQ(1u, calcAsked().size()); + EXPECT_EQ(_ready.bucket(1), calcAsked()[0]); + _calc->resetAsked(); + remReady(_ready.bucket(1)); + changeCalc(); // not cancelled. No active bucket move + EXPECT_EQ(0u, calcAsked().size()); + _calc->resetAsked(); + _bmj.scanAndMove(2, 1); + EXPECT_EQ(0u, docsMoved().size()); + EXPECT_EQ(2u, calcAsked().size()); + EXPECT_EQ(_ready.bucket(2), calcAsked()[0]); + EXPECT_EQ(_notReady.bucket(3), calcAsked()[1]); + _bmj.scanAndMove(2, 3); + EXPECT_TRUE(_bmj.done()); + EXPECT_EQ(3u, docsMoved().size()); + EXPECT_EQ(4u, calcAsked().size()); + EXPECT_EQ(_notReady.bucket(4), calcAsked()[2]); + EXPECT_EQ(_ready.bucket(1), calcAsked()[3]); } -TEST_F("require that last bucket is moved before reporting done", ControllerFixture) +TEST_F(ControllerFixture, require_that_last_bucket_is_moved_before_reporting_done) { // bucket 4 should be moved - f.addReady(f._ready.bucket(1)); - f.addReady(f._ready.bucket(2)); - f.addReady(f._notReady.bucket(4)); - f._bmj.scanAndMove(4, 1); - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(1u, f.docsMoved().size()); - EXPECT_EQUAL(4u, f.calcAsked().size()); - f._bmj.scanAndMove(0, 2); - EXPECT_TRUE(f._bmj.done()); - EXPECT_EQUAL(3u, f.docsMoved().size()); - EXPECT_EQUAL(4u, f.calcAsked().size()); + addReady(_ready.bucket(1)); + addReady(_ready.bucket(2)); + addReady(_notReady.bucket(4)); + _bmj.scanAndMove(4, 1); + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(1u, docsMoved().size()); + EXPECT_EQ(4u, calcAsked().size()); + _bmj.scanAndMove(0, 2); + EXPECT_TRUE(_bmj.done()); + EXPECT_EQ(3u, docsMoved().size()); + EXPECT_EQ(4u, calcAsked().size()); } -TEST_F("require that frozen bucket is not moved until thawed", ControllerFixture) +TEST_F(ControllerFixture, require_that_frozen_bucket_is_not_moved_until_thawed) { // bucket 1 should be moved but is frozen - f.addReady(f._ready.bucket(2)); - f.addFrozen(f._ready.bucket(1)); - f._bmj.scanAndMove(4, 3); // scan all, delay frozen bucket 1 - f.remFrozen(f._ready.bucket(1)); - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(0u, f.docsMoved().size()); - EXPECT_EQUAL(0u, f.bucketsModified().size()); - f._bmj.scanAndMove(0, 3); // move delayed and thawed bucket 1 - EXPECT_TRUE(f._bmj.done()); - EXPECT_EQUAL(3u, f.docsMoved().size()); - EXPECT_EQUAL(1u, f.bucketsModified().size()); - EXPECT_EQUAL(f._ready.bucket(1), f.bucketsModified()[0]); + addReady(_ready.bucket(2)); + addFrozen(_ready.bucket(1)); + _bmj.scanAndMove(4, 3); // scan all, delay frozen bucket 1 + remFrozen(_ready.bucket(1)); + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(0u, docsMoved().size()); + EXPECT_EQ(0u, bucketsModified().size()); + _bmj.scanAndMove(0, 3); // move delayed and thawed bucket 1 + EXPECT_TRUE(_bmj.done()); + EXPECT_EQ(3u, docsMoved().size()); + EXPECT_EQ(1u, bucketsModified().size()); + EXPECT_EQ(_ready.bucket(1), bucketsModified()[0]); } -TEST_F("require that thawed bucket is moved before other buckets", ControllerFixture) +TEST_F(ControllerFixture, require_that_thawed_bucket_is_moved_before_other_buckets) { // bucket 2 should be moved but is frozen. // bucket 3 & 4 should also be moved - f.addReady(f._ready.bucket(1)); - f.addReady(f._notReady.bucket(3)); - f.addReady(f._notReady.bucket(4)); - f.addFrozen(f._ready.bucket(2)); - f._bmj.scanAndMove(3, 2); // delay bucket 2, move bucket 3 - f.remFrozen(f._ready.bucket(2)); - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(2u, f.docsMoved().size()); - EXPECT_EQUAL(1u, f.bucketsModified().size()); - EXPECT_EQUAL(f._notReady.bucket(3), f.bucketsModified()[0]); - f._bmj.scanAndMove(2, 2); // move thawed bucket 2 - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(4u, f.docsMoved().size()); - EXPECT_EQUAL(2u, f.bucketsModified().size()); - EXPECT_EQUAL(f._ready.bucket(2), f.bucketsModified()[1]); - f._bmj.scanAndMove(1, 4); // move bucket 4 - EXPECT_TRUE(f._bmj.done()); - EXPECT_EQUAL(7u, f.docsMoved().size()); - EXPECT_EQUAL(3u, f.bucketsModified().size()); - EXPECT_EQUAL(f._notReady.bucket(4), f.bucketsModified()[2]); + addReady(_ready.bucket(1)); + addReady(_notReady.bucket(3)); + addReady(_notReady.bucket(4)); + addFrozen(_ready.bucket(2)); + _bmj.scanAndMove(3, 2); // delay bucket 2, move bucket 3 + remFrozen(_ready.bucket(2)); + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(2u, docsMoved().size()); + EXPECT_EQ(1u, bucketsModified().size()); + EXPECT_EQ(_notReady.bucket(3), bucketsModified()[0]); + _bmj.scanAndMove(2, 2); // move thawed bucket 2 + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(4u, docsMoved().size()); + EXPECT_EQ(2u, bucketsModified().size()); + EXPECT_EQ(_ready.bucket(2), bucketsModified()[1]); + _bmj.scanAndMove(1, 4); // move bucket 4 + EXPECT_TRUE(_bmj.done()); + EXPECT_EQ(7u, docsMoved().size()); + EXPECT_EQ(3u, bucketsModified().size()); + EXPECT_EQ(_notReady.bucket(4), bucketsModified()[2]); } -TEST_F("require that re-frozen thawed bucket is not moved until re-thawed", ControllerFixture) +TEST_F(ControllerFixture, require_that_re_frozen_thawed_bucket_is_not_moved_until_re_thawed) { // bucket 1 should be moved but is re-frozen - f.addReady(f._ready.bucket(2)); - f.addFrozen(f._ready.bucket(1)); - f._bmj.scanAndMove(1, 0); // scan, delay frozen bucket 1 - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(0u, f.docsMoved().size()); - EXPECT_EQUAL(0u, f.bucketsModified().size()); - EXPECT_EQUAL(1u, f.calcAsked().size()); - EXPECT_EQUAL(f._ready.bucket(1), f.calcAsked()[0]); - f.remFrozen(f._ready.bucket(1)); - f.addFrozen(f._ready.bucket(1)); - f._bmj.scanAndMove(1, 0); // scan, but nothing to move - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(0u, f.docsMoved().size()); - EXPECT_EQUAL(0u, f.bucketsModified().size()); - EXPECT_EQUAL(3u, f.calcAsked().size()); - EXPECT_EQUAL(f._ready.bucket(1), f.calcAsked()[1]); - EXPECT_EQUAL(f._ready.bucket(2), f.calcAsked()[2]); - f.remFrozen(f._ready.bucket(1)); - f._bmj.scanAndMove(3, 4); // move delayed and thawed bucket 1 - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(3u, f.docsMoved().size()); - EXPECT_EQUAL(1u, f.bucketsModified().size()); - EXPECT_EQUAL(f._ready.bucket(1), f.bucketsModified()[0]); - EXPECT_EQUAL(4u, f.calcAsked().size()); - EXPECT_EQUAL(f._ready.bucket(1), f.calcAsked()[3]); - f._bmj.scanAndMove(2, 0); // scan the rest - EXPECT_TRUE(f._bmj.done()); - EXPECT_EQUAL(3u, f.docsMoved().size()); - EXPECT_EQUAL(1u, f.bucketsModified().size()); - EXPECT_EQUAL(6u, f.calcAsked().size()); + addReady(_ready.bucket(2)); + addFrozen(_ready.bucket(1)); + _bmj.scanAndMove(1, 0); // scan, delay frozen bucket 1 + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(0u, docsMoved().size()); + EXPECT_EQ(0u, bucketsModified().size()); + EXPECT_EQ(1u, calcAsked().size()); + EXPECT_EQ(_ready.bucket(1), calcAsked()[0]); + remFrozen(_ready.bucket(1)); + addFrozen(_ready.bucket(1)); + _bmj.scanAndMove(1, 0); // scan, but nothing to move + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(0u, docsMoved().size()); + EXPECT_EQ(0u, bucketsModified().size()); + EXPECT_EQ(3u, calcAsked().size()); + EXPECT_EQ(_ready.bucket(1), calcAsked()[1]); + EXPECT_EQ(_ready.bucket(2), calcAsked()[2]); + remFrozen(_ready.bucket(1)); + _bmj.scanAndMove(3, 4); // move delayed and thawed bucket 1 + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(3u, docsMoved().size()); + EXPECT_EQ(1u, bucketsModified().size()); + EXPECT_EQ(_ready.bucket(1), bucketsModified()[0]); + EXPECT_EQ(4u, calcAsked().size()); + EXPECT_EQ(_ready.bucket(1), calcAsked()[3]); + _bmj.scanAndMove(2, 0); // scan the rest + EXPECT_TRUE(_bmj.done()); + EXPECT_EQ(3u, docsMoved().size()); + EXPECT_EQ(1u, bucketsModified().size()); + EXPECT_EQ(6u, calcAsked().size()); } -TEST_F("require that thawed bucket is not moved if new calculator does not say so", ControllerFixture) +TEST_F(ControllerFixture, require_that_thawed_bucket_is_not_moved_if_new_calculator_does_not_say_so) { // bucket 3 should be moved - f.addReady(f._ready.bucket(1)); - f.addReady(f._ready.bucket(2)); - f.addReady(f._notReady.bucket(3)); - f.addFrozen(f._notReady.bucket(3)); - f._bmj.scanAndMove(4, 3); // scan all, delay frozen bucket 3 - f.remFrozen(f._notReady.bucket(3)); - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(0u, f.docsMoved().size()); - EXPECT_EQUAL(0u, f.bucketsModified().size()); - EXPECT_EQUAL(4u, f.calcAsked().size()); - f.changeCalc(); - f.remReady(f._notReady.bucket(3)); - f._bmj.scanAndMove(0, 3); // consider delayed bucket 3 - EXPECT_EQUAL(0u, f.docsMoved().size()); - EXPECT_EQUAL(0u, f.bucketsModified().size()); - EXPECT_EQUAL(1u, f.calcAsked().size()); - EXPECT_EQUAL(f._notReady.bucket(3), f.calcAsked()[0]); + addReady(_ready.bucket(1)); + addReady(_ready.bucket(2)); + addReady(_notReady.bucket(3)); + addFrozen(_notReady.bucket(3)); + _bmj.scanAndMove(4, 3); // scan all, delay frozen bucket 3 + remFrozen(_notReady.bucket(3)); + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(0u, docsMoved().size()); + EXPECT_EQ(0u, bucketsModified().size()); + EXPECT_EQ(4u, calcAsked().size()); + changeCalc(); + remReady(_notReady.bucket(3)); + _bmj.scanAndMove(0, 3); // consider delayed bucket 3 + EXPECT_EQ(0u, docsMoved().size()); + EXPECT_EQ(0u, bucketsModified().size()); + EXPECT_EQ(1u, calcAsked().size()); + EXPECT_EQ(_notReady.bucket(3), calcAsked()[0]); } -TEST_F("require that current bucket mover is cancelled if bucket is frozen", ControllerFixture) +TEST_F(ControllerFixture, require_that_current_bucket_mover_is_cancelled_if_bucket_is_frozen) { // bucket 3 should be moved - f.addReady(f._ready.bucket(1)); - f.addReady(f._ready.bucket(2)); - f.addReady(f._notReady.bucket(3)); - f._bmj.scanAndMove(3, 1); // move 1 doc from bucket 3 - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(1u, f.docsMoved().size()); - EXPECT_EQUAL(0u, f.bucketsModified().size()); - EXPECT_EQUAL(3u, f.calcAsked().size()); - EXPECT_EQUAL(f._ready.bucket(1), f.calcAsked()[0]); - EXPECT_EQUAL(f._ready.bucket(2), f.calcAsked()[1]); - EXPECT_EQUAL(f._notReady.bucket(3), f.calcAsked()[2]); - - f.addFrozen(f._notReady.bucket(3)); - f._bmj.scanAndMove(1, 3); // done scanning - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(1u, f.docsMoved().size()); - EXPECT_EQUAL(0u, f.bucketsModified().size()); - EXPECT_EQUAL(3u, f.calcAsked().size()); - - f._bmj.scanAndMove(1, 3); // done scanning - f.remFrozen(f._notReady.bucket(3)); - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(1u, f.docsMoved().size()); - EXPECT_EQUAL(0u, f.bucketsModified().size()); - EXPECT_EQUAL(4u, f.calcAsked().size()); - - EXPECT_EQUAL(f._notReady.bucket(4), f.calcAsked()[3]); - f._bmj.scanAndMove(0, 2); // move all docs from bucket 3 again - EXPECT_TRUE(f._bmj.done()); - EXPECT_EQUAL(3u, f.docsMoved().size()); - EXPECT_EQUAL(1u, f.bucketsModified().size()); - EXPECT_EQUAL(f._notReady.bucket(3), f.bucketsModified()[0]); - EXPECT_EQUAL(5u, f.calcAsked().size()); - EXPECT_EQUAL(f._notReady.bucket(3), f.calcAsked()[4]); + addReady(_ready.bucket(1)); + addReady(_ready.bucket(2)); + addReady(_notReady.bucket(3)); + _bmj.scanAndMove(3, 1); // move 1 doc from bucket 3 + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(1u, docsMoved().size()); + EXPECT_EQ(0u, bucketsModified().size()); + EXPECT_EQ(3u, calcAsked().size()); + EXPECT_EQ(_ready.bucket(1), calcAsked()[0]); + EXPECT_EQ(_ready.bucket(2), calcAsked()[1]); + EXPECT_EQ(_notReady.bucket(3), calcAsked()[2]); + + addFrozen(_notReady.bucket(3)); + _bmj.scanAndMove(1, 3); // done scanning + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(1u, docsMoved().size()); + EXPECT_EQ(0u, bucketsModified().size()); + EXPECT_EQ(3u, calcAsked().size()); + + _bmj.scanAndMove(1, 3); // done scanning + remFrozen(_notReady.bucket(3)); + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(1u, docsMoved().size()); + EXPECT_EQ(0u, bucketsModified().size()); + EXPECT_EQ(4u, calcAsked().size()); + + EXPECT_EQ(_notReady.bucket(4), calcAsked()[3]); + _bmj.scanAndMove(0, 2); // move all docs from bucket 3 again + EXPECT_TRUE(_bmj.done()); + EXPECT_EQ(3u, docsMoved().size()); + EXPECT_EQ(1u, bucketsModified().size()); + EXPECT_EQ(_notReady.bucket(3), bucketsModified()[0]); + EXPECT_EQ(5u, calcAsked().size()); + EXPECT_EQ(_notReady.bucket(3), calcAsked()[4]); } -TEST_F("require that current bucket mover is not cancelled if another bucket is frozen", ControllerFixture) +TEST_F(ControllerFixture, require_that_current_bucket_mover_is_not_cancelled_if_another_bucket_is_frozen) { // bucket 3 and 4 should be moved - f.addReady(f._ready.bucket(1)); - f.addReady(f._ready.bucket(2)); - f.addReady(f._notReady.bucket(3)); - f.addReady(f._notReady.bucket(4)); - f._bmj.scanAndMove(3, 1); // move 1 doc from bucket 3 - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(1u, f.docsMoved().size()); - EXPECT_EQUAL(0u, f.bucketsModified().size()); - EXPECT_EQUAL(3u, f.calcAsked().size()); - f.addFrozen(f._notReady.bucket(4)); - f._bmj.scanAndMove(1, 2); // move rest of docs from bucket 3 - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(2u, f.docsMoved().size()); - EXPECT_EQUAL(1u, f.bucketsModified().size()); - EXPECT_EQUAL(f._notReady.bucket(3), f.bucketsModified()[0]); - EXPECT_EQUAL(3u, f.calcAsked().size()); + addReady(_ready.bucket(1)); + addReady(_ready.bucket(2)); + addReady(_notReady.bucket(3)); + addReady(_notReady.bucket(4)); + _bmj.scanAndMove(3, 1); // move 1 doc from bucket 3 + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(1u, docsMoved().size()); + EXPECT_EQ(0u, bucketsModified().size()); + EXPECT_EQ(3u, calcAsked().size()); + addFrozen(_notReady.bucket(4)); + _bmj.scanAndMove(1, 2); // move rest of docs from bucket 3 + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(2u, docsMoved().size()); + EXPECT_EQ(1u, bucketsModified().size()); + EXPECT_EQ(_notReady.bucket(3), bucketsModified()[0]); + EXPECT_EQ(3u, calcAsked().size()); } -TEST_F("require that active bucket is not moved from ready to not ready until being not active", ControllerFixture) +TEST_F(ControllerFixture, require_that_active_bucket_is_not_moved_from_ready_to_not_ready_until_being_not_active) { // bucket 1 should be moved but is active - f.addReady(f._ready.bucket(2)); - f.activateBucket(f._ready.bucket(1)); - f._bmj.scanAndMove(4, 3); // scan all, delay active bucket 1 - EXPECT_TRUE(f._bmj.done()); - EXPECT_EQUAL(0u, f.docsMoved().size()); - EXPECT_EQUAL(0u, f.bucketsModified().size()); - - f.deactivateBucket(f._ready.bucket(1)); - EXPECT_FALSE(f._bmj.done()); - f._bmj.scanAndMove(0, 3); // move delayed and de-activated bucket 1 - EXPECT_TRUE(f._bmj.done()); - EXPECT_EQUAL(3u, f.docsMoved().size()); - EXPECT_EQUAL(1u, f.bucketsModified().size()); - EXPECT_EQUAL(f._ready.bucket(1), f.bucketsModified()[0]); + addReady(_ready.bucket(2)); + activateBucket(_ready.bucket(1)); + _bmj.scanAndMove(4, 3); // scan all, delay active bucket 1 + EXPECT_TRUE(_bmj.done()); + EXPECT_EQ(0u, docsMoved().size()); + EXPECT_EQ(0u, bucketsModified().size()); + + deactivateBucket(_ready.bucket(1)); + EXPECT_FALSE(_bmj.done()); + _bmj.scanAndMove(0, 3); // move delayed and de-activated bucket 1 + EXPECT_TRUE(_bmj.done()); + EXPECT_EQ(3u, docsMoved().size()); + EXPECT_EQ(1u, bucketsModified().size()); + EXPECT_EQ(_ready.bucket(1), bucketsModified()[0]); } -TEST_F("require that de-activated bucket is moved before other buckets", OnlyReadyControllerFixture) +TEST_F(OnlyReadyControllerFixture, require_that_de_activated_bucket_is_moved_before_other_buckets) { // bucket 1, 2, 3 should be moved (but bucket 1 is active) - f.addReady(f._ready.bucket(4)); - f.activateBucket(f._ready.bucket(1)); - f._bmj.scanAndMove(2, 4); // delay bucket 1, move bucket 2 - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(2u, f.docsMoved().size()); - EXPECT_EQUAL(1u, f.bucketsModified().size()); - EXPECT_EQUAL(f._ready.bucket(2), f.bucketsModified()[0]); - - f.deactivateBucket(f._ready.bucket(1)); - f._bmj.scanAndMove(2, 4); // move de-activated bucket 1 - EXPECT_FALSE(f._bmj.done()); - EXPECT_EQUAL(3u, f.docsMoved().size()); - EXPECT_EQUAL(2u, f.bucketsModified().size()); - EXPECT_EQUAL(f._ready.bucket(1), f.bucketsModified()[1]); - - f._bmj.scanAndMove(2, 4); // move bucket 3 - // EXPECT_TRUE(f._bmj.done()); // TODO(geirst): fix this - EXPECT_EQUAL(6u, f.docsMoved().size()); - EXPECT_EQUAL(3u, f.bucketsModified().size()); - EXPECT_EQUAL(f._ready.bucket(3), f.bucketsModified()[2]); + addReady(_ready.bucket(4)); + activateBucket(_ready.bucket(1)); + _bmj.scanAndMove(2, 4); // delay bucket 1, move bucket 2 + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(2u, docsMoved().size()); + EXPECT_EQ(1u, bucketsModified().size()); + EXPECT_EQ(_ready.bucket(2), bucketsModified()[0]); + + deactivateBucket(_ready.bucket(1)); + _bmj.scanAndMove(2, 4); // move de-activated bucket 1 + EXPECT_FALSE(_bmj.done()); + EXPECT_EQ(3u, docsMoved().size()); + EXPECT_EQ(2u, bucketsModified().size()); + EXPECT_EQ(_ready.bucket(1), bucketsModified()[1]); + + _bmj.scanAndMove(2, 4); // move bucket 3 + // EXPECT_TRUE(_bmj.done()); // TODO(geirst): fix this + EXPECT_EQ(6u, docsMoved().size()); + EXPECT_EQ(3u, bucketsModified().size()); + EXPECT_EQ(_ready.bucket(3), bucketsModified()[2]); } -TEST_F("require that de-activated bucket is not moved if new calculator does not say so", ControllerFixture) +TEST_F(ControllerFixture, require_that_de_activated_bucket_is_not_moved_if_new_calculator_does_not_say_so) { // bucket 1 should be moved - f.addReady(f._ready.bucket(2)); - f.activateBucket(f._ready.bucket(1)); - f._bmj.scanAndMove(4, 3); // scan all, delay active bucket 1 - EXPECT_EQUAL(0u, f.docsMoved().size()); - EXPECT_EQUAL(0u, f.bucketsModified().size()); - - f.deactivateBucket(f._ready.bucket(1)); - f.addReady(f._ready.bucket(1)); - f.changeCalc(); - f._bmj.scanAndMove(0, 3); // consider delayed bucket 3 - EXPECT_EQUAL(0u, f.docsMoved().size()); - EXPECT_EQUAL(0u, f.bucketsModified().size()); - EXPECT_EQUAL(1u, f.calcAsked().size()); - EXPECT_EQUAL(f._ready.bucket(1), f.calcAsked()[0]); + addReady(_ready.bucket(2)); + activateBucket(_ready.bucket(1)); + _bmj.scanAndMove(4, 3); // scan all, delay active bucket 1 + EXPECT_EQ(0u, docsMoved().size()); + EXPECT_EQ(0u, bucketsModified().size()); + + deactivateBucket(_ready.bucket(1)); + addReady(_ready.bucket(1)); + changeCalc(); + _bmj.scanAndMove(0, 3); // consider delayed bucket 3 + EXPECT_EQ(0u, docsMoved().size()); + EXPECT_EQ(0u, bucketsModified().size()); + EXPECT_EQ(1u, calcAsked().size()); + EXPECT_EQ(_ready.bucket(1), calcAsked()[0]); } -TEST_F("require that de-activated bucket is not moved if frozen as well", ControllerFixture) +TEST_F(ControllerFixture, require_that_de_activated_bucket_is_not_moved_if_frozen_as_well) { // bucket 1 should be moved - f.addReady(f._ready.bucket(2)); - f.activateBucket(f._ready.bucket(1)); - f._bmj.scanAndMove(4, 3); // scan all, delay active bucket 1 - EXPECT_EQUAL(0u, f.docsMoved().size()); - EXPECT_EQUAL(0u, f.bucketsModified().size()); - - f.addFrozen(f._ready.bucket(1)); - f.deactivateBucket(f._ready.bucket(1)); - f._bmj.scanAndMove(0, 3); // bucket 1 de-activated but frozen - EXPECT_EQUAL(0u, f.docsMoved().size()); - EXPECT_EQUAL(0u, f.bucketsModified().size()); - - f.remFrozen(f._ready.bucket(1)); - f._bmj.scanAndMove(0, 3); // handle thawed bucket 1 - EXPECT_EQUAL(3u, f.docsMoved().size()); - EXPECT_EQUAL(1u, f.bucketsModified().size()); - EXPECT_EQUAL(f._ready.bucket(1), f.bucketsModified()[0]); + addReady(_ready.bucket(2)); + activateBucket(_ready.bucket(1)); + _bmj.scanAndMove(4, 3); // scan all, delay active bucket 1 + EXPECT_EQ(0u, docsMoved().size()); + EXPECT_EQ(0u, bucketsModified().size()); + + addFrozen(_ready.bucket(1)); + deactivateBucket(_ready.bucket(1)); + _bmj.scanAndMove(0, 3); // bucket 1 de-activated but frozen + EXPECT_EQ(0u, docsMoved().size()); + EXPECT_EQ(0u, bucketsModified().size()); + + remFrozen(_ready.bucket(1)); + _bmj.scanAndMove(0, 3); // handle thawed bucket 1 + EXPECT_EQ(3u, docsMoved().size()); + EXPECT_EQ(1u, bucketsModified().size()); + EXPECT_EQ(_ready.bucket(1), bucketsModified()[0]); } -TEST_F("require that thawed bucket is not moved if active as well", ControllerFixture) +TEST_F(ControllerFixture, require_that_thawed_bucket_is_not_moved_if_active_as_well) { // bucket 1 should be moved - f.addReady(f._ready.bucket(2)); - f.addFrozen(f._ready.bucket(1)); - f._bmj.scanAndMove(4, 3); // scan all, delay frozen bucket 1 - EXPECT_EQUAL(0u, f.docsMoved().size()); - EXPECT_EQUAL(0u, f.bucketsModified().size()); - - f.activateBucket(f._ready.bucket(1)); - f.remFrozen(f._ready.bucket(1)); - f._bmj.scanAndMove(0, 3); // bucket 1 thawed but active - EXPECT_EQUAL(0u, f.docsMoved().size()); - EXPECT_EQUAL(0u, f.bucketsModified().size()); - - f.deactivateBucket(f._ready.bucket(1)); - f._bmj.scanAndMove(0, 3); // handle de-activated bucket 1 - EXPECT_EQUAL(3u, f.docsMoved().size()); - EXPECT_EQUAL(1u, f.bucketsModified().size()); - EXPECT_EQUAL(f._ready.bucket(1), f.bucketsModified()[0]); + addReady(_ready.bucket(2)); + addFrozen(_ready.bucket(1)); + _bmj.scanAndMove(4, 3); // scan all, delay frozen bucket 1 + EXPECT_EQ(0u, docsMoved().size()); + EXPECT_EQ(0u, bucketsModified().size()); + + activateBucket(_ready.bucket(1)); + remFrozen(_ready.bucket(1)); + _bmj.scanAndMove(0, 3); // bucket 1 thawed but active + EXPECT_EQ(0u, docsMoved().size()); + EXPECT_EQ(0u, bucketsModified().size()); + + deactivateBucket(_ready.bucket(1)); + _bmj.scanAndMove(0, 3); // handle de-activated bucket 1 + EXPECT_EQ(3u, docsMoved().size()); + EXPECT_EQ(1u, bucketsModified().size()); + EXPECT_EQ(_ready.bucket(1), bucketsModified()[0]); } -TEST_F("ready bucket not moved to not ready if node is marked as retired", ControllerFixture) +TEST_F(ControllerFixture, ready_bucket_not_moved_to_not_ready_if_node_is_marked_as_retired) { - f._calc->setNodeRetired(true); + _calc->setNodeRetired(true); // Bucket 2 would be moved from ready to not ready in a non-retired case, but not when retired. - f.addReady(f._ready.bucket(1)); - f._bmj.scanAndMove(4, 3); - EXPECT_TRUE(f._bmj.done()); - EXPECT_EQUAL(0u, f.docsMoved().size()); + addReady(_ready.bucket(1)); + _bmj.scanAndMove(4, 3); + EXPECT_TRUE(_bmj.done()); + EXPECT_EQ(0u, docsMoved().size()); } // Technically this should never happen since a retired node is never in the ideal state, // but test this case for the sake of completion. -TEST_F("inactive not ready bucket not moved to ready if node is marked as retired", ControllerFixture) -{ - f._calc->setNodeRetired(true); - f.addReady(f._ready.bucket(1)); - f.addReady(f._ready.bucket(2)); - f.addReady(f._notReady.bucket(3)); - f._bmj.scanAndMove(4, 3); - EXPECT_TRUE(f._bmj.done()); - EXPECT_EQUAL(0u, f.docsMoved().size()); +TEST_F(ControllerFixture, inactive_not_ready_bucket_not_moved_to_ready_if_node_is_marked_as_retired) +{ + _calc->setNodeRetired(true); + addReady(_ready.bucket(1)); + addReady(_ready.bucket(2)); + addReady(_notReady.bucket(3)); + _bmj.scanAndMove(4, 3); + EXPECT_TRUE(_bmj.done()); + EXPECT_EQ(0u, docsMoved().size()); } -TEST_F("explicitly active not ready bucket can be moved to ready even if node is marked as retired", ControllerFixture) -{ - f._calc->setNodeRetired(true); - f.addReady(f._ready.bucket(1)); - f.addReady(f._ready.bucket(2)); - f.addReady(f._notReady.bucket(3)); - f.activateBucket(f._notReady.bucket(3)); - f._bmj.scanAndMove(4, 3); - EXPECT_FALSE(f._bmj.done()); - ASSERT_EQUAL(2u, f.docsMoved().size()); - assertEqual(f._notReady.bucket(3), f._notReady.docs(3)[0], 2, 1, f.docsMoved()[0]); - assertEqual(f._notReady.bucket(3), f._notReady.docs(3)[1], 2, 1, f.docsMoved()[1]); - ASSERT_EQUAL(1u, f.bucketsModified().size()); - EXPECT_EQUAL(f._notReady.bucket(3), f.bucketsModified()[0]); +TEST_F(ControllerFixture, explicitly_active_not_ready_bucket_can_be_moved_to_ready_even_if_node_is_marked_as_retired) +{ + _calc->setNodeRetired(true); + addReady(_ready.bucket(1)); + addReady(_ready.bucket(2)); + addReady(_notReady.bucket(3)); + activateBucket(_notReady.bucket(3)); + _bmj.scanAndMove(4, 3); + EXPECT_FALSE(_bmj.done()); + ASSERT_EQ(2u, docsMoved().size()); + assertEqual(_notReady.bucket(3), _notReady.docs(3)[0], 2, 1, docsMoved()[0]); + assertEqual(_notReady.bucket(3), _notReady.docs(3)[1], 2, 1, docsMoved()[1]); + ASSERT_EQ(1u, bucketsModified().size()); + EXPECT_EQ(_notReady.bucket(3), bucketsModified()[0]); } -TEST_F("require that notifyCreateBucket causes bucket to be reconsidered by job", ControllerFixture) -{ - EXPECT_FALSE(f._bmj.done()); - f.addReady(f._ready.bucket(1)); - f.addReady(f._ready.bucket(2)); - f.runLoop(); - EXPECT_TRUE(f._bmj.done()); - EXPECT_TRUE(f.docsMoved().empty()); - EXPECT_TRUE(f.bucketsModified().empty()); - f.addReady(f._notReady.bucket(3)); // bucket 3 now ready, no notify - EXPECT_TRUE(f._bmj.done()); // move job still believes work done - f._bmj.notifyCreateBucket(f._notReady.bucket(3)); // reconsider bucket 3 - EXPECT_FALSE(f._bmj.done()); - f.runLoop(); - EXPECT_TRUE(f._bmj.done()); - EXPECT_EQUAL(1u, f.bucketsModified().size()); - EXPECT_EQUAL(2u, f.docsMoved().size()); +TEST_F(ControllerFixture, require_that_notifyCreateBucket_causes_bucket_to_be_reconsidered_by_job) +{ + EXPECT_FALSE(_bmj.done()); + addReady(_ready.bucket(1)); + addReady(_ready.bucket(2)); + runLoop(); + EXPECT_TRUE(_bmj.done()); + EXPECT_TRUE(docsMoved().empty()); + EXPECT_TRUE(bucketsModified().empty()); + addReady(_notReady.bucket(3)); // bucket 3 now ready, no notify + EXPECT_TRUE(_bmj.done()); // move job still believes work done + _bmj.notifyCreateBucket(_notReady.bucket(3)); // reconsider bucket 3 + EXPECT_FALSE(_bmj.done()); + runLoop(); + EXPECT_TRUE(_bmj.done()); + EXPECT_EQ(1u, bucketsModified().size()); + EXPECT_EQ(2u, docsMoved().size()); } struct ResourceLimitControllerFixture : public ControllerFixture @@ -775,55 +776,59 @@ struct ResourceLimitControllerFixture : public ControllerFixture void testJobStopping(DiskMemUsageState blockingUsageState) { // Bucket 1 should be moved addReady(_ready.bucket(2)); - // Note: This depends on f._bmj.run() moving max 1 documents + // Note: This depends on _bmj.run() moving max 1 documents EXPECT_TRUE(!_bmj.run()); - EXPECT_EQUAL(1u, docsMoved().size()); - EXPECT_EQUAL(0u, bucketsModified().size()); + EXPECT_EQ(1u, docsMoved().size()); + EXPECT_EQ(0u, bucketsModified().size()); // Notify that we've over limit _diskMemUsageNotifier.notify(blockingUsageState); EXPECT_TRUE(_bmj.run()); - EXPECT_EQUAL(1u, docsMoved().size()); - EXPECT_EQUAL(0u, bucketsModified().size()); + EXPECT_EQ(1u, docsMoved().size()); + EXPECT_EQ(0u, bucketsModified().size()); // Notify that we've under limit _diskMemUsageNotifier.notify(DiskMemUsageState()); EXPECT_TRUE(!_bmj.run()); - EXPECT_EQUAL(2u, docsMoved().size()); - EXPECT_EQUAL(0u, bucketsModified().size()); + EXPECT_EQ(2u, docsMoved().size()); + EXPECT_EQ(0u, bucketsModified().size()); } void testJobNotStopping(DiskMemUsageState blockingUsageState) { // Bucket 1 should be moved addReady(_ready.bucket(2)); - // Note: This depends on f._bmj.run() moving max 1 documents + // Note: This depends on _bmj.run() moving max 1 documents EXPECT_TRUE(!_bmj.run()); - EXPECT_EQUAL(1u, docsMoved().size()); - EXPECT_EQUAL(0u, bucketsModified().size()); + EXPECT_EQ(1u, docsMoved().size()); + EXPECT_EQ(0u, bucketsModified().size()); // Notify that we've over limit, but not over adjusted limit _diskMemUsageNotifier.notify(blockingUsageState); EXPECT_TRUE(!_bmj.run()); - EXPECT_EQUAL(2u, docsMoved().size()); - EXPECT_EQUAL(0u, bucketsModified().size()); + EXPECT_EQ(2u, docsMoved().size()); + EXPECT_EQ(0u, bucketsModified().size()); } }; -TEST_F("require that bucket move stops when disk limit is reached", ResourceLimitControllerFixture) +struct ResourceLimitControllerFixture_1_2 : public ResourceLimitControllerFixture { + ResourceLimitControllerFixture_1_2() : ResourceLimitControllerFixture(1.2) {} +}; + +TEST_F(ResourceLimitControllerFixture, require_that_bucket_move_stops_when_disk_limit_is_reached) { - f.testJobStopping(DiskMemUsageState(ResourceUsageState(0.7, 0.8), ResourceUsageState())); + testJobStopping(DiskMemUsageState(ResourceUsageState(0.7, 0.8), ResourceUsageState())); } -TEST_F("require that bucket move stops when memory limit is reached", ResourceLimitControllerFixture) +TEST_F(ResourceLimitControllerFixture, require_that_bucket_move_stops_when_memory_limit_is_reached) { - f.testJobStopping(DiskMemUsageState(ResourceUsageState(), ResourceUsageState(0.7, 0.8))); + testJobStopping(DiskMemUsageState(ResourceUsageState(), ResourceUsageState(0.7, 0.8))); } -TEST_F("require that bucket move uses resource limit factor for disk resource limit", ResourceLimitControllerFixture(1.2)) +TEST_F(ResourceLimitControllerFixture_1_2, require_that_bucket_move_uses_resource_limit_factor_for_disk_resource_limit) { - f.testJobNotStopping(DiskMemUsageState(ResourceUsageState(0.7, 0.8), ResourceUsageState())); + testJobNotStopping(DiskMemUsageState(ResourceUsageState(0.7, 0.8), ResourceUsageState())); } -TEST_F("require that bucket move uses resource limit factor for memory resource limit", ResourceLimitControllerFixture(1.2)) +TEST_F(ResourceLimitControllerFixture_1_2, require_that_bucket_move_uses_resource_limit_factor_for_memory_resource_limit) { - f.testJobNotStopping(DiskMemUsageState(ResourceUsageState(), ResourceUsageState(0.7, 0.8))); + testJobNotStopping(DiskMemUsageState(ResourceUsageState(), ResourceUsageState(0.7, 0.8))); } struct MaxOutstandingMoveOpsFixture : public ControllerFixture @@ -852,54 +857,58 @@ struct MaxOutstandingMoveOpsFixture : public ControllerFixture EXPECT_FALSE(_bmj.isBlocked()); } void assertDocsMoved(uint32_t expDocsMovedCnt, uint32_t expMoveContextsCnt) { - EXPECT_EQUAL(expDocsMovedCnt, docsMoved().size()); - EXPECT_EQUAL(expMoveContextsCnt, _moveHandler._moveDoneContexts.size()); + EXPECT_EQ(expDocsMovedCnt, docsMoved().size()); + EXPECT_EQ(expMoveContextsCnt, _moveHandler._moveDoneContexts.size()); } void unblockJob(uint32_t expRunnerCnt) { _moveHandler.clearMoveDoneContexts(); // unblocks job and try to execute it via runner - EXPECT_EQUAL(expRunnerCnt, _runner.runCount); + EXPECT_EQ(expRunnerCnt, _runner.runCount); EXPECT_FALSE(_bmj.isBlocked()); } +}; +struct MaxOutstandingMoveOpsFixture_1 : public MaxOutstandingMoveOpsFixture { + MaxOutstandingMoveOpsFixture_1() : MaxOutstandingMoveOpsFixture(1) {} }; -TEST_F("require that bucket move job is blocked if it has too many outstanding move operations (max=1)", MaxOutstandingMoveOpsFixture(1)) +struct MaxOutstandingMoveOpsFixture_2 : public MaxOutstandingMoveOpsFixture { + MaxOutstandingMoveOpsFixture_2() : MaxOutstandingMoveOpsFixture(2) {} +}; + +TEST_F(MaxOutstandingMoveOpsFixture_1, require_that_bucket_move_job_is_blocked_if_it_has_too_many_outstanding_move_operations__max_1) { - TEST_DO(f.assertRunToBlocked()); - TEST_DO(f.assertDocsMoved(1, 1)); - TEST_DO(f.assertRunToBlocked()); - TEST_DO(f.assertDocsMoved(1, 1)); + assertRunToBlocked(); + assertDocsMoved(1, 1); + assertRunToBlocked(); + assertDocsMoved(1, 1); - TEST_DO(f.unblockJob(1)); - TEST_DO(f.assertRunToBlocked()); - TEST_DO(f.assertDocsMoved(2, 1)); + unblockJob(1); + assertRunToBlocked(); + assertDocsMoved(2, 1); - TEST_DO(f.unblockJob(2)); - TEST_DO(f.assertRunToBlocked()); - TEST_DO(f.assertDocsMoved(3, 1)); + unblockJob(2); + assertRunToBlocked(); + assertDocsMoved(3, 1); - TEST_DO(f.unblockJob(3)); - TEST_DO(f.assertRunToFinished()); - TEST_DO(f.assertDocsMoved(3, 0)); + unblockJob(3); + assertRunToFinished(); + assertDocsMoved(3, 0); } -TEST_F("require that bucket move job is blocked if it has too many outstanding move operations (max=2)", MaxOutstandingMoveOpsFixture(2)) +TEST_F(MaxOutstandingMoveOpsFixture_2, require_that_bucket_move_job_is_blocked_if_it_has_too_many_outstanding_move_operations_max_2) { - TEST_DO(f.assertRunToNotBlocked()); - TEST_DO(f.assertDocsMoved(1, 1)); + assertRunToNotBlocked(); + assertDocsMoved(1, 1); - TEST_DO(f.assertRunToBlocked()); - TEST_DO(f.assertDocsMoved(2, 2)); + assertRunToBlocked(); + assertDocsMoved(2, 2); - TEST_DO(f.unblockJob(1)); - TEST_DO(f.assertRunToNotBlocked()); - TEST_DO(f.assertDocsMoved(3, 1)); + unblockJob(1); + assertRunToNotBlocked(); + assertDocsMoved(3, 1); - TEST_DO(f.assertRunToFinished()); - TEST_DO(f.assertDocsMoved(3, 1)); + assertRunToFinished(); + assertDocsMoved(3, 1); } -TEST_MAIN() -{ - TEST_RUN_ALL(); -} +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/documentmover_test.cpp b/searchcore/src/tests/proton/documentdb/documentbucketmover/documentmover_test.cpp index 8b304bba4da..565050f1052 100644 --- a/searchcore/src/tests/proton/documentdb/documentbucketmover/documentmover_test.cpp +++ b/searchcore/src/tests/proton/documentdb/documentbucketmover/documentmover_test.cpp @@ -1,7 +1,7 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "bucketmover_common.h" -#include <vespa/vespalib/testkit/testapp.h> +#include <vespa/vespalib/gtest/gtest.h> #include <vespa/log/log.h> LOG_SETUP("document_bucket_mover_test"); @@ -21,14 +21,14 @@ struct MySubDbTwoBuckets : public MySubDb builder.createDocs(1, 1, 6); builder.createDocs(2, 6, 9); insertDocs(builder.getDocs()); - ASSERT_NOT_EQUAL(bucket(1), bucket(2)); - ASSERT_EQUAL(5u, docs(1).size()); - ASSERT_EQUAL(3u, docs(2).size()); - ASSERT_EQUAL(9u, _realRetriever->_docs.size()); + assert(bucket(1) != bucket(2)); + assert(5u == docs(1).size()); + assert(3u == docs(2).size()); + assert(9u == _realRetriever->_docs.size()); } }; -struct MoveFixture +struct DocumentMoverTest : ::testing::Test { test::UserDocumentsBuilder _builder; std::shared_ptr<BucketDBOwner> _bucketDB; @@ -38,7 +38,7 @@ struct MoveFixture BucketDBOwner _bucketDb; MyMoveHandler _handler; PendingLidTracker _pendingLidsForCommit; - MoveFixture() + DocumentMoverTest() : _builder(), _bucketDB(std::make_shared<BucketDBOwner>()), _limiter(), @@ -64,7 +64,7 @@ struct MoveFixture } }; -TEST("require that initial bucket mover is done") +TEST_F(DocumentMoverTest, require_that_initial_bucket_mover_is_done) { MyMoveOperationLimiter limiter; DocumentBucketMover mover(limiter); @@ -73,68 +73,65 @@ TEST("require that initial bucket mover is done") EXPECT_TRUE(mover.bucketDone()); } -TEST_F("require that we can move all documents", MoveFixture) +TEST_F(DocumentMoverTest, require_that_we_can_move_all_documents) { - f.setupForBucket(f._source.bucket(1), 6, 9); - EXPECT_TRUE(f.moveDocuments(5)); - EXPECT_TRUE(f._mover.bucketDone()); - EXPECT_EQUAL(5u, f._handler._moves.size()); - EXPECT_EQUAL(5u, f._limiter.beginOpCount); + setupForBucket(_source.bucket(1), 6, 9); + EXPECT_TRUE(moveDocuments(5)); + EXPECT_TRUE(_mover.bucketDone()); + EXPECT_EQ(5u, _handler._moves.size()); + EXPECT_EQ(5u, _limiter.beginOpCount); for (size_t i = 0; i < 5u; ++i) { - assertEqual(f._source.bucket(1), f._source.docs(1)[0], 6, 9, f._handler._moves[0]); + assertEqual(_source.bucket(1), _source.docs(1)[0], 6, 9, _handler._moves[0]); } } -TEST_F("require that move is stalled if document is pending commit", MoveFixture) +TEST_F(DocumentMoverTest, require_that_move_is_stalled_if_document_is_pending_commit) { - f.setupForBucket(f._source.bucket(1), 6, 9); + setupForBucket(_source.bucket(1), 6, 9); { - IPendingLidTracker::Token token = f._pendingLidsForCommit.produce(1); - EXPECT_FALSE(f.moveDocuments(5)); - EXPECT_FALSE(f._mover.bucketDone()); + IPendingLidTracker::Token token = _pendingLidsForCommit.produce(1); + EXPECT_FALSE(moveDocuments(5)); + EXPECT_FALSE(_mover.bucketDone()); } - EXPECT_TRUE(f.moveDocuments(5)); - EXPECT_TRUE(f._mover.bucketDone()); - EXPECT_EQUAL(5u, f._handler._moves.size()); - EXPECT_EQUAL(5u, f._limiter.beginOpCount); + EXPECT_TRUE(moveDocuments(5)); + EXPECT_TRUE(_mover.bucketDone()); + EXPECT_EQ(5u, _handler._moves.size()); + EXPECT_EQ(5u, _limiter.beginOpCount); for (size_t i = 0; i < 5u; ++i) { - assertEqual(f._source.bucket(1), f._source.docs(1)[0], 6, 9, f._handler._moves[0]); + assertEqual(_source.bucket(1), _source.docs(1)[0], 6, 9, _handler._moves[0]); } } -TEST_F("require that bucket is cached when IDocumentMoveHandler handles move operation", MoveFixture) +TEST_F(DocumentMoverTest, require_that_bucket_is_cached_when_IDocumentMoveHandler_handles_move_operation) { - f.setupForBucket(f._source.bucket(1), 6, 9); - EXPECT_TRUE(f.moveDocuments(5)); - EXPECT_TRUE(f._mover.bucketDone()); - EXPECT_EQUAL(5u, f._handler._moves.size()); - EXPECT_EQUAL(5u, f._handler._numCachedBuckets); - EXPECT_FALSE(f._bucketDb.takeGuard()->isCachedBucket(f._source.bucket(1))); + setupForBucket(_source.bucket(1), 6, 9); + EXPECT_TRUE(moveDocuments(5)); + EXPECT_TRUE(_mover.bucketDone()); + EXPECT_EQ(5u, _handler._moves.size()); + EXPECT_EQ(5u, _handler._numCachedBuckets); + EXPECT_FALSE(_bucketDb.takeGuard()->isCachedBucket(_source.bucket(1))); } -TEST_F("require that we can move documents in several steps", MoveFixture) +TEST_F(DocumentMoverTest, require_that_we_can_move_documents_in_several_steps) { - f.setupForBucket(f._source.bucket(1), 6, 9); - f.moveDocuments(2); - EXPECT_FALSE(f._mover.bucketDone()); - EXPECT_EQUAL(2u, f._handler._moves.size()); - assertEqual(f._source.bucket(1), f._source.docs(1)[0], 6, 9, f._handler._moves[0]); - assertEqual(f._source.bucket(1), f._source.docs(1)[1], 6, 9, f._handler._moves[1]); - EXPECT_TRUE(f.moveDocuments(2)); - EXPECT_FALSE(f._mover.bucketDone()); - EXPECT_EQUAL(4u, f._handler._moves.size()); - assertEqual(f._source.bucket(1), f._source.docs(1)[2], 6, 9, f._handler._moves[2]); - assertEqual(f._source.bucket(1), f._source.docs(1)[3], 6, 9, f._handler._moves[3]); - EXPECT_TRUE(f.moveDocuments(2)); - EXPECT_TRUE(f._mover.bucketDone()); - EXPECT_EQUAL(5u, f._handler._moves.size()); - assertEqual(f._source.bucket(1), f._source.docs(1)[4], 6, 9, f._handler._moves[4]); - EXPECT_TRUE(f.moveDocuments(2)); - EXPECT_TRUE(f._mover.bucketDone()); - EXPECT_EQUAL(5u, f._handler._moves.size()); + setupForBucket(_source.bucket(1), 6, 9); + moveDocuments(2); + EXPECT_FALSE(_mover.bucketDone()); + EXPECT_EQ(2u, _handler._moves.size()); + assertEqual(_source.bucket(1), _source.docs(1)[0], 6, 9, _handler._moves[0]); + assertEqual(_source.bucket(1), _source.docs(1)[1], 6, 9, _handler._moves[1]); + EXPECT_TRUE(moveDocuments(2)); + EXPECT_FALSE(_mover.bucketDone()); + EXPECT_EQ(4u, _handler._moves.size()); + assertEqual(_source.bucket(1), _source.docs(1)[2], 6, 9, _handler._moves[2]); + assertEqual(_source.bucket(1), _source.docs(1)[3], 6, 9, _handler._moves[3]); + EXPECT_TRUE(moveDocuments(2)); + EXPECT_TRUE(_mover.bucketDone()); + EXPECT_EQ(5u, _handler._moves.size()); + assertEqual(_source.bucket(1), _source.docs(1)[4], 6, 9, _handler._moves[4]); + EXPECT_TRUE(moveDocuments(2)); + EXPECT_TRUE(_mover.bucketDone()); + EXPECT_EQ(5u, _handler._moves.size()); } -TEST_MAIN() -{ - TEST_RUN_ALL(); -} +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/scaniterator_test.cpp b/searchcore/src/tests/proton/documentdb/documentbucketmover/scaniterator_test.cpp index 5da5d5ff21e..75f5f7c7427 100644 --- a/searchcore/src/tests/proton/documentdb/documentbucketmover/scaniterator_test.cpp +++ b/searchcore/src/tests/proton/documentdb/documentbucketmover/scaniterator_test.cpp @@ -1,7 +1,7 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "bucketmover_common.h" -#include <vespa/vespalib/testkit/testapp.h> +#include <vespa/vespalib/gtest/gtest.h> #include <vespa/log/log.h> LOG_SETUP("document_bucket_mover_test"); @@ -13,14 +13,14 @@ using document::BucketId; using ScanItr = bucketdb::ScanIterator; using ScanPass = ScanItr::Pass; -struct ScanFixtureBase +struct ScanTestBase : public ::testing::Test { test::UserDocumentsBuilder _builder; std::shared_ptr<BucketDBOwner> _bucketDB; MySubDb _ready; MySubDb _notReady; - ScanFixtureBase(); - ~ScanFixtureBase(); + ScanTestBase(); + ~ScanTestBase(); ScanItr getItr() { return ScanItr(_bucketDB->takeGuard(), BucketId()); @@ -31,17 +31,17 @@ struct ScanFixtureBase } }; -ScanFixtureBase::ScanFixtureBase() +ScanTestBase::ScanTestBase() : _builder(), _bucketDB(std::make_shared<BucketDBOwner>()), _ready(_builder.getRepo(), _bucketDB, 1, SubDbType::READY), _notReady(_builder.getRepo(), _bucketDB, 2, SubDbType::NOTREADY) {} -ScanFixtureBase::~ScanFixtureBase() = default; +ScanTestBase::~ScanTestBase() = default; -struct ScanFixture : public ScanFixtureBase +struct ScanTest : public ScanTestBase { - ScanFixture() : ScanFixtureBase() + ScanTest() : ScanTestBase() { _builder.createDocs(6, 1, 2); _builder.createDocs(8, 2, 3); @@ -54,9 +54,9 @@ struct ScanFixture : public ScanFixtureBase } }; -struct OnlyNotReadyScanFixture : public ScanFixtureBase +struct OnlyNotReadyScanTest : public ScanTestBase { - OnlyNotReadyScanFixture() : ScanFixtureBase() + OnlyNotReadyScanTest() : ScanTestBase() { _builder.createDocs(2, 1, 2); _builder.createDocs(4, 2, 3); @@ -64,9 +64,9 @@ struct OnlyNotReadyScanFixture : public ScanFixtureBase } }; -struct OnlyReadyScanFixture : public ScanFixtureBase +struct OnlyReadyScanTest : public ScanTestBase { - OnlyReadyScanFixture() : ScanFixtureBase() + OnlyReadyScanTest() : ScanTestBase() { _builder.createDocs(6, 1, 2); _builder.createDocs(8, 2, 3); @@ -103,94 +103,91 @@ void assertEquals(const BucketVector &exp, ScanItr &itr, SubDbType subDbType) for (size_t i = 0; i < exp.size(); ++i) { advanceToFirstBucketWithDocs(itr, subDbType); EXPECT_TRUE(itr.valid()); - EXPECT_EQUAL(exp[i], itr.getBucket()); + EXPECT_EQ(exp[i], itr.getBucket()); ++itr; } advanceToFirstBucketWithDocs(itr, subDbType); EXPECT_FALSE(itr.valid()); } -TEST_F("require that we can iterate all buckets from start to end", ScanFixture) +TEST_F(ScanTest, require_that_we_can_iterate_all_buckets_from_start_to_end) { { - ScanItr itr = f.getItr(); + ScanItr itr = getItr(); assertEquals(BucketVector(). - add(f._notReady.bucket(2)). - add(f._notReady.bucket(4)), itr, SubDbType::NOTREADY); + add(_notReady.bucket(2)). + add(_notReady.bucket(4)), itr, SubDbType::NOTREADY); } { - ScanItr itr = f.getItr(); + ScanItr itr = getItr(); assertEquals(BucketVector(). - add(f._ready.bucket(6)). - add(f._ready.bucket(8)), itr, SubDbType::READY); + add(_ready.bucket(6)). + add(_ready.bucket(8)), itr, SubDbType::READY); } } -TEST_F("require that we can iterate from the middle of not ready buckets", ScanFixture) +TEST_F(ScanTest, require_that_we_can_iterate_from_the_middle_of_not_ready_buckets) { - BucketId bucket = f._notReady.bucket(2); + BucketId bucket = _notReady.bucket(2); { - ScanItr itr = f.getItr(bucket, bucket, ScanPass::FIRST); + ScanItr itr = getItr(bucket, bucket, ScanPass::FIRST); assertEquals(BucketVector(). - add(f._notReady.bucket(4)), itr, SubDbType::NOTREADY); + add(_notReady.bucket(4)), itr, SubDbType::NOTREADY); } { - ScanItr itr = f.getItr(BucketId(), bucket, ScanPass::SECOND); + ScanItr itr = getItr(BucketId(), bucket, ScanPass::SECOND); assertEquals(BucketVector(). - add(f._notReady.bucket(2)), itr, SubDbType::NOTREADY); + add(_notReady.bucket(2)), itr, SubDbType::NOTREADY); } { - ScanItr itr = f.getItr(); + ScanItr itr = getItr(); assertEquals(BucketVector(). - add(f._ready.bucket(6)). - add(f._ready.bucket(8)), itr, SubDbType::READY); + add(_ready.bucket(6)). + add(_ready.bucket(8)), itr, SubDbType::READY); } } -TEST_F("require that we can iterate from the middle of ready buckets", ScanFixture) +TEST_F(ScanTest, require_that_we_can_iterate_from_the_middle_of_ready_buckets) { - BucketId bucket = f._ready.bucket(6); + BucketId bucket = _ready.bucket(6); { - ScanItr itr = f.getItr(); + ScanItr itr = getItr(); assertEquals(BucketVector(). - add(f._notReady.bucket(2)). - add(f._notReady.bucket(4)), itr, SubDbType::NOTREADY); + add(_notReady.bucket(2)). + add(_notReady.bucket(4)), itr, SubDbType::NOTREADY); } { - ScanItr itr = f.getItr(bucket, bucket, ScanPass::FIRST); + ScanItr itr = getItr(bucket, bucket, ScanPass::FIRST); assertEquals(BucketVector(). - add(f._ready.bucket(8)), itr, SubDbType::READY); + add(_ready.bucket(8)), itr, SubDbType::READY); } { - ScanItr itr = f.getItr(BucketId(), bucket, ScanPass::SECOND); + ScanItr itr = getItr(BucketId(), bucket, ScanPass::SECOND); assertEquals(BucketVector(). - add(f._ready.bucket(6)), itr, SubDbType::READY); + add(_ready.bucket(6)), itr, SubDbType::READY); } } -TEST_F("require that we can iterate only not ready buckets", OnlyNotReadyScanFixture) +TEST_F(OnlyNotReadyScanTest, require_that_we_can_iterate_only_not_ready_buckets) { - ScanItr itr = f.getItr(); + ScanItr itr = getItr(); assertEquals(BucketVector(). - add(f._notReady.bucket(2)). - add(f._notReady.bucket(4)), itr, SubDbType::NOTREADY); + add(_notReady.bucket(2)). + add(_notReady.bucket(4)), itr, SubDbType::NOTREADY); } -TEST_F("require that we can iterate only ready buckets", OnlyReadyScanFixture) +TEST_F(OnlyReadyScanTest, require_that_we_can_iterate_only_ready_buckets) { - ScanItr itr = f.getItr(); + ScanItr itr = getItr(); assertEquals(BucketVector(). - add(f._ready.bucket(6)). - add(f._ready.bucket(8)), itr, SubDbType::READY); + add(_ready.bucket(6)). + add(_ready.bucket(8)), itr, SubDbType::READY); } -TEST_F("require that we can iterate zero buckets", ScanFixtureBase) +TEST_F(ScanTestBase, require_that_we_can_iterate_zero_buckets) { - ScanItr itr = f.getItr(); + ScanItr itr = getItr(); EXPECT_FALSE(itr.valid()); } -TEST_MAIN() -{ - TEST_RUN_ALL(); -} +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_handler_test.cpp b/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_handler_test.cpp index 2603c041db0..993cb9226c5 100644 --- a/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_handler_test.cpp +++ b/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_handler_test.cpp @@ -57,4 +57,4 @@ TEST_F(HandlerTest, createMoveOperation_works_as_expected) EXPECT_EQ(timestamp, op->getTimestamp()); } -GTEST_MAIN_RUN_ALL_TESTS()
\ No newline at end of file +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/storage/src/tests/persistence/filestorage/service_layer_host_info_reporter_test.cpp b/storage/src/tests/persistence/filestorage/service_layer_host_info_reporter_test.cpp index a54c5c8ccf9..e5ba04f789b 100644 --- a/storage/src/tests/persistence/filestorage/service_layer_host_info_reporter_test.cpp +++ b/storage/src/tests/persistence/filestorage/service_layer_host_info_reporter_test.cpp @@ -50,6 +50,9 @@ struct ServiceLayerHostInfoReporterTest : ::testing::Test { void notify(double disk_usage, double memory_usage) { notify(disk_usage, memory_usage, {0.0, ""}, {0.0, ""}); } + void set_noise_level(double level) { + _reporter.set_noise_level(level); + } size_t requested_almost_immediate_replies() { return _state_manager.requested_almost_immediate_node_state_replies(); } ResourceUsage get_old_usage() { return _reporter.get_old_resource_usage(); } @@ -102,26 +105,45 @@ TEST_F(ServiceLayerHostInfoReporterTest, request_almost_immediate_node_state_as_ EXPECT_EQ(ResourceUsage(0.8, 0.7, {0.1, attr_es_name}, {0.2, attr_mv_name}), get_usage()); } +TEST_F(ServiceLayerHostInfoReporterTest, can_set_noise_level) +{ + set_noise_level(0.02); + notify(0.5, 0.4); + EXPECT_EQ(1, requested_almost_immediate_replies()); + EXPECT_EQ(ResourceUsage(0.5, 0.4), get_old_usage()); + EXPECT_EQ(ResourceUsage(0.5, 0.4), get_usage()); + // the difference in disk usage is below the noise level + notify(0.519, 0.4); + EXPECT_EQ(1, requested_almost_immediate_replies()); + EXPECT_EQ(ResourceUsage(0.5, 0.4), get_old_usage()); + EXPECT_EQ(ResourceUsage(0.519, 0.4), get_usage()); + // the difference in disk usage is above the noise level + notify(0.521, 0.4); + EXPECT_EQ(2, requested_almost_immediate_replies()); + EXPECT_EQ(ResourceUsage(0.521, 0.4), get_old_usage()); + EXPECT_EQ(ResourceUsage(0.521, 0.4), get_usage()); +} + TEST_F(ServiceLayerHostInfoReporterTest, - first_valid_attribute_enum_store_sample_triggers_immediate_node_state_when_below_slack_diff) + first_valid_attribute_enum_store_sample_triggers_immediate_node_state_when_below_noise_level) { - // TODO: Assert this is below slack diff when that becomes configurable. - constexpr double usage_below_slack_diff = 0.00001; - notify(0.0, 0.0, {usage_below_slack_diff, attr_es_name}, {}); + set_noise_level(0.02); + constexpr double usage_below_noise_level = 0.019; + notify(0.0, 0.0, {usage_below_noise_level, attr_es_name}, {}); EXPECT_EQ(1, requested_almost_immediate_replies()); - EXPECT_EQ(ResourceUsage(0.0, 0.0, {usage_below_slack_diff, attr_es_name}, {}), get_old_usage()); - EXPECT_EQ(ResourceUsage(0.0, 0.0, {usage_below_slack_diff, attr_es_name}, {}), get_usage()); + EXPECT_EQ(ResourceUsage(0.0, 0.0, {usage_below_noise_level, attr_es_name}, {}), get_old_usage()); + EXPECT_EQ(ResourceUsage(0.0, 0.0, {usage_below_noise_level, attr_es_name}, {}), get_usage()); } TEST_F(ServiceLayerHostInfoReporterTest, - first_valid_attribute_multi_value_sample_triggers_immediate_node_state_when_below_slack_diff) + first_valid_attribute_multi_value_sample_triggers_immediate_node_state_when_below_noise_level) { - // TODO: Assert this is below slack diff when that becomes configurable. - constexpr double usage_below_slack_diff = 0.00001; - notify(0.0, 0.0, {}, {usage_below_slack_diff, attr_mv_name}); + set_noise_level(0.02); + constexpr double usage_below_noise_level = 0.019; + notify(0.0, 0.0, {}, {usage_below_noise_level, attr_mv_name}); EXPECT_EQ(1, requested_almost_immediate_replies()); - EXPECT_EQ(ResourceUsage(0.0, 0.0, {}, {usage_below_slack_diff, attr_mv_name}), get_old_usage()); - EXPECT_EQ(ResourceUsage(0.0, 0.0, {}, {usage_below_slack_diff, attr_mv_name}), get_usage()); + EXPECT_EQ(ResourceUsage(0.0, 0.0, {}, {usage_below_noise_level, attr_mv_name}), get_old_usage()); + EXPECT_EQ(ResourceUsage(0.0, 0.0, {}, {usage_below_noise_level, attr_mv_name}), get_usage()); } TEST_F(ServiceLayerHostInfoReporterTest, json_report_generated) diff --git a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp index 7c175686359..eee509e79f8 100644 --- a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp +++ b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp @@ -190,6 +190,7 @@ FileStorManager::configure(std::unique_ptr<vespa::config::content::StorFilestorC bool liveUpdate = ! _threads.empty(); _use_async_message_handling_on_schedule = config->useAsyncMessageHandlingOnSchedule; + _host_info_reporter.set_noise_level(config->resourceUsageReporterNoiseLevel); if (!liveUpdate) { _config = std::move(config); diff --git a/storage/src/vespa/storage/persistence/filestorage/service_layer_host_info_reporter.cpp b/storage/src/vespa/storage/persistence/filestorage/service_layer_host_info_reporter.cpp index 88cdcc2b42f..97244582f50 100644 --- a/storage/src/vespa/storage/persistence/filestorage/service_layer_host_info_reporter.cpp +++ b/storage/src/vespa/storage/persistence/filestorage/service_layer_host_info_reporter.cpp @@ -16,8 +16,6 @@ using End = vespalib::JsonStream::End; namespace { -constexpr double diff_slack = 0.001; - const vespalib::string memory_label("memory"); const vespalib::string disk_label("disk"); const vespalib::string attribute_enum_store_label("attribute-enum-store"); @@ -38,7 +36,7 @@ void write_attribute_usage(vespalib::JsonStream& output, const vespalib::string output << End(); } -bool want_immediate_report(const spi::ResourceUsage& old_usage, const spi::ResourceUsage& new_usage) +bool want_immediate_report(const spi::ResourceUsage& old_usage, const spi::ResourceUsage& new_usage, double noise_level) { auto disk_usage_diff = fabs(new_usage.get_disk_usage() - old_usage.get_disk_usage()); auto memory_usage_diff = fabs(new_usage.get_memory_usage() - old_usage.get_memory_usage()); @@ -48,22 +46,24 @@ bool want_immediate_report(const spi::ResourceUsage& old_usage, const spi::Resou new_usage.get_attribute_enum_store_usage().valid(); bool multivalue_got_valid = !old_usage.get_attribute_multivalue_usage().valid() && new_usage.get_attribute_multivalue_usage().valid(); - return ((disk_usage_diff > diff_slack) || - (memory_usage_diff > diff_slack) || - (enum_store_diff > diff_slack) || - (multivalue_diff > diff_slack) || + return ((disk_usage_diff > noise_level) || + (memory_usage_diff > noise_level) || + (enum_store_diff > noise_level) || + (multivalue_diff > noise_level) || enum_store_got_valid || multivalue_got_valid); } } -ServiceLayerHostInfoReporter::ServiceLayerHostInfoReporter(NodeStateUpdater& node_state_updater) +ServiceLayerHostInfoReporter::ServiceLayerHostInfoReporter(NodeStateUpdater& node_state_updater, + double noise_level) : HostReporter(), spi::ResourceUsageListener(), _node_state_updater(node_state_updater), _lock(), - _old_resource_usage() + _old_resource_usage(), + _noise_level(noise_level) { } @@ -72,6 +72,12 @@ ServiceLayerHostInfoReporter::~ServiceLayerHostInfoReporter() spi::ResourceUsageListener::reset(); // detach } +void +ServiceLayerHostInfoReporter::set_noise_level(double level) +{ + _noise_level.store(level, std::memory_order_relaxed); +} + namespace { vespalib::string @@ -87,9 +93,10 @@ to_string(const spi::ResourceUsage& usage) void ServiceLayerHostInfoReporter::update_resource_usage(const spi::ResourceUsage& resource_usage) { - bool immediate_report = want_immediate_report(_old_resource_usage, resource_usage); - LOG(debug, "update_resource_usage(): immediate_report=%s, old_usage=%s, new_usage=%s", - (immediate_report ? "true" : "false"), to_string(_old_resource_usage).c_str(), + double noise_level = _noise_level.load(std::memory_order_relaxed); + bool immediate_report = want_immediate_report(_old_resource_usage, resource_usage, noise_level); + LOG(debug, "update_resource_usage(): immediate_report=%s, noise_level=%f, old_usage=%s, new_usage=%s", + (immediate_report ? "true" : "false"), noise_level, to_string(_old_resource_usage).c_str(), to_string(resource_usage).c_str()); if (immediate_report) { _old_resource_usage = resource_usage; diff --git a/storage/src/vespa/storage/persistence/filestorage/service_layer_host_info_reporter.h b/storage/src/vespa/storage/persistence/filestorage/service_layer_host_info_reporter.h index b58e047d0af..cc3af71fc26 100644 --- a/storage/src/vespa/storage/persistence/filestorage/service_layer_host_info_reporter.h +++ b/storage/src/vespa/storage/persistence/filestorage/service_layer_host_info_reporter.h @@ -3,6 +3,7 @@ #include <vespa/persistence/spi/resource_usage_listener.h> #include <vespa/storage/common/hostreporter/hostreporter.h> +#include <atomic> #include <mutex> namespace storage { @@ -18,15 +19,18 @@ class ServiceLayerHostInfoReporter : public HostReporter, NodeStateUpdater& _node_state_updater; std::mutex _lock; spi::ResourceUsage _old_resource_usage; + std::atomic<double> _noise_level; void update_resource_usage(const spi::ResourceUsage& resource_usage) override; public: - ServiceLayerHostInfoReporter(NodeStateUpdater& node_state_updater); + ServiceLayerHostInfoReporter(NodeStateUpdater& node_state_updater, + double noise_level = 0.001); ServiceLayerHostInfoReporter(const ServiceLayerHostInfoReporter&) = delete; ServiceLayerHostInfoReporter& operator=(const ServiceLayerHostInfoReporter&) = delete; ~ServiceLayerHostInfoReporter() override; + void set_noise_level(double level); void report(vespalib::JsonStream& output) override; const spi::ResourceUsage &get_old_resource_usage() noexcept { return _old_resource_usage; } }; |