From 49842b8051a7540d7bfe6f849c5499a3a9891ce3 Mon Sep 17 00:00:00 2001 From: Artem Prigoda Date: Mon, 17 Feb 2025 09:49:59 +0100 Subject: [PATCH 1/4] Remove `@UpdateForV9` annotation from `ReindexRequest#failOnSizeSpecified` We do need this check to make sure we don't silently accept requests with ignored `size` parameter specified since `max_docs` is optional. We want to notify users that they must use `max_docs` and specifying `size` is a mistake. This behaviour is tested in the `reindex/20_validation/specifying size fails` test. --- .../java/org/elasticsearch/index/reindex/ReindexRequest.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/reindex/ReindexRequest.java b/server/src/main/java/org/elasticsearch/index/reindex/ReindexRequest.java index f9d027e0c9c1c..cf68d419a3ef9 100644 --- a/server/src/main/java/org/elasticsearch/index/reindex/ReindexRequest.java +++ b/server/src/main/java/org/elasticsearch/index/reindex/ReindexRequest.java @@ -21,7 +21,6 @@ import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.core.TimeValue; -import org.elasticsearch.core.UpdateForV9; import org.elasticsearch.features.NodeFeature; import org.elasticsearch.index.VersionType; import org.elasticsearch.index.query.QueryBuilder; @@ -499,8 +498,6 @@ static void setMaxDocsValidateIdentical(AbstractBulkByScrollRequest request, } } - @UpdateForV9(owner = UpdateForV9.Owner.DISTRIBUTED_INDEXING) - // do we still need this ref to [max_docs] or can we remove the field entirely so it's rejected with the default message? private static void failOnSizeSpecified() { throw new IllegalArgumentException("invalid parameter [size], use [max_docs] instead"); } From 9914525dfc685b84ed6be2db11a3b28149aff4f2 Mon Sep 17 00:00:00 2001 From: Artem Prigoda Date: Mon, 17 Feb 2025 15:00:04 +0100 Subject: [PATCH 2/4] Deprecate special handling of ReindexRequest#size --- modules/reindex/build.gradle | 4 ++++ .../test/reindex/20_validation.yml | 22 ------------------- .../index/reindex/ReindexRequest.java | 8 ------- 3 files changed, 4 insertions(+), 30 deletions(-) diff --git a/modules/reindex/build.gradle b/modules/reindex/build.gradle index 05cd906f61160..8ea52de5a23a8 100644 --- a/modules/reindex/build.gradle +++ b/modules/reindex/build.gradle @@ -167,3 +167,7 @@ if (OS.current() == OS.WINDOWS) { } } } + +tasks.named("yamlRestCompatTestTransform").configure { task -> + task.skipTest("reindex/20_validation/specifying size fails", "size is rejected in 9.0") +} diff --git a/modules/reindex/src/yamlRestTest/resources/rest-api-spec/test/reindex/20_validation.yml b/modules/reindex/src/yamlRestTest/resources/rest-api-spec/test/reindex/20_validation.yml index c96414e46f7e7..dcfffcc0f30e0 100644 --- a/modules/reindex/src/yamlRestTest/resources/rest-api-spec/test/reindex/20_validation.yml +++ b/modules/reindex/src/yamlRestTest/resources/rest-api-spec/test/reindex/20_validation.yml @@ -94,28 +94,6 @@ body: conflicts: cat ---- -"specifying size fails": - - requires: - cluster_features: ["gte_v8.0.0"] - reason: "size supported until 8" - - - do: - index: - index: test - id: "1" - body: { "text": "test" } - - - do: - catch: /invalid parameter \[size\], use \[max_docs\] instead/ - reindex: - body: - source: - index: test - dest: - index: dest - size: 1 - --- "invalid max_docs in body fails": - requires: diff --git a/server/src/main/java/org/elasticsearch/index/reindex/ReindexRequest.java b/server/src/main/java/org/elasticsearch/index/reindex/ReindexRequest.java index cf68d419a3ef9..c3e429af08a4e 100644 --- a/server/src/main/java/org/elasticsearch/index/reindex/ReindexRequest.java +++ b/server/src/main/java/org/elasticsearch/index/reindex/ReindexRequest.java @@ -354,10 +354,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws ); PARSER.declareInt(ReindexRequest::setMaxDocsValidateIdentical, new ParseField("max_docs")); - - // avoid silently accepting an ignored size. - PARSER.declareInt((r, s) -> failOnSizeSpecified(), new ParseField("size")); - PARSER.declareField((p, v, c) -> v.setScript(Script.parse(p)), new ParseField("script"), ObjectParser.ValueType.OBJECT); PARSER.declareString(ReindexRequest::setConflicts, new ParseField("conflicts")); } @@ -497,8 +493,4 @@ static void setMaxDocsValidateIdentical(AbstractBulkByScrollRequest request, request.setMaxDocs(maxDocs); } } - - private static void failOnSizeSpecified() { - throw new IllegalArgumentException("invalid parameter [size], use [max_docs] instead"); - } } From 2620796d7f4fce54932e84eb61859489cfd21e62 Mon Sep 17 00:00:00 2001 From: Artem Prigoda Date: Mon, 17 Feb 2025 15:47:43 +0100 Subject: [PATCH 3/4] Adjust regex for catching both new and old size error messages --- modules/reindex/build.gradle | 4 ---- .../test/reindex/20_validation.yml | 22 +++++++++++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/modules/reindex/build.gradle b/modules/reindex/build.gradle index 8ea52de5a23a8..05cd906f61160 100644 --- a/modules/reindex/build.gradle +++ b/modules/reindex/build.gradle @@ -167,7 +167,3 @@ if (OS.current() == OS.WINDOWS) { } } } - -tasks.named("yamlRestCompatTestTransform").configure { task -> - task.skipTest("reindex/20_validation/specifying size fails", "size is rejected in 9.0") -} diff --git a/modules/reindex/src/yamlRestTest/resources/rest-api-spec/test/reindex/20_validation.yml b/modules/reindex/src/yamlRestTest/resources/rest-api-spec/test/reindex/20_validation.yml index dcfffcc0f30e0..cb155bc2219ab 100644 --- a/modules/reindex/src/yamlRestTest/resources/rest-api-spec/test/reindex/20_validation.yml +++ b/modules/reindex/src/yamlRestTest/resources/rest-api-spec/test/reindex/20_validation.yml @@ -94,6 +94,28 @@ body: conflicts: cat +--- +"specifying size fails": + - requires: + cluster_features: ["gte_v8.0.0"] + reason: "size supported until 8" + + - do: + index: + index: test + id: "1" + body: { "text": "test" } + + - do: + catch: /(invalid parameter \[size\], use \[max_docs\] instead|unknown field \[size\])/ + reindex: + body: + source: + index: test + dest: + index: dest + size: 1 + --- "invalid max_docs in body fails": - requires: From 064ede8f957a886485eab0bda304ab8f7601db74 Mon Sep 17 00:00:00 2001 From: Artem Prigoda Date: Mon, 17 Feb 2025 17:10:47 +0100 Subject: [PATCH 4/4] Don't run `reindex/20_validation/specifying size fails` in 8.x compatibility mode --- modules/reindex/build.gradle | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/modules/reindex/build.gradle b/modules/reindex/build.gradle index 05cd906f61160..8ea52de5a23a8 100644 --- a/modules/reindex/build.gradle +++ b/modules/reindex/build.gradle @@ -167,3 +167,7 @@ if (OS.current() == OS.WINDOWS) { } } } + +tasks.named("yamlRestCompatTestTransform").configure { task -> + task.skipTest("reindex/20_validation/specifying size fails", "size is rejected in 9.0") +}