Merge pull request #21713 from vespa-engine/jonmv/shard-ownership-confirmation

Shard ownership confirmation requests by 24

4 files changed, 17 insertions, 4 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/TenantAndApplicationId.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/TenantAndApplicationId.java
index 189ff9b1025..695c8fb6764 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/TenantAndApplicationId.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/TenantAndApplicationId.java
@@ -11,7 +11,7 @@ import java.util.Objects;
 /**
  * Tenant and application name pair.
  *
- * TODO jonmv: rename to ApplicationId if ApplicationId is renamed.
+ * @author jonmv
  */
 public class TenantAndApplicationId implements Comparable<TenantAndApplicationId> {
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java
index 308bfbb0408..fbd224dd8fd 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java
@@ -32,11 +32,18 @@ public class ApplicationOwnershipConfirmer extends ControllerMaintainer {
 
     private final OwnershipIssues ownershipIssues;
     private final ApplicationController applications;
+    private final int shards;
 
     public ApplicationOwnershipConfirmer(Controller controller, Duration interval, OwnershipIssues ownershipIssues) {
+        this(controller, interval, ownershipIssues, 24);
+    }
+
+    public ApplicationOwnershipConfirmer(Controller controller, Duration interval, OwnershipIssues ownershipIssues, int shards) {
         super(controller, interval);
         this.ownershipIssues = ownershipIssues;
         this.applications = controller.applications();
+        if (shards <= 0) throw new IllegalArgumentException("shards must be a positive number, but got " + shards);
+        this.shards = shards;
     }
 
     @Override
@@ -57,10 +64,10 @@ public class ApplicationOwnershipConfirmer extends ControllerMaintainer {
                        .asList()
                        .stream()
                        .filter(application -> application.createdAt().isBefore(controller().clock().instant().minus(Duration.ofDays(90))))
+                       .filter(application -> isInCurrentShard(application.id()))
                        .forEach(application -> {
                            try {
                                attempts.incrementAndGet();
-                               // TODO jvenstad: Makes sense to require, and run this only in main?
                                tenantOf(application.id()).contact().flatMap(contact -> {
                                    return ownershipIssues.confirmOwnership(application.ownershipIssueId(),
                                                                            summaryOf(application.id()),
@@ -76,6 +83,12 @@ public class ApplicationOwnershipConfirmer extends ControllerMaintainer {
         return asSuccessFactor(attempts.get(), failures.get());
     }
 
+    private boolean isInCurrentShard(TenantAndApplicationId id) {
+        double participants = Math.max(1, controller().curator().cluster().size());
+        long ticksSinceEpoch = Math.round((controller().clock().millis() * participants / interval().toMillis()));
+        return (ticksSinceEpoch + id.hashCode()) % shards == 0;
+    }
+
     private ApplicationSummary summaryOf(TenantAndApplicationId application) {
         var app = applications.requireApplication(application);
         var metrics = new HashMap<DeploymentId, ApplicationSummary.Metric>();
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 bde39aa3dd6..deafcd35e9b 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
@@ -142,7 +142,7 @@ public class ControllerMaintenance extends AbstractComponent {
             this.versionStatusUpdater = duration(3, MINUTES);
             this.readyJobsTrigger = duration(1, MINUTES);
             this.deploymentMetricsMaintainer = duration(10, MINUTES);
-            this.applicationOwnershipConfirmer = duration(12, HOURS);
+            this.applicationOwnershipConfirmer = duration(3, HOURS);
             this.systemUpgrader = duration(2, MINUTES);
             this.jobRunner = duration(system.isCd() ? 45 : 90, SECONDS);
             this.osVersionStatusUpdater = duration(2, MINUTES);
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java
index 09ee1f8fe51..1e258e8febc 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java
@@ -34,7 +34,7 @@ public class ApplicationOwnershipConfirmerTest {
     public void setup() {
         tester = new DeploymentTester();
         issues = new MockOwnershipIssues();
-        confirmer = new ApplicationOwnershipConfirmer(tester.controller(), Duration.ofDays(1), issues);
+        confirmer = new ApplicationOwnershipConfirmer(tester.controller(), Duration.ofDays(1), issues, 1);
     }
 
     @Test