Register SLM run before snapshotting to save stats (#110216)

The SLM health indicator relies on the policyMetadata.getInvocationsSinceLastSuccess to determine if the last several snapshots have failed. If a snapshot fails and the master is shutdown before setting invocationsSinceLastSuccess, the fact that failure occurred will be lost.

To solve this, before snapshotting, we register that a snapshot is about to run, in the cluster state custom metadata. If the run fails, and invocationsSinceLastSuccess is not updated before a master shutdown, the fact that the failure occurred will not be lost. On completion of a subsequent snapshot run, SnapshotLifecycleTask will observe that there exists a registered snapshot which is no longer running. It will infer that the snapshot failed, and update invocationsSinceLastSuccess and other stats accordingly.

A few parts of this change touch general snapshot code, and are worth noting:
* Snapshots can only be uniquely identified with a uuid which had previously been generated in SnapshotService. This uuid is needed when there is a snapshot failure, but was not available to SLM as it was only returned in the SnapshotInfo after a success. To make this available, the uuid is now generated in the CreateSnapshotRequest constructor and passed to SnapshotService. In mixed version clusters, there exists a special case where the uuid is still generated in SnapshotService.
* If a snapshot were registered before calling snapshot service, there would a small period of time when a snapshot is registered but not yet stored in SnapshotsInProgress. During this time, another snapshot from the same policy might incorrectly infer that the snapshot failed and update its stats accordingly. To avoid this race condition, we register snapshots within SnapshotService in the same cluster update which updates SnapshotsInProgress.

Author: parkertimmins

docs/changelog/110216.yaml

@@ -0,0 +1,5 @@
+pr: 110216
+summary: Register SLM run before snapshotting to save stats
+area: ILM+SLM
+type: enhancement
+issues: []

server/src/main/java/org/elasticsearch/TransportVersions.java

@@ -183,6 +183,7 @@ static TransportVersion def(int id) {
     public static final TransportVersion FIX_VECTOR_SIMILARITY_INNER_HITS = def(8_713_00_0);
     public static final TransportVersion INDEX_REQUEST_UPDATE_BY_DOC_ORIGIN = def(8_714_00_0);
     public static final TransportVersion ESQL_ATTRIBUTE_CACHED_SERIALIZATION = def(8_715_00_0);
+    public static final TransportVersion REGISTER_SLM_STATS = def(8_716_00_0);
 
     /*
      * STOP! READ THIS FIRST! No, really,

server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequest.java

@@ -17,6 +17,7 @@
 import org.elasticsearch.action.support.master.MasterNodeRequest;
 import org.elasticsearch.cluster.metadata.DataStream;
 import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.UUIDs;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
@@ -82,6 +83,9 @@ public class CreateSnapshotRequest extends MasterNodeRequest<CreateSnapshotReque
     @Nullable
     private Map<String, Object> userMetadata;
 
+    @Nullable
+    private String uuid = null;
+
     public CreateSnapshotRequest(TimeValue masterNodeTimeout) {
         super(masterNodeTimeout);
     }
@@ -96,6 +100,7 @@ public CreateSnapshotRequest(TimeValue masterNodeTimeout, String repository, Str
         this(masterNodeTimeout);
         this.snapshot = snapshot;
         this.repository = repository;
+        this.uuid = UUIDs.randomBase64UUID();
     }
 
     public CreateSnapshotRequest(StreamInput in) throws IOException {
@@ -112,6 +117,7 @@ public CreateSnapshotRequest(StreamInput in) throws IOException {
         waitForCompletion = in.readBoolean();
         partial = in.readBoolean();
         userMetadata = in.readGenericMap();
+        uuid = in.getTransportVersion().onOrAfter(TransportVersions.REGISTER_SLM_STATS) ? in.readOptionalString() : null;
     }
 
     @Override
@@ -129,6 +135,9 @@ public void writeTo(StreamOutput out) throws IOException {
         out.writeBoolean(waitForCompletion);
         out.writeBoolean(partial);
         out.writeGenericMap(userMetadata);
+        if (out.getTransportVersion().onOrAfter(TransportVersions.REGISTER_SLM_STATS)) {
+            out.writeOptionalString(uuid);
+        }
     }
 
     @Override
@@ -364,6 +373,35 @@ public CreateSnapshotRequest userMetadata(@Nullable Map<String, Object> userMeta
         return this;
     }
 
+    /**
+     * Set a uuid to identify snapshot.
+     * If no uuid is specified, one will be created within SnapshotService
+     */
+    public CreateSnapshotRequest uuid(String uuid) {
+        this.uuid = uuid;
+        return this;
+    }
+
+    /**
+     * Get the uuid, generating it if one does not yet exist.
+     * Because the uuid can be set, this method is NOT thread-safe.
+     * <p>
+     * The uuid was previously generated in SnapshotService.createSnapshot
+     * but was moved to the CreateSnapshotRequest constructor so that the caller could
+     * uniquely identify the snapshot. Unfortunately, in a mixed-version cluster,
+     * the CreateSnapshotRequest could be created on a node which does not yet
+     * generate the uuid in the constructor. In this case, the uuid
+     * must be generated when it is first accessed with this getter.
+     *
+     * @return the uuid that will be used for the snapshot
+     */
+    public String uuid() {
+        if (this.uuid == null) {
+            this.uuid = UUIDs.randomBase64UUID();
+        }
+        return this.uuid;
+    }
+
     /**
      * @return Which plugin states should be included in the snapshot
      */
@@ -469,12 +507,13 @@ public boolean equals(Object o) {
             && Objects.equals(indicesOptions, that.indicesOptions)
             && Arrays.equals(featureStates, that.featureStates)
             && Objects.equals(masterNodeTimeout(), that.masterNodeTimeout())
-            && Objects.equals(userMetadata, that.userMetadata);
+            && Objects.equals(userMetadata, that.userMetadata)
+            && Objects.equals(uuid, that.uuid);
     }
 
     @Override
     public int hashCode() {
-        int result = Objects.hash(snapshot, repository, indicesOptions, partial, includeGlobalState, waitForCompletion, userMetadata);
+        int result = Objects.hash(snapshot, repository, indicesOptions, partial, includeGlobalState, waitForCompletion, userMetadata, uuid);
         result = 31 * result + Arrays.hashCode(indices);
         result = 31 * result + Arrays.hashCode(featureStates);
         return result;
@@ -505,6 +544,8 @@ public String toString() {
             + masterNodeTimeout()
             + ", metadata="
             + userMetadata
+            + ", uuid="
+            + uuid
             + '}';
     }
 }

server/src/main/java/org/elasticsearch/cluster/ClusterModule.java

@@ -76,6 +76,7 @@
 import org.elasticsearch.persistent.PersistentTasksNodeService;
 import org.elasticsearch.plugins.ClusterPlugin;
 import org.elasticsearch.script.ScriptMetadata;
+import org.elasticsearch.snapshots.RegisteredPolicySnapshots;
 import org.elasticsearch.snapshots.SnapshotsInfoService;
 import org.elasticsearch.tasks.Task;
 import org.elasticsearch.tasks.TaskResultsService;
@@ -234,6 +235,12 @@ public static List<Entry> getNamedWriteables() {
         registerMetadataCustom(entries, NodesShutdownMetadata.TYPE, NodesShutdownMetadata::new, NodesShutdownMetadata::readDiffFrom);
         registerMetadataCustom(entries, FeatureMigrationResults.TYPE, FeatureMigrationResults::new, FeatureMigrationResults::readDiffFrom);
         registerMetadataCustom(entries, DesiredNodesMetadata.TYPE, DesiredNodesMetadata::new, DesiredNodesMetadata::readDiffFrom);
+        registerMetadataCustom(
+            entries,
+            RegisteredPolicySnapshots.TYPE,
+            RegisteredPolicySnapshots::new,
+            RegisteredPolicySnapshots.RegisteredSnapshotsDiff::new
+        );
 
         // Task Status (not Diffable)
         entries.add(new Entry(Task.Status.class, PersistentTasksNodeService.Status.NAME, PersistentTasksNodeService.Status::new));