Skip to content

Commit 7b0573e

Browse files
DaveCTurnerDaveCTurner
authored
Remove trappy master-node timeouts in o.e.x.core.action (#136826)
Migrates the set-reset-mode and set-upgrade-mode actions to use explicit master-node timeouts. Also moves the test-only `XContent` parsers into the test suite. Relates #107984
1 parent 4d7621a commit 7b0573e

File tree

15 files changed

+179
-118
lines changed

15 files changed

+179
-118
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/action/SetResetModeActionRequest.java‎

Lines changed: 17 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@
88
package org.elasticsearch.xpack.core.action;
99

1010
import org.elasticsearch.action.support.master.AcknowledgedRequest;
11+
import org.elasticsearch.action.support.master.MasterNodeRequest;
1112
import org.elasticsearch.common.io.stream.StreamInput;
1213
import org.elasticsearch.common.io.stream.StreamOutput;
13-
import org.elasticsearch.xcontent.ConstructingObjectParser;
14-
import org.elasticsearch.xcontent.ParseField;
14+
import org.elasticsearch.core.TimeValue;
1515
import org.elasticsearch.xcontent.ToXContent;
1616
import org.elasticsearch.xcontent.ToXContentObject;
1717
import org.elasticsearch.xcontent.XContentBuilder;
@@ -20,31 +20,23 @@
2020
import java.util.Objects;
2121

2222
public class SetResetModeActionRequest extends AcknowledgedRequest<SetResetModeActionRequest> implements ToXContentObject {
23-
public static SetResetModeActionRequest enabled() {
24-
return new SetResetModeActionRequest(true, false);
23+
24+
public static SetResetModeActionRequest enabled(TimeValue masterNodeTimeout) {
25+
return new SetResetModeActionRequest(masterNodeTimeout, true, false);
2526
}
2627

27-
public static SetResetModeActionRequest disabled(boolean deleteMetadata) {
28-
return new SetResetModeActionRequest(false, deleteMetadata);
28+
public static SetResetModeActionRequest disabled(TimeValue masterNodeTimeout, boolean deleteMetadata) {
29+
return new SetResetModeActionRequest(masterNodeTimeout, false, deleteMetadata);
2930
}
3031

3132
private final boolean enabled;
3233
private final boolean deleteMetadata;
3334

34-
private static final ParseField ENABLED = new ParseField("enabled");
35-
private static final ParseField DELETE_METADATA = new ParseField("delete_metadata");
36-
public static final ConstructingObjectParser<SetResetModeActionRequest, Void> PARSER = new ConstructingObjectParser<>(
37-
"set_reset_mode_action_request",
38-
a -> new SetResetModeActionRequest((Boolean) a[0], (Boolean) a[1])
39-
);
40-
41-
static {
42-
PARSER.declareBoolean(ConstructingObjectParser.constructorArg(), ENABLED);
43-
PARSER.declareBoolean(ConstructingObjectParser.optionalConstructorArg(), DELETE_METADATA);
44-
}
35+
static final String ENABLED_FIELD_NAME = "enabled";
36+
static final String DELETE_METADATA_FIELD_NAME = "delete_metadata";
4537

46-
SetResetModeActionRequest(boolean enabled, Boolean deleteMetadata) {
47-
super(TRAPPY_IMPLICIT_DEFAULT_MASTER_NODE_TIMEOUT, DEFAULT_ACK_TIMEOUT);
38+
SetResetModeActionRequest(TimeValue masterNodeTimeout, boolean enabled, Boolean deleteMetadata) {
39+
super(masterNodeTimeout, MasterNodeRequest.INFINITE_MASTER_NODE_TIMEOUT /* wait indefinitely for acks */);
4840
this.enabled = enabled;
4941
this.deleteMetadata = deleteMetadata != null && deleteMetadata;
5042
}
@@ -72,7 +64,7 @@ public void writeTo(StreamOutput out) throws IOException {
7264

7365
@Override
7466
public int hashCode() {
75-
return Objects.hash(enabled, deleteMetadata);
67+
return Objects.hash(masterNodeTimeout(), enabled, deleteMetadata);
7668
}
7769

7870
@Override
@@ -84,15 +76,17 @@ public boolean equals(Object obj) {
8476
return false;
8577
}
8678
SetResetModeActionRequest other = (SetResetModeActionRequest) obj;
87-
return Objects.equals(enabled, other.enabled) && Objects.equals(deleteMetadata, other.deleteMetadata);
79+
return Objects.equals(masterNodeTimeout(), other.masterNodeTimeout())
80+
&& Objects.equals(enabled, other.enabled)
81+
&& Objects.equals(deleteMetadata, other.deleteMetadata);
8882
}
8983

9084
@Override
9185
public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException {
9286
builder.startObject();
93-
builder.field(ENABLED.getPreferredName(), enabled);
87+
builder.field(ENABLED_FIELD_NAME, enabled);
9488
if (enabled == false) {
95-
builder.field(DELETE_METADATA.getPreferredName(), deleteMetadata);
89+
builder.field(DELETE_METADATA_FIELD_NAME, deleteMetadata);
9690
}
9791
builder.endObject();
9892
return builder;

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/action/SetUpgradeModeActionRequest.java‎

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@
1010
import org.elasticsearch.action.support.master.AcknowledgedRequest;
1111
import org.elasticsearch.common.io.stream.StreamInput;
1212
import org.elasticsearch.common.io.stream.StreamOutput;
13-
import org.elasticsearch.xcontent.ConstructingObjectParser;
14-
import org.elasticsearch.xcontent.ParseField;
13+
import org.elasticsearch.core.TimeValue;
1514
import org.elasticsearch.xcontent.ToXContentObject;
1615
import org.elasticsearch.xcontent.XContentBuilder;
1716

@@ -22,18 +21,8 @@ public class SetUpgradeModeActionRequest extends AcknowledgedRequest<SetUpgradeM
2221

2322
private final boolean enabled;
2423

25-
private static final ParseField ENABLED = new ParseField("enabled");
26-
public static final ConstructingObjectParser<SetUpgradeModeActionRequest, Void> PARSER = new ConstructingObjectParser<>(
27-
"set_upgrade_mode_action_request",
28-
a -> new SetUpgradeModeActionRequest((Boolean) a[0])
29-
);
30-
31-
static {
32-
PARSER.declareBoolean(ConstructingObjectParser.constructorArg(), ENABLED);
33-
}
34-
35-
public SetUpgradeModeActionRequest(boolean enabled) {
36-
super(TRAPPY_IMPLICIT_DEFAULT_MASTER_NODE_TIMEOUT, DEFAULT_ACK_TIMEOUT);
24+
public SetUpgradeModeActionRequest(TimeValue masterNodeTimeout, TimeValue ackTimeout, boolean enabled) {
25+
super(masterNodeTimeout, ackTimeout);
3726
this.enabled = enabled;
3827
}
3928

@@ -54,7 +43,7 @@ public void writeTo(StreamOutput out) throws IOException {
5443

5544
@Override
5645
public int hashCode() {
57-
return Objects.hash(enabled);
46+
return Objects.hash(masterNodeTimeout(), ackTimeout(), enabled);
5847
}
5948

6049
@Override
@@ -66,13 +55,15 @@ public boolean equals(Object obj) {
6655
return false;
6756
}
6857
SetUpgradeModeActionRequest other = (SetUpgradeModeActionRequest) obj;
69-
return enabled == other.enabled();
58+
return Objects.equals(masterNodeTimeout(), other.masterNodeTimeout())
59+
&& Objects.equals(ackTimeout(), other.ackTimeout())
60+
&& enabled == other.enabled();
7061
}
7162

7263
@Override
7364
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
7465
builder.startObject();
75-
builder.field(ENABLED.getPreferredName(), enabled);
66+
builder.field("enabled", enabled);
7667
builder.endObject();
7768
return builder;
7869
}

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/SetUpgradeModeAction.java‎

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,7 @@
99
import org.elasticsearch.action.ActionType;
1010
import org.elasticsearch.action.support.master.AcknowledgedResponse;
1111
import org.elasticsearch.common.io.stream.StreamInput;
12-
import org.elasticsearch.xcontent.ConstructingObjectParser;
13-
import org.elasticsearch.xcontent.ParseField;
12+
import org.elasticsearch.core.TimeValue;
1413
import org.elasticsearch.xpack.core.action.SetUpgradeModeActionRequest;
1514

1615
import java.io.IOException;
@@ -26,18 +25,8 @@ private SetUpgradeModeAction() {
2625

2726
public static class Request extends SetUpgradeModeActionRequest {
2827

29-
private static final ParseField ENABLED = new ParseField("enabled");
30-
public static final ConstructingObjectParser<Request, Void> PARSER = new ConstructingObjectParser<>(
31-
NAME,
32-
a -> new Request((Boolean) a[0])
33-
);
34-
35-
static {
36-
PARSER.declareBoolean(ConstructingObjectParser.constructorArg(), ENABLED);
37-
}
38-
39-
public Request(boolean enabled) {
40-
super(enabled);
28+
public Request(TimeValue masterNodeTimeout, TimeValue ackTimeout, boolean enabled) {
29+
super(masterNodeTimeout, ackTimeout, enabled);
4130
}
4231

4332
public Request(StreamInput in) throws IOException {

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/action/AbstractTransportSetUpgradeModeActionTests.java‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -174,19 +174,29 @@ private static class TestTransportSetUpgradeModeAction extends AbstractTransport
174174
}
175175

176176
public void runWithoutWaiting(boolean upgrade) throws Exception {
177-
masterOperation(mock(), new SetUpgradeModeActionRequest(upgrade), ClusterState.EMPTY_STATE, ActionListener.noop());
177+
masterOperation(
178+
mock(),
179+
new SetUpgradeModeActionRequest(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, upgrade),
180+
ClusterState.EMPTY_STATE,
181+
ActionListener.noop()
182+
);
178183
}
179184

180185
public Tuple<AcknowledgedResponse, Exception> run(boolean upgrade) throws Exception {
181186
AtomicReference<Tuple<AcknowledgedResponse, Exception>> response = new AtomicReference<>();
182187
CountDownLatch latch = new CountDownLatch(1);
183-
masterOperation(mock(), new SetUpgradeModeActionRequest(upgrade), ClusterState.EMPTY_STATE, ActionListener.wrap(r -> {
184-
response.set(Tuple.tuple(r, null));
185-
latch.countDown();
186-
}, e -> {
187-
response.set(Tuple.tuple(null, e));
188-
latch.countDown();
189-
}));
188+
masterOperation(
189+
mock(),
190+
new SetUpgradeModeActionRequest(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, upgrade),
191+
ClusterState.EMPTY_STATE,
192+
ActionListener.wrap(r -> {
193+
response.set(Tuple.tuple(r, null));
194+
latch.countDown();
195+
}, e -> {
196+
response.set(Tuple.tuple(null, e));
197+
latch.countDown();
198+
})
199+
);
190200
assertTrue("Failed to run TestTransportSetUpgradeModeAction in 10s", latch.await(10, TimeUnit.SECONDS));
191201
return response.get();
192202
}

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/action/SetResetModeActionRequestTests.java‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,16 @@
88

99
import org.elasticsearch.common.io.stream.Writeable;
1010
import org.elasticsearch.test.AbstractXContentSerializingTestCase;
11+
import org.elasticsearch.xcontent.ConstructingObjectParser;
12+
import org.elasticsearch.xcontent.ParseField;
1113
import org.elasticsearch.xcontent.XContentParser;
1214

1315
public class SetResetModeActionRequestTests extends AbstractXContentSerializingTestCase<SetResetModeActionRequest> {
1416

1517
@Override
1618
protected SetResetModeActionRequest createTestInstance() {
1719
boolean enabled = randomBoolean();
18-
return new SetResetModeActionRequest(enabled, enabled == false && randomBoolean());
20+
return new SetResetModeActionRequest(TEST_REQUEST_TIMEOUT, enabled, enabled == false && randomBoolean());
1921
}
2022

2123
@Override
@@ -30,6 +32,20 @@ protected Writeable.Reader<SetResetModeActionRequest> instanceReader() {
3032

3133
@Override
3234
protected SetResetModeActionRequest doParseInstance(XContentParser parser) {
33-
return SetResetModeActionRequest.PARSER.apply(parser, null);
35+
return PARSER.apply(parser, null);
3436
}
37+
38+
public static final ConstructingObjectParser<SetResetModeActionRequest, Void> PARSER = new ConstructingObjectParser<>(
39+
"set_reset_mode_action_request",
40+
a -> new SetResetModeActionRequest(TEST_REQUEST_TIMEOUT, (Boolean) a[0], (Boolean) a[1])
41+
);
42+
43+
static {
44+
PARSER.declareBoolean(ConstructingObjectParser.constructorArg(), new ParseField(SetResetModeActionRequest.ENABLED_FIELD_NAME));
45+
PARSER.declareBoolean(
46+
ConstructingObjectParser.optionalConstructorArg(),
47+
new ParseField(SetResetModeActionRequest.DELETE_METADATA_FIELD_NAME)
48+
);
49+
}
50+
3551
}

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/SetUpgradeModeActionRequestTests.java‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,26 @@
88

99
import org.elasticsearch.common.io.stream.Writeable;
1010
import org.elasticsearch.test.AbstractXContentSerializingTestCase;
11+
import org.elasticsearch.xcontent.ConstructingObjectParser;
12+
import org.elasticsearch.xcontent.ParseField;
1113
import org.elasticsearch.xcontent.XContentParser;
1214
import org.elasticsearch.xpack.core.ml.action.SetUpgradeModeAction.Request;
1315

1416
public class SetUpgradeModeActionRequestTests extends AbstractXContentSerializingTestCase<Request> {
1517

18+
private static final ParseField ENABLED = new ParseField("enabled");
19+
public static final ConstructingObjectParser<Request, Void> PARSER = new ConstructingObjectParser<>(
20+
SetUpgradeModeAction.NAME,
21+
a -> new Request(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, (Boolean) a[0])
22+
);
23+
24+
static {
25+
PARSER.declareBoolean(ConstructingObjectParser.constructorArg(), ENABLED);
26+
}
27+
1628
@Override
1729
protected Request createTestInstance() {
18-
return new Request(randomBoolean());
30+
return new Request(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, randomBoolean());
1931
}
2032

2133
@Override
@@ -30,6 +42,7 @@ protected Writeable.Reader<Request> instanceReader() {
3042

3143
@Override
3244
protected Request doParseInstance(XContentParser parser) {
33-
return Request.PARSER.apply(parser, null);
45+
return PARSER.apply(parser, null);
3446
}
47+
3548
}

‎x-pack/plugin/ml/qa/native-multi-node-tests/src/javaRestTest/java/org/elasticsearch/xpack/ml/integration/MlNativeIntegTestCase.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -283,8 +283,10 @@ protected void cleanUpResources() {
283283
}
284284

285285
protected void setUpgradeModeTo(boolean enabled) {
286-
AcknowledgedResponse response = client().execute(SetUpgradeModeAction.INSTANCE, new SetUpgradeModeAction.Request(enabled))
287-
.actionGet();
286+
AcknowledgedResponse response = client().execute(
287+
SetUpgradeModeAction.INSTANCE,
288+
new SetUpgradeModeAction.Request(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, enabled)
289+
).actionGet();
288290
assertThat(response.isAcknowledged(), is(true));
289291
assertThat(upgradeMode(), is(enabled));
290292
}

0 commit comments

Comments
 (0)