ResourceMeterMaintainer refreshes metering at set intervals (#15688)

5 files changed, 67 insertions, 2 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/resource/MeteringClient.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/resource/MeteringClient.java
index 1047e1a02a4..e8c7a9a1654 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/resource/MeteringClient.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/resource/MeteringClient.java
@@ -21,4 +21,6 @@ public interface MeteringClient {
 
     List<ResourceSnapshot> getSnapshotHistoryForTenant(TenantName tenantName, YearMonth yearMonth);
 
+    void refresh();
+
 }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockMeteringClient.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockMeteringClient.java
index ec3f564e930..6ccab4f60fe 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockMeteringClient.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockMeteringClient.java
@@ -24,6 +24,7 @@ public class MockMeteringClient implements MeteringClient {
 
     private Collection<ResourceSnapshot> resources = new ArrayList<>();
     private Optional<MeteringData> meteringData;
+    private boolean isRefreshed = false;
 
     @Override
     public void consume(Collection<ResourceSnapshot> resources){
@@ -43,6 +44,11 @@ public class MockMeteringClient implements MeteringClient {
         return new ArrayList<>(resources);
     }
 
+    @Override
+    public void refresh() {
+        isRefreshed = true;
+    }
+
     public Collection<ResourceSnapshot> consumedResources() {
         return this.resources;
     }
@@ -51,4 +57,8 @@ public class MockMeteringClient implements MeteringClient {
         this.meteringData = Optional.of(meteringData);
         this.resources = meteringData.getSnapshotHistory().entrySet().stream().map(Map.Entry::getValue).flatMap(List::stream).collect(Collectors.toList());
     }
+
+    public boolean isRefreshed() {
+        return isRefreshed;
+    }
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java
index f460561df08..5496eab049f 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java
@@ -11,14 +11,17 @@ import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.NodeRepository;
 import com.yahoo.vespa.hosted.controller.api.integration.resource.MeteringClient;
 import com.yahoo.vespa.hosted.controller.api.integration.resource.ResourceSnapshot;
+import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
 import com.yahoo.yolean.Exceptions;
 
 import java.time.Clock;
 import java.time.Duration;
+import java.time.Instant;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.TimeoutException;
 import java.util.logging.Level;
 import java.util.stream.Collectors;
 
@@ -33,9 +36,11 @@ public class ResourceMeterMaintainer extends ControllerMaintainer {
     private final Metric metric;
     private final NodeRepository nodeRepository;
     private final MeteringClient meteringClient;
+    private final CuratorDb curator;
 
     private static final String METERING_LAST_REPORTED = "metering_last_reported";
     private static final String METERING_TOTAL_REPORTED = "metering_total_reported";
+    private static final int METERING_REFRESH_INTERVAL_SECONDS = 1800;
 
     @SuppressWarnings("WeakerAccess")
     public ResourceMeterMaintainer(Controller controller,
@@ -47,6 +52,7 @@ public class ResourceMeterMaintainer extends ControllerMaintainer {
         this.nodeRepository = controller.serviceRegistry().configServer().nodeRepository();
         this.metric = metric;
         this.meteringClient = meteringClient;
+        this.curator = controller.curator();
     }
 
     @Override
@@ -71,6 +77,15 @@ public class ResourceMeterMaintainer extends ControllerMaintainer {
                    resourceSnapshots.stream()
                            .mapToDouble(r -> r.getCpuCores() + r.getMemoryGb() + r.getDiskGb()).sum(),
                    metric.createContext(Collections.emptyMap()));
+
+        try (var lock = curator.lockMeteringRefreshTime()) {
+            if (needsRefresh(curator.readMeteringRefreshTime())) {
+                meteringClient.refresh();
+                curator.writeMeteringRefreshTime(clock.millis());
+            }
+        } catch (TimeoutException ignored) {
+            // If it's locked, it means we're currently refreshing
+        }
     }
 
     private Collection<ResourceSnapshot> getAllResourceSnapshots() {
@@ -117,4 +132,10 @@ public class ResourceMeterMaintainer extends ControllerMaintainer {
         return METERABLE_NODE_STATES.contains(node.state());
     }
 
+    private boolean needsRefresh(long lastRefreshTimestamp) {
+        return clock.instant()
+                .minusSeconds(METERING_REFRESH_INTERVAL_SECONDS)
+                .isAfter(Instant.ofEpochMilli(lastRefreshTimestamp));
+    }
+
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java
index eda4f713732..bc0170ee695 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java
@@ -188,6 +188,10 @@ public class CuratorDb {
         return curator.lock(lockRoot.append("nameServiceQueue"), defaultLockTimeout);
     }
 
+    public Lock lockMeteringRefreshTime() throws TimeoutException {
+        return tryLock(lockRoot.append("meteringRefreshTime"));
+    }
+
     // -------------- Helpers ------------------------------------------
 
     /** Try locking with a low timeout, meaning it is OK to fail lock acquisition.
@@ -519,6 +523,18 @@ public class CuratorDb {
         return allEndpointCertificateMetadata;
     }
 
+    // -------------- Metering view refresh times ----------------------------
+
+    public void writeMeteringRefreshTime(long timestamp) {
+        curator.set(meteringRefreshPath(), Long.toString(timestamp).getBytes());
+    }
+
+    public long readMeteringRefreshTime() {
+        return curator.getData(meteringRefreshPath())
+                .map(String::new).map(Long::parseLong)
+                .orElse(0l);
+    }
+
     // -------------- Paths ---------------------------------------------------
 
     private Path lockPath(TenantName tenant) {
@@ -640,4 +656,8 @@ public class CuratorDb {
         return endpointCertificateRoot.append(id.serializedForm());
     }
 
+    private static Path meteringRefreshPath() {
+        return root.append("meteringRefreshTime");
+    }
+
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainerTest.java
index ffc735c1990..34d461c6152 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainerTest.java
@@ -15,12 +15,13 @@ import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock;
 import org.junit.Test;
 
 import java.time.Duration;
+import java.time.Instant;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
 import java.util.stream.Collectors;
 
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.*;
 
 /**
  * @author olaa
@@ -34,8 +35,9 @@ public class ResourceMeterMaintainerTest {
     @Test
     public void testMaintainer() {
         setUpZones();
-
         ResourceMeterMaintainer resourceMeterMaintainer = new ResourceMeterMaintainer(tester.controller(), Duration.ofMinutes(5), metrics, snapshotConsumer);
+        long lastRefreshTime = tester.clock().millis();
+        tester.curator().writeMeteringRefreshTime(lastRefreshTime);
         resourceMeterMaintainer.maintain();
         Collection<ResourceSnapshot> consumedResources = snapshotConsumer.consumedResources();
 
@@ -54,6 +56,16 @@ public class ResourceMeterMaintainerTest {
 
         assertEquals(tester.clock().millis()/1000, metrics.getMetric("metering_last_reported"));
         assertEquals(2224.0d, (Double) metrics.getMetric("metering_total_reported"), Double.MIN_VALUE);
+
+        // Metering is not refreshed
+        assertFalse(snapshotConsumer.isRefreshed());
+        assertEquals(lastRefreshTime, tester.curator().readMeteringRefreshTime());
+
+        var millisAdvanced = 3600 * 1000;
+        tester.clock().advance(Duration.ofMillis(millisAdvanced));
+        resourceMeterMaintainer.maintain();
+        assertTrue(snapshotConsumer.isRefreshed());
+        assertEquals(lastRefreshTime + millisAdvanced, tester.curator().readMeteringRefreshTime());
     }
 
     private void setUpZones() {