Skip to content

Commit 70b5ac0

Browse files
martijnvgmartijnvg
committed
[CCR] Add validation for max_retry_delay (#33648)
1 parent a04db2e commit 70b5ac0

File tree

5 files changed

+105
-11
lines changed

5 files changed

+105
-11
lines changed

‎x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/FollowIndexRequestTests.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,18 @@
55
*/
66
package org.elasticsearch.xpack.ccr.action;
77

8+
import org.elasticsearch.action.ActionRequestValidationException;
89
import org.elasticsearch.common.unit.TimeValue;
910
import org.elasticsearch.common.xcontent.XContentParser;
1011
import org.elasticsearch.test.AbstractStreamableXContentTestCase;
1112
import org.elasticsearch.xpack.core.ccr.action.FollowIndexAction;
1213

1314
import java.io.IOException;
1415

16+
import static org.hamcrest.Matchers.containsString;
17+
import static org.hamcrest.Matchers.notNullValue;
18+
import static org.hamcrest.Matchers.nullValue;
19+
1520
public class FollowIndexRequestTests extends AbstractStreamableXContentTestCase<FollowIndexAction.Request> {
1621

1722
@Override
@@ -39,4 +44,21 @@ static FollowIndexAction.Request createTestRequest() {
3944
randomIntBetween(1, Integer.MAX_VALUE), randomNonNegativeLong(), randomIntBetween(1, Integer.MAX_VALUE),
4045
randomIntBetween(1, Integer.MAX_VALUE), TimeValue.timeValueMillis(500), TimeValue.timeValueMillis(500));
4146
}
47+
48+
public void testValidate() {
49+
FollowIndexAction.Request request = new FollowIndexAction.Request("index1", "index2", null, null, null, null,
50+
null, TimeValue.ZERO, null);
51+
ActionRequestValidationException validationException = request.validate();
52+
assertThat(validationException, notNullValue());
53+
assertThat(validationException.getMessage(), containsString("[max_retry_delay] must be positive but was [0ms]"));
54+
55+
request = new FollowIndexAction.Request("index1", "index2", null, null, null, null, null, TimeValue.timeValueMinutes(10), null);
56+
validationException = request.validate();
57+
assertThat(validationException, notNullValue());
58+
assertThat(validationException.getMessage(), containsString("[max_retry_delay] must be less than [5m] but was [10m]"));
59+
60+
request = new FollowIndexAction.Request("index1", "index2", null, null, null, null, null, TimeValue.timeValueMinutes(1), null);
61+
validationException = request.validate();
62+
assertThat(validationException, nullValue());
63+
}
4264
}

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ccr/AutoFollowMetadata.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ public static class AutoFollowPattern implements Writeable, ToXContentObject {
169169
public static final ParseField MAX_BATCH_SIZE_IN_BYTES = new ParseField("max_batch_size_in_bytes");
170170
public static final ParseField MAX_CONCURRENT_WRITE_BATCHES = new ParseField("max_concurrent_write_batches");
171171
public static final ParseField MAX_WRITE_BUFFER_SIZE = new ParseField("max_write_buffer_size");
172-
public static final ParseField MAX_RETRY_DELAY = new ParseField("retry_timeout");
172+
public static final ParseField MAX_RETRY_DELAY = new ParseField("max_retry_delay");
173173
public static final ParseField IDLE_SHARD_RETRY_DELAY = new ParseField("idle_shard_retry_delay");
174174

175175
@SuppressWarnings("unchecked")

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

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
import java.io.IOException;
2626
import java.util.Objects;
2727

28+
import static org.elasticsearch.action.ValidateActions.addValidationError;
29+
2830
public final class FollowIndexAction extends Action<
2931
FollowIndexAction.Request,
3032
AcknowledgedResponse,
@@ -38,8 +40,9 @@ public final class FollowIndexAction extends Action<
3840
public static final int DEFAULT_MAX_CONCURRENT_READ_BATCHES = 1;
3941
public static final int DEFAULT_MAX_CONCURRENT_WRITE_BATCHES = 1;
4042
public static final long DEFAULT_MAX_BATCH_SIZE_IN_BYTES = Long.MAX_VALUE;
41-
public static final TimeValue DEFAULT_RETRY_TIMEOUT = new TimeValue(500);
42-
public static final TimeValue DEFAULT_IDLE_SHARD_RETRY_DELAY = TimeValue.timeValueSeconds(10);
43+
static final TimeValue DEFAULT_MAX_RETRY_DELAY = new TimeValue(500);
44+
static final TimeValue DEFAULT_IDLE_SHARD_RETRY_DELAY = TimeValue.timeValueSeconds(10);
45+
static final TimeValue MAX_RETRY_DELAY = TimeValue.timeValueMinutes(5);
4346

4447
private FollowIndexAction() {
4548
super(NAME);
@@ -59,7 +62,7 @@ public static class Request extends ActionRequest implements ToXContentObject {
5962
private static final ParseField MAX_BATCH_SIZE_IN_BYTES = new ParseField("max_batch_size_in_bytes");
6063
private static final ParseField MAX_CONCURRENT_WRITE_BATCHES = new ParseField("max_concurrent_write_batches");
6164
private static final ParseField MAX_WRITE_BUFFER_SIZE = new ParseField("max_write_buffer_size");
62-
private static final ParseField MAX_RETRY_DELAY = new ParseField("max_retry_delay");
65+
private static final ParseField MAX_RETRY_DELAY_FIELD = new ParseField("max_retry_delay");
6366
private static final ParseField IDLE_SHARD_RETRY_DELAY = new ParseField("idle_shard_retry_delay");
6467
private static final ConstructingObjectParser<Request, String> PARSER = new ConstructingObjectParser<>(NAME, true,
6568
(args, followerIndex) -> {
@@ -80,8 +83,8 @@ public static class Request extends ActionRequest implements ToXContentObject {
8083
PARSER.declareInt(ConstructingObjectParser.optionalConstructorArg(), MAX_WRITE_BUFFER_SIZE);
8184
PARSER.declareField(
8285
ConstructingObjectParser.optionalConstructorArg(),
83-
(p, c) -> TimeValue.parseTimeValue(p.text(), MAX_RETRY_DELAY.getPreferredName()),
84-
MAX_RETRY_DELAY,
86+
(p, c) -> TimeValue.parseTimeValue(p.text(), MAX_RETRY_DELAY_FIELD.getPreferredName()),
87+
MAX_RETRY_DELAY_FIELD,
8588
ObjectParser.ValueType.STRING);
8689
PARSER.declareField(
8790
ConstructingObjectParser.optionalConstructorArg(),
@@ -207,7 +210,7 @@ public Request(
207210
throw new IllegalArgumentException(MAX_WRITE_BUFFER_SIZE.getPreferredName() + " must be larger than 0");
208211
}
209212

210-
final TimeValue actualRetryTimeout = maxRetryDelay == null ? DEFAULT_RETRY_TIMEOUT : maxRetryDelay;
213+
final TimeValue actualRetryTimeout = maxRetryDelay == null ? DEFAULT_MAX_RETRY_DELAY : maxRetryDelay;
211214
final TimeValue actualIdleShardRetryDelay = idleShardRetryDelay == null ? DEFAULT_IDLE_SHARD_RETRY_DELAY : idleShardRetryDelay;
212215

213216
this.leaderIndex = leaderIndex;
@@ -227,7 +230,20 @@ public Request() {
227230

228231
@Override
229232
public ActionRequestValidationException validate() {
230-
return null;
233+
ActionRequestValidationException validationException = null;
234+
235+
if (maxRetryDelay.millis() <= 0) {
236+
String message = "[" + MAX_RETRY_DELAY_FIELD.getPreferredName() + "] must be positive but was [" +
237+
maxRetryDelay.getStringRep() + "]";
238+
validationException = addValidationError(message, validationException);
239+
}
240+
if (maxRetryDelay.millis() > FollowIndexAction.MAX_RETRY_DELAY.millis()) {
241+
String message = "[" + MAX_RETRY_DELAY_FIELD.getPreferredName() + "] must be less than [" + MAX_RETRY_DELAY +
242+
"] but was [" + maxRetryDelay.getStringRep() + "]";
243+
validationException = addValidationError(message, validationException);
244+
}
245+
246+
return validationException;
231247
}
232248

233249
@Override
@@ -269,7 +285,7 @@ public XContentBuilder toXContent(final XContentBuilder builder, final Params pa
269285
builder.field(MAX_WRITE_BUFFER_SIZE.getPreferredName(), maxWriteBufferSize);
270286
builder.field(MAX_CONCURRENT_READ_BATCHES.getPreferredName(), maxConcurrentReadBatches);
271287
builder.field(MAX_CONCURRENT_WRITE_BATCHES.getPreferredName(), maxConcurrentWriteBatches);
272-
builder.field(MAX_RETRY_DELAY.getPreferredName(), maxRetryDelay.getStringRep());
288+
builder.field(MAX_RETRY_DELAY_FIELD.getPreferredName(), maxRetryDelay.getStringRep());
273289
builder.field(IDLE_SHARD_RETRY_DELAY.getPreferredName(), idleShardRetryDelay.getStringRep());
274290
}
275291
builder.endObject();

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

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,25 @@ public static Request fromXContent(XContentParser parser, String remoteClusterAl
102102
public ActionRequestValidationException validate() {
103103
ActionRequestValidationException validationException = null;
104104
if (leaderClusterAlias == null) {
105-
validationException = addValidationError("leaderClusterAlias is missing", validationException);
105+
validationException = addValidationError("[" + LEADER_CLUSTER_ALIAS_FIELD.getPreferredName() +
106+
"] is missing", validationException);
106107
}
107108
if (leaderIndexPatterns == null || leaderIndexPatterns.isEmpty()) {
108-
validationException = addValidationError("leaderIndexPatterns is missing", validationException);
109+
validationException = addValidationError("[" + LEADER_INDEX_PATTERNS_FIELD.getPreferredName() +
110+
"] is missing", validationException);
111+
}
112+
if (maxRetryDelay != null) {
113+
if (maxRetryDelay.millis() <= 0) {
114+
String message = "[" + AutoFollowPattern.MAX_RETRY_DELAY.getPreferredName() + "] must be positive but was [" +
115+
maxRetryDelay.getStringRep() + "]";
116+
validationException = addValidationError(message, validationException);
117+
}
118+
if (maxRetryDelay.millis() > FollowIndexAction.MAX_RETRY_DELAY.millis()) {
119+
String message = "[" + AutoFollowPattern.MAX_RETRY_DELAY.getPreferredName() + "] must be less than [" +
120+
FollowIndexAction.MAX_RETRY_DELAY +
121+
"] but was [" + maxRetryDelay.getStringRep() + "]";
122+
validationException = addValidationError(message, validationException);
123+
}
109124
}
110125
return validationException;
111126
}

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,18 @@
55
*/
66
package org.elasticsearch.xpack.core.ccr.action;
77

8+
import org.elasticsearch.action.ActionRequestValidationException;
89
import org.elasticsearch.common.unit.TimeValue;
910
import org.elasticsearch.common.xcontent.XContentParser;
1011
import org.elasticsearch.test.AbstractStreamableXContentTestCase;
1112

1213
import java.io.IOException;
1314
import java.util.Arrays;
15+
import java.util.Collections;
16+
17+
import static org.hamcrest.Matchers.containsString;
18+
import static org.hamcrest.Matchers.notNullValue;
19+
import static org.hamcrest.Matchers.nullValue;
1420

1521
public class PutAutoFollowPatternRequestTests extends AbstractStreamableXContentTestCase<PutAutoFollowPatternAction.Request> {
1622

@@ -60,4 +66,39 @@ protected PutAutoFollowPatternAction.Request createTestInstance() {
6066
}
6167
return request;
6268
}
69+
70+
public void testValidate() {
71+
PutAutoFollowPatternAction.Request request = new PutAutoFollowPatternAction.Request();
72+
ActionRequestValidationException validationException = request.validate();
73+
assertThat(validationException, notNullValue());
74+
assertThat(validationException.getMessage(), containsString("[leader_cluster_alias] is missing"));
75+
76+
request.setLeaderClusterAlias("_alias");
77+
validationException = request.validate();
78+
assertThat(validationException, notNullValue());
79+
assertThat(validationException.getMessage(), containsString("[leader_index_patterns] is missing"));
80+
81+
request.setLeaderIndexPatterns(Collections.emptyList());
82+
validationException = request.validate();
83+
assertThat(validationException, notNullValue());
84+
assertThat(validationException.getMessage(), containsString("[leader_index_patterns] is missing"));
85+
86+
request.setLeaderIndexPatterns(Collections.singletonList("logs-*"));
87+
validationException = request.validate();
88+
assertThat(validationException, nullValue());
89+
90+
request.setMaxRetryDelay(TimeValue.ZERO);
91+
validationException = request.validate();
92+
assertThat(validationException, notNullValue());
93+
assertThat(validationException.getMessage(), containsString("[max_retry_delay] must be positive but was [0ms]"));
94+
95+
request.setMaxRetryDelay(TimeValue.timeValueMinutes(10));
96+
validationException = request.validate();
97+
assertThat(validationException, notNullValue());
98+
assertThat(validationException.getMessage(), containsString("[max_retry_delay] must be less than [5m] but was [10m]"));
99+
100+
request.setMaxRetryDelay(TimeValue.timeValueMinutes(1));
101+
validationException = request.validate();
102+
assertThat(validationException, nullValue());
103+
}
63104
}

0 commit comments

Comments
 (0)