From 380451f26e55dc7fcfbee5055de7128104afc0ce Mon Sep 17 00:00:00 2001 From: Gao Binlong Date: Thu, 14 Mar 2024 14:33:16 +0800 Subject: [PATCH 1/5] Update supported version for adding primary_only parameter to force-merge API Signed-off-by: Gao Binlong --- CHANGELOG.md | 1 + .../20_wait_for_completion.yml | 33 ++++++++++++++++--- .../indices/forcemerge/ForceMergeRequest.java | 4 +-- .../forcemerge/ForceMergeRequestTests.java | 12 +++---- 4 files changed, 38 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 19a9a7461500f..aee2e762cb028 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -96,6 +96,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Bug] Check phase name before SearchRequestOperationsListener onPhaseStart ([#12035](https://github.com/opensearch-project/OpenSearch/pull/12035)) - Fix Span operation names generated from RestActions ([#12005](https://github.com/opensearch-project/OpenSearch/pull/12005)) - Fix error in RemoteSegmentStoreDirectory when debug logging is enabled ([#12328](https://github.com/opensearch-project/OpenSearch/pull/12328)) +- Update supported version for the primary_only parameter in force-merge API ### Security diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.forcemerge/20_wait_for_completion.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.forcemerge/20_wait_for_completion.yml index efa239547e84a..a0bddd1cbd13f 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.forcemerge/20_wait_for_completion.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.forcemerge/20_wait_for_completion.yml @@ -4,15 +4,17 @@ # will return a task immediately and the merge process will run in background. - skip: - version: " - 2.99.99" - reason: "only available in 3.0+" - features: allowed_warnings + version: " - 2.6.99, 2.13.0 - " + reason: "wait_for_completion was introduced in 2.7.0 and task description was changed in 2.13.0" + features: allowed_warnings, node_selector - do: indices.create: index: test_index - do: + node_selector: + version: " 2.7.0 - 2.12.99" indices.forcemerge: index: test_index wait_for_completion: false @@ -25,8 +27,31 @@ wait_for_completion: true task_id: $taskId - match: { task.action: "indices:admin/forcemerge" } - - match: { task.description: "Force-merge indices [test_index], maxSegments[1], onlyExpungeDeletes[false], flush[true], primaryOnly[false]" } + - match: { task.description: "Force-merge indices [test_index], maxSegments[1], onlyExpungeDeletes[false], flush[true]" } + +--- +"Force merge index with wait_for_completion after task description changed": + - skip: + version: " - 2.12.99 " + reason: "task description was changed in 2.13.0" + features: allowed_warnings, node_selector + + - do: + node_selector: + version: " 2.13.0 - " + indices.forcemerge: + index: test_index + wait_for_completion: false + max_num_segments: 1 + - match: { task: /^.+$/ } + - set: { task: taskId } + - do: + tasks.get: + wait_for_completion: true + task_id: $taskId + - match: { task.action: "indices:admin/forcemerge" } + - match: { task.description: "Force-merge indices [test_index], maxSegments[1], onlyExpungeDeletes[false], flush[true], primaryOnly[false]" } # .tasks index is created when the force-merge operation completes, so we should delete .tasks index finally, # if not, the .tasks index may introduce unexpected warnings and then cause other test cases to fail. # Delete the .tasks index directly will also introduce warning, but currently we don't have such APIs which can delete one diff --git a/server/src/main/java/org/opensearch/action/admin/indices/forcemerge/ForceMergeRequest.java b/server/src/main/java/org/opensearch/action/admin/indices/forcemerge/ForceMergeRequest.java index bf6ee9ca43755..3efc4db21afbc 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/forcemerge/ForceMergeRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/forcemerge/ForceMergeRequest.java @@ -102,7 +102,7 @@ public ForceMergeRequest(StreamInput in) throws IOException { maxNumSegments = in.readInt(); onlyExpungeDeletes = in.readBoolean(); flush = in.readBoolean(); - if (in.getVersion().onOrAfter(Version.V_3_0_0)) { + if (in.getVersion().onOrAfter(Version.V_2_13_0)) { primaryOnly = in.readBoolean(); } if (in.getVersion().onOrAfter(FORCE_MERGE_UUID_VERSION)) { @@ -219,7 +219,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeInt(maxNumSegments); out.writeBoolean(onlyExpungeDeletes); out.writeBoolean(flush); - if (out.getVersion().onOrAfter(Version.V_3_0_0)) { + if (out.getVersion().onOrAfter(Version.V_2_13_0)) { out.writeBoolean(primaryOnly); } if (out.getVersion().onOrAfter(FORCE_MERGE_UUID_VERSION)) { diff --git a/server/src/test/java/org/opensearch/action/admin/indices/forcemerge/ForceMergeRequestTests.java b/server/src/test/java/org/opensearch/action/admin/indices/forcemerge/ForceMergeRequestTests.java index a80141c52b6b4..3ac044d23f191 100644 --- a/server/src/test/java/org/opensearch/action/admin/indices/forcemerge/ForceMergeRequestTests.java +++ b/server/src/test/java/org/opensearch/action/admin/indices/forcemerge/ForceMergeRequestTests.java @@ -109,9 +109,11 @@ public void testBwcSerialization() throws Exception { assertEquals(sample.maxNumSegments(), deserializedRequest.maxNumSegments()); assertEquals(sample.onlyExpungeDeletes(), deserializedRequest.onlyExpungeDeletes()); assertEquals(sample.flush(), deserializedRequest.flush()); - if (compatibleVersion.onOrAfter(Version.V_3_0_0)) { + if (compatibleVersion.onOrAfter(Version.V_2_13_0)) { assertEquals(sample.primaryOnly(), deserializedRequest.primaryOnly()); - assertEquals(sample.forceMergeUUID(), deserializedRequest.forceMergeUUID()); + if (compatibleVersion.onOrAfter(Version.V_3_0_0)) { + assertEquals(sample.forceMergeUUID(), deserializedRequest.forceMergeUUID()); + } } } } @@ -127,7 +129,7 @@ public void testBwcSerialization() throws Exception { out.writeInt(sample.maxNumSegments()); out.writeBoolean(sample.onlyExpungeDeletes()); out.writeBoolean(sample.flush()); - if (compatibleVersion.onOrAfter(Version.V_3_0_0)) { + if (compatibleVersion.onOrAfter(Version.V_2_13_0)) { out.writeBoolean(sample.primaryOnly()); } if (compatibleVersion.onOrAfter(Version.V_3_0_0)) { @@ -145,9 +147,7 @@ public void testBwcSerialization() throws Exception { assertEquals(sample.maxNumSegments(), deserializedRequest.maxNumSegments()); assertEquals(sample.onlyExpungeDeletes(), deserializedRequest.onlyExpungeDeletes()); assertEquals(sample.flush(), deserializedRequest.flush()); - if (compatibleVersion.onOrAfter(Version.V_3_0_0)) { - assertEquals(sample.primaryOnly(), deserializedRequest.primaryOnly()); - } + assertEquals(sample.primaryOnly(), deserializedRequest.primaryOnly()); assertEquals(sample.forceMergeUUID(), deserializedRequest.forceMergeUUID()); } From bc44f08ab758773969a3e5439ef5c7ee42126175 Mon Sep 17 00:00:00 2001 From: Gao Binlong Date: Thu, 14 Mar 2024 14:42:39 +0800 Subject: [PATCH 2/5] Modify change log Signed-off-by: Gao Binlong --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aee2e762cb028..e7eef68c1dab9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -96,7 +96,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Bug] Check phase name before SearchRequestOperationsListener onPhaseStart ([#12035](https://github.com/opensearch-project/OpenSearch/pull/12035)) - Fix Span operation names generated from RestActions ([#12005](https://github.com/opensearch-project/OpenSearch/pull/12005)) - Fix error in RemoteSegmentStoreDirectory when debug logging is enabled ([#12328](https://github.com/opensearch-project/OpenSearch/pull/12328)) -- Update supported version for the primary_only parameter in force-merge API +- Update supported version for the primary_only parameter in force-merge API ([#12657](https://github.com/opensearch-project/OpenSearch/pull/12657)) ### Security From f8539eeba4a1394a5b375cb50060f0d777d06ee4 Mon Sep 17 00:00:00 2001 From: Gao Binlong Date: Fri, 15 Mar 2024 13:58:19 +0800 Subject: [PATCH 3/5] Remove some unused code Signed-off-by: Gao Binlong --- .../forcemerge/ForceMergeRequestTests.java | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/server/src/test/java/org/opensearch/action/admin/indices/forcemerge/ForceMergeRequestTests.java b/server/src/test/java/org/opensearch/action/admin/indices/forcemerge/ForceMergeRequestTests.java index 3ac044d23f191..f7e5783ed7820 100644 --- a/server/src/test/java/org/opensearch/action/admin/indices/forcemerge/ForceMergeRequestTests.java +++ b/server/src/test/java/org/opensearch/action/admin/indices/forcemerge/ForceMergeRequestTests.java @@ -32,8 +32,10 @@ package org.opensearch.action.admin.indices.forcemerge; import org.opensearch.Version; +import org.opensearch.action.support.IndicesOptions; import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.tasks.TaskId; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.VersionUtils; @@ -100,20 +102,26 @@ public void testBwcSerialization() throws Exception { out.setVersion(compatibleVersion); sample.writeTo(out); - final ForceMergeRequest deserializedRequest; try (StreamInput in = out.bytes().streamInput()) { in.setVersion(Version.CURRENT); - deserializedRequest = new ForceMergeRequest(in); - } - - assertEquals(sample.maxNumSegments(), deserializedRequest.maxNumSegments()); - assertEquals(sample.onlyExpungeDeletes(), deserializedRequest.onlyExpungeDeletes()); - assertEquals(sample.flush(), deserializedRequest.flush()); - if (compatibleVersion.onOrAfter(Version.V_2_13_0)) { - assertEquals(sample.primaryOnly(), deserializedRequest.primaryOnly()); + TaskId.readFromStream(in); + in.readStringArray(); + IndicesOptions.readIndicesOptions(in); + int maxNumSegments = in.readInt(); + boolean onlyExpungeDeletes = in.readBoolean(); + boolean flush = in.readBoolean(); + boolean primaryOnly = in.readBoolean(); + String forceMergeUUID; if (compatibleVersion.onOrAfter(Version.V_3_0_0)) { - assertEquals(sample.forceMergeUUID(), deserializedRequest.forceMergeUUID()); + forceMergeUUID = in.readString(); + } else { + forceMergeUUID = in.readOptionalString(); } + assertEquals(sample.maxNumSegments(), maxNumSegments); + assertEquals(sample.onlyExpungeDeletes(), onlyExpungeDeletes); + assertEquals(sample.flush(), flush); + assertEquals(sample.primaryOnly(), primaryOnly); + assertEquals(sample.forceMergeUUID(), forceMergeUUID); } } } @@ -129,9 +137,7 @@ public void testBwcSerialization() throws Exception { out.writeInt(sample.maxNumSegments()); out.writeBoolean(sample.onlyExpungeDeletes()); out.writeBoolean(sample.flush()); - if (compatibleVersion.onOrAfter(Version.V_2_13_0)) { - out.writeBoolean(sample.primaryOnly()); - } + out.writeBoolean(sample.primaryOnly()); if (compatibleVersion.onOrAfter(Version.V_3_0_0)) { out.writeString(sample.forceMergeUUID()); } else { @@ -149,7 +155,6 @@ public void testBwcSerialization() throws Exception { assertEquals(sample.flush(), deserializedRequest.flush()); assertEquals(sample.primaryOnly(), deserializedRequest.primaryOnly()); assertEquals(sample.forceMergeUUID(), deserializedRequest.forceMergeUUID()); - } } } From 4c45a9e9525668aa999b093554a28d9e3bcee7ef Mon Sep 17 00:00:00 2001 From: Gao Binlong Date: Fri, 15 Mar 2024 13:59:05 +0800 Subject: [PATCH 4/5] Remove change log Signed-off-by: Gao Binlong --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e7eef68c1dab9..19a9a7461500f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -96,7 +96,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Bug] Check phase name before SearchRequestOperationsListener onPhaseStart ([#12035](https://github.com/opensearch-project/OpenSearch/pull/12035)) - Fix Span operation names generated from RestActions ([#12005](https://github.com/opensearch-project/OpenSearch/pull/12005)) - Fix error in RemoteSegmentStoreDirectory when debug logging is enabled ([#12328](https://github.com/opensearch-project/OpenSearch/pull/12328)) -- Update supported version for the primary_only parameter in force-merge API ([#12657](https://github.com/opensearch-project/OpenSearch/pull/12657)) ### Security From b3048fed8cd335863b58148e0d7389553d596c59 Mon Sep 17 00:00:00 2001 From: Gao Binlong Date: Mon, 18 Mar 2024 11:31:32 +0800 Subject: [PATCH 5/5] Fix test issue Signed-off-by: Gao Binlong --- .../forcemerge/ForceMergeRequestTests.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/server/src/test/java/org/opensearch/action/admin/indices/forcemerge/ForceMergeRequestTests.java b/server/src/test/java/org/opensearch/action/admin/indices/forcemerge/ForceMergeRequestTests.java index f7e5783ed7820..03cf38548a8cd 100644 --- a/server/src/test/java/org/opensearch/action/admin/indices/forcemerge/ForceMergeRequestTests.java +++ b/server/src/test/java/org/opensearch/action/admin/indices/forcemerge/ForceMergeRequestTests.java @@ -97,13 +97,13 @@ public void testSerialization() throws Exception { public void testBwcSerialization() throws Exception { { final ForceMergeRequest sample = randomRequest(); - final Version compatibleVersion = VersionUtils.randomCompatibleVersion(random(), Version.CURRENT); + final Version version = VersionUtils.randomCompatibleVersion(random(), Version.CURRENT); try (BytesStreamOutput out = new BytesStreamOutput()) { - out.setVersion(compatibleVersion); + out.setVersion(version); sample.writeTo(out); try (StreamInput in = out.bytes().streamInput()) { - in.setVersion(Version.CURRENT); + in.setVersion(version); TaskId.readFromStream(in); in.readStringArray(); IndicesOptions.readIndicesOptions(in); @@ -112,7 +112,7 @@ public void testBwcSerialization() throws Exception { boolean flush = in.readBoolean(); boolean primaryOnly = in.readBoolean(); String forceMergeUUID; - if (compatibleVersion.onOrAfter(Version.V_3_0_0)) { + if (version.onOrAfter(Version.V_3_0_0)) { forceMergeUUID = in.readString(); } else { forceMergeUUID = in.readOptionalString(); @@ -128,9 +128,9 @@ public void testBwcSerialization() throws Exception { { final ForceMergeRequest sample = randomRequest(); - final Version compatibleVersion = VersionUtils.randomCompatibleVersion(random(), Version.CURRENT); + final Version version = VersionUtils.randomCompatibleVersion(random(), Version.CURRENT); try (BytesStreamOutput out = new BytesStreamOutput()) { - out.setVersion(Version.CURRENT); + out.setVersion(version); sample.getParentTask().writeTo(out); out.writeStringArray(sample.indices()); sample.indicesOptions().writeIndicesOptions(out); @@ -138,7 +138,7 @@ public void testBwcSerialization() throws Exception { out.writeBoolean(sample.onlyExpungeDeletes()); out.writeBoolean(sample.flush()); out.writeBoolean(sample.primaryOnly()); - if (compatibleVersion.onOrAfter(Version.V_3_0_0)) { + if (version.onOrAfter(Version.V_3_0_0)) { out.writeString(sample.forceMergeUUID()); } else { out.writeOptionalString(sample.forceMergeUUID()); @@ -146,7 +146,7 @@ public void testBwcSerialization() throws Exception { final ForceMergeRequest deserializedRequest; try (StreamInput in = out.bytes().streamInput()) { - in.setVersion(compatibleVersion); + in.setVersion(version); deserializedRequest = new ForceMergeRequest(in); }