From 77c599c0405c9b79185074e371742a9af21aa462 Mon Sep 17 00:00:00 2001 From: Gao Binlong Date: Sat, 23 Nov 2024 17:55:16 +0800 Subject: [PATCH 1/4] Deprecate performing update operation with default pipeline or final pipeline Signed-off-by: Gao Binlong --- CHANGELOG.md | 1 + .../test/update/100_ingest_pipeline.yml | 102 ++++++++++++++++++ .../action/update/TransportUpdateAction.java | 14 ++- 3 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/update/100_ingest_pipeline.yml diff --git a/CHANGELOG.md b/CHANGELOG.md index 70245afda0dd1..341d802101cce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Changed ### Deprecated +- Performing update operation with default pipeline or final pipeline is deprecated ### Removed diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/update/100_ingest_pipeline.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/update/100_ingest_pipeline.yml new file mode 100644 index 0000000000000..56501270d6750 --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/update/100_ingest_pipeline.yml @@ -0,0 +1,102 @@ +setup: + - do: + ingest.put_pipeline: + id: "pipeline1" + body: > + { + "description": "_description", + "processors": [ + { + "set" : { + "field" : "field1", + "value": "value1" + } + } + ] + } + - do: + indices.create: + index: test_1 + body: + settings: + index.default_pipeline: "pipeline1" + - do: + indices.create: + index: test_2 + body: + settings: + index.final_pipeline: "pipeline1" +--- +teardown: + - do: + ingest.delete_pipeline: + id: "pipeline1" + ignore: 404 + + - do: + indices.delete: + index: test_1 + - do: + indices.delete: + index: test_2 +--- +"update operation with predefined default or final pipeline returns warning header": + - skip: + version: " - 3.0.0" + reason: "this change is added in 3.0.0" + features: allowed_warnings + - do: + index: + index: test_1 + id: 1 + body: { foo: bar } + + - match: { _seq_no: 0 } + - match: { _version: 1 } + - match: { _primary_term: 1 } + - match: { result: created } + + - do: + allowed_warnings: + - "the index [test_1] has a default ingest pipeline or a final ingest pipeline, but performing update operation with ingest pipeline causes unexpected result, this support will be removed in 3.0.0" + update: + index: test_1 + id: 1 + _source: true + body: + doc: { foo: bar1 } + + - match: { _seq_no: 1 } + - match: { _primary_term: 1 } + - match: { _version: 2 } + - match: { result: updated } + - match: { get._source.foo: bar1 } + - match: { get._source.field1: value1 } + + - do: + index: + index: test_2 + id: 1 + body: { foo: bar } + + - match: { _seq_no: 0 } + - match: { _version: 1 } + - match: { _primary_term: 1 } + - match: { result: created } + + - do: + allowed_warnings: + - "the index [test_2] has a default ingest pipeline or a final ingest pipeline, but performing update operation with ingest pipeline causes unexpected result, this support will be removed in 3.0.0" + update: + index: test_2 + id: 1 + _source: true + body: + doc: { foo: bar1 } + + - match: { _seq_no: 1 } + - match: { _primary_term: 1 } + - match: { _version: 2 } + - match: { result: updated } + - match: { get._source.foo: bar1 } + - match: { get._source.field1: value1 } diff --git a/server/src/main/java/org/opensearch/action/update/TransportUpdateAction.java b/server/src/main/java/org/opensearch/action/update/TransportUpdateAction.java index 819112eb497f6..978e833a701ce 100644 --- a/server/src/main/java/org/opensearch/action/update/TransportUpdateAction.java +++ b/server/src/main/java/org/opensearch/action/update/TransportUpdateAction.java @@ -57,6 +57,8 @@ import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.collect.Tuple; import org.opensearch.common.inject.Inject; +import org.opensearch.common.logging.DeprecationLogger; +import org.opensearch.common.settings.Settings; import org.opensearch.common.xcontent.XContentHelper; import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.bytes.BytesReference; @@ -67,6 +69,7 @@ import org.opensearch.core.xcontent.MediaType; import org.opensearch.index.IndexNotFoundException; import org.opensearch.index.IndexService; +import org.opensearch.index.IndexSettings; import org.opensearch.index.engine.VersionConflictEngineException; import org.opensearch.index.shard.IndexShard; import org.opensearch.index.shard.IndexingStats.Stats.DocStatusStats; @@ -90,7 +93,7 @@ * @opensearch.internal */ public class TransportUpdateAction extends TransportInstanceSingleOperationAction { - + private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(TransportUpdateAction.class); private final AutoCreateIndex autoCreateIndex; private final UpdateHelper updateHelper; private final IndicesService indicesService; @@ -276,6 +279,15 @@ protected void shardOperation(final UpdateRequest request, final ActionListener< IndexRequest indexRequest = result.action(); // we fetch it from the index request so we don't generate the bytes twice, its already done in the index request final BytesReference indexSourceBytes = indexRequest.source(); + final Settings indexSettings = indexService.getIndexSettings().getSettings(); + if (IndexSettings.DEFAULT_PIPELINE.exists(indexSettings) || IndexSettings.FINAL_PIPELINE.exists(indexSettings)) { + deprecationLogger.deprecate( + "update_operation_with_ingest_pipeline", + "the index [" + + indexRequest.index() + + "] has a default ingest pipeline or a final ingest pipeline, but performing update operation with ingest pipeline causes unexpected result, this support will be removed in 3.0.0" + ); + } client.bulk(toSingleItemBulkRequest(indexRequest), wrapBulkResponse(ActionListener.wrap(response -> { UpdateResponse update = new UpdateResponse( response.getShardInfo(), From 859ed0157fcdc47089938539088b4a622c02fd50 Mon Sep 17 00:00:00 2001 From: Gao Binlong Date: Mon, 2 Dec 2024 13:42:40 +0800 Subject: [PATCH 2/4] Modify the warning message Signed-off-by: Gao Binlong --- .../resources/rest-api-spec/test/update/100_ingest_pipeline.yml | 2 +- .../org/opensearch/action/update/TransportUpdateAction.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/update/100_ingest_pipeline.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/update/100_ingest_pipeline.yml index 56501270d6750..480b3ae990cad 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/update/100_ingest_pipeline.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/update/100_ingest_pipeline.yml @@ -86,7 +86,7 @@ teardown: - do: allowed_warnings: - - "the index [test_2] has a default ingest pipeline or a final ingest pipeline, but performing update operation with ingest pipeline causes unexpected result, this support will be removed in 3.0.0" + - "the index [test_2] has a default ingest pipeline or a final ingest pipeline, the support of the ingest pipelines for update operation causes unexpected result and will be removed in 3.0.0" update: index: test_2 id: 1 diff --git a/server/src/main/java/org/opensearch/action/update/TransportUpdateAction.java b/server/src/main/java/org/opensearch/action/update/TransportUpdateAction.java index 978e833a701ce..52378142ae1dd 100644 --- a/server/src/main/java/org/opensearch/action/update/TransportUpdateAction.java +++ b/server/src/main/java/org/opensearch/action/update/TransportUpdateAction.java @@ -285,7 +285,7 @@ protected void shardOperation(final UpdateRequest request, final ActionListener< "update_operation_with_ingest_pipeline", "the index [" + indexRequest.index() - + "] has a default ingest pipeline or a final ingest pipeline, but performing update operation with ingest pipeline causes unexpected result, this support will be removed in 3.0.0" + + "] has a default ingest pipeline or a final ingest pipeline, the support of the ingest pipelines for update operation causes unexpected result and will be removed in 3.0.0" ); } client.bulk(toSingleItemBulkRequest(indexRequest), wrapBulkResponse(ActionListener.wrap(response -> { From 84e37ec333321dc3ce57a872cef16db9273601d8 Mon Sep 17 00:00:00 2001 From: Gao Binlong Date: Mon, 2 Dec 2024 16:56:08 +0800 Subject: [PATCH 3/4] Modify changelog Signed-off-by: Gao Binlong --- CHANGELOG.md | 2 +- .../resources/rest-api-spec/test/update/100_ingest_pipeline.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f138ea9389bfd..57c2f97405fd2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,7 +43,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Indexed IP field supports `terms_query` with more than 1025 IP masks [#16391](https://github.com/opensearch-project/OpenSearch/pull/16391) ### Deprecated -- Performing update operation with default pipeline or final pipeline is deprecated +- Performing update operation with default pipeline or final pipeline is deprecated ([#16712](https://github.com/opensearch-project/OpenSearch/pull/16712)) ### Removed diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/update/100_ingest_pipeline.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/update/100_ingest_pipeline.yml index 480b3ae990cad..0996fe3b4a08e 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/update/100_ingest_pipeline.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/update/100_ingest_pipeline.yml @@ -58,7 +58,7 @@ teardown: - do: allowed_warnings: - - "the index [test_1] has a default ingest pipeline or a final ingest pipeline, but performing update operation with ingest pipeline causes unexpected result, this support will be removed in 3.0.0" + - "the index [test_1] has a default ingest pipeline or a final ingest pipeline, the support of the ingest pipelines for update operation causes unexpected result and will be removed in 3.0.0" update: index: test_1 id: 1 From b5c1a07b200e7e58c6863a1fead8987846e0218b Mon Sep 17 00:00:00 2001 From: Gao Binlong Date: Mon, 2 Dec 2024 17:11:59 +0800 Subject: [PATCH 4/4] Fix test issue Signed-off-by: Gao Binlong --- .../resources/rest-api-spec/test/ingest/75_update.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename rest-api-spec/src/main/resources/rest-api-spec/test/update/100_ingest_pipeline.yml => modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/75_update.yml (98%) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/update/100_ingest_pipeline.yml b/modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/75_update.yml similarity index 98% rename from rest-api-spec/src/main/resources/rest-api-spec/test/update/100_ingest_pipeline.yml rename to modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/75_update.yml index 0996fe3b4a08e..a66b6293110cf 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/update/100_ingest_pipeline.yml +++ b/modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/75_update.yml @@ -42,7 +42,7 @@ teardown: --- "update operation with predefined default or final pipeline returns warning header": - skip: - version: " - 3.0.0" + version: " - 2.99.99" reason: "this change is added in 3.0.0" features: allowed_warnings - do: