Simplify common history check

6 files changed, 27 insertions, 30 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Expirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Expirer.java
index a3b4917147e..3ca2ee32313 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Expirer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Expirer.java
@@ -9,7 +9,6 @@ import java.time.Clock;
 import java.time.Duration;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Optional;
 import java.util.logging.Logger;
 
 /**
@@ -45,8 +44,7 @@ public abstract class Expirer extends Maintainer {
     protected void maintain() {
         List<Node> expired = new ArrayList<>();
         for (Node node : nodeRepository().getNodes(fromState)) {
-            Optional<History.Event> event = node.history().event(eventType);
-            if (event.isPresent() && event.get().at().plus(expiryTime).isBefore(clock.instant()))
+            if (node.history().hasEventBefore(eventType, clock.instant().minus(expiryTime)))
                 expired.add(node);
         }
         if ( ! expired.isEmpty())
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java
index fc6560b9f1b..3bc8857be07 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java
@@ -86,9 +86,8 @@ public class FailedExpirer extends Maintainer {
     /** Get failed nodes that have expired according to given expiry */
     private List<Node> getExpiredNodes(Duration expiry) {
         return nodeRepository.getNodes(Node.State.failed).stream()
-                             .filter(node -> node.history().event(History.Event.Type.failed)
-                                                 .map(event -> event.at().plus(expiry).isBefore(clock.instant()))
-                                                 .orElse(false))
+                             .filter(node -> node.history()
+                                     .hasEventBefore(History.Event.Type.failed, clock.instant().minus(expiry)))
                              .collect(Collectors.toList());
     }
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java
index e5d1bb98936..5631899c68b 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java
@@ -95,8 +95,8 @@ public class NodeFailer extends Maintainer {
 
         // Active nodes
         for (Node node : determineActiveNodeDownStatus()) {
-            Instant graceTimeEnd = node.history().event(History.Event.Type.down).get().at().plus(downTimeLimit);
-            if (graceTimeEnd.isBefore(clock.instant()) && ! applicationSuspended(node) && failAllowedFor(node.type()))
+            Instant graceTimeEnd = clock.instant().minus(downTimeLimit);
+            if (node.history().hasEventBefore(History.Event.Type.down, graceTimeEnd) && ! applicationSuspended(node) && failAllowedFor(node.type()))
                 if (!throttle(node)) failActive(node, "Node has been down longer than " + downTimeLimit);
         }
     }
