Don't store full bundle objects in cluster state history

Bundles have a lot of sub-objects per state, so in systems with a
high amount of node entries, this adds unnecessary pressure on the
heap. Instead, store the string representations of the bundle and
the string representation of the diff to the previous state version
(if any). This is also inherently faster than computing the diffs
on-demand on every status page render.

Also remove mutable `official` field from `ClusterState`. Not worth
violating immutability of an object just to get some prettier (but
with high likelihood actually more confusing) status page rendering.

1 files changed, 61 insertions, 11 deletions

diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStateHistoryEntry.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStateHistoryEntry.java
index 65c0f210fd5..17c6d6b721f 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStateHistoryEntry.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStateHistoryEntry.java
@@ -5,25 +5,74 @@ import com.yahoo.vdslib.state.ClusterState;
 
 import java.util.Map;
 import java.util.Objects;
+import java.util.TreeMap;
 import java.util.stream.Collectors;
 
+/**
+ * Represents the absolute cluster states (baseline + derived) for a given version
+ * as well as the relative diffs (baseline + derived) since the previous version.
+ */
 public class ClusterStateHistoryEntry {
 
-    private final ClusterStateBundle state;
+    public final static String BASELINE = "-";
+
     private final long time;
+    private final Map<String, String> states = new TreeMap<>();
+    private final Map<String, String> diffs = new TreeMap<>();
+
+    ClusterStateHistoryEntry(ClusterStateBundle state, long time) {
+        this.time = time;
+        populateStateStrings(state);
+        // No diffs for the first entry in the history.
+    }
 
-    ClusterStateHistoryEntry(final ClusterStateBundle state, final long time) {
-        this.state = state;
+    ClusterStateHistoryEntry(ClusterStateBundle state, ClusterStateBundle prevState, long time) {
         this.time = time;
+        populateStateStrings(state);
+        populateDiffStrings(state, prevState);
+    }
+
+    private void populateStateStrings(ClusterStateBundle state) {
+        states.put(BASELINE, state.getBaselineClusterState().toString());
+        var derivedStates = state.getDerivedBucketSpaceStates().entrySet().stream()
+                .collect(Collectors.toMap(
+                        entry -> entry.getKey(),
+                        entry -> entry.getValue().getClusterState().toString()));
+        states.putAll(derivedStates);
+    }
+
+    private void populateDiffStrings(ClusterStateBundle state, ClusterStateBundle prevState) {
+        // Yes, it's correct to get the diff by doing old.getHtmlDifference(new) rather than the other way around.
+        diffs.put(BASELINE, prevState.getBaselineClusterState().getHtmlDifference(state.getBaselineClusterState()));
+        var spaceDiffs = state.getDerivedBucketSpaceStates().entrySet().stream()
+                .collect(Collectors.toMap(
+                        entry -> entry.getKey(),
+                        entry -> derivedStateOf(prevState, entry.getKey()).getHtmlDifference(entry.getValue().getClusterState())));
+        diffs.putAll(spaceDiffs);
+    }
+
+    public static ClusterStateHistoryEntry makeFirstEntry(ClusterStateBundle state, long time) {
+        return new ClusterStateHistoryEntry(state, time);
+    }
+
+    public static ClusterStateHistoryEntry makeSuccessor(ClusterStateBundle state, ClusterStateBundle prevState, long time) {
+        return new ClusterStateHistoryEntry(state, prevState, time);
+    }
+
+    private static ClusterState derivedStateOf(ClusterStateBundle state, String space) {
+        return state.getDerivedBucketSpaceStates().getOrDefault(space, AnnotatedClusterState.emptyState()).getClusterState();
+    }
+
+    public Map<String, String> getRawStates() {
+        return states;
     }
 
-    public ClusterState getBaselineState() {
-        return state.getBaselineClusterState();
+    public String getStateString(String space) {
+        return states.getOrDefault(space, "");
     }
 
-    public Map<String, ClusterState> getDerivedBucketSpaceStates() {
-        return state.getDerivedBucketSpaceStates().entrySet().stream()
-                .collect(Collectors.toMap(entry -> entry.getKey(), entry -> entry.getValue().getClusterState()));
+    public String getDiffString(String space) {
+        return diffs.getOrDefault(space, "");
     }
 
     public long time() {
@@ -36,18 +85,19 @@ public class ClusterStateHistoryEntry {
         if (o == null || getClass() != o.getClass()) return false;
         ClusterStateHistoryEntry that = (ClusterStateHistoryEntry) o;
         return time == that.time &&
-                Objects.equals(state, that.state);
+                Objects.equals(states, that.states) &&
+                Objects.equals(diffs, that.diffs);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(state, time);
+        return Objects.hash(time, states, diffs);
     }
 
     // String representation only used for test expectation failures and debugging output.
     // Actual status page history entry rendering emits formatted date/time.
     public String toString() {
-        return String.format("state '%s' at time %d", state, time);
+        return String.format("state '%s' at time %d", getStateString(BASELINE), time);
     }
 
 }