Skip to content

Commit 571cf52

Browse files
martijnvgmartijnvg
authored
Clean up duplicate follow config parameter code (#37688) (#38443)
Introduced FollowParameters class that put follow, resume follow, put auto follow pattern requests and follow info response classes reuse. The FollowParameters class had the fields, getters etc. for the common parameters that all these APIs have. Also binary and xcontent serialization / parsing is handled by this class. The follow, resume follow, put auto follow pattern request classes originally used optional non primitive fields, so FollowParameters has that too and the follow info api can handle that now too. Also the followerIndex field can in production only be specified via the url path. If it is also specified via the request body then it must have the same value as is specified in the url path. This option only existed to xcontent testing. However the AbstractSerializingTestCase base class now also supports createXContextTestInstance() to provide a different test instance when testing xcontent, so allowing followerIndex to be specified via the request body is no longer needed. By moving the followerIndex field from Body to ResumeFollowAction.Request class and not allowing the followerIndex field to be specified via the request body the Body class is redundant and can be removed. The ResumeFollowAction.Request class can then directly use the FollowParameters class. For consistency I also removed the ability to specified followerIndex in the put follow api and the name in put auto follow pattern api via the request body.
1 parent 5512574 commit 571cf52

26 files changed

+714
-1022
lines changed

‎client/rest-high-level/src/main/java/org/elasticsearch/client/ccr/PutFollowRequest.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ public final class PutFollowRequest extends FollowConfig implements Validatable,
3232

3333
static final ParseField REMOTE_CLUSTER_FIELD = new ParseField("remote_cluster");
3434
static final ParseField LEADER_INDEX_FIELD = new ParseField("leader_index");
35-
static final ParseField FOLLOWER_INDEX_FIELD = new ParseField("follower_index");
3635

3736
private final String remoteCluster;
3837
private final String leaderIndex;
@@ -55,7 +54,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
5554
builder.startObject();
5655
builder.field(REMOTE_CLUSTER_FIELD.getPreferredName(), remoteCluster);
5756
builder.field(LEADER_INDEX_FIELD.getPreferredName(), leaderIndex);
58-
builder.field(FOLLOWER_INDEX_FIELD.getPreferredName(), followerIndex);
5957
toXContentFragment(builder, params);
6058
builder.endObject();
6159
return builder;

‎client/rest-high-level/src/main/java/org/elasticsearch/client/ccr/ResumeFollowRequest.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@
2626
import java.io.IOException;
2727
import java.util.Objects;
2828

29-
import static org.elasticsearch.client.ccr.PutFollowRequest.FOLLOWER_INDEX_FIELD;
30-
3129
public final class ResumeFollowRequest extends FollowConfig implements Validatable, ToXContentObject {
3230

3331
private final String followerIndex;
@@ -39,7 +37,6 @@ public ResumeFollowRequest(String followerIndex) {
3937
@Override
4038
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
4139
builder.startObject();
42-
builder.field(FOLLOWER_INDEX_FIELD.getPreferredName(), followerIndex);
4340
toXContentFragment(builder, params);
4441
builder.endObject();
4542
return builder;

‎client/rest-high-level/src/test/java/org/elasticsearch/client/ccr/PutFollowRequestTests.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,11 @@
3131
public class PutFollowRequestTests extends AbstractXContentTestCase<PutFollowRequest> {
3232

3333
private static final ConstructingObjectParser<PutFollowRequest, Void> PARSER = new ConstructingObjectParser<>("test_parser",
34-
true, (args) -> new PutFollowRequest((String) args[0], (String) args[1], (String) args[2]));
34+
true, (args) -> new PutFollowRequest((String) args[0], (String) args[1], "followerIndex"));
3535

3636
static {
3737
PARSER.declareString(ConstructingObjectParser.constructorArg(), PutFollowRequest.REMOTE_CLUSTER_FIELD);
3838
PARSER.declareString(ConstructingObjectParser.constructorArg(), PutFollowRequest.LEADER_INDEX_FIELD);
39-
PARSER.declareString(ConstructingObjectParser.constructorArg(), PutFollowRequest.FOLLOWER_INDEX_FIELD);
4039
PARSER.declareInt(PutFollowRequest::setMaxReadRequestOperationCount, PutFollowRequest.MAX_READ_REQUEST_OPERATION_COUNT);
4140
PARSER.declareField(
4241
PutFollowRequest::setMaxReadRequestSize,
@@ -82,7 +81,7 @@ protected boolean supportsUnknownFields() {
8281
@Override
8382
protected PutFollowRequest createTestInstance() {
8483
PutFollowRequest putFollowRequest =
85-
new PutFollowRequest(randomAlphaOfLength(4), randomAlphaOfLength(4), randomAlphaOfLength(4));
84+
new PutFollowRequest(randomAlphaOfLength(4), randomAlphaOfLength(4), "followerIndex");
8685
if (randomBoolean()) {
8786
putFollowRequest.setMaxOutstandingReadRequests(randomIntBetween(0, Integer.MAX_VALUE));
8887
}

‎client/rest-high-level/src/test/java/org/elasticsearch/client/ccr/ResumeFollowRequestTests.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121

2222
import org.elasticsearch.common.unit.ByteSizeValue;
2323
import org.elasticsearch.common.unit.TimeValue;
24-
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
2524
import org.elasticsearch.common.xcontent.ObjectParser;
2625
import org.elasticsearch.common.xcontent.XContentParser;
2726
import org.elasticsearch.test.AbstractXContentTestCase;
@@ -30,11 +29,10 @@
3029

3130
public class ResumeFollowRequestTests extends AbstractXContentTestCase<ResumeFollowRequest> {
3231

33-
private static final ConstructingObjectParser<ResumeFollowRequest, Void> PARSER = new ConstructingObjectParser<>("test_parser",
34-
true, (args) -> new ResumeFollowRequest((String) args[0]));
32+
private static final ObjectParser<ResumeFollowRequest, Void> PARSER = new ObjectParser<>("test_parser",
33+
true, () -> new ResumeFollowRequest("followerIndex"));
3534

3635
static {
37-
PARSER.declareString(ConstructingObjectParser.constructorArg(), PutFollowRequest.FOLLOWER_INDEX_FIELD);
3836
PARSER.declareInt(ResumeFollowRequest::setMaxReadRequestOperationCount, FollowConfig.MAX_READ_REQUEST_OPERATION_COUNT);
3937
PARSER.declareField(
4038
ResumeFollowRequest::setMaxReadRequestSize,
@@ -79,7 +77,7 @@ protected boolean supportsUnknownFields() {
7977

8078
@Override
8179
protected ResumeFollowRequest createTestInstance() {
82-
ResumeFollowRequest resumeFollowRequest = new ResumeFollowRequest(randomAlphaOfLength(4));
80+
ResumeFollowRequest resumeFollowRequest = new ResumeFollowRequest("followerIndex");
8381
if (randomBoolean()) {
8482
resumeFollowRequest.setMaxOutstandingReadRequests(randomIntBetween(0, Integer.MAX_VALUE));
8583
}

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

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040
import org.elasticsearch.xpack.core.ccr.AutoFollowMetadata.AutoFollowPattern;
4141
import org.elasticsearch.xpack.core.ccr.AutoFollowStats;
4242
import org.elasticsearch.xpack.core.ccr.action.PutFollowAction;
43-
import org.elasticsearch.xpack.core.ccr.action.ResumeFollowAction;
4443

4544
import java.util.ArrayList;
4645
import java.util.Collections;
@@ -511,23 +510,20 @@ private void followLeaderIndex(String autoFollowPattenName,
511510
final String leaderIndexName = indexToFollow.getName();
512511
final String followIndexName = getFollowerIndexName(pattern, leaderIndexName);
513512

514-
ResumeFollowAction.Request followRequest = new ResumeFollowAction.Request();
515-
followRequest.setFollowerIndex(followIndexName);
516-
followRequest.setMaxReadRequestOperationCount(pattern.getMaxReadRequestOperationCount());
517-
followRequest.setMaxReadRequestSize(pattern.getMaxReadRequestSize());
518-
followRequest.setMaxOutstandingReadRequests(pattern.getMaxOutstandingReadRequests());
519-
followRequest.setMaxWriteRequestOperationCount(pattern.getMaxWriteRequestOperationCount());
520-
followRequest.setMaxWriteRequestSize(pattern.getMaxWriteRequestSize());
521-
followRequest.setMaxOutstandingWriteRequests(pattern.getMaxOutstandingWriteRequests());
522-
followRequest.setMaxWriteBufferCount(pattern.getMaxWriteBufferCount());
523-
followRequest.setMaxWriteBufferSize(pattern.getMaxWriteBufferSize());
524-
followRequest.setMaxRetryDelay(pattern.getMaxRetryDelay());
525-
followRequest.setReadPollTimeout(pattern.getPollTimeout());
526-
527513
PutFollowAction.Request request = new PutFollowAction.Request();
528514
request.setRemoteCluster(remoteCluster);
529515
request.setLeaderIndex(indexToFollow.getName());
530-
request.setFollowRequest(followRequest);
516+
request.setFollowerIndex(followIndexName);
517+
request.getParameters().setMaxReadRequestOperationCount(pattern.getMaxReadRequestOperationCount());
518+
request.getParameters().setMaxReadRequestSize(pattern.getMaxReadRequestSize());
519+
request.getParameters().setMaxOutstandingReadRequests(pattern.getMaxOutstandingReadRequests());
520+
request.getParameters().setMaxWriteRequestOperationCount(pattern.getMaxWriteRequestOperationCount());
521+
request.getParameters().setMaxWriteRequestSize(pattern.getMaxWriteRequestSize());
522+
request.getParameters().setMaxOutstandingWriteRequests(pattern.getMaxOutstandingWriteRequests());
523+
request.getParameters().setMaxWriteBufferCount(pattern.getMaxWriteBufferCount());
524+
request.getParameters().setMaxWriteBufferSize(pattern.getMaxWriteBufferSize());
525+
request.getParameters().setMaxRetryDelay(pattern.getMaxRetryDelay());
526+
request.getParameters().setReadPollTimeout(pattern.getPollTimeout());
531527

532528
// Execute if the create and follow api call succeeds:
533529
Runnable successHandler = () -> {

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ protected Boolean newResponse(final boolean acknowledged) {
7272

7373
@Override
7474
public ClusterState execute(final ClusterState currentState) throws Exception {
75-
String followIndex = request.getFollowRequest().getFollowerIndex();
75+
String followIndex = request.getFollowerIndex();
7676
IndexMetaData currentIndex = currentState.metaData().index(followIndex);
7777
if (currentIndex != null) {
7878
throw new ResourceAlreadyExistsException(currentIndex.getIndex());
@@ -112,10 +112,10 @@ public ClusterState execute(final ClusterState currentState) throws Exception {
112112
ClusterState updatedState = builder.build();
113113

114114
RoutingTable.Builder routingTableBuilder = RoutingTable.builder(updatedState.routingTable())
115-
.addAsNew(updatedState.metaData().index(request.getFollowRequest().getFollowerIndex()));
115+
.addAsNew(updatedState.metaData().index(request.getFollowerIndex()));
116116
updatedState = allocationService.reroute(
117117
ClusterState.builder(updatedState).routingTable(routingTableBuilder.build()).build(),
118-
"follow index [" + request.getFollowRequest().getFollowerIndex() + "] created");
118+
"follow index [" + request.getFollowerIndex() + "] created");
119119

120120
logger.info("[{}] creating index, cause [ccr_create_and_follow], shards [{}]/[{}]",
121121
followIndex, followIMD.getNumberOfShards(), followIMD.getNumberOfReplicas());
@@ -126,10 +126,13 @@ public ClusterState execute(final ClusterState currentState) throws Exception {
126126
}
127127

128128
private void initiateFollowing(final PutFollowAction.Request request, final ActionListener<PutFollowAction.Response> listener) {
129-
activeShardsObserver.waitForActiveShards(new String[]{request.getFollowRequest().getFollowerIndex()},
129+
activeShardsObserver.waitForActiveShards(new String[]{request.getFollowerIndex()},
130130
ActiveShardCount.DEFAULT, request.timeout(), result -> {
131131
if (result) {
132-
client.execute(ResumeFollowAction.INSTANCE, request.getFollowRequest(), ActionListener.wrap(
132+
ResumeFollowAction.Request resumeFollowRequest = new ResumeFollowAction.Request();
133+
resumeFollowRequest.setFollowerIndex(request.getFollowerIndex());
134+
resumeFollowRequest.setParameters(request.getParameters());
135+
client.execute(ResumeFollowAction.INSTANCE, resumeFollowRequest, ActionListener.wrap(
133136
r -> listener.onResponse(new PutFollowAction.Response(true, true, r.isAcknowledged())),
134137
listener::onFailure
135138
));

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

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
import org.elasticsearch.transport.TransportService;
2424
import org.elasticsearch.xpack.ccr.Ccr;
2525
import org.elasticsearch.xpack.core.ccr.action.FollowInfoAction;
26-
import org.elasticsearch.xpack.core.ccr.action.FollowInfoAction.Response.FollowParameters;
26+
import org.elasticsearch.xpack.core.ccr.action.FollowParameters;
2727
import org.elasticsearch.xpack.core.ccr.action.FollowInfoAction.Response.FollowerInfo;
2828
import org.elasticsearch.xpack.core.ccr.action.FollowInfoAction.Response.Status;
2929

@@ -99,18 +99,17 @@ static List<FollowerInfo> getFollowInfos(List<String> concreteFollowerIndices, C
9999
String leaderIndex = ccrCustomData.get(Ccr.CCR_CUSTOM_METADATA_LEADER_INDEX_NAME_KEY);
100100
if (result.isPresent()) {
101101
ShardFollowTask params = result.get();
102-
FollowParameters followParameters = new FollowParameters(
103-
params.getMaxReadRequestOperationCount(),
104-
params.getMaxReadRequestSize(),
105-
params.getMaxOutstandingReadRequests(),
106-
params.getMaxWriteRequestOperationCount(),
107-
params.getMaxWriteRequestSize(),
108-
params.getMaxOutstandingWriteRequests(),
109-
params.getMaxWriteBufferCount(),
110-
params.getMaxWriteBufferSize(),
111-
params.getMaxRetryDelay(),
112-
params.getReadPollTimeout()
113-
);
102+
FollowParameters followParameters = new FollowParameters();
103+
followParameters.setMaxOutstandingReadRequests(params.getMaxOutstandingReadRequests());
104+
followParameters.setMaxOutstandingWriteRequests(params.getMaxOutstandingWriteRequests());
105+
followParameters.setMaxReadRequestOperationCount(params.getMaxReadRequestOperationCount());
106+
followParameters.setMaxWriteRequestOperationCount(params.getMaxWriteRequestOperationCount());
107+
followParameters.setMaxReadRequestSize(params.getMaxReadRequestSize());
108+
followParameters.setMaxWriteRequestSize(params.getMaxWriteRequestSize());
109+
followParameters.setMaxWriteBufferCount(params.getMaxWriteBufferCount());
110+
followParameters.setMaxWriteBufferSize(params.getMaxWriteBufferSize());
111+
followParameters.setMaxRetryDelay(params.getMaxRetryDelay());
112+
followParameters.setReadPollTimeout(params.getReadPollTimeout());
114113
followerInfos.add(new FollowerInfo(followerIndex, remoteCluster, leaderIndex, Status.ACTIVE, followParameters));
115114
} else {
116115
followerInfos.add(new FollowerInfo(followerIndex, remoteCluster, leaderIndex, Status.PAUSED, null));

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

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,7 @@ static ClusterState innerPut(PutAutoFollowPatternAction.Request request,
149149
markExistingIndicesAsAutoFollowedForNewPatterns(request.getLeaderIndexPatterns(), remoteClusterState.metaData(),
150150
previousPattern, followedIndexUUIDs);
151151
} else {
152-
markExistingIndicesAsAutoFollowed(request.getLeaderIndexPatterns(), remoteClusterState.metaData(),
153-
followedIndexUUIDs);
152+
markExistingIndicesAsAutoFollowed(request.getLeaderIndexPatterns(), remoteClusterState.metaData(), followedIndexUUIDs);
154153
}
155154

156155
if (filteredHeaders != null) {
@@ -161,16 +160,16 @@ static ClusterState innerPut(PutAutoFollowPatternAction.Request request,
161160
request.getRemoteCluster(),
162161
request.getLeaderIndexPatterns(),
163162
request.getFollowIndexNamePattern(),
164-
request.getMaxReadRequestOperationCount(),
165-
request.getMaxReadRequestSize(),
166-
request.getMaxConcurrentReadBatches(),
167-
request.getMaxWriteRequestOperationCount(),
168-
request.getMaxWriteRequestSize(),
169-
request.getMaxConcurrentWriteBatches(),
170-
request.getMaxWriteBufferCount(),
171-
request.getMaxWriteBufferSize(),
172-
request.getMaxRetryDelay(),
173-
request.getReadPollTimeout());
163+
request.getParameters().getMaxReadRequestOperationCount(),
164+
request.getParameters().getMaxReadRequestSize(),
165+
request.getParameters().getMaxOutstandingReadRequests(),
166+
request.getParameters().getMaxWriteRequestOperationCount(),
167+
request.getParameters().getMaxWriteRequestSize(),
168+
request.getParameters().getMaxOutstandingWriteRequests(),
169+
request.getParameters().getMaxWriteBufferCount(),
170+
request.getParameters().getMaxWriteBufferSize(),
171+
request.getParameters().getMaxRetryDelay(),
172+
request.getParameters().getReadPollTimeout());
174173
patterns.put(request.getName(), autoFollowPattern);
175174
ClusterState.Builder newState = ClusterState.builder(localState);
176175
newState.metaData(MetaData.builder(localState.getMetaData())

0 commit comments

Comments
 (0)