@@ -108,11 +108,9 @@ public class NodeFailer extends Maintainer {
             Optional<Instant> lastLocalRequest = hostLivenessTracker.lastRequestFrom(node.hostname());
             if ( ! lastLocalRequest.isPresent()) continue;
 
-            Optional<History.Event> recordedRequest = node.history().event(History.Event.Type.requested);
-            if ( ! recordedRequest.isPresent() || recordedRequest.get().at().isBefore(lastLocalRequest.get())) {
-                History updatedHistory = node.history().with(new History.Event(History.Event.Type.requested,
-                        Agent.system,
-                        lastLocalRequest.get()));
+            if (! node.history().hasEventAfter(History.Event.Type.requested, lastLocalRequest.get())) {
+                History updatedHistory = node.history()
+                        .with(new History.Event(History.Event.Type.requested, Agent.system, lastLocalRequest.get()));
                 nodeRepository().write(node.with(updatedHistory));
             }
         }
@@ -150,13 +148,11 @@ public class NodeFailer extends Maintainer {
     }
 
     private boolean wasMadeReadyBefore(Node node, Instant instant) {
-        Optional<History.Event> readiedEvent = node.history().event(History.Event.Type.readied);
-        return readiedEvent.map(event -> event.at().isBefore(instant)).orElse(false);
+        return node.history().hasEventBefore(History.Event.Type.readied, instant);
     }
 
     private boolean hasRecordedRequestAfter(Node node, Instant instant) {
-        Optional<History.Event> lastRequest = node.history().event(History.Event.Type.requested);
-        return lastRequest.map(event -> event.at().isAfter(instant)).orElse(false);
+        return node.history().hasEventAfter(History.Event.Type.requested, instant);
     }
 
     private boolean applicationSuspended(Node node) {
@@ -289,10 +285,7 @@ public class NodeFailer extends Maintainer {
         Instant startOfThrottleWindow = clock.instant().minus(throttlePolicy.throttleWindow);
         List<Node> nodes = nodeRepository().getNodes();
         long recentlyFailedNodes = nodes.stream()
-                .map(n -> n.history().event(History.Event.Type.failed))
-                .filter(Optional::isPresent)
-                .map(Optional::get)
-                .filter(failedEvent -> failedEvent.at().isAfter(startOfThrottleWindow))
+                .filter(n -> n.history().hasEventAfter(History.Event.Type.failed, startOfThrottleWindow))
                 .count();
         int allowedFailedNodes = (int) Math.max(nodes.size() * throttlePolicy.fractionAllowedToFail,
                                                 throttlePolicy.minimumAllowedToFail);
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRebooter.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRebooter.java
index 6b538946e09..4b2fcbcb257 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRebooter.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRebooter.java
@@ -10,7 +10,6 @@ import com.yahoo.vespa.hosted.provision.node.filter.NodeListFilter;
 import java.time.Clock;
 import java.time.Duration;
 import java.util.List;
-import java.util.Optional;
 import java.util.Random;
 
 /**
@@ -47,8 +46,7 @@ public class NodeRebooter extends Maintainer {
     }
     
     private boolean shouldReboot(Node node) {
-        Optional<History.Event> lastReboot = node.history().event(History.Event.Type.rebooted);
-        if (lastReboot.isPresent() && lastReboot.get().at().plus(rebootInterval).isAfter(clock.instant()))
+        if (node.history().hasEventAfter(History.Event.Type.rebooted, clock.instant().minus(rebootInterval)))
             return false;
         else // schedule with a probability such that reboots of nodes are spread roughly over the reboot interval
             return random.nextDouble() < (double) interval().getSeconds() / (double)rebootInterval.getSeconds();
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java
index 7bc7d3ee732..86d4caef94b 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java
@@ -4,7 +4,6 @@ package com.yahoo.vespa.hosted.provision.maintenance;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.Deployer;
 import com.yahoo.config.provision.Deployment;
-import com.yahoo.config.provision.NodeType;
 import com.yahoo.vespa.applicationmodel.HostName;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
@@ -14,7 +13,6 @@ import com.yahoo.vespa.orchestrator.Orchestrator;
 
 import java.time.Clock;
 import java.time.Duration;
-import java.time.Instant;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -99,10 +97,7 @@ public class RetiredExpirer extends Maintainer {
                     .allMatch(child -> child.state() == Node.State.parked || child.state() == Node.State.failed);
         }
 
-        Optional<Instant> timeOfRetiredEvent = node.history().event(History.Event.Type.retired).map(History.Event::at);
-        Optional<Instant> retireAfter = timeOfRetiredEvent.map(retiredEvent -> retiredEvent.plus(retiredExpiry));
-        boolean shouldRetireNowBecauseExpired = retireAfter.map(time -> time.isBefore(clock.instant())).orElse(false);
-        if (shouldRetireNowBecauseExpired) {
+        if (node.history().hasEventBefore(History.Event.Type.retired, clock.instant().minus(retiredExpiry))) {
             return true;
         }
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/History.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/History.java
index df21950c69f..304e207a5e4 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/History.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/History.java
@@ -39,6 +39,20 @@ public class History {
     /** Returns this event if it is present in this history */
     public Optional<Event> event(Event.Type type) { return Optional.ofNullable(events.get(type)); }
 
+    /** Returns true if a given event is registered in this history after the given time */
+    public boolean hasEventAfter(Event.Type type, Instant time) {
+        return event(type)
+                .map(event -> event.at().isAfter(time))
+                .orElse(false);
+    }
+
+    /** Returns true if a given event is registered in this history before the given time */
+    public boolean hasEventBefore(Event.Type type, Instant time) {
+        return event(type)
+                .map(event -> event.at().isBefore(time))
+                .orElse(false);
+    }
+
     public Collection<Event> events() { return events.values(); }
 
     /** Returns a copy of this history with the given event added */