Skip to content

Commit 53ba253

Browse files
martijnvgmartijnvg
authored
[CCR] Add validation for max_retry_delay (#33648)
1 parent b9d0c8f commit 53ba253

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
@@ -23,6 +23,8 @@
2323
import java.io.IOException;
2424
import java.util.Objects;
2525

26+
import static org.elasticsearch.action.ValidateActions.addValidationError;
27+
2628
public final class FollowIndexAction extends Action<AcknowledgedResponse> {
2729

2830
public static final FollowIndexAction INSTANCE = new FollowIndexAction();
@@ -33,8 +35,9 @@ public final class FollowIndexAction extends Action<AcknowledgedResponse> {
3335
public static final int DEFAULT_MAX_CONCURRENT_READ_BATCHES = 1;
3436
public static final int DEFAULT_MAX_CONCURRENT_WRITE_BATCHES = 1;
3537
public static final long DEFAULT_MAX_BATCH_SIZE_IN_BYTES = Long.MAX_VALUE;
36-
public static final TimeValue DEFAULT_RETRY_TIMEOUT = new TimeValue(500);
37-
public static final TimeValue DEFAULT_IDLE_SHARD_RETRY_DELAY = TimeValue.timeValueSeconds(10);
38+
static final TimeValue DEFAULT_MAX_RETRY_DELAY = new TimeValue(500);
39+
static final TimeValue DEFAULT_IDLE_SHARD_RETRY_DELAY = TimeValue.timeValueSeconds(10);
40+
static final TimeValue MAX_RETRY_DELAY = TimeValue.timeValueMinutes(5);
3841

3942
private FollowIndexAction() {
4043
super(NAME);
@@ -54,7 +57,7 @@ public static class Request extends ActionRequest implements ToXContentObject {
5457
private static final ParseField MAX_BATCH_SIZE_IN_BYTES = new ParseField("max_batch_size_in_bytes");
5558
private static final ParseField MAX_CONCURRENT_WRITE_BATCHES = new ParseField("max_concurrent_write_batches");
5659
private static final ParseField MAX_WRITE_BUFFER_SIZE = new ParseField("max_write_buffer_size");
57-
private static final ParseField MAX_RETRY_DELAY = new ParseField("max_retry_delay");
60+
private static final ParseField MAX_RETRY_DELAY_FIELD = new ParseField("max_retry_delay");
5861
private static final ParseField IDLE_SHARD_RETRY_DELAY = new ParseField("idle_shard_retry_delay");
5962
private static final ConstructingObjectParser<Request, String> PARSER = new ConstructingObjectParser<>(NAME, true,
6063
(args, followerIndex) -> {
@@ -75,8 +78,8 @@ public static class Request extends ActionRequest implements ToXContentObject {
7578
PARSER.declareInt(ConstructingObjectParser.optionalConstructorArg(), MAX_WRITE_BUFFER_SIZE);
7679
PARSER.declareField(
7780
ConstructingObjectParser.optionalConstructorArg(),
78-
(p, c) -> TimeValue.parseTimeValue(p.text(), MAX_RETRY_DELAY.getPreferredName()),
79-
MAX_RETRY_DELAY,
81+
(p, c) -> TimeValue.parseTimeValue(p.text(), MAX_RETRY_DELAY_FIELD.getPreferredName()),
82+
MAX_RETRY_DELAY_FIELD,
8083
ObjectParser.ValueType.STRING);
8184
PARSER.declareField(
8285
ConstructingObjectParser.optionalConstructorArg(),
@@ -202,7 +205,7 @@ public Request(
202205
throw new IllegalArgumentException(MAX_WRITE_BUFFER_SIZE.getPreferredName() + " must be larger than 0");
203206
}
204207

205-
final TimeValue actualRetryTimeout = maxRetryDelay == null ? DEFAULT_RETRY_TIMEOUT : maxRetryDelay;
208+
final TimeValue actualRetryTimeout = maxRetryDelay == null ? DEFAULT_MAX_RETRY_DELAY : maxRetryDelay;
206209
final TimeValue actualIdleShardRetryDelay = idleShardRetryDelay == null ? DEFAULT_IDLE_SHARD_RETRY_DELAY : idleShardRetryDelay;
207210

208211
this.leaderIndex = leaderIndex;
@@ -222,7 +225,20 @@ public Request() {
222225

223226
@Override
224227
public ActionRequestValidationException validate() {
225-
return null;
228+
ActionRequestValidationException validationException = null;
229+
230+
if (maxRetryDelay.millis() <= 0) {
231+
String message = "[" + MAX_RETRY_DELAY_FIELD.getPreferredName() + "] must be positive but was [" +
232+
maxRetryDelay.getStringRep() + "]";
233+
validationException = addValidationError(message, validationException);
234+
}
235+
if (maxRetryDelay.millis() > FollowIndexAction.MAX_RETRY_DELAY.millis()) {
236+
String message = "[" + MAX_RETRY_DELAY_FIELD.getPreferredName() + "] must be less than [" + MAX_RETRY_DELAY +
237+
"] but was [" + maxRetryDelay.getStringRep() + "]";
238+
validationException = addValidationError(message, validationException);
239+
}
240+
241+
return validationException;
226242
}
227243

228244
@Override
@@ -264,7 +280,7 @@ public XContentBuilder toXContent(final XContentBuilder builder, final Params pa
264280
builder.field(MAX_WRITE_BUFFER_SIZE.getPreferredName(), maxWriteBufferSize);
265281
builder.field(MAX_CONCURRENT_READ_BATCHES.getPreferredName(), maxConcurrentReadBatches);
266282
builder.field(MAX_CONCURRENT_WRITE_BATCHES.getPreferredName(), maxConcurrentWriteBatches);
267-
builder.field(MAX_RETRY_DELAY.getPreferredName(), maxRetryDelay.getStringRep());
283+
builder.field(MAX_RETRY_DELAY_FIELD.getPreferredName(), maxRetryDelay.getStringRep());
268284
builder.field(IDLE_SHARD_RETRY_DELAY.getPreferredName(), idleShardRetryDelay.getStringRep());
269285
}
270286
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
@@ -94,10 +94,25 @@ public static Request fromXContent(XContentParser parser, String remoteClusterAl
9494
public ActionRequestValidationException validate() {
9595
ActionRequestValidationException validationException = null;
9696
if (leaderClusterAlias == null) {
97-
validationException = addValidationError("leaderClusterAlias is missing", validationException);
97+
validationException = addValidationError("[" + LEADER_CLUSTER_ALIAS_FIELD.getPreferredName() +
98+
"] is missing", validationException);
9899
}
99100
if (leaderIndexPatterns == null || leaderIndexPatterns.isEmpty()) {
100-
validationException = addValidationError("leaderIndexPatterns is missing", validationException);
101+
validationException = addValidationError("[" + LEADER_INDEX_PATTERNS_FIELD.getPreferredName() +
102+
"] is missing", validationException);
103+
}
104+
if (maxRetryDelay != null) {
105+
if (maxRetryDelay.millis() <= 0) {
106+
String message = "[" + AutoFollowPattern.MAX_RETRY_DELAY.getPreferredName() + "] must be positive but was [" +
107+
maxRetryDelay.getStringRep() + "]";
108+
validationException = addValidationError(message, validationException);
109+
}
110+
if (maxRetryDelay.millis() > FollowIndexAction.MAX_RETRY_DELAY.millis()) {
111+
String message = "[" + AutoFollowPattern.MAX_RETRY_DELAY.getPreferredName() + "] must be less than [" +
112+
FollowIndexAction.MAX_RETRY_DELAY +
113+
"] but was [" + maxRetryDelay.getStringRep() + "]";
114+
validationException = addValidationError(message, validationException);
115+
}
101116
}
102117
return validationException;
103118
}

‎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)