summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJon Marius Venstad <jonmv@users.noreply.github.com>2020-04-17 23:05:37 +0200
committerGitHub <noreply@github.com>2020-04-17 23:05:37 +0200
commit3d477471159147bd5c79e11756ef0617d833993d (patch)
tree9c9dd53610eea03643ac8aa18d673d4325312dd5
parent2e5b18e1c72390b8f9a0fc75f86ecf885bf517c1 (diff)
parentc16e37ada9df0cca4e686ee546442e02a3cd1ed9 (diff)
Merge pull request #12960 from vespa-engine/jonmv/reduce-initial-maintainer-delay
Reduce initial maintainer delay in controller
-rw-r--r--controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockContactRetriever.java11
-rw-r--r--controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java22
-rw-r--r--controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Maintainer.java5
-rw-r--r--controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainerTest.java26
-rw-r--r--controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MaintainerTest.java17
-rw-r--r--node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Maintainer.java5
-rw-r--r--node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MaintainerTest.java19
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));
}
}