Skip to content

Commit 19bfdf0

Browse files
author
Christoph Büscher
committed
Fix AbstractBulkByScrollRequest slices parameter via Rest
Currently the AbstractBulkByScrollRequest accepts slice values of 0 via its `setSlices` method, denoting the "auto" slicing behaviour that is usable by settting the "slices=auto" parameter on rest requests. When using the High Level Rest Client, however, we send the 0 value as an integer, which is then rejected as invalid by `AbstractBulkByScrollRequest#parseSlices`. Instead of making parsing of the rest request more lenient, this PR opts for changing the RequestConverter logic in the client to translate 0 values to "auto" on the rest requests. Closes elastic#53044
1 parent 9a2a59b commit 19bfdf0

File tree

3 files changed

+46
-10
lines changed

3 files changed

+46
-10
lines changed

client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -557,8 +557,13 @@ private static Request prepareReindexRequest(ReindexRequest reindexRequest, bool
557557
.withRefresh(reindexRequest.isRefresh())
558558
.withTimeout(reindexRequest.getTimeout())
559559
.withWaitForActiveShards(reindexRequest.getWaitForActiveShards())
560-
.withRequestsPerSecond(reindexRequest.getRequestsPerSecond())
561-
.withSlices(reindexRequest.getSlices());
560+
.withRequestsPerSecond(reindexRequest.getRequestsPerSecond());
561+
if (reindexRequest.getSlices() == 0) {
562+
// translate to "auto" value
563+
params.withAutoSlices();
564+
} else {
565+
params.withSlices(reindexRequest.getSlices());
566+
}
562567

563568
if (reindexRequest.getScrollTime() != null) {
564569
params.putParam("scroll", reindexRequest.getScrollTime());
@@ -579,8 +584,13 @@ private static Request prepareDeleteByQueryRequest(DeleteByQueryRequest deleteBy
579584
.withWaitForActiveShards(deleteByQueryRequest.getWaitForActiveShards())
580585
.withRequestsPerSecond(deleteByQueryRequest.getRequestsPerSecond())
581586
.withIndicesOptions(deleteByQueryRequest.indicesOptions())
582-
.withWaitForCompletion(waitForCompletion)
583-
.withSlices(deleteByQueryRequest.getSlices());
587+
.withWaitForCompletion(waitForCompletion);
588+
if (deleteByQueryRequest.getSlices() == 0) {
589+
// translate to "auto" value
590+
params.withAutoSlices();
591+
} else {
592+
params.withSlices(deleteByQueryRequest.getSlices());
593+
}
584594
if (deleteByQueryRequest.isAbortOnVersionConflict() == false) {
585595
params.putParam("conflicts", "proceed");
586596
}
@@ -608,8 +618,13 @@ static Request updateByQuery(UpdateByQueryRequest updateByQueryRequest) throws I
608618
.withTimeout(updateByQueryRequest.getTimeout())
609619
.withWaitForActiveShards(updateByQueryRequest.getWaitForActiveShards())
610620
.withRequestsPerSecond(updateByQueryRequest.getRequestsPerSecond())
611-
.withIndicesOptions(updateByQueryRequest.indicesOptions())
612-
.withSlices(updateByQueryRequest.getSlices());
621+
.withIndicesOptions(updateByQueryRequest.indicesOptions());
622+
if (updateByQueryRequest.getSlices() == 0) {
623+
// translate to "auto" value
624+
params.withAutoSlices();
625+
} else {
626+
params.withSlices(updateByQueryRequest.getSlices());
627+
}
613628
if (updateByQueryRequest.isAbortOnVersionConflict() == false) {
614629
params.putParam("conflicts", "proceed");
615630
}
@@ -916,6 +931,10 @@ Params withSlices(int slices) {
916931
return putParam("slices", String.valueOf(slices));
917932
}
918933

934+
Params withAutoSlices() {
935+
return putParam("slices", AbstractBulkByScrollRequest.AUTO_SLICES_VALUE);
936+
}
937+
919938
Params withStoredFields(String[] storedFields) {
920939
if (storedFields != null && storedFields.length > 0) {
921940
return putParam("stored_fields", String.join(",", storedFields));

client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
package org.elasticsearch.client;
2121

22+
import com.carrotsearch.randomizedtesting.annotations.Repeat;
23+
2224
import org.apache.http.HttpEntity;
2325
import org.apache.http.client.methods.HttpDelete;
2426
import org.apache.http.client.methods.HttpGet;
@@ -80,6 +82,7 @@
8082
import org.elasticsearch.index.rankeval.RankEvalSpec;
8183
import org.elasticsearch.index.rankeval.RatedRequest;
8284
import org.elasticsearch.index.rankeval.RestRankEvalAction;
85+
import org.elasticsearch.index.reindex.AbstractBulkByScrollRequest;
8386
import org.elasticsearch.index.reindex.DeleteByQueryRequest;
8487
import org.elasticsearch.index.reindex.ReindexRequest;
8588
import org.elasticsearch.index.reindex.RemoteInfo;
@@ -132,6 +135,7 @@
132135
import static org.hamcrest.Matchers.hasKey;
133136
import static org.hamcrest.Matchers.nullValue;
134137

138+
@Repeat(iterations = 100)
135139
public class RequestConvertersTests extends ESTestCase {
136140
public void testPing() {
137141
Request request = RequestConverters.ping();
@@ -437,9 +441,13 @@ public void testReindex() throws IOException {
437441
reindexRequest.setSourceQuery(new TermQueryBuilder("foo", "fooval"));
438442
}
439443
if (randomBoolean()) {
440-
int slices = randomInt(100);
444+
int slices = randomIntBetween(0,4);
441445
reindexRequest.setSlices(slices);
442-
expectedParams.put("slices", String.valueOf(slices));
446+
if (slices == 0) {
447+
expectedParams.put("slices", AbstractBulkByScrollRequest.AUTO_SLICES_VALUE);
448+
} else {
449+
expectedParams.put("slices", Integer.toString(slices));
450+
}
443451
} else {
444452
expectedParams.put("slices", "1");
445453
}
@@ -500,7 +508,11 @@ public void testUpdateByQuery() throws IOException {
500508
}
501509
if (randomBoolean()) {
502510
int slices = randomIntBetween(0, 4);
503-
expectedParams.put("slices", Integer.toString(slices));
511+
if (slices == 0) {
512+
expectedParams.put("slices", AbstractBulkByScrollRequest.AUTO_SLICES_VALUE);
513+
} else {
514+
expectedParams.put("slices", Integer.toString(slices));
515+
}
504516
updateByQueryRequest.setSlices(slices);
505517
} else {
506518
expectedParams.put("slices", "1");
@@ -556,7 +568,11 @@ public void testDeleteByQuery() throws IOException {
556568
}
557569
if (randomBoolean()) {
558570
int slices = randomIntBetween(0, 4);
559-
expectedParams.put("slices", Integer.toString(slices));
571+
if (slices == 0) {
572+
expectedParams.put("slices", AbstractBulkByScrollRequest.AUTO_SLICES_VALUE);
573+
} else {
574+
expectedParams.put("slices", Integer.toString(slices));
575+
}
560576
deleteByQueryRequest.setSlices(slices);
561577
} else {
562578
expectedParams.put("slices", "1");

server/src/main/java/org/elasticsearch/index/reindex/AbstractBulkByScrollRequest.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,7 @@ public TimeValue getScrollTime() {
375375

376376
/**
377377
* The number of slices this task should be divided into. Defaults to 1 meaning the task isn't sliced into subtasks.
378+
* A value of 0 is equivalent to the "auto" slices parameter of the Rest API.
378379
*/
379380
public Self setSlices(int slices) {
380381
if (slices < 0) {

0 commit comments

Comments
 (0)