Skip to content

Commit 810e86b

Browse files
thecoopthecoop
authored
Backport serialization fix of put/delete shutdown requests to 8.14 (#108251)
Backport of #107862 to 8.14
1 parent 176061e commit 810e86b

File tree

5 files changed

+311
-0
lines changed

5 files changed

+311
-0
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ static TransportVersion def(int id) {
133133
public static final TransportVersion INDEX_REQUEST_NORMALIZED_BYTES_PARSED = def(8_593_00_0);
134134
public static final TransportVersion INGEST_GRAPH_STRUCTURE_EXCEPTION = def(8_594_00_0);
135135
public static final TransportVersion ML_MODEL_IN_SERVICE_SETTINGS = def(8_595_00_0);
136+
public static final TransportVersion SHUTDOWN_REQUEST_TIMEOUTS_FIX_8_13 = def(8_595_00_1);
136137
// 8.14.0+
137138
public static final TransportVersion RANDOM_AGG_SHARD_SEED = def(8_596_00_0);
138139
public static final TransportVersion ESQL_TIMINGS = def(8_597_00_0);
@@ -175,6 +176,7 @@ static TransportVersion def(int id) {
175176
public static final TransportVersion ML_INFERENCE_AZURE_OPENAI_EMBEDDINGS = def(8_634_00_0);
176177
public static final TransportVersion ILM_SHRINK_ENABLE_WRITE = def(8_635_00_0);
177178
public static final TransportVersion GEOIP_CACHE_STATS = def(8_636_00_0);
179+
public static final TransportVersion SHUTDOWN_REQUEST_TIMEOUTS_FIX_8_14 = def(8_636_00_1);
178180

179181
/*
180182
* STOP! READ THIS FIRST! No, really,

‎x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/DeleteShutdownNodeAction.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,15 @@
77

88
package org.elasticsearch.xpack.shutdown;
99

10+
import org.elasticsearch.TransportVersions;
1011
import org.elasticsearch.action.ActionRequestValidationException;
1112
import org.elasticsearch.action.ActionType;
1213
import org.elasticsearch.action.support.master.AcknowledgedRequest;
1314
import org.elasticsearch.action.support.master.AcknowledgedResponse;
1415
import org.elasticsearch.common.Strings;
1516
import org.elasticsearch.common.io.stream.StreamInput;
1617
import org.elasticsearch.common.io.stream.StreamOutput;
18+
import org.elasticsearch.tasks.TaskId;
1719

1820
import java.io.IOException;
1921

@@ -35,11 +37,22 @@ public Request(String nodeId) {
3537
}
3638

3739
public Request(StreamInput in) throws IOException {
40+
if (in.getTransportVersion().isPatchFrom(TransportVersions.SHUTDOWN_REQUEST_TIMEOUTS_FIX_8_13)
41+
|| in.getTransportVersion().onOrAfter(TransportVersions.SHUTDOWN_REQUEST_TIMEOUTS_FIX_8_14)) {
42+
// effectively super(in):
43+
setParentTask(TaskId.readFromStream(in));
44+
masterNodeTimeout(in.readTimeValue());
45+
timeout(in.readTimeValue());
46+
}
3847
this.nodeId = in.readString();
3948
}
4049

4150
@Override
4251
public void writeTo(StreamOutput out) throws IOException {
52+
if (out.getTransportVersion().isPatchFrom(TransportVersions.SHUTDOWN_REQUEST_TIMEOUTS_FIX_8_13)
53+
|| out.getTransportVersion().onOrAfter(TransportVersions.SHUTDOWN_REQUEST_TIMEOUTS_FIX_8_14)) {
54+
super.writeTo(out);
55+
}
4356
out.writeString(this.nodeId);
4457
}
4558

‎x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/PutShutdownNodeAction.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
package org.elasticsearch.xpack.shutdown;
99

10+
import org.elasticsearch.TransportVersions;
1011
import org.elasticsearch.action.ActionRequestValidationException;
1112
import org.elasticsearch.action.ActionType;
1213
import org.elasticsearch.action.support.master.AcknowledgedRequest;
@@ -17,6 +18,7 @@
1718
import org.elasticsearch.common.io.stream.StreamOutput;
1819
import org.elasticsearch.core.Nullable;
1920
import org.elasticsearch.core.TimeValue;
21+
import org.elasticsearch.tasks.TaskId;
2022
import org.elasticsearch.xcontent.ConstructingObjectParser;
2123
import org.elasticsearch.xcontent.ParseField;
2224
import org.elasticsearch.xcontent.XContentParser;
@@ -96,6 +98,13 @@ public Request(
9698
}
9799

98100
public Request(StreamInput in) throws IOException {
101+
if (in.getTransportVersion().isPatchFrom(TransportVersions.SHUTDOWN_REQUEST_TIMEOUTS_FIX_8_13)
102+
|| in.getTransportVersion().onOrAfter(TransportVersions.SHUTDOWN_REQUEST_TIMEOUTS_FIX_8_14)) {
103+
// effectively super(in):
104+
setParentTask(TaskId.readFromStream(in));
105+
masterNodeTimeout(in.readTimeValue());
106+
timeout(in.readTimeValue());
107+
}
99108
this.nodeId = in.readString();
100109
this.type = in.readEnum(SingleNodeShutdownMetadata.Type.class);
101110
this.reason = in.readString();
@@ -114,6 +123,10 @@ public Request(StreamInput in) throws IOException {
114123

115124
@Override
116125
public void writeTo(StreamOutput out) throws IOException {
126+
if (out.getTransportVersion().isPatchFrom(TransportVersions.SHUTDOWN_REQUEST_TIMEOUTS_FIX_8_13)
127+
|| out.getTransportVersion().onOrAfter(TransportVersions.SHUTDOWN_REQUEST_TIMEOUTS_FIX_8_14)) {
128+
super.writeTo(out);
129+
}
117130
out.writeString(nodeId);
118131
if (out.getTransportVersion().before(REPLACE_SHUTDOWN_TYPE_ADDED_VERSION)
119132
&& this.type == SingleNodeShutdownMetadata.Type.REPLACE) {

‎x-pack/plugin/shutdown/src/test/java/org/elasticsearch/xpack/shutdown/DeleteShutdownRequestTests.java

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.xpack.shutdown;
9+
10+
import org.elasticsearch.common.io.stream.StreamOutput;
11+
import org.elasticsearch.common.io.stream.Writeable;
12+
import org.elasticsearch.core.TimeValue;
13+
import org.elasticsearch.tasks.TaskId;
14+
import org.elasticsearch.test.AbstractWireSerializingTestCase;
15+
import org.elasticsearch.test.ESTestCase;
16+
17+
import java.io.IOException;
18+
19+
public class DeleteShutdownRequestTests extends AbstractWireSerializingTestCase<DeleteShutdownRequestTests.RequestWrapper> {
20+
21+
/**
22+
* Wraps a {@link DeleteShutdownNodeAction.Request} to add proper equality checks
23+
*/
24+
record RequestWrapper(String nodeId, TaskId parentTask, TimeValue masterNodeTimeout, TimeValue ackTimeout) implements Writeable {
25+
@Override
26+
public void writeTo(StreamOutput out) throws IOException {
27+
final var request = new DeleteShutdownNodeAction.Request(nodeId);
28+
request.setParentTask(parentTask);
29+
request.timeout(ackTimeout);
30+
request.masterNodeTimeout(masterNodeTimeout);
31+
request.writeTo(out);
32+
}
33+
}
34+
35+
@Override
36+
protected Writeable.Reader<RequestWrapper> instanceReader() {
37+
return in -> {
38+
final var request = new DeleteShutdownNodeAction.Request(in);
39+
return new RequestWrapper(request.getNodeId(), request.getParentTask(), request.masterNodeTimeout(), request.ackTimeout());
40+
};
41+
}
42+
43+
@Override
44+
protected RequestWrapper createTestInstance() {
45+
return new RequestWrapper(
46+
randomIdentifier(),
47+
randomTaskId(),
48+
TimeValue.parseTimeValue(randomTimeValue(), getTestName()),
49+
TimeValue.parseTimeValue(randomTimeValue(), getTestName())
50+
);
51+
}
52+
53+
private static TaskId randomTaskId() {
54+
return randomBoolean() ? TaskId.EMPTY_TASK_ID : new TaskId(randomIdentifier(), randomNonNegativeLong());
55+
}
56+
57+
@Override
58+
protected RequestWrapper mutateInstance(RequestWrapper instance) {
59+
return switch (between(1, 4)) {
60+
case 1 -> new RequestWrapper(
61+
randomValueOtherThan(instance.nodeId, ESTestCase::randomIdentifier),
62+
instance.parentTask,
63+
instance.ackTimeout,
64+
instance.masterNodeTimeout
65+
);
66+
case 2 -> new RequestWrapper(
67+
instance.nodeId,
68+
randomValueOtherThan(instance.parentTask, DeleteShutdownRequestTests::randomTaskId),
69+
instance.ackTimeout,
70+
instance.masterNodeTimeout
71+
);
72+
case 3 -> new RequestWrapper(
73+
instance.nodeId,
74+
instance.parentTask,
75+
randomValueOtherThan(instance.ackTimeout, () -> TimeValue.parseTimeValue(randomTimeValue(), getTestName())),
76+
instance.masterNodeTimeout
77+
);
78+
case 4 -> new RequestWrapper(
79+
instance.nodeId,
80+
instance.parentTask,
81+
instance.ackTimeout,
82+
randomValueOtherThan(instance.masterNodeTimeout, () -> TimeValue.parseTimeValue(randomTimeValue(), getTestName()))
83+
);
84+
default -> throw new AssertionError("impossible");
85+
};
86+
}
87+
}

‎x-pack/plugin/shutdown/src/test/java/org/elasticsearch/xpack/shutdown/PutShutdownRequestTests.java

Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.xpack.shutdown;
9+
10+
import org.elasticsearch.cluster.metadata.SingleNodeShutdownMetadata;
11+
import org.elasticsearch.common.io.stream.StreamOutput;
12+
import org.elasticsearch.common.io.stream.Writeable;
13+
import org.elasticsearch.core.TimeValue;
14+
import org.elasticsearch.tasks.TaskId;
15+
import org.elasticsearch.test.AbstractWireSerializingTestCase;
16+
import org.elasticsearch.test.ESTestCase;
17+
18+
import java.io.IOException;
19+
20+
public class PutShutdownRequestTests extends AbstractWireSerializingTestCase<PutShutdownRequestTests.RequestWrapper> {
21+
22+
/**
23+
* Wraps a {@link org.elasticsearch.xpack.shutdown.PutShutdownNodeAction.Request} to add proper equality checks
24+
*/
25+
record RequestWrapper(
26+
String nodeId,
27+
SingleNodeShutdownMetadata.Type type,
28+
String reason,
29+
TimeValue allocationDelay,
30+
String targetNodeName,
31+
TimeValue gracePeriod,
32+
TaskId parentTask,
33+
TimeValue masterNodeTimeout,
34+
TimeValue ackTimeout
35+
) implements Writeable {
36+
@Override
37+
public void writeTo(StreamOutput out) throws IOException {
38+
final var request = new PutShutdownNodeAction.Request(nodeId, type, reason, allocationDelay, targetNodeName, gracePeriod);
39+
request.setParentTask(parentTask);
40+
request.timeout(ackTimeout);
41+
request.masterNodeTimeout(masterNodeTimeout);
42+
request.writeTo(out);
43+
}
44+
}
45+
46+
@Override
47+
protected Writeable.Reader<RequestWrapper> instanceReader() {
48+
return in -> {
49+
final var request = new PutShutdownNodeAction.Request(in);
50+
return new RequestWrapper(
51+
request.getNodeId(),
52+
request.getType(),
53+
request.getReason(),
54+
request.getAllocationDelay(),
55+
request.getTargetNodeName(),
56+
request.getGracePeriod(),
57+
request.getParentTask(),
58+
request.masterNodeTimeout(),
59+
request.ackTimeout()
60+
);
61+
};
62+
}
63+
64+
@Override
65+
protected RequestWrapper createTestInstance() {
66+
return new RequestWrapper(
67+
randomIdentifier(),
68+
randomFrom(SingleNodeShutdownMetadata.Type.values()),
69+
randomIdentifier(),
70+
randomOptionalTimeValue(),
71+
randomOptionalIdentifier(),
72+
randomOptionalTimeValue(),
73+
randomTaskId(),
74+
TimeValue.parseTimeValue(randomTimeValue(), getTestName()),
75+
TimeValue.parseTimeValue(randomTimeValue(), getTestName())
76+
);
77+
}
78+
79+
private static String randomOptionalIdentifier() {
80+
return randomBoolean() ? null : randomIdentifier();
81+
}
82+
83+
private TimeValue randomOptionalTimeValue() {
84+
return randomBoolean() ? null : TimeValue.parseTimeValue(randomTimeValue(), getTestName());
85+
}
86+
87+
private static TaskId randomTaskId() {
88+
return randomBoolean() ? TaskId.EMPTY_TASK_ID : new TaskId(randomIdentifier(), randomNonNegativeLong());
89+
}
90+
91+
@Override
92+
protected RequestWrapper mutateInstance(RequestWrapper instance) {
93+
return switch (between(1, 9)) {
94+
case 1 -> new RequestWrapper(
95+
randomValueOtherThan(instance.nodeId, ESTestCase::randomIdentifier),
96+
instance.type,
97+
instance.reason,
98+
instance.allocationDelay,
99+
instance.targetNodeName,
100+
instance.gracePeriod,
101+
instance.parentTask,
102+
instance.ackTimeout,
103+
instance.masterNodeTimeout
104+
);
105+
case 2 -> new RequestWrapper(
106+
instance.nodeId,
107+
randomValueOtherThan(instance.type, () -> randomFrom(SingleNodeShutdownMetadata.Type.values())),
108+
instance.reason,
109+
instance.allocationDelay,
110+
instance.targetNodeName,
111+
instance.gracePeriod,
112+
instance.parentTask,
113+
instance.ackTimeout,
114+
instance.masterNodeTimeout
115+
);
116+
case 3 -> new RequestWrapper(
117+
instance.nodeId,
118+
instance.type,
119+
randomValueOtherThan(instance.reason, ESTestCase::randomIdentifier),
120+
instance.allocationDelay,
121+
instance.targetNodeName,
122+
instance.gracePeriod,
123+
instance.parentTask,
124+
instance.ackTimeout,
125+
instance.masterNodeTimeout
126+
);
127+
case 4 -> new RequestWrapper(
128+
instance.nodeId,
129+
instance.type,
130+
instance.reason,
131+
randomValueOtherThan(instance.allocationDelay, this::randomOptionalTimeValue),
132+
instance.targetNodeName,
133+
instance.gracePeriod,
134+
instance.parentTask,
135+
instance.ackTimeout,
136+
instance.masterNodeTimeout
137+
);
138+
case 5 -> new RequestWrapper(
139+
instance.nodeId,
140+
instance.type,
141+
instance.reason,
142+
instance.allocationDelay,
143+
randomValueOtherThan(instance.targetNodeName, PutShutdownRequestTests::randomOptionalIdentifier),
144+
instance.gracePeriod,
145+
instance.parentTask,
146+
instance.ackTimeout,
147+
instance.masterNodeTimeout
148+
);
149+
case 6 -> new RequestWrapper(
150+
instance.nodeId,
151+
instance.type,
152+
instance.reason,
153+
instance.allocationDelay,
154+
instance.targetNodeName,
155+
randomValueOtherThan(instance.gracePeriod, this::randomOptionalTimeValue),
156+
instance.parentTask,
157+
instance.ackTimeout,
158+
instance.masterNodeTimeout
159+
);
160+
case 7 -> new RequestWrapper(
161+
instance.nodeId,
162+
instance.type,
163+
instance.reason,
164+
instance.allocationDelay,
165+
instance.targetNodeName,
166+
instance.gracePeriod,
167+
randomValueOtherThan(instance.parentTask, PutShutdownRequestTests::randomTaskId),
168+
instance.ackTimeout,
169+
instance.masterNodeTimeout
170+
);
171+
case 8 -> new RequestWrapper(
172+
instance.nodeId,
173+
instance.type,
174+
instance.reason,
175+
instance.allocationDelay,
176+
instance.targetNodeName,
177+
instance.gracePeriod,
178+
instance.parentTask,
179+
randomValueOtherThan(instance.ackTimeout, () -> TimeValue.parseTimeValue(randomTimeValue(), getTestName())),
180+
instance.masterNodeTimeout
181+
);
182+
case 9 -> new RequestWrapper(
183+
instance.nodeId,
184+
instance.type,
185+
instance.reason,
186+
instance.allocationDelay,
187+
instance.targetNodeName,
188+
instance.gracePeriod,
189+
instance.parentTask,
190+
instance.ackTimeout,
191+
randomValueOtherThan(instance.masterNodeTimeout, () -> TimeValue.parseTimeValue(randomTimeValue(), getTestName()))
192+
);
193+
default -> throw new AssertionError("impossible");
194+
};
195+
}
196+
}

0 commit comments

Comments
 (0)