From 08b8450b628fc546e73f0f47d0f333e946443ac0 Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Mon, 13 Jan 2020 10:58:29 -0500 Subject: [PATCH] Deprecate synced flush (#50835) A normal flush has the same effect as a synced flush on Elasticsearch 7.6 or later. It's deprecated in 7.6 and will be removed in 8.0. Relates #50776 --- .../elasticsearch/client/IndicesClient.java | 6 +++ .../elasticsearch/client/IndicesClientIT.java | 7 ++- .../client/RestHighLevelClientTests.java | 3 +- .../IndicesClientDocumentationIT.java | 8 +++- docs/reference/indices/synced-flush.asciidoc | 4 ++ docs/reference/upgrade/synced-flush.asciidoc | 4 ++ .../upgrades/FullClusterRestartIT.java | 2 +- .../elasticsearch/upgrades/RecoveryIT.java | 2 +- .../api/indices.flush_synced.json | 2 +- .../test/cat.shards/10_basic.yml | 48 ------------------- .../test/indices.flush/10_basic.yml | 40 +++++++++------- .../indices/flush/SyncedFlushService.java | 7 +++ .../test/rest/ESRestTestCase.java | 16 +++++++ 13 files changed, 76 insertions(+), 73 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesClient.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesClient.java index 4cdc59dd3bd35..e00a9c0da8d50 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesClient.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesClient.java @@ -423,7 +423,10 @@ public Cancellable flushAsync(FlushRequest flushRequest, RequestOptions options, * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized * @return the response * @throws IOException in case there is a problem sending the request or parsing back the response + * @deprecated synced flush is deprecated and will be removed in 8.0. + * Use {@link #flush(FlushRequest, RequestOptions)} instead. */ + @Deprecated public SyncedFlushResponse flushSynced(SyncedFlushRequest syncedFlushRequest, RequestOptions options) throws IOException { return restHighLevelClient.performRequestAndParseEntity(syncedFlushRequest, IndicesRequestConverters::flushSynced, options, SyncedFlushResponse::fromXContent, emptySet()); @@ -437,7 +440,10 @@ public SyncedFlushResponse flushSynced(SyncedFlushRequest syncedFlushRequest, Re * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized * @param listener the listener to be notified upon request completion * @return cancellable that may be used to cancel the request + * @deprecated synced flush is deprecated and will be removed in 8.0. + * Use {@link #flushAsync(FlushRequest, RequestOptions, ActionListener)} instead. */ + @Deprecated public Cancellable flushSyncedAsync(SyncedFlushRequest syncedFlushRequest, RequestOptions options, ActionListener listener) { return restHighLevelClient.performRequestAsyncAndParseEntity(syncedFlushRequest, IndicesRequestConverters::flushSynced, options, diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java index e712604266a31..842d348115845 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java @@ -97,6 +97,7 @@ import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryBuilders; +import org.elasticsearch.indices.flush.SyncedFlushService; import org.elasticsearch.rest.RestStatus; import java.io.IOException; @@ -768,7 +769,8 @@ public void testSyncedFlush() throws IOException { createIndex(index, settings); SyncedFlushRequest syncedFlushRequest = new SyncedFlushRequest(index); SyncedFlushResponse flushResponse = - execute(syncedFlushRequest, highLevelClient().indices()::flushSynced, highLevelClient().indices()::flushSyncedAsync); + execute(syncedFlushRequest, highLevelClient().indices()::flushSynced, highLevelClient().indices()::flushSyncedAsync, + expectWarnings(SyncedFlushService.SYNCED_FLUSH_DEPRECATION_MESSAGE)); assertThat(flushResponse.totalShards(), equalTo(1)); assertThat(flushResponse.successfulShards(), equalTo(1)); assertThat(flushResponse.failedShards(), equalTo(0)); @@ -783,7 +785,8 @@ public void testSyncedFlush() throws IOException { execute( syncedFlushRequest, highLevelClient().indices()::flushSynced, - highLevelClient().indices()::flushSyncedAsync + highLevelClient().indices()::flushSyncedAsync, + expectWarnings(SyncedFlushService.SYNCED_FLUSH_DEPRECATION_MESSAGE) ) ); assertEquals(RestStatus.NOT_FOUND, exception.status()); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java index 4ec048ad1e4f5..c33c7212388ad 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java @@ -839,7 +839,8 @@ public void testApiNamingConventions() throws Exception { // looking like it doesn't have a valid implementatation when it does. apiUnsupported.remove("indices.get_template"); - + // Synced flush is deprecated + apiUnsupported.remove("indices.flush_synced"); for (Map.Entry> entry : methods.entrySet()) { String apiName = entry.getKey(); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java index 480091e1e4a03..02d426e4b6c6d 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java @@ -1018,7 +1018,9 @@ public void testSyncedFlushIndex() throws Exception { // end::flush-synced-request-indicesOptions // tag::flush-synced-execute - SyncedFlushResponse flushSyncedResponse = client.indices().flushSynced(request, RequestOptions.DEFAULT); + SyncedFlushResponse flushSyncedResponse = client.indices().flushSynced(request, expectWarnings( + "Synced flush is deprecated and will be removed in 8.0. Use flush at _/flush or /{index}/_flush instead." + )); // end::flush-synced-execute // tag::flush-synced-response @@ -1062,7 +1064,9 @@ public void onFailure(Exception e) { listener = new LatchedActionListener<>(listener, latch); // tag::flush-synced-execute-async - client.indices().flushSyncedAsync(request, RequestOptions.DEFAULT, listener); // <1> + client.indices().flushSyncedAsync(request, expectWarnings( + "Synced flush is deprecated and will be removed in 8.0. Use flush at _/flush or /{index}/_flush instead." + ), listener); // <1> // end::flush-synced-execute-async assertTrue(latch.await(30L, TimeUnit.SECONDS)); diff --git a/docs/reference/indices/synced-flush.asciidoc b/docs/reference/indices/synced-flush.asciidoc index cb2c40793091a..8e3a85aceca33 100644 --- a/docs/reference/indices/synced-flush.asciidoc +++ b/docs/reference/indices/synced-flush.asciidoc @@ -4,6 +4,10 @@ Synced flush ++++ +deprecated::[7.6, synced-flush is deprecated and will be removed in 8.0. +Use <> instead. A <> has the +same effect as a synced flush on Elasticsearch 7.6 or later] + Performs a synced flush on one or more indices. [source,console] diff --git a/docs/reference/upgrade/synced-flush.asciidoc b/docs/reference/upgrade/synced-flush.asciidoc index 59bdea15ba742..a67c171f99875 100644 --- a/docs/reference/upgrade/synced-flush.asciidoc +++ b/docs/reference/upgrade/synced-flush.asciidoc @@ -3,8 +3,12 @@ -------------------------------------------------- POST _flush/synced -------------------------------------------------- +// TEST[skip: will fail as synced flush is deprecated] When you perform a synced flush, check the response to make sure there are no failures. Synced flush operations that fail due to pending indexing operations are listed in the response body, although the request itself still returns a 200 OK status. If there are failures, reissue the request. + +Note that synced flush is deprecated and will be removed in 8.0. A flush +has the same effect as a synced flush on Elasticsearch 7.6 or later. diff --git a/qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/FullClusterRestartIT.java b/qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/FullClusterRestartIT.java index 21ae48e9c77d4..9c7d7af4ca6c8 100644 --- a/qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/FullClusterRestartIT.java +++ b/qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/FullClusterRestartIT.java @@ -684,7 +684,7 @@ public void testRecovery() throws Exception { // We had a bug before where we failed to perform peer recovery with sync_id from 5.x to 6.x. // We added this synced flush so we can exercise different paths of recovery code. try { - client().performRequest(new Request("POST", index + "/_flush/synced")); + performSyncedFlush(index); } catch (ResponseException ignored) { // synced flush is optional here } diff --git a/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/RecoveryIT.java b/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/RecoveryIT.java index 88dabadbd09fd..f29bfddde0420 100644 --- a/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/RecoveryIT.java +++ b/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/RecoveryIT.java @@ -562,7 +562,7 @@ private void syncedFlush(String index) throws Exception { // A synced-flush request considers the global checkpoint sync as an going operation because it acquires a shard permit. assertBusy(() -> { try { - Response resp = client().performRequest(new Request("POST", index + "/_flush/synced")); + Response resp = performSyncedFlush(index); Map result = ObjectPath.createFromResponse(resp).evaluate("_shards"); assertThat(result.get("failed"), equalTo(0)); } catch (ResponseException ex) { diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/indices.flush_synced.json b/rest-api-spec/src/main/resources/rest-api-spec/api/indices.flush_synced.json index e7c98d66451bd..a7b4541c9623a 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/indices.flush_synced.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/indices.flush_synced.json @@ -2,7 +2,7 @@ "indices.flush_synced":{ "documentation":{ "url":"https://www.elastic.co/guide/en/elasticsearch/reference/master/indices-synced-flush-api.html", - "description":"Performs a synced flush operation on one or more indices." + "description":"Performs a synced flush operation on one or more indices. Synced flush is deprecated and will be removed in 8.0. Use flush instead" }, "stability":"stable", "url":{ diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/cat.shards/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/cat.shards/10_basic.yml index 6632d912cd57e..de5e632975752 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/cat.shards/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/cat.shards/10_basic.yml @@ -88,54 +88,6 @@ - match: $body: | /^$/ - - - do: - indices.create: - index: sync_id_test - body: - settings: - number_of_shards: 5 - number_of_replicas: 0 - - - do: - indices.flush_synced: - index: sync_id_test - - - is_false: _shards.failed - - - do: - cat.shards: - index: sync_id_test - h: index,state,sync_id -# 20 chars for sync ids with 5.x which uses time-based uuids and 22 with 6.x which uses random uuids - - match: - $body: | - /^(sync_id_test\s+STARTED\s+[A-Za-z0-9_\-]{20,22}\n){5}$/ - - - do: - indices.delete: - index: sync_id_test - - - do: - indices.create: - index: sync_id_no_flush_test - body: - settings: - number_of_shards: 5 - number_of_replicas: 0 - - - do: - cat.shards: - index: sync_id_no_flush_test - h: index,state,sync_id - - match: - $body: | - /^(sync_id_no_flush_test\s+STARTED\s+\n){5}$/ - - - do: - indices.delete: - index: sync_id_no_flush_test - - do: indices.create: index: index1 diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.flush/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.flush/10_basic.yml index ff99d20e9b761..e3681c834dbad 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.flush/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.flush/10_basic.yml @@ -1,26 +1,32 @@ --- "Index synced flush rest test": - - do: - indices.create: - index: testing - body: - settings: - index: - number_of_replicas: 0 + - skip: + version: " - 7.5.99" + reason: "synced flush is deprecated in 7.6" + features: "warnings" + - do: + indices.create: + index: testing + body: + settings: + index: + number_of_replicas: 0 - - do: - cluster.health: - wait_for_status: green - - do: - indices.flush_synced: - index: testing + - do: + cluster.health: + wait_for_status: green + - do: + warnings: + - Synced flush is deprecated and will be removed in 8.0. Use flush at _/flush or /{index}/_flush instead. + indices.flush_synced: + index: testing - - is_false: _shards.failed + - is_false: _shards.failed - - do: - indices.stats: {level: shards} + - do: + indices.stats: {level: shards} - - is_true: indices.testing.shards.0.0.commit.user_data.sync_id + - is_true: indices.testing.shards.0.0.commit.user_data.sync_id --- "Flush stats": diff --git a/server/src/main/java/org/elasticsearch/indices/flush/SyncedFlushService.java b/server/src/main/java/org/elasticsearch/indices/flush/SyncedFlushService.java index c0e0d513b33d7..a0fc2e153b90e 100644 --- a/server/src/main/java/org/elasticsearch/indices/flush/SyncedFlushService.java +++ b/server/src/main/java/org/elasticsearch/indices/flush/SyncedFlushService.java @@ -40,6 +40,7 @@ import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.util.concurrent.ConcurrentCollections; import org.elasticsearch.common.util.concurrent.CountDown; import org.elasticsearch.index.Index; @@ -74,6 +75,11 @@ public class SyncedFlushService { private static final Logger logger = LogManager.getLogger(SyncedFlushService.class); + private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(logger); + + public static final String SYNCED_FLUSH_DEPRECATION_MESSAGE = + "Synced flush is deprecated and will be removed in 8.0. Use flush at _/flush or /{index}/_flush instead."; + private static final String PRE_SYNCED_FLUSH_ACTION_NAME = "internal:indices/flush/synced/pre"; private static final String SYNCED_FLUSH_ACTION_NAME = "internal:indices/flush/synced/sync"; private static final String IN_FLIGHT_OPS_ACTION_NAME = "internal:indices/flush/synced/in_flight"; @@ -109,6 +115,7 @@ public void attemptSyncedFlush(final String[] aliasesOrIndices, IndicesOptions indicesOptions, final ActionListener listener) { final ClusterState state = clusterService.state(); + DEPRECATION_LOGGER.deprecatedAndMaybeLog("synced_flush", SYNCED_FLUSH_DEPRECATION_MESSAGE); final Index[] concreteIndices = indexNameExpressionResolver.concreteIndices(state, indicesOptions, aliasesOrIndices); final Map> results = ConcurrentCollections.newConcurrentMap(); int numberOfShards = 0; diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java index a1968332c734b..988975619fd89 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java @@ -54,6 +54,7 @@ import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.seqno.ReplicationTracker; +import org.elasticsearch.indices.flush.SyncedFlushService; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.snapshots.SnapshotState; import org.elasticsearch.test.ESTestCase; @@ -1212,4 +1213,19 @@ protected static Version minimumNodeVersion() throws IOException { assertNotNull(minVersion); return minVersion; } + + protected static Response performSyncedFlush(String indexName) throws IOException { + final Request request = new Request("POST", indexName + "/_flush/synced"); + final List expectedWarnings = Collections.singletonList(SyncedFlushService.SYNCED_FLUSH_DEPRECATION_MESSAGE); + if (nodeVersions.stream().allMatch(version -> version.onOrAfter(Version.V_7_6_0))) { + final Builder options = RequestOptions.DEFAULT.toBuilder(); + options.setWarningsHandler(warnings -> warnings.equals(expectedWarnings) == false); + request.setOptions(options); + } else if (nodeVersions.stream().anyMatch(version -> version.onOrAfter(Version.V_7_6_0))) { + final Builder options = RequestOptions.DEFAULT.toBuilder(); + options.setWarningsHandler(warnings -> warnings.isEmpty() == false && warnings.equals(expectedWarnings) == false); + request.setOptions(options); + } + return client().performRequest(request); + } }