Skip to content

Commit

Permalink
Encapsulate MasterNodeRequest#masterNodeTimeout (elastic#107999)
Browse files Browse the repository at this point in the history
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 elastic#107984
  • Loading branch information
DaveCTurner authored Apr 30, 2024
1 parent 61a3415 commit a2d9cc6
Show file tree
Hide file tree
Showing 8 changed files with 32 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -495,7 +495,7 @@ public String toString() {
+ ", waitForCompletion="
+ waitForCompletion
+ ", masterNodeTimeout="
+ masterNodeTimeout
+ masterNodeTimeout()
+ ", metadata="
+ userMetadata
+ '}';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import org.elasticsearch.core.TimeValue;

import java.io.IOException;
import java.util.Objects;

/**
* A based request for master based operation.
Expand All @@ -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();
Expand All @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() + "]";
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -234,7 +234,7 @@ public int hashCode() {
snapshotIndexName,
indexSettings,
waitForCompletion,
masterNodeTimeout,
masterNodeTimeout(),
storage
);
result = 31 * result + Arrays.hashCode(ignoreIndexSettings);
Expand Down

0 comments on commit a2d9cc6

Please sign in to comment.