From a2d9cc6473b7c433243391b35a634aca6a7c62af Mon Sep 17 00:00:00 2001
From: David Turner <david.turner@elastic.co>
Date: Tue, 30 Apr 2024 07:39:39 +0100
Subject: [PATCH] Encapsulate `MasterNodeRequest#masterNodeTimeout` (#107999)

There's no good reason for this field to have `protected` visibility,
and we definitely don't want subclasses to be able to set it to `null`.
This commit makes it `private`.

Relates #107984
---
 .../cluster/reroute/ClusterRerouteRequest.java      |  4 ++--
 .../snapshots/create/CreateSnapshotRequest.java     |  4 ++--
 .../admin/cluster/state/ClusterStateRequest.java    |  2 +-
 .../indices/settings/put/UpdateSettingsRequest.java | 12 ++++++++++--
 .../action/support/master/MasterNodeRequest.java    | 13 +++++++++++--
 .../UpdateIndexShardSnapshotStatusRequest.java      |  3 +--
 .../xpack/core/ccr/action/CcrStatsAction.java       |  6 +++---
 .../MountSearchableSnapshotRequest.java             |  4 ++--
 8 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteRequest.java
index 5aeef6b19298e..b355d3c50400e 100644
--- a/server/src/main/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteRequest.java
+++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteRequest.java
@@ -131,12 +131,12 @@ public boolean equals(Object obj) {
             && Objects.equals(explain, other.explain)
             && Objects.equals(ackTimeout(), other.ackTimeout())
             && Objects.equals(retryFailed, other.retryFailed)
-            && Objects.equals(masterNodeTimeout, other.masterNodeTimeout);
+            && Objects.equals(masterNodeTimeout(), other.masterNodeTimeout());
     }
 
     @Override
     public int hashCode() {
         // Override equals and hashCode for testing
-        return Objects.hash(commands, dryRun, explain, ackTimeout(), retryFailed, masterNodeTimeout);
+        return Objects.hash(commands, dryRun, explain, ackTimeout(), retryFailed, masterNodeTimeout());
     }
 }
diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequest.java
index 2e8a28d412e26..9127092bdb13a 100644
--- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequest.java
+++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequest.java
@@ -461,7 +461,7 @@ public boolean equals(Object o) {
             && Arrays.equals(indices, that.indices)
             && Objects.equals(indicesOptions, that.indicesOptions)
             && Arrays.equals(featureStates, that.featureStates)
-            && Objects.equals(masterNodeTimeout, that.masterNodeTimeout)
+            && Objects.equals(masterNodeTimeout(), that.masterNodeTimeout())
             && Objects.equals(userMetadata, that.userMetadata);
     }
 
@@ -495,7 +495,7 @@ public String toString() {
             + ", waitForCompletion="
             + waitForCompletion
             + ", masterNodeTimeout="
-            + masterNodeTimeout
+            + masterNodeTimeout()
             + ", metadata="
             + userMetadata
             + '}';
diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/state/ClusterStateRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/state/ClusterStateRequest.java
index e9de49dcbf5b4..d29996711d722 100644
--- a/server/src/main/java/org/elasticsearch/action/admin/cluster/state/ClusterStateRequest.java
+++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/state/ClusterStateRequest.java
@@ -222,7 +222,7 @@ public String getDescription() {
         if (indices.length > 0) {
             stringBuilder.append("indices ").append(Arrays.toString(indices)).append(", ");
         }
-        stringBuilder.append("master timeout [").append(masterNodeTimeout).append("]]");
+        stringBuilder.append("master timeout [").append(masterNodeTimeout()).append("]]");
         return stringBuilder.toString();
     }
 
diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/put/UpdateSettingsRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/put/UpdateSettingsRequest.java
index 666419edc1bf0..7fa2e11317a43 100644
--- a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/put/UpdateSettingsRequest.java
+++ b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/put/UpdateSettingsRequest.java
@@ -254,7 +254,7 @@ public boolean equals(Object o) {
             return false;
         }
         UpdateSettingsRequest that = (UpdateSettingsRequest) o;
-        return masterNodeTimeout.equals(that.masterNodeTimeout)
+        return masterNodeTimeout().equals(that.masterNodeTimeout())
             && ackTimeout().equals(that.ackTimeout())
             && Objects.equals(settings, that.settings)
             && Objects.equals(indicesOptions, that.indicesOptions)
@@ -265,7 +265,15 @@ && ackTimeout().equals(that.ackTimeout())
 
     @Override
     public int hashCode() {
-        return Objects.hash(masterNodeTimeout, ackTimeout(), settings, indicesOptions, preserveExisting, reopen, Arrays.hashCode(indices));
+        return Objects.hash(
+            masterNodeTimeout(),
+            ackTimeout(),
+            settings,
+            indicesOptions,
+            preserveExisting,
+            reopen,
+            Arrays.hashCode(indices)
+        );
     }
 
 }
diff --git a/server/src/main/java/org/elasticsearch/action/support/master/MasterNodeRequest.java b/server/src/main/java/org/elasticsearch/action/support/master/MasterNodeRequest.java
index 6459f6c1b458a..063dbb0397de8 100644
--- a/server/src/main/java/org/elasticsearch/action/support/master/MasterNodeRequest.java
+++ b/server/src/main/java/org/elasticsearch/action/support/master/MasterNodeRequest.java
@@ -14,6 +14,7 @@
 import org.elasticsearch.core.TimeValue;
 
 import java.io.IOException;
