diff options
author | Jon Marius Venstad <jonmv@users.noreply.github.com> | 2020-04-17 23:05:37 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-04-17 23:05:37 +0200 |
commit | 3d477471159147bd5c79e11756ef0617d833993d (patch) | |
tree | 9c9dd53610eea03643ac8aa18d673d4325312dd5 | |
parent | 2e5b18e1c72390b8f9a0fc75f86ecf885bf517c1 (diff) | |
parent | c16e37ada9df0cca4e686ee546442e02a3cd1ed9 (diff) |
Merge pull request #12960 from vespa-engine/jonmv/reduce-initial-maintainer-delay
Reduce initial maintainer delay in controller
7 files changed, 66 insertions, 39 deletions
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockContactRetriever.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockContactRetriever.java index dd0c143ae28..ab39d8e5660 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockContactRetriever.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockContactRetriever.java @@ -8,23 +8,28 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.Optional; +import java.util.function.Supplier; /** * @author olaa */ public class MockContactRetriever implements ContactRetriever{ - private final Map<PropertyId, Contact> contacts = new HashMap<>(); + private final Map<PropertyId, Supplier<Contact>> contacts = new HashMap<>(); @Override public Contact getContact(Optional<PropertyId> propertyId) { - return contacts.getOrDefault(propertyId.get(), contact()); + return contacts.getOrDefault(propertyId.get(), this::contact).get(); } - public void addContact(PropertyId propertyId, Contact contact) { + public void addContact(PropertyId propertyId, Supplier<Contact> contact) { contacts.put(propertyId, contact); } + public void addContact(PropertyId propertyId, Contact contact) { + contacts.put(propertyId, () -> contact); + } + public Contact contact() { return new Contact(URI.create("contacts.tld"), URI.create("properties.tld"), URI.create("issues.tld"), Collections.emptyList(), "queue", Optional.of("component")); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java index 7e354a02832..d1f2087ce12 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java @@ -6,14 +6,18 @@ import com.yahoo.log.LogLevel; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.LockedTenant; import com.yahoo.vespa.hosted.controller.TenantController; +import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; import com.yahoo.vespa.hosted.controller.api.integration.organization.ContactRetriever; import com.yahoo.vespa.hosted.controller.tenant.Tenant; import com.yahoo.yolean.Exceptions; import java.time.Duration; +import java.util.Optional; import java.util.function.Predicate; import java.util.logging.Logger; +import static java.util.logging.Level.INFO; + /** * Periodically fetch and store contact information for tenants. * @@ -34,13 +38,21 @@ public class ContactInformationMaintainer extends Maintainer { protected void maintain() { TenantController tenants = controller().tenants(); for (Tenant tenant : tenants.asList()) { + log.log(INFO, "Updating contact information for " + tenant); try { switch (tenant.type()) { - case athenz: tenants.lockIfPresent(tenant.name(), LockedTenant.Athenz.class, lockedTenant -> - tenants.store(lockedTenant.with(contactRetriever.getContact(lockedTenant.get().propertyId())))); - return; - case cloud: return; - default: throw new IllegalArgumentException("Unexpected tenant type '" + tenant.type() + "'."); + case athenz: + tenants.lockIfPresent(tenant.name(), LockedTenant.Athenz.class, lockedTenant -> { + Contact contact = contactRetriever.getContact(lockedTenant.get().propertyId()); + log.log(INFO, "Contact found for " + tenant + " was " + + (Optional.of(contact).equals(tenant.contact()) ? "un" : "") + "changed"); + tenants.store(lockedTenant.with(contact)); + }); + break; + case cloud: + break; + default: + throw new IllegalArgumentException("Unexpected tenant type '" + tenant.type() + "'."); } } catch (Exception e) { log.log(LogLevel.WARNING, "Failed to update contact information for " + tenant + ": " + diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Maintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Maintainer.java index cec44c5d3a0..3e283d6c54a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Maintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Maintainer.java @@ -52,7 +52,7 @@ public abstract class Maintainer extends AbstractComponent implements Runnable { this.name = name; this.activeSystems = Set.copyOf(activeSystems); - service = new ScheduledThreadPoolExecutor(1); + service = new ScheduledThreadPoolExecutor(1, r -> new Thread(r, name() + "-worker")); long delay = staggeredDelay(controller.curator().cluster(), controller.hostname(), controller.clock().instant(), interval); service.scheduleAtFixedRate(this, delay, interval.toMillis(), TimeUnit.MILLISECONDS); jobControl.started(name()); @@ -115,8 +115,7 @@ public abstract class Maintainer extends AbstractComponent implements Runnable { return interval.toMillis(); long offset = cluster.indexOf(host) * interval.toMillis() / cluster.size(); - long timeUntilNextRun = Math.floorMod(offset - now.toEpochMilli(), interval.toMillis()); - return timeUntilNextRun + interval.toMillis() / cluster.size(); + return Math.floorMod(offset - now.toEpochMilli(), interval.toMillis()); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainerTest.java index 0db718080c2..32d2e2f35f4 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainerTest.java @@ -37,22 +37,22 @@ public class ContactInformationMaintainerTest { @Test public void updates_contact_information() { - long propertyId = 1; - TenantName name = tester.createTenant("tenant1", "domain1", propertyId); - Supplier<AthenzTenant> tenant = () -> (AthenzTenant) tester.controller().tenants().require(name); - assertFalse("No contact information initially", tenant.get().contact().isPresent()); + PropertyId propertyId1 = new PropertyId("1"); + PropertyId propertyId2 = new PropertyId("2"); + TenantName name1 = tester.createTenant("tenant1", "domain1", 1L); + TenantName name2 = tester.createTenant("zenant1", "domain2", 2L); + Supplier<AthenzTenant> tenant1 = () -> (AthenzTenant) tester.controller().tenants().require(name1); + Supplier<AthenzTenant> tenant2 = () -> (AthenzTenant) tester.controller().tenants().require(name2); + assertFalse("No contact information initially", tenant1.get().contact().isPresent()); + assertFalse("No contact information initially", tenant2.get().contact().isPresent()); Contact contact = testContact(); - registerContact(propertyId, contact); - maintainer.run(); + tester.serviceRegistry().contactRetriever().addContact(propertyId1, () -> { throw new RuntimeException("ERROR"); }); + tester.serviceRegistry().contactRetriever().addContact(propertyId2, () -> contact); + maintainer.maintain(); - assertTrue("Contact information added", tenant.get().contact().isPresent()); - assertEquals(contact, tenant.get().contact().get()); - } - - private void registerContact(long propertyId, Contact contact) { - PropertyId p = new PropertyId(String.valueOf(propertyId)); - tester.serviceRegistry().contactRetrieverMock().addContact(p, contact); + assertEquals("No contact information added due to error", Optional.empty(), tenant1.get().contact()); + assertEquals("Contact information added", Optional.of(contact), tenant2.get().contact()); } private static Contact testContact() { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MaintainerTest.java index 3aa1f2b5af2..efd5d61ce56 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MaintainerTest.java @@ -38,17 +38,22 @@ public class MaintainerTest { @Test public void staggering() { List<HostName> cluster = List.of(HostName.from("cfg1"), HostName.from("cfg2"), HostName.from("cfg3")); - Instant now = Instant.ofEpochMilli(1001); Duration interval = Duration.ofMillis(300); - assertEquals(299, Maintainer.staggeredDelay(cluster, HostName.from("cfg1"), now, interval)); - assertEquals(399, Maintainer.staggeredDelay(cluster, HostName.from("cfg2"), now, interval)); - assertEquals(199, Maintainer.staggeredDelay(cluster, HostName.from("cfg3"), now, interval)); + Instant now = Instant.ofEpochMilli(1000); + assertEquals(200, Maintainer.staggeredDelay(cluster, HostName.from("cfg1"), now, interval)); + assertEquals( 0, Maintainer.staggeredDelay(cluster, HostName.from("cfg2"), now, interval)); + assertEquals(100, Maintainer.staggeredDelay(cluster, HostName.from("cfg3"), now, interval)); - now = Instant.ofEpochMilli(1101); + now = Instant.ofEpochMilli(1001); assertEquals(199, Maintainer.staggeredDelay(cluster, HostName.from("cfg1"), now, interval)); assertEquals(299, Maintainer.staggeredDelay(cluster, HostName.from("cfg2"), now, interval)); - assertEquals(399, Maintainer.staggeredDelay(cluster, HostName.from("cfg3"), now, interval)); + assertEquals( 99, Maintainer.staggeredDelay(cluster, HostName.from("cfg3"), now, interval)); + + now = Instant.ofEpochMilli(1101); + assertEquals( 99, Maintainer.staggeredDelay(cluster, HostName.from("cfg1"), now, interval)); + assertEquals(199, Maintainer.staggeredDelay(cluster, HostName.from("cfg2"), now, interval)); + assertEquals(299, Maintainer.staggeredDelay(cluster, HostName.from("cfg3"), now, interval)); assertEquals(300, Maintainer.staggeredDelay(cluster, HostName.from("cfg0"), now, interval)); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Maintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Maintainer.java index e01f7ea7bf5..17c73282b4b 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Maintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Maintainer.java @@ -42,7 +42,7 @@ public abstract class Maintainer extends AbstractComponent implements Runnable { HostName hostname = HostName.from(com.yahoo.net.HostName.getLocalhost()); long delay = staggeredDelay(nodeRepository.database().cluster(), hostname, nodeRepository.clock().instant(), interval); - service = new ScheduledThreadPoolExecutor(1); + service = new ScheduledThreadPoolExecutor(1, r -> new Thread(r, name() + "-worker")); service.scheduleAtFixedRate(this, delay, interval.toMillis(), TimeUnit.MILLISECONDS); jobControl.started(name(), this); } @@ -113,8 +113,7 @@ public abstract class Maintainer extends AbstractComponent implements Runnable { return interval.toMillis(); long offset = cluster.indexOf(host) * interval.toMillis() / cluster.size(); - long timeUntilNextRun = Math.floorMod(offset - now.toEpochMilli(), interval.toMillis()); - return timeUntilNextRun + interval.toMillis() / cluster.size(); + return Math.floorMod(offset - now.toEpochMilli(), interval.toMillis()); } } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MaintainerTest.java index c19b54a8cab..7ce64093491 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MaintainerTest.java @@ -19,15 +19,22 @@ public class MaintainerTest { @Test public void staggering() { List<HostName> cluster = Arrays.asList(HostName.from("cfg1"), HostName.from("cfg2"), HostName.from("cfg3")); - Instant now = Instant.ofEpochMilli(1001); Duration interval = Duration.ofMillis(300); - assertEquals(299, Maintainer.staggeredDelay(cluster, HostName.from("cfg1"), now, interval)); - assertEquals(399, Maintainer.staggeredDelay(cluster, HostName.from("cfg2"), now, interval)); - assertEquals(199, Maintainer.staggeredDelay(cluster, HostName.from("cfg3"), now, interval)); - now = Instant.ofEpochMilli(1101); + Instant now = Instant.ofEpochMilli(1000); + assertEquals(200, Maintainer.staggeredDelay(cluster, HostName.from("cfg1"), now, interval)); + assertEquals( 0, Maintainer.staggeredDelay(cluster, HostName.from("cfg2"), now, interval)); + assertEquals(100, Maintainer.staggeredDelay(cluster, HostName.from("cfg3"), now, interval)); + + now = Instant.ofEpochMilli(1001); assertEquals(199, Maintainer.staggeredDelay(cluster, HostName.from("cfg1"), now, interval)); assertEquals(299, Maintainer.staggeredDelay(cluster, HostName.from("cfg2"), now, interval)); - assertEquals(399, Maintainer.staggeredDelay(cluster, HostName.from("cfg3"), now, interval)); + assertEquals( 99, Maintainer.staggeredDelay(cluster, HostName.from("cfg3"), now, interval)); + + now = Instant.ofEpochMilli(1101); + assertEquals( 99, Maintainer.staggeredDelay(cluster, HostName.from("cfg1"), now, interval)); + assertEquals(199, Maintainer.staggeredDelay(cluster, HostName.from("cfg2"), now, interval)); + assertEquals(299, Maintainer.staggeredDelay(cluster, HostName.from("cfg3"), now, interval)); + assertEquals(300, Maintainer.staggeredDelay(cluster, HostName.from("cfg0"), now, interval)); } } |