+import java.util.Objects;
 
 /**
  * A based request for master based operation.
@@ -22,10 +23,18 @@ public abstract class MasterNodeRequest<Request extends MasterNodeRequest<Reques
 
     public static final TimeValue DEFAULT_MASTER_NODE_TIMEOUT = TimeValue.timeValueSeconds(30);
 
-    protected TimeValue masterNodeTimeout = DEFAULT_MASTER_NODE_TIMEOUT;
+    private TimeValue masterNodeTimeout = DEFAULT_MASTER_NODE_TIMEOUT;
 
     protected MasterNodeRequest() {}
 
+    /**
+     * @param masterNodeTimeout Specifies how long to wait when the master has not been discovered yet, or is disconnected, or is busy
+     *                          processing other tasks. The value {@link TimeValue#MINUS_ONE} means to wait forever.
+     */
+    protected MasterNodeRequest(TimeValue masterNodeTimeout) {
+        this.masterNodeTimeout = Objects.requireNonNull(masterNodeTimeout);
+    }
+
     protected MasterNodeRequest(StreamInput in) throws IOException {
         super(in);
         masterNodeTimeout = in.readTimeValue();
@@ -44,7 +53,7 @@ public void writeTo(StreamOutput out) throws IOException {
      */
     @SuppressWarnings("unchecked")
     public final Request masterNodeTimeout(TimeValue timeout) {
-        this.masterNodeTimeout = timeout;
+        this.masterNodeTimeout = Objects.requireNonNull(timeout);
         return (Request) this;
     }
 
diff --git a/server/src/main/java/org/elasticsearch/snapshots/UpdateIndexShardSnapshotStatusRequest.java b/server/src/main/java/org/elasticsearch/snapshots/UpdateIndexShardSnapshotStatusRequest.java
index ccf28b9ee9e2e..b0c00a4cc710d 100644
--- a/server/src/main/java/org/elasticsearch/snapshots/UpdateIndexShardSnapshotStatusRequest.java
+++ b/server/src/main/java/org/elasticsearch/snapshots/UpdateIndexShardSnapshotStatusRequest.java
@@ -34,11 +34,10 @@ public UpdateIndexShardSnapshotStatusRequest(StreamInput in) throws IOException
     }
 
     public UpdateIndexShardSnapshotStatusRequest(Snapshot snapshot, ShardId shardId, SnapshotsInProgress.ShardSnapshotStatus status) {
+        super(TimeValue.MAX_VALUE); // By default, keep trying to post snapshot status messages to avoid snapshot processes getting stuck.
         this.snapshot = snapshot;
         this.shardId = shardId;
         this.status = status;
-        // By default, we keep trying to post snapshot status messages to avoid snapshot processes getting stuck.
-        this.masterNodeTimeout = TimeValue.timeValueNanos(Long.MAX_VALUE);
     }
 
     @Override
diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ccr/action/CcrStatsAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ccr/action/CcrStatsAction.java
index 771071c6e1029..b12f7bf2dc06a 100644
--- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ccr/action/CcrStatsAction.java
+++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ccr/action/CcrStatsAction.java
@@ -77,17 +77,17 @@ public boolean equals(Object o) {
                 return false;
             }
             Request that = (Request) o;
-            return Objects.equals(this.timeout, that.timeout) && Objects.equals(this.masterNodeTimeout, that.masterNodeTimeout);
+            return Objects.equals(this.timeout, that.timeout) && Objects.equals(this.masterNodeTimeout(), that.masterNodeTimeout());
         }
 
         @Override
         public int hashCode() {
-            return Objects.hash(this.timeout, this.masterNodeTimeout);
+            return Objects.hash(this.timeout, this.masterNodeTimeout());
         }
 
         @Override
         public String toString() {
-            return "CcrStatsAction.Request[timeout=" + timeout + ", masterNodeTimeout=" + masterNodeTimeout + "]";
+            return "CcrStatsAction.Request[timeout=" + timeout + ", masterNodeTimeout=" + masterNodeTimeout() + "]";
         }
     }
 
diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/searchablesnapshots/MountSearchableSnapshotRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/searchablesnapshots/MountSearchableSnapshotRequest.java
index 76b0634b55b39..3cb7b5b07fc1b 100644
--- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/searchablesnapshots/MountSearchableSnapshotRequest.java
+++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/searchablesnapshots/MountSearchableSnapshotRequest.java
@@ -222,7 +222,7 @@ public boolean equals(Object o) {
             && Objects.equals(snapshotIndexName, that.snapshotIndexName)
             && Objects.equals(indexSettings, that.indexSettings)
             && Arrays.equals(ignoreIndexSettings, that.ignoreIndexSettings)
-            && Objects.equals(masterNodeTimeout, that.masterNodeTimeout);
+            && Objects.equals(masterNodeTimeout(), that.masterNodeTimeout());
     }
 
     @Override
@@ -234,7 +234,7 @@ public int hashCode() {
             snapshotIndexName,
             indexSettings,
             waitForCompletion,
-            masterNodeTimeout,
+            masterNodeTimeout(),
             storage
         );
         result = 31 * result + Arrays.hashCode(ignoreIndexSettings);