diff --git a/client/rest-high-level/src/main/java/org/opensearch/client/IndicesClient.java b/client/rest-high-level/src/main/java/org/opensearch/client/IndicesClient.java index dcff976106b4e..360c0481e619f 100644 --- a/client/rest-high-level/src/main/java/org/opensearch/client/IndicesClient.java +++ b/client/rest-high-level/src/main/java/org/opensearch/client/IndicesClient.java @@ -40,7 +40,6 @@ import org.opensearch.action.admin.indices.delete.DeleteIndexRequest; import org.opensearch.action.admin.indices.flush.FlushRequest; import org.opensearch.action.admin.indices.flush.FlushResponse; -import org.opensearch.action.admin.indices.flush.SyncedFlushRequest; import org.opensearch.action.admin.indices.forcemerge.ForceMergeRequest; import org.opensearch.action.admin.indices.forcemerge.ForceMergeResponse; import org.opensearch.action.admin.indices.open.OpenIndexRequest; @@ -931,53 +930,6 @@ public Cancellable flushAsync(FlushRequest flushRequest, RequestOptions options, ); } - /** - * Initiate a synced flush manually using the synced flush API. - * - * @param syncedFlushRequest the request - * @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() - ); - } - - /** - * Asynchronously initiate a synced flush manually using the synced flush API. - * - * @param syncedFlushRequest the request - * @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, - SyncedFlushResponse::fromXContent, - listener, - emptySet() - ); - } - /** * Retrieve the settings of one or more indices. * diff --git a/client/rest-high-level/src/main/java/org/opensearch/client/IndicesRequestConverters.java b/client/rest-high-level/src/main/java/org/opensearch/client/IndicesRequestConverters.java index 727e91fc210cd..9979d18635d05 100644 --- a/client/rest-high-level/src/main/java/org/opensearch/client/IndicesRequestConverters.java +++ b/client/rest-high-level/src/main/java/org/opensearch/client/IndicesRequestConverters.java @@ -42,7 +42,6 @@ import org.opensearch.action.admin.indices.cache.clear.ClearIndicesCacheRequest; import org.opensearch.action.admin.indices.delete.DeleteIndexRequest; import org.opensearch.action.admin.indices.flush.FlushRequest; -import org.opensearch.action.admin.indices.flush.SyncedFlushRequest; import org.opensearch.action.admin.indices.forcemerge.ForceMergeRequest; import org.opensearch.action.admin.indices.open.OpenIndexRequest; import org.opensearch.action.admin.indices.refresh.RefreshRequest; @@ -322,15 +321,6 @@ static Request flush(FlushRequest flushRequest) { return request; } - static Request flushSynced(SyncedFlushRequest syncedFlushRequest) { - String[] indices = syncedFlushRequest.indices() == null ? Strings.EMPTY_ARRAY : syncedFlushRequest.indices(); - Request request = new Request(HttpPost.METHOD_NAME, RequestConverters.endpoint(indices, "_flush/synced")); - RequestConverters.Params parameters = new RequestConverters.Params(); - parameters.withIndicesOptions(syncedFlushRequest.indicesOptions()); - request.addParameters(parameters.asMap()); - return request; - } - static Request forceMerge(ForceMergeRequest forceMergeRequest) { String[] indices = forceMergeRequest.indices() == null ? Strings.EMPTY_ARRAY : forceMergeRequest.indices(); Request request = new Request(HttpPost.METHOD_NAME, RequestConverters.endpoint(indices, "_forcemerge")); diff --git a/client/rest-high-level/src/main/java/org/opensearch/client/SyncedFlushResponse.java b/client/rest-high-level/src/main/java/org/opensearch/client/SyncedFlushResponse.java deleted file mode 100644 index a0c94fb75579e..0000000000000 --- a/client/rest-high-level/src/main/java/org/opensearch/client/SyncedFlushResponse.java +++ /dev/null @@ -1,346 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -/* - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.client; - -import org.opensearch.common.ParseField; -import org.opensearch.common.ParsingException; -import org.opensearch.common.xcontent.ConstructingObjectParser; -import org.opensearch.common.xcontent.ToXContentFragment; -import org.opensearch.common.xcontent.ToXContentObject; -import org.opensearch.common.xcontent.XContentBuilder; -import org.opensearch.common.xcontent.XContentLocation; -import org.opensearch.common.xcontent.XContentParser; -import org.opensearch.common.xcontent.XContentParser.Token; - -import java.io.IOException; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -import static org.opensearch.common.xcontent.ConstructingObjectParser.constructorArg; -import static org.opensearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg; -import static org.opensearch.common.xcontent.XContentParserUtils.ensureExpectedToken; - -public class SyncedFlushResponse implements ToXContentObject { - - public static final String SHARDS_FIELD = "_shards"; - - private ShardCounts totalCounts; - private Map indexResults; - - SyncedFlushResponse(ShardCounts totalCounts, Map indexResults) { - this.totalCounts = new ShardCounts(totalCounts.total, totalCounts.successful, totalCounts.failed); - this.indexResults = Collections.unmodifiableMap(indexResults); - } - - /** - * @return The total number of shard copies that were processed across all indexes - */ - public int totalShards() { - return totalCounts.total; - } - - /** - * @return The number of successful shard copies that were processed across all indexes - */ - public int successfulShards() { - return totalCounts.successful; - } - - /** - * @return The number of failed shard copies that were processed across all indexes - */ - public int failedShards() { - return totalCounts.failed; - } - - /** - * @return A map of results for each index where the keys of the map are the index names - * and the values are the results encapsulated in {@link IndexResult}. - */ - public Map getIndexResults() { - return indexResults; - } - - ShardCounts getShardCounts() { - return totalCounts; - } - - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(); - builder.startObject(SHARDS_FIELD); - totalCounts.toXContent(builder, params); - builder.endObject(); - for (Map.Entry entry : indexResults.entrySet()) { - String indexName = entry.getKey(); - IndexResult indexResult = entry.getValue(); - builder.startObject(indexName); - indexResult.toXContent(builder, params); - builder.endObject(); - } - builder.endObject(); - return builder; - } - - public static SyncedFlushResponse fromXContent(XContentParser parser) throws IOException { - ensureExpectedToken(Token.START_OBJECT, parser.nextToken(), parser); - ShardCounts totalCounts = null; - Map indexResults = new HashMap<>(); - XContentLocation startLoc = parser.getTokenLocation(); - while (parser.nextToken().equals(Token.FIELD_NAME)) { - if (parser.currentName().equals(SHARDS_FIELD)) { - ensureExpectedToken(Token.START_OBJECT, parser.nextToken(), parser); - totalCounts = ShardCounts.fromXContent(parser); - } else { - String indexName = parser.currentName(); - IndexResult indexResult = IndexResult.fromXContent(parser); - indexResults.put(indexName, indexResult); - } - } - if (totalCounts != null) { - return new SyncedFlushResponse(totalCounts, indexResults); - } else { - throw new ParsingException(startLoc, "Unable to reconstruct object. Total counts for shards couldn't be parsed."); - } - } - - /** - * Encapsulates the number of total successful and failed shard copies - */ - public static final class ShardCounts implements ToXContentFragment { - - public static final String TOTAL_FIELD = "total"; - public static final String SUCCESSFUL_FIELD = "successful"; - public static final String FAILED_FIELD = "failed"; - - private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( - "shardcounts", - a -> new ShardCounts((Integer) a[0], (Integer) a[1], (Integer) a[2]) - ); - static { - PARSER.declareInt(constructorArg(), new ParseField(TOTAL_FIELD)); - PARSER.declareInt(constructorArg(), new ParseField(SUCCESSFUL_FIELD)); - PARSER.declareInt(constructorArg(), new ParseField(FAILED_FIELD)); - } - - private int total; - private int successful; - private int failed; - - ShardCounts(int total, int successful, int failed) { - this.total = total; - this.successful = successful; - this.failed = failed; - } - - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.field(TOTAL_FIELD, total); - builder.field(SUCCESSFUL_FIELD, successful); - builder.field(FAILED_FIELD, failed); - return builder; - } - - public static ShardCounts fromXContent(XContentParser parser) throws IOException { - return PARSER.parse(parser, null); - } - - public boolean equals(ShardCounts other) { - if (other != null) { - return other.total == this.total && other.successful == this.successful && other.failed == this.failed; - } else { - return false; - } - } - - } - - /** - * Description for the flush/synced results for a particular index. - * This includes total, successful and failed copies along with failure description for each failed copy. - */ - public static final class IndexResult implements ToXContentFragment { - - public static final String TOTAL_FIELD = "total"; - public static final String SUCCESSFUL_FIELD = "successful"; - public static final String FAILED_FIELD = "failed"; - public static final String FAILURES_FIELD = "failures"; - - @SuppressWarnings("unchecked") - private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( - "indexresult", - a -> new IndexResult((Integer) a[0], (Integer) a[1], (Integer) a[2], (List) a[3]) - ); - static { - PARSER.declareInt(constructorArg(), new ParseField(TOTAL_FIELD)); - PARSER.declareInt(constructorArg(), new ParseField(SUCCESSFUL_FIELD)); - PARSER.declareInt(constructorArg(), new ParseField(FAILED_FIELD)); - PARSER.declareObjectArray(optionalConstructorArg(), ShardFailure.PARSER, new ParseField(FAILURES_FIELD)); - } - - private ShardCounts counts; - private List failures; - - IndexResult(int total, int successful, int failed, List failures) { - counts = new ShardCounts(total, successful, failed); - if (failures != null) { - this.failures = Collections.unmodifiableList(failures); - } else { - this.failures = Collections.unmodifiableList(new ArrayList<>()); - } - } - - /** - * @return The total number of shard copies that were processed for this index. - */ - public int totalShards() { - return counts.total; - } - - /** - * @return The number of successful shard copies that were processed for this index. - */ - public int successfulShards() { - return counts.successful; - } - - /** - * @return The number of failed shard copies that were processed for this index. - */ - public int failedShards() { - return counts.failed; - } - - /** - * @return A list of {@link ShardFailure} objects that describe each of the failed shard copies for this index. - */ - public List failures() { - return failures; - } - - ShardCounts getShardCounts() { - return counts; - } - - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - counts.toXContent(builder, params); - if (failures.size() > 0) { - builder.startArray(FAILURES_FIELD); - for (ShardFailure failure : failures) { - failure.toXContent(builder, params); - } - builder.endArray(); - } - return builder; - } - - public static IndexResult fromXContent(XContentParser parser) throws IOException { - return PARSER.parse(parser, null); - } - } - - /** - * Description of a failed shard copy for an index. - */ - public static final class ShardFailure implements ToXContentFragment { - - public static String SHARD_ID_FIELD = "shard"; - public static String FAILURE_REASON_FIELD = "reason"; - public static String ROUTING_FIELD = "routing"; - - private int shardId; - private String failureReason; - private Map routing; - - @SuppressWarnings("unchecked") - static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( - "shardfailure", - a -> new ShardFailure((Integer) a[0], (String) a[1], (Map) a[2]) - ); - static { - PARSER.declareInt(constructorArg(), new ParseField(SHARD_ID_FIELD)); - PARSER.declareString(constructorArg(), new ParseField(FAILURE_REASON_FIELD)); - PARSER.declareObject(optionalConstructorArg(), (parser, c) -> parser.map(), new ParseField(ROUTING_FIELD)); - } - - ShardFailure(int shardId, String failureReason, Map routing) { - this.shardId = shardId; - this.failureReason = failureReason; - if (routing != null) { - this.routing = Collections.unmodifiableMap(routing); - } else { - this.routing = Collections.unmodifiableMap(new HashMap<>()); - } - } - - /** - * @return Id of the shard whose copy failed - */ - public int getShardId() { - return shardId; - } - - /** - * @return Reason for failure of the shard copy - */ - public String getFailureReason() { - return failureReason; - } - - /** - * @return Additional information about the failure. - */ - public Map getRouting() { - return routing; - } - - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(); - builder.field(SHARD_ID_FIELD, shardId); - builder.field(FAILURE_REASON_FIELD, failureReason); - if (routing.size() > 0) { - builder.field(ROUTING_FIELD, routing); - } - builder.endObject(); - return builder; - } - - public static ShardFailure fromXContent(XContentParser parser) throws IOException { - return PARSER.parse(parser, null); - } - } -} diff --git a/client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java b/client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java index d33abb0552776..043a75d28a301 100644 --- a/client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java +++ b/client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java @@ -46,7 +46,6 @@ import org.opensearch.action.admin.indices.delete.DeleteIndexRequest; import org.opensearch.action.admin.indices.flush.FlushRequest; import org.opensearch.action.admin.indices.flush.FlushResponse; -import org.opensearch.action.admin.indices.flush.SyncedFlushRequest; import org.opensearch.action.admin.indices.forcemerge.ForceMergeRequest; import org.opensearch.action.admin.indices.forcemerge.ForceMergeResponse; import org.opensearch.action.admin.indices.open.OpenIndexRequest; @@ -126,7 +125,6 @@ import org.opensearch.index.mapper.MapperService; import org.opensearch.index.query.QueryBuilder; import org.opensearch.index.query.QueryBuilders; -import org.opensearch.indices.flush.SyncedFlushService; import org.opensearch.rest.RestStatus; import org.opensearch.rest.action.admin.indices.RestCreateIndexAction; import org.opensearch.rest.action.admin.indices.RestGetFieldMappingAction; @@ -1080,39 +1078,6 @@ public void testFlush() throws IOException { } } - public void testSyncedFlush() throws IOException { - { - String index = "index"; - Settings settings = Settings.builder().put("number_of_shards", 1).put("number_of_replicas", 0).build(); - createIndex(index, settings); - SyncedFlushRequest syncedFlushRequest = new SyncedFlushRequest(index); - SyncedFlushResponse flushResponse = execute( - syncedFlushRequest, - highLevelClient().indices()::flushSynced, - highLevelClient().indices()::flushSyncedAsync, - expectWarningsOnce(SyncedFlushService.SYNCED_FLUSH_DEPRECATION_MESSAGE) - ); - assertThat(flushResponse.totalShards(), equalTo(1)); - assertThat(flushResponse.successfulShards(), equalTo(1)); - assertThat(flushResponse.failedShards(), equalTo(0)); - } - { - String nonExistentIndex = "non_existent_index"; - assertFalse(indexExists(nonExistentIndex)); - SyncedFlushRequest syncedFlushRequest = new SyncedFlushRequest(nonExistentIndex); - OpenSearchException exception = expectThrows( - OpenSearchException.class, - () -> execute( - syncedFlushRequest, - highLevelClient().indices()::flushSynced, - highLevelClient().indices()::flushSyncedAsync, - expectWarningsOnce(SyncedFlushService.SYNCED_FLUSH_DEPRECATION_MESSAGE) - ) - ); - assertEquals(RestStatus.NOT_FOUND, exception.status()); - } - } - public void testClearCache() throws IOException { { String index = "index"; diff --git a/client/rest-high-level/src/test/java/org/opensearch/client/IndicesRequestConvertersTests.java b/client/rest-high-level/src/test/java/org/opensearch/client/IndicesRequestConvertersTests.java index 28728a95ae976..0ea2280b386eb 100644 --- a/client/rest-high-level/src/test/java/org/opensearch/client/IndicesRequestConvertersTests.java +++ b/client/rest-high-level/src/test/java/org/opensearch/client/IndicesRequestConvertersTests.java @@ -45,7 +45,6 @@ import org.opensearch.action.admin.indices.cache.clear.ClearIndicesCacheRequest; import org.opensearch.action.admin.indices.delete.DeleteIndexRequest; import org.opensearch.action.admin.indices.flush.FlushRequest; -import org.opensearch.action.admin.indices.flush.SyncedFlushRequest; import org.opensearch.action.admin.indices.forcemerge.ForceMergeRequest; import org.opensearch.action.admin.indices.open.OpenIndexRequest; import org.opensearch.action.admin.indices.refresh.RefreshRequest; @@ -750,33 +749,6 @@ public void testFlush() { Assert.assertThat(request.getMethod(), equalTo(HttpPost.METHOD_NAME)); } - public void testSyncedFlush() { - String[] indices = OpenSearchTestCase.randomBoolean() ? null : RequestConvertersTests.randomIndicesNames(0, 5); - SyncedFlushRequest syncedFlushRequest; - if (OpenSearchTestCase.randomBoolean()) { - syncedFlushRequest = new SyncedFlushRequest(indices); - } else { - syncedFlushRequest = new SyncedFlushRequest(); - syncedFlushRequest.indices(indices); - } - Map expectedParams = new HashMap<>(); - RequestConvertersTests.setRandomIndicesOptions( - syncedFlushRequest::indicesOptions, - syncedFlushRequest::indicesOptions, - expectedParams - ); - Request request = IndicesRequestConverters.flushSynced(syncedFlushRequest); - StringJoiner endpoint = new StringJoiner("/", "/", ""); - if (indices != null && indices.length > 0) { - endpoint.add(String.join(",", indices)); - } - endpoint.add("_flush/synced"); - Assert.assertThat(request.getEndpoint(), equalTo(endpoint.toString())); - Assert.assertThat(request.getParameters(), equalTo(expectedParams)); - Assert.assertThat(request.getEntity(), nullValue()); - Assert.assertThat(request.getMethod(), equalTo(HttpPost.METHOD_NAME)); - } - public void testForceMerge() { String[] indices = OpenSearchTestCase.randomBoolean() ? null : RequestConvertersTests.randomIndicesNames(0, 5); ForceMergeRequest forceMergeRequest; diff --git a/client/rest-high-level/src/test/java/org/opensearch/client/SyncedFlushResponseTests.java b/client/rest-high-level/src/test/java/org/opensearch/client/SyncedFlushResponseTests.java deleted file mode 100644 index e56e78d5d9caf..0000000000000 --- a/client/rest-high-level/src/test/java/org/opensearch/client/SyncedFlushResponseTests.java +++ /dev/null @@ -1,258 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -/* - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.client; - -import com.carrotsearch.hppc.ObjectIntHashMap; -import com.carrotsearch.hppc.ObjectIntMap; -import org.opensearch.cluster.routing.ShardRouting; -import org.opensearch.cluster.routing.ShardRoutingState; -import org.opensearch.cluster.routing.TestShardRouting; -import org.opensearch.common.bytes.BytesReference; -import org.opensearch.common.xcontent.DeprecationHandler; -import org.opensearch.common.xcontent.ToXContent; -import org.opensearch.common.xcontent.XContentBuilder; -import org.opensearch.common.xcontent.XContentParser; -import org.opensearch.common.xcontent.XContentType; -import org.opensearch.index.shard.ShardId; -import org.opensearch.indices.flush.ShardsSyncedFlushResult; -import org.opensearch.indices.flush.SyncedFlushService; -import org.opensearch.test.OpenSearchTestCase; - -import java.io.IOException; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; - -public class SyncedFlushResponseTests extends OpenSearchTestCase { - - public void testXContentSerialization() throws IOException { - final XContentType xContentType = randomFrom(XContentType.values()); - TestPlan plan = createTestPlan(); - - XContentBuilder serverResponsebuilder = XContentBuilder.builder(xContentType.xContent()); - assertNotNull(plan.result); - serverResponsebuilder.startObject(); - plan.result.toXContent(serverResponsebuilder, ToXContent.EMPTY_PARAMS); - serverResponsebuilder.endObject(); - XContentBuilder clientResponsebuilder = XContentBuilder.builder(xContentType.xContent()); - assertNotNull(plan.result); - plan.clientResult.toXContent(clientResponsebuilder, ToXContent.EMPTY_PARAMS); - Map serverContentMap = convertFailureListToSet( - serverResponsebuilder.generator() - .contentType() - .xContent() - .createParser( - xContentRegistry(), - DeprecationHandler.THROW_UNSUPPORTED_OPERATION, - BytesReference.bytes(serverResponsebuilder).streamInput() - ) - .map() - ); - Map clientContentMap = convertFailureListToSet( - clientResponsebuilder.generator() - .contentType() - .xContent() - .createParser( - xContentRegistry(), - DeprecationHandler.THROW_UNSUPPORTED_OPERATION, - BytesReference.bytes(clientResponsebuilder).streamInput() - ) - .map() - ); - assertEquals(serverContentMap, clientContentMap); - } - - public void testXContentDeserialization() throws IOException { - final XContentType xContentType = randomFrom(XContentType.values()); - TestPlan plan = createTestPlan(); - XContentBuilder builder = XContentBuilder.builder(xContentType.xContent()); - builder.startObject(); - plan.result.toXContent(builder, ToXContent.EMPTY_PARAMS); - builder.endObject(); - XContentParser parser = builder.generator() - .contentType() - .xContent() - .createParser(xContentRegistry(), DeprecationHandler.THROW_UNSUPPORTED_OPERATION, BytesReference.bytes(builder).streamInput()); - SyncedFlushResponse originalResponse = plan.clientResult; - SyncedFlushResponse parsedResponse = SyncedFlushResponse.fromXContent(parser); - assertNotNull(parsedResponse); - assertShardCounts(originalResponse.getShardCounts(), parsedResponse.getShardCounts()); - for (Map.Entry entry : originalResponse.getIndexResults().entrySet()) { - String index = entry.getKey(); - SyncedFlushResponse.IndexResult responseResult = entry.getValue(); - SyncedFlushResponse.IndexResult parsedResult = parsedResponse.getIndexResults().get(index); - assertNotNull(responseResult); - assertNotNull(parsedResult); - assertShardCounts(responseResult.getShardCounts(), parsedResult.getShardCounts()); - assertEquals(responseResult.failures().size(), parsedResult.failures().size()); - for (SyncedFlushResponse.ShardFailure responseShardFailure : responseResult.failures()) { - assertTrue(containsFailure(parsedResult.failures(), responseShardFailure)); - } - } - } - - static class TestPlan { - SyncedFlushResponse.ShardCounts totalCounts; - Map countsPerIndex = new HashMap<>(); - ObjectIntMap expectedFailuresPerIndex = new ObjectIntHashMap<>(); - org.opensearch.action.admin.indices.flush.SyncedFlushResponse result; - SyncedFlushResponse clientResult; - } - - TestPlan createTestPlan() throws IOException { - final TestPlan testPlan = new TestPlan(); - final Map> indicesResults = new HashMap<>(); - Map indexResults = new HashMap<>(); - final XContentType xContentType = randomFrom(XContentType.values()); - final int indexCount = randomIntBetween(1, 10); - int totalShards = 0; - int totalSuccessful = 0; - int totalFailed = 0; - for (int i = 0; i < indexCount; i++) { - final String index = "index_" + i; - int shards = randomIntBetween(1, 4); - int replicas = randomIntBetween(0, 2); - int successful = 0; - int failed = 0; - int failures = 0; - List shardsResults = new ArrayList<>(); - List shardFailures = new ArrayList<>(); - for (int shard = 0; shard < shards; shard++) { - final ShardId shardId = new ShardId(index, "_na_", shard); - if (randomInt(5) < 2) { - // total shard failure - failed += replicas + 1; - failures++; - shardsResults.add(new ShardsSyncedFlushResult(shardId, replicas + 1, "simulated total failure")); - shardFailures.add(new SyncedFlushResponse.ShardFailure(shardId.id(), "simulated total failure", new HashMap<>())); - } else { - Map shardResponses = new HashMap<>(); - for (int copy = 0; copy < replicas + 1; copy++) { - final ShardRouting shardRouting = TestShardRouting.newShardRouting( - index, - shard, - "node_" + shardId + "_" + copy, - null, - copy == 0, - ShardRoutingState.STARTED - ); - if (randomInt(5) < 2) { - // shard copy failure - failed++; - failures++; - shardResponses.put(shardRouting, new SyncedFlushService.ShardSyncedFlushResponse("copy failure " + shardId)); - // Building the shardRouting map here. - XContentBuilder builder = XContentBuilder.builder(xContentType.xContent()); - Map routing = shardRouting.toXContent(builder, ToXContent.EMPTY_PARAMS) - .generator() - .contentType() - .xContent() - .createParser( - xContentRegistry(), - DeprecationHandler.THROW_UNSUPPORTED_OPERATION, - BytesReference.bytes(builder).streamInput() - ) - .map(); - shardFailures.add(new SyncedFlushResponse.ShardFailure(shardId.id(), "copy failure " + shardId, routing)); - } else { - successful++; - shardResponses.put(shardRouting, new SyncedFlushService.ShardSyncedFlushResponse((String) null)); - } - } - shardsResults.add(new ShardsSyncedFlushResult(shardId, "_sync_id_" + shard, replicas + 1, shardResponses)); - } - } - indicesResults.put(index, shardsResults); - indexResults.put(index, new SyncedFlushResponse.IndexResult(shards * (replicas + 1), successful, failed, shardFailures)); - testPlan.countsPerIndex.put(index, new SyncedFlushResponse.ShardCounts(shards * (replicas + 1), successful, failed)); - testPlan.expectedFailuresPerIndex.put(index, failures); - totalFailed += failed; - totalShards += shards * (replicas + 1); - totalSuccessful += successful; - } - testPlan.result = new org.opensearch.action.admin.indices.flush.SyncedFlushResponse(indicesResults); - testPlan.totalCounts = new SyncedFlushResponse.ShardCounts(totalShards, totalSuccessful, totalFailed); - testPlan.clientResult = new SyncedFlushResponse( - new SyncedFlushResponse.ShardCounts(totalShards, totalSuccessful, totalFailed), - indexResults - ); - return testPlan; - } - - public boolean containsFailure(List failures, SyncedFlushResponse.ShardFailure origFailure) { - for (SyncedFlushResponse.ShardFailure failure : failures) { - if (failure.getShardId() == origFailure.getShardId() - && failure.getFailureReason().equals(origFailure.getFailureReason()) - && failure.getRouting().equals(origFailure.getRouting())) { - return true; - } - } - return false; - } - - public void assertShardCounts(SyncedFlushResponse.ShardCounts first, SyncedFlushResponse.ShardCounts second) { - if (first == null) { - assertNull(second); - } else { - assertTrue(first.equals(second)); - } - } - - public Map convertFailureListToSet(Map input) { - Map retMap = new HashMap<>(); - for (Map.Entry entry : input.entrySet()) { - if (entry.getKey().equals(SyncedFlushResponse.SHARDS_FIELD)) { - retMap.put(entry.getKey(), entry.getValue()); - } else { - // This was an index entry. - @SuppressWarnings("unchecked") - Map indexResult = (Map) entry.getValue(); - Map retResult = new HashMap<>(); - for (Map.Entry entry2 : indexResult.entrySet()) { - if (entry2.getKey().equals(SyncedFlushResponse.IndexResult.FAILURES_FIELD)) { - @SuppressWarnings("unchecked") - List failures = (List) entry2.getValue(); - Set retSet = new HashSet<>(failures); - retResult.put(entry.getKey(), retSet); - } else { - retResult.put(entry2.getKey(), entry2.getValue()); - } - } - retMap.put(entry.getKey(), retResult); - } - } - return retMap; - } -} diff --git a/client/rest-high-level/src/test/java/org/opensearch/client/documentation/IndicesClientDocumentationIT.java b/client/rest-high-level/src/test/java/org/opensearch/client/documentation/IndicesClientDocumentationIT.java index ad2b0d1e603bb..3fbe7f63b09a2 100644 --- a/client/rest-high-level/src/test/java/org/opensearch/client/documentation/IndicesClientDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/opensearch/client/documentation/IndicesClientDocumentationIT.java @@ -44,7 +44,6 @@ import org.opensearch.action.admin.indices.delete.DeleteIndexRequest; import org.opensearch.action.admin.indices.flush.FlushRequest; import org.opensearch.action.admin.indices.flush.FlushResponse; -import org.opensearch.action.admin.indices.flush.SyncedFlushRequest; import org.opensearch.action.admin.indices.forcemerge.ForceMergeRequest; import org.opensearch.action.admin.indices.forcemerge.ForceMergeResponse; import org.opensearch.action.admin.indices.open.OpenIndexRequest; @@ -69,7 +68,6 @@ import org.opensearch.client.GetAliasesResponse; import org.opensearch.client.RequestOptions; import org.opensearch.client.RestHighLevelClient; -import org.opensearch.client.SyncedFlushResponse; import org.opensearch.client.indices.AnalyzeRequest; import org.opensearch.client.indices.AnalyzeResponse; import org.opensearch.client.indices.CloseIndexRequest; @@ -1012,94 +1010,6 @@ public void onFailure(Exception e) { } } - @SuppressWarnings("unused") - public void testSyncedFlushIndex() throws Exception { - RestHighLevelClient client = highLevelClient(); - - { - createIndex("index1", Settings.EMPTY); - } - - { - // tag::flush-synced-request - SyncedFlushRequest request = new SyncedFlushRequest("index1"); // <1> - SyncedFlushRequest requestMultiple = new SyncedFlushRequest("index1", "index2"); // <2> - SyncedFlushRequest requestAll = new SyncedFlushRequest(); // <3> - // end::flush-synced-request - - // tag::flush-synced-request-indicesOptions - request.indicesOptions(IndicesOptions.lenientExpandOpen()); // <1> - // end::flush-synced-request-indicesOptions - - // tag::flush-synced-execute - 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 - int totalShards = flushSyncedResponse.totalShards(); // <1> - int successfulShards = flushSyncedResponse.successfulShards(); // <2> - int failedShards = flushSyncedResponse.failedShards(); // <3> - - for (Map.Entry responsePerIndexEntry: - flushSyncedResponse.getIndexResults().entrySet()) { - String indexName = responsePerIndexEntry.getKey(); // <4> - SyncedFlushResponse.IndexResult indexResult = responsePerIndexEntry.getValue(); - int totalShardsForIndex = indexResult.totalShards(); // <5> - int successfulShardsForIndex = indexResult.successfulShards(); // <6> - int failedShardsForIndex = indexResult.failedShards(); // <7> - if (failedShardsForIndex > 0) { - for (SyncedFlushResponse.ShardFailure failureEntry: indexResult.failures()) { - int shardId = failureEntry.getShardId(); // <8> - String failureReason = failureEntry.getFailureReason(); // <9> - Map routing = failureEntry.getRouting(); // <10> - } - } - } - // end::flush-synced-response - - // tag::flush-synced-execute-listener - ActionListener listener = new ActionListener() { - @Override - public void onResponse(SyncedFlushResponse refreshResponse) { - // <1> - } - - @Override - public void onFailure(Exception e) { - // <2> - } - }; - // end::flush-synced-execute-listener - - // Replace the empty listener by a blocking listener in test - final CountDownLatch latch = new CountDownLatch(1); - listener = new LatchedActionListener<>(listener, latch); - - // tag::flush-synced-execute-async - 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)); - } - - { - // tag::flush-synced-notfound - try { - SyncedFlushRequest request = new SyncedFlushRequest("does_not_exist"); - client.indices().flushSynced(request, RequestOptions.DEFAULT); - } catch (OpenSearchException exception) { - if (exception.status() == RestStatus.NOT_FOUND) { - // <1> - } - } - // end::flush-synced-notfound - } - } - public void testGetSettings() throws Exception { RestHighLevelClient client = highLevelClient(); diff --git a/qa/full-cluster-restart/src/test/java/org/opensearch/upgrades/FullClusterRestartIT.java b/qa/full-cluster-restart/src/test/java/org/opensearch/upgrades/FullClusterRestartIT.java index 8f1ff3002362b..b2e0130489880 100644 --- a/qa/full-cluster-restart/src/test/java/org/opensearch/upgrades/FullClusterRestartIT.java +++ b/qa/full-cluster-restart/src/test/java/org/opensearch/upgrades/FullClusterRestartIT.java @@ -737,7 +737,7 @@ public void testRecovery() throws Exception { assertOK(client().performRequest(flushRequest)); if (randomBoolean()) { - performSyncedFlush(index, randomBoolean()); + syncedFlush(index, randomBoolean()); } if (shouldHaveTranslog) { // Update a few documents so we are sure to have a translog @@ -1429,7 +1429,7 @@ public void testRecoveryWithTranslogRetentionDisabled() throws Exception { if (randomBoolean()) { flush(index, randomBoolean()); } else if (randomBoolean()) { - performSyncedFlush(index, randomBoolean()); + syncedFlush(index, randomBoolean()); } saveInfoDocument("doc_count", Integer.toString(numDocs)); } diff --git a/qa/mixed-cluster/src/test/java/org/opensearch/backwards/IndexingIT.java b/qa/mixed-cluster/src/test/java/org/opensearch/backwards/IndexingIT.java index 8e21998b50525..64f3114f1d891 100644 --- a/qa/mixed-cluster/src/test/java/org/opensearch/backwards/IndexingIT.java +++ b/qa/mixed-cluster/src/test/java/org/opensearch/backwards/IndexingIT.java @@ -35,13 +35,17 @@ import org.opensearch.LegacyESVersion; import org.opensearch.Version; import org.opensearch.client.Request; +import org.opensearch.client.RequestOptions; import org.opensearch.client.Response; +import org.opensearch.client.ResponseException; import org.opensearch.client.RestClient; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.common.Strings; import org.opensearch.common.settings.Settings; import org.opensearch.common.xcontent.json.JsonXContent; +import org.opensearch.common.xcontent.support.XContentMapValues; import org.opensearch.index.seqno.SeqNoStats; +import org.opensearch.rest.RestStatus; import org.opensearch.rest.action.document.RestGetAction; import org.opensearch.rest.action.document.RestIndexAction; import org.opensearch.test.rest.OpenSearchRestTestCase; @@ -49,11 +53,13 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.stream.Collectors; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.equalTo; public class IndexingIT extends OpenSearchRestTestCase { @@ -295,6 +301,59 @@ public void testUpdateSnapshotStatus() throws Exception { request.setJsonEntity("{\"indices\": \"" + index + "\"}"); } + public void testSyncedFlushTransition() throws Exception { + Nodes nodes = buildNodeAndVersions(); + assumeTrue("bwc version is on 1.x or Legacy 7.x", nodes.getBWCVersion().before(Version.V_2_0_0)); + assumeFalse("no new node found", nodes.getNewNodes().isEmpty()); + assumeFalse("no bwc node found", nodes.getBWCNodes().isEmpty()); + // Allocate shards to new nodes then verify synced flush requests processed by old nodes/new nodes + String newNodes = nodes.getNewNodes().stream().map(Node::getNodeName).collect(Collectors.joining(",")); + int numShards = randomIntBetween(1, 10); + int numOfReplicas = randomIntBetween(0, nodes.getNewNodes().size() - 1); + int totalShards = numShards * (numOfReplicas + 1); + final String index = "test_synced_flush"; + createIndex(index, Settings.builder() + .put(IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), numShards) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numOfReplicas) + .put("index.routing.allocation.include._name", newNodes).build()); + ensureGreen(index); + indexDocs(index, randomIntBetween(0, 100), between(1, 100)); + try (RestClient oldNodeClient = buildClient(restClientSettings(), + nodes.getBWCNodes().stream().map(Node::getPublishAddress).toArray(HttpHost[]::new))) { + Request request = new Request("POST", index + "/_flush/synced"); + assertBusy(() -> { + ResponseException responseException = expectThrows(ResponseException.class, () -> oldNodeClient.performRequest(request)); + assertThat(responseException.getResponse().getStatusLine().getStatusCode(), equalTo(RestStatus.CONFLICT.getStatus())); + assertThat(responseException.getResponse().getWarnings(), + contains("Synced flush is deprecated and will be removed in 8.0. Use flush at _/flush or /{index}/_flush instead.")); + Map result = ObjectPath.createFromResponse(responseException.getResponse()).evaluate("_shards"); + assertThat(result.get("total"), equalTo(totalShards)); + assertThat(result.get("successful"), equalTo(0)); + assertThat(result.get("failed"), equalTo(totalShards)); + }); + Map stats = entityAsMap(client().performRequest(new Request("GET", index + "/_stats?level=shards"))); + assertThat(XContentMapValues.extractValue("indices." + index + ".total.translog.uncommitted_operations", stats), equalTo(0)); + } + indexDocs(index, randomIntBetween(0, 100), between(1, 100)); + try (RestClient newNodeClient = buildClient(restClientSettings(), + nodes.getNewNodes().stream().map(Node::getPublishAddress).toArray(HttpHost[]::new))) { + Request request = new Request("POST", index + "/_flush/synced"); + List warningMsg = Arrays.asList("Synced flush was removed and a normal flush was performed instead. " + + "This transition will be removed in a future version."); + RequestOptions.Builder requestOptionsBuilder = RequestOptions.DEFAULT.toBuilder(); + requestOptionsBuilder.setWarningsHandler(warnings -> warnings.equals(warningMsg) == false); + request.setOptions(requestOptionsBuilder); + assertBusy(() -> { + Map result = ObjectPath.createFromResponse(newNodeClient.performRequest(request)).evaluate("_shards"); + assertThat(result.get("total"), equalTo(totalShards)); + assertThat(result.get("successful"), equalTo(totalShards)); + assertThat(result.get("failed"), equalTo(0)); + }); + Map stats = entityAsMap(client().performRequest(new Request("GET", index + "/_stats?level=shards"))); + assertThat(XContentMapValues.extractValue("indices." + index + ".total.translog.uncommitted_operations", stats), equalTo(0)); + } + } + private void assertCount(final String index, final String preference, final int expectedCount) throws IOException { Request request = new Request("GET", index + "/_count"); request.addParameter("preference", preference); diff --git a/qa/rolling-upgrade/src/test/java/org/opensearch/upgrades/RecoveryIT.java b/qa/rolling-upgrade/src/test/java/org/opensearch/upgrades/RecoveryIT.java index 0e0dedf00b929..5870ec171ae30 100644 --- a/qa/rolling-upgrade/src/test/java/org/opensearch/upgrades/RecoveryIT.java +++ b/qa/rolling-upgrade/src/test/java/org/opensearch/upgrades/RecoveryIT.java @@ -322,13 +322,13 @@ public void testRelocationWithConcurrentIndexing() throws Exception { throw new IllegalStateException("unknown type " + CLUSTER_TYPE); } if (randomBoolean()) { - performSyncedFlush(index, randomBoolean()); + syncedFlush(index, randomBoolean()); ensureGlobalCheckpointSynced(index); } } public void testRecovery() throws Exception { - final String index = "recover_with_soft_deletes"; + final String index = "test_recovery"; if (CLUSTER_TYPE == ClusterType.OLD) { Settings.Builder settings = Settings.builder() .put(IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), 1) @@ -360,6 +360,9 @@ public void testRecovery() throws Exception { } } } + if (randomBoolean()) { + syncedFlush(index, randomBoolean()); + } ensureGreen(index); } @@ -671,7 +674,7 @@ public void testUpdateDoc() throws Exception { assertThat(XContentMapValues.extractValue("_source.updated_field", doc), equalTo(updates.get(docId))); } if (randomBoolean()) { - performSyncedFlush(index, randomBoolean()); + syncedFlush(index, randomBoolean()); ensureGlobalCheckpointSynced(index); } } diff --git a/qa/translog-policy/src/test/java/org/opensearch/upgrades/TranslogPolicyIT.java b/qa/translog-policy/src/test/java/org/opensearch/upgrades/TranslogPolicyIT.java index 72400a5705162..5ae9944429d21 100644 --- a/qa/translog-policy/src/test/java/org/opensearch/upgrades/TranslogPolicyIT.java +++ b/qa/translog-policy/src/test/java/org/opensearch/upgrades/TranslogPolicyIT.java @@ -141,7 +141,7 @@ public void testRecoverReplica() throws Exception { if (randomBoolean()) { flush(index, randomBoolean()); } else if (randomBoolean()) { - performSyncedFlush(index, randomBoolean()); + syncedFlush(index, randomBoolean()); } } ensureGreen(index); 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 deleted file mode 100644 index 134ba93395b40..0000000000000 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/indices.flush_synced.json +++ /dev/null @@ -1,62 +0,0 @@ -{ - "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. Synced flush is deprecated and will be removed in 8.0. Use flush instead" - }, - "stability":"stable", - "url":{ - "paths":[ - { - "path":"/_flush/synced", - "methods":[ - "POST", - "GET" - ], - "deprecated":{ - "version":"7.6.0", - "description":"Synced flush is deprecated and will be removed in 8.0. Use flush instead." - } - }, - { - "path":"/{index}/_flush/synced", - "methods":[ - "POST", - "GET" - ], - "parts":{ - "index":{ - "type":"list", - "description":"A comma-separated list of index names; use `_all` or empty string for all indices" - } - }, - "deprecated":{ - "version":"7.6.0", - "description":"Synced flush is deprecated and will be removed in 8.0. Use flush instead." - } - } - ] - }, - "params":{ - "ignore_unavailable":{ - "type":"boolean", - "description":"Whether specified concrete indices should be ignored when unavailable (missing or closed)" - }, - "allow_no_indices":{ - "type":"boolean", - "description":"Whether to ignore if a wildcard indices expression resolves into no concrete indices. (This includes `_all` string or when no indices have been specified)" - }, - "expand_wildcards":{ - "type":"enum", - "options":[ - "open", - "closed", - "none", - "all" - ], - "default":"open", - "description":"Whether to expand wildcard expression to concrete indices that are open, closed or both." - } - } - } -} 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 781d133153605..4ac82ec60e738 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,33 +1,3 @@ ---- -"Index synced flush rest test": - - skip: - version: " - 7.5.99" - reason: "synced flush is deprecated in 7.6" - features: "allowed_warnings" - - do: - indices.create: - index: testing - body: - settings: - index: - number_of_replicas: 0 - - - do: - cluster.health: - wait_for_status: green - - do: - allowed_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 - - - do: - indices.stats: {level: shards} - - - is_true: indices.testing.shards.0.0.commit.user_data.sync_id - --- "Flush stats": - skip: diff --git a/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/create/ShrinkIndexIT.java b/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/create/ShrinkIndexIT.java index d87bbbb0926c5..a53c28d170a93 100644 --- a/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/create/ShrinkIndexIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/create/ShrinkIndexIT.java @@ -667,7 +667,7 @@ public void testShrinkCommitsMergeOnIdle() throws Exception { IndexService indexShards = service.indexService(target.getIndex()); IndexShard shard = indexShards.getShard(0); assertTrue(shard.isActive()); - shard.checkIdle(0); + shard.flushOnIdle(0); assertFalse(shard.isActive()); } } diff --git a/server/src/internalClusterTest/java/org/opensearch/gateway/ReplicaShardAllocatorIT.java b/server/src/internalClusterTest/java/org/opensearch/gateway/ReplicaShardAllocatorIT.java index 178fe49b1f0e7..e9414fd651ca0 100644 --- a/server/src/internalClusterTest/java/org/opensearch/gateway/ReplicaShardAllocatorIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/gateway/ReplicaShardAllocatorIT.java @@ -33,7 +33,6 @@ package org.opensearch.gateway; import org.opensearch.LegacyESVersion; -import org.opensearch.action.admin.indices.flush.SyncedFlushResponse; import org.opensearch.action.admin.indices.stats.ShardStats; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.node.DiscoveryNode; @@ -196,10 +195,6 @@ public void testRecentPrimaryInformation() throws Exception { .mapToObj(n -> client().prepareIndex(indexName, "_doc").setSource("f", "v")) .collect(Collectors.toList()) ); - assertBusy(() -> { - SyncedFlushResponse syncedFlushResponse = client().admin().indices().prepareSyncedFlush(indexName).get(); - assertThat(syncedFlushResponse.successfulShards(), equalTo(2)); - }); internalCluster().stopRandomNode(InternalTestCluster.nameFilter(nodeWithReplica)); if (randomBoolean()) { indexRandom( @@ -548,12 +543,6 @@ public void testSimulateRecoverySourceOnOldNode() throws Exception { if (randomBoolean()) { client().admin().indices().prepareFlush(indexName).get(); } - if (randomBoolean()) { - assertBusy(() -> { - SyncedFlushResponse syncedFlushResponse = client().admin().indices().prepareSyncedFlush(indexName).get(); - assertThat(syncedFlushResponse.successfulShards(), equalTo(1)); - }); - } internalCluster().startDataOnlyNode(); MockTransportService transportService = (MockTransportService) internalCluster().getInstance(TransportService.class, source); Semaphore failRecovery = new Semaphore(1); @@ -587,10 +576,11 @@ public void testSimulateRecoverySourceOnOldNode() throws Exception { transportService.clearAllRules(); } - private void ensureActivePeerRecoveryRetentionLeasesAdvanced(String indexName) throws Exception { + public static void ensureActivePeerRecoveryRetentionLeasesAdvanced(String indexName) throws Exception { + final ClusterService clusterService = internalCluster().clusterService(); assertBusy(() -> { Index index = resolveIndex(indexName); - Set activeRetentionLeaseIds = clusterService().state() + Set activeRetentionLeaseIds = clusterService.state() .routingTable() .index(index) .shard(0) diff --git a/server/src/internalClusterTest/java/org/opensearch/index/shard/IndexShardIT.java b/server/src/internalClusterTest/java/org/opensearch/index/shard/IndexShardIT.java index 59e719c3e16a8..ac45940b9913c 100644 --- a/server/src/internalClusterTest/java/org/opensearch/index/shard/IndexShardIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/index/shard/IndexShardIT.java @@ -38,7 +38,6 @@ import org.opensearch.action.ActionListener; import org.opensearch.action.admin.cluster.node.stats.NodeStats; import org.opensearch.action.admin.cluster.node.stats.NodesStatsResponse; -import org.opensearch.action.admin.indices.stats.IndexStats; import org.opensearch.action.index.IndexRequest; import org.opensearch.action.search.SearchRequest; import org.opensearch.action.search.SearchResponse; @@ -179,24 +178,6 @@ public void testLockTryingToDelete() throws Exception { } } - public void testMarkAsInactiveTriggersSyncedFlush() throws Exception { - assertAcked( - client().admin() - .indices() - .prepareCreate("test") - .setSettings(Settings.builder().put(SETTING_NUMBER_OF_SHARDS, 1).put(SETTING_NUMBER_OF_REPLICAS, 0)) - ); - client().prepareIndex("test", "test").setSource("{}", XContentType.JSON).get(); - ensureGreen("test"); - IndicesService indicesService = getInstanceFromNode(IndicesService.class); - indicesService.indexService(resolveIndex("test")).getShardOrNull(0).checkIdle(0); - assertBusy(() -> { - IndexStats indexStats = client().admin().indices().prepareStats("test").clear().setTranslog(true).get().getIndex("test"); - assertThat(indexStats.getTotal().translog.getUncommittedOperations(), equalTo(0)); - indicesService.indexService(resolveIndex("test")).getShardOrNull(0).checkIdle(0); - }); - } - public void testDurableFlagHasEffect() throws Exception { createIndex("test"); ensureGreen(); diff --git a/server/src/internalClusterTest/java/org/opensearch/indices/flush/FlushIT.java b/server/src/internalClusterTest/java/org/opensearch/indices/flush/FlushIT.java deleted file mode 100644 index 277e83fa51379..0000000000000 --- a/server/src/internalClusterTest/java/org/opensearch/indices/flush/FlushIT.java +++ /dev/null @@ -1,495 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -/* - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.indices.flush; - -import org.apache.lucene.index.Term; -import org.opensearch.action.ActionListener; -import org.opensearch.action.admin.indices.flush.FlushRequest; -import org.opensearch.action.admin.indices.flush.FlushResponse; -import org.opensearch.action.admin.indices.flush.SyncedFlushResponse; -import org.opensearch.action.admin.indices.stats.IndexStats; -import org.opensearch.action.admin.indices.stats.ShardStats; -import org.opensearch.action.support.ActiveShardCount; -import org.opensearch.cluster.ClusterState; -import org.opensearch.cluster.metadata.IndexMetadata; -import org.opensearch.cluster.routing.ShardRouting; -import org.opensearch.cluster.routing.allocation.command.MoveAllocationCommand; -import org.opensearch.common.UUIDs; -import org.opensearch.common.ValidationException; -import org.opensearch.common.settings.Settings; -import org.opensearch.common.unit.ByteSizeUnit; -import org.opensearch.common.unit.ByteSizeValue; -import org.opensearch.common.xcontent.XContentType; -import org.opensearch.index.Index; -import org.opensearch.index.IndexService; -import org.opensearch.index.IndexSettings; -import org.opensearch.index.engine.Engine; -import org.opensearch.index.engine.InternalEngine; -import org.opensearch.index.engine.InternalEngineTests; -import org.opensearch.index.mapper.ParsedDocument; -import org.opensearch.index.mapper.Uid; -import org.opensearch.index.seqno.SequenceNumbers; -import org.opensearch.index.shard.IndexShard; -import org.opensearch.index.shard.IndexShardTestCase; -import org.opensearch.index.shard.ShardId; -import org.opensearch.indices.IndexingMemoryController; -import org.opensearch.indices.IndicesService; -import org.opensearch.plugins.Plugin; -import org.opensearch.test.OpenSearchIntegTestCase; -import org.opensearch.test.InternalSettingsPlugin; -import org.opensearch.test.InternalTestCluster; - -import java.io.IOException; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.stream.Collectors; - -import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.emptyArray; -import static org.hamcrest.Matchers.emptyIterable; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.not; -import static org.hamcrest.Matchers.nullValue; - -@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST) -public class FlushIT extends OpenSearchIntegTestCase { - - @Override - protected Collection> nodePlugins() { - return Collections.singletonList(InternalSettingsPlugin.class); - } - - public void testWaitIfOngoing() throws InterruptedException { - createIndex("test"); - ensureGreen("test"); - final int numIters = scaledRandomIntBetween(10, 30); - for (int i = 0; i < numIters; i++) { - for (int j = 0; j < 10; j++) { - client().prepareIndex("test", "test").setSource("{}", XContentType.JSON).get(); - } - final CountDownLatch latch = new CountDownLatch(10); - final CopyOnWriteArrayList errors = new CopyOnWriteArrayList<>(); - for (int j = 0; j < 10; j++) { - client().admin().indices().prepareFlush("test").execute(new ActionListener() { - @Override - public void onResponse(FlushResponse flushResponse) { - try { - // don't use assertAllSuccessful it uses a randomized context that belongs to a different thread - assertThat( - "Unexpected ShardFailures: " + Arrays.toString(flushResponse.getShardFailures()), - flushResponse.getFailedShards(), - equalTo(0) - ); - latch.countDown(); - } catch (Exception ex) { - onFailure(ex); - } - - } - - @Override - public void onFailure(Exception e) { - errors.add(e); - latch.countDown(); - } - }); - } - latch.await(); - assertThat(errors, emptyIterable()); - } - } - - public void testRejectIllegalFlushParameters() { - createIndex("test"); - int numDocs = randomIntBetween(0, 10); - for (int i = 0; i < numDocs; i++) { - client().prepareIndex("test", "_doc").setSource("{}", XContentType.JSON).get(); - } - assertThat( - expectThrows( - ValidationException.class, - () -> client().admin().indices().flush(new FlushRequest().force(true).waitIfOngoing(false)).actionGet() - ).getMessage(), - containsString("wait_if_ongoing must be true for a force flush") - ); - assertThat( - client().admin().indices().flush(new FlushRequest().force(true).waitIfOngoing(true)).actionGet().getShardFailures(), - emptyArray() - ); - assertThat( - client().admin().indices().flush(new FlushRequest().force(false).waitIfOngoing(randomBoolean())).actionGet().getShardFailures(), - emptyArray() - ); - } - - public void testSyncedFlush() throws Exception { - internalCluster().ensureAtLeastNumDataNodes(2); - prepareCreate("test").setSettings(Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)).get(); - ensureGreen(); - - final Index index = client().admin().cluster().prepareState().get().getState().metadata().index("test").getIndex(); - - IndexStats indexStats = client().admin().indices().prepareStats("test").get().getIndex("test"); - for (ShardStats shardStats : indexStats.getShards()) { - assertNull(shardStats.getCommitStats().getUserData().get(Engine.SYNC_COMMIT_ID)); - } - - ShardsSyncedFlushResult result; - if (randomBoolean()) { - logger.info("--> sync flushing shard 0"); - result = SyncedFlushUtil.attemptSyncedFlush(logger, internalCluster(), new ShardId(index, 0)); - } else { - logger.info("--> sync flushing index [test]"); - SyncedFlushResponse indicesResult = client().admin().indices().prepareSyncedFlush("test").get(); - result = indicesResult.getShardsResultPerIndex().get("test").get(0); - } - assertFalse(result.failed()); - assertThat(result.totalShards(), equalTo(indexStats.getShards().length)); - assertThat(result.successfulShards(), equalTo(indexStats.getShards().length)); - - indexStats = client().admin().indices().prepareStats("test").get().getIndex("test"); - String syncId = result.syncId(); - for (ShardStats shardStats : indexStats.getShards()) { - final String shardSyncId = shardStats.getCommitStats().getUserData().get(Engine.SYNC_COMMIT_ID); - assertThat(shardSyncId, equalTo(syncId)); - } - - // now, start new node and relocate a shard there and see if sync id still there - String newNodeName = internalCluster().startNode(); - ClusterState clusterState = client().admin().cluster().prepareState().get().getState(); - ShardRouting shardRouting = clusterState.getRoutingTable().index("test").shard(0).iterator().next(); - String currentNodeName = clusterState.nodes().resolveNode(shardRouting.currentNodeId()).getName(); - assertFalse(currentNodeName.equals(newNodeName)); - internalCluster().client() - .admin() - .cluster() - .prepareReroute() - .add(new MoveAllocationCommand("test", 0, currentNodeName, newNodeName)) - .get(); - - client().admin().cluster().prepareHealth().setWaitForNoRelocatingShards(true).get(); - indexStats = client().admin().indices().prepareStats("test").get().getIndex("test"); - for (ShardStats shardStats : indexStats.getShards()) { - assertNotNull(shardStats.getCommitStats().getUserData().get(Engine.SYNC_COMMIT_ID)); - } - - client().admin() - .indices() - .prepareUpdateSettings("test") - .setSettings(Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0).build()) - .get(); - ensureGreen("test"); - indexStats = client().admin().indices().prepareStats("test").get().getIndex("test"); - for (ShardStats shardStats : indexStats.getShards()) { - assertNotNull(shardStats.getCommitStats().getUserData().get(Engine.SYNC_COMMIT_ID)); - } - client().admin() - .indices() - .prepareUpdateSettings("test") - .setSettings(Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, internalCluster().numDataNodes() - 1).build()) - .get(); - ensureGreen("test"); - indexStats = client().admin().indices().prepareStats("test").get().getIndex("test"); - for (ShardStats shardStats : indexStats.getShards()) { - assertNotNull(shardStats.getCommitStats().getUserData().get(Engine.SYNC_COMMIT_ID)); - } - } - - public void testSyncedFlushWithConcurrentIndexing() throws Exception { - - internalCluster().ensureAtLeastNumDataNodes(3); - createIndex("test"); - - client().admin() - .indices() - .prepareUpdateSettings("test") - .setSettings( - Settings.builder() - .put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), new ByteSizeValue(1, ByteSizeUnit.PB)) - .put("index.refresh_interval", -1) - .put("index.number_of_replicas", internalCluster().numDataNodes() - 1) - ) - .get(); - ensureGreen(); - final AtomicBoolean stop = new AtomicBoolean(false); - final AtomicInteger numDocs = new AtomicInteger(0); - Thread indexingThread = new Thread() { - @Override - public void run() { - while (stop.get() == false) { - client().prepareIndex().setIndex("test").setType("_doc").setSource("{}", XContentType.JSON).get(); - numDocs.incrementAndGet(); - } - } - }; - indexingThread.start(); - - IndexStats indexStats = client().admin().indices().prepareStats("test").get().getIndex("test"); - for (ShardStats shardStats : indexStats.getShards()) { - assertNull(shardStats.getCommitStats().getUserData().get(Engine.SYNC_COMMIT_ID)); - } - logger.info("--> trying sync flush"); - SyncedFlushResponse syncedFlushResult = client().admin().indices().prepareSyncedFlush("test").get(); - logger.info("--> sync flush done"); - stop.set(true); - indexingThread.join(); - indexStats = client().admin().indices().prepareStats("test").get().getIndex("test"); - assertFlushResponseEqualsShardStats(indexStats.getShards(), syncedFlushResult.getShardsResultPerIndex().get("test")); - refresh(); - assertThat(client().prepareSearch().setSize(0).get().getHits().getTotalHits().value, equalTo((long) numDocs.get())); - logger.info("indexed {} docs", client().prepareSearch().setSize(0).get().getHits().getTotalHits().value); - logClusterState(); - internalCluster().fullRestart(); - ensureGreen(); - assertThat(client().prepareSearch().setSize(0).get().getHits().getTotalHits().value, equalTo((long) numDocs.get())); - } - - private void assertFlushResponseEqualsShardStats(ShardStats[] shardsStats, List syncedFlushResults) { - - for (final ShardStats shardStats : shardsStats) { - for (final ShardsSyncedFlushResult shardResult : syncedFlushResults) { - if (shardStats.getShardRouting().getId() == shardResult.shardId().getId()) { - for (Map.Entry singleResponse : shardResult.shardResponses() - .entrySet()) { - if (singleResponse.getKey().currentNodeId().equals(shardStats.getShardRouting().currentNodeId())) { - if (singleResponse.getValue().success()) { - logger.info( - "{} sync flushed on node {}", - singleResponse.getKey().shardId(), - singleResponse.getKey().currentNodeId() - ); - assertNotNull(shardStats.getCommitStats().getUserData().get(Engine.SYNC_COMMIT_ID)); - } else { - logger.info( - "{} sync flush failed for on node {}", - singleResponse.getKey().shardId(), - singleResponse.getKey().currentNodeId() - ); - assertNull(shardStats.getCommitStats().getUserData().get(Engine.SYNC_COMMIT_ID)); - } - } - } - } - } - } - } - - public void testUnallocatedShardsDoesNotHang() throws InterruptedException { - // create an index but disallow allocation - prepareCreate("test").setWaitForActiveShards(ActiveShardCount.NONE) - .setSettings(Settings.builder().put("index.routing.allocation.include._name", "nonexistent")) - .get(); - - // this should not hang but instead immediately return with empty result set - List shardsResult = client().admin() - .indices() - .prepareSyncedFlush("test") - .get() - .getShardsResultPerIndex() - .get("test"); - // just to make sure the test actually tests the right thing - int numShards = client().admin() - .indices() - .prepareGetSettings("test") - .get() - .getIndexToSettings() - .get("test") - .getAsInt(IndexMetadata.SETTING_NUMBER_OF_SHARDS, -1); - assertThat(shardsResult.size(), equalTo(numShards)); - assertThat(shardsResult.get(0).failureReason(), equalTo("no active shards")); - } - - private void indexDoc(Engine engine, String id) throws IOException { - final ParsedDocument doc = InternalEngineTests.createParsedDoc(id, null); - final Engine.IndexResult indexResult = engine.index( - new Engine.Index( - new Term("_id", Uid.encodeId(doc.id())), - doc, - ((InternalEngine) engine).getProcessedLocalCheckpoint() + 1, - 1L, - 1L, - null, - Engine.Operation.Origin.REPLICA, - System.nanoTime(), - -1L, - false, - SequenceNumbers.UNASSIGNED_SEQ_NO, - 0 - ) - ); - assertThat(indexResult.getFailure(), nullValue()); - engine.syncTranslog(); - } - - public void testSyncedFlushSkipOutOfSyncReplicas() throws Exception { - internalCluster().ensureAtLeastNumDataNodes(between(2, 3)); - final int numberOfReplicas = internalCluster().numDataNodes() - 1; - assertAcked( - prepareCreate("test").setSettings( - Settings.builder() - .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) - .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numberOfReplicas) - ).get() - ); - ensureGreen(); - final Index index = clusterService().state().metadata().index("test").getIndex(); - final ShardId shardId = new ShardId(index, 0); - final int numDocs = between(1, 10); - for (int i = 0; i < numDocs; i++) { - index("test", "doc", Integer.toString(i)); - } - final List indexShards = internalCluster().nodesInclude("test") - .stream() - .map(node -> internalCluster().getInstance(IndicesService.class, node).getShardOrNull(shardId)) - .collect(Collectors.toList()); - // Index extra documents to one replica - synced-flush should fail on that replica. - final IndexShard outOfSyncReplica = randomValueOtherThanMany(s -> s.routingEntry().primary(), () -> randomFrom(indexShards)); - final int extraDocs = between(1, 10); - for (int i = 0; i < extraDocs; i++) { - indexDoc(IndexShardTestCase.getEngine(outOfSyncReplica), "extra_" + i); - } - final ShardsSyncedFlushResult partialResult = SyncedFlushUtil.attemptSyncedFlush(logger, internalCluster(), shardId); - assertThat(partialResult.totalShards(), equalTo(numberOfReplicas + 1)); - assertThat(partialResult.successfulShards(), equalTo(numberOfReplicas)); - assertThat( - partialResult.shardResponses().get(outOfSyncReplica.routingEntry()).failureReason, - equalTo( - "ongoing indexing operations: num docs on replica [" + (numDocs + extraDocs) + "]; num docs on primary [" + numDocs + "]" - ) - ); - // Index extra documents to all shards - synced-flush should be ok. - for (IndexShard indexShard : indexShards) { - // Do reindex documents to the out of sync replica to avoid trigger merges - if (indexShard != outOfSyncReplica) { - for (int i = 0; i < extraDocs; i++) { - indexDoc(IndexShardTestCase.getEngine(indexShard), "extra_" + i); - } - } - } - final ShardsSyncedFlushResult fullResult = SyncedFlushUtil.attemptSyncedFlush(logger, internalCluster(), shardId); - assertThat(fullResult.totalShards(), equalTo(numberOfReplicas + 1)); - assertThat(fullResult.successfulShards(), equalTo(numberOfReplicas + 1)); - } - - public void testDoNotRenewSyncedFlushWhenAllSealed() throws Exception { - internalCluster().ensureAtLeastNumDataNodes(between(2, 3)); - final int numberOfReplicas = internalCluster().numDataNodes() - 1; - assertAcked( - prepareCreate("test").setSettings( - Settings.builder() - .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) - .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numberOfReplicas) - ).get() - ); - ensureGreen(); - final Index index = clusterService().state().metadata().index("test").getIndex(); - final ShardId shardId = new ShardId(index, 0); - final int numDocs = between(1, 10); - for (int i = 0; i < numDocs; i++) { - index("test", "doc", Integer.toString(i)); - } - final ShardsSyncedFlushResult firstSeal = SyncedFlushUtil.attemptSyncedFlush(logger, internalCluster(), shardId); - assertThat(firstSeal.successfulShards(), equalTo(numberOfReplicas + 1)); - // Do not renew synced-flush - final ShardsSyncedFlushResult secondSeal = SyncedFlushUtil.attemptSyncedFlush(logger, internalCluster(), shardId); - assertThat(secondSeal.successfulShards(), equalTo(numberOfReplicas + 1)); - assertThat(secondSeal.syncId(), equalTo(firstSeal.syncId())); - // Shards were updated, renew synced flush. - final int moreDocs = between(1, 10); - for (int i = 0; i < moreDocs; i++) { - index("test", "doc", "more-" + i); - } - final ShardsSyncedFlushResult thirdSeal = SyncedFlushUtil.attemptSyncedFlush(logger, internalCluster(), shardId); - assertThat(thirdSeal.successfulShards(), equalTo(numberOfReplicas + 1)); - assertThat(thirdSeal.syncId(), not(equalTo(firstSeal.syncId()))); - // Manually remove or change sync-id, renew synced flush. - IndexShard shard = internalCluster().getInstance(IndicesService.class, randomFrom(internalCluster().nodesInclude("test"))) - .getShardOrNull(shardId); - if (randomBoolean()) { - // Change the existing sync-id of a single shard. - shard.syncFlush(UUIDs.randomBase64UUID(random()), shard.commitStats().getRawCommitId()); - assertThat(shard.commitStats().syncId(), not(equalTo(thirdSeal.syncId()))); - } else { - // Flush will create a new commit without sync-id - shard.flush(new FlushRequest(shardId.getIndexName()).force(true).waitIfOngoing(true)); - assertThat(shard.commitStats().syncId(), nullValue()); - } - final ShardsSyncedFlushResult forthSeal = SyncedFlushUtil.attemptSyncedFlush(logger, internalCluster(), shardId); - assertThat(forthSeal.successfulShards(), equalTo(numberOfReplicas + 1)); - assertThat(forthSeal.syncId(), not(equalTo(thirdSeal.syncId()))); - } - - public void testFlushOnInactive() throws Exception { - final String indexName = "flush_on_inactive"; - List dataNodes = internalCluster().startDataOnlyNodes( - 2, - Settings.builder().put(IndexingMemoryController.SHARD_INACTIVE_TIME_SETTING.getKey(), randomTimeValue(10, 1000, "ms")).build() - ); - assertAcked( - client().admin() - .indices() - .prepareCreate(indexName) - .setSettings( - Settings.builder() - .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) - .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1) - .put(IndexSettings.INDEX_TRANSLOG_SYNC_INTERVAL_SETTING.getKey(), randomTimeValue(200, 500, "ms")) - .put(IndexService.GLOBAL_CHECKPOINT_SYNC_INTERVAL_SETTING.getKey(), randomTimeValue(50, 200, "ms")) - .put("index.routing.allocation.include._name", String.join(",", dataNodes)) - .build() - ) - ); - ensureGreen(indexName); - int numDocs = randomIntBetween(1, 10); - for (int i = 0; i < numDocs; i++) { - client().prepareIndex(indexName, "_doc").setSource("f", "v").get(); - } - if (randomBoolean()) { - internalCluster().restartNode(randomFrom(dataNodes), new InternalTestCluster.RestartCallback()); - ensureGreen(indexName); - } - assertBusy(() -> { - for (ShardStats shardStats : client().admin().indices().prepareStats(indexName).get().getShards()) { - assertThat(shardStats.getStats().getTranslog().getUncommittedOperations(), equalTo(0)); - } - }, 30, TimeUnit.SECONDS); - } -} diff --git a/server/src/internalClusterTest/java/org/opensearch/indices/recovery/IndexRecoveryIT.java b/server/src/internalClusterTest/java/org/opensearch/indices/recovery/IndexRecoveryIT.java index 52e7e7b55bbd6..3bab909d3b7f3 100644 --- a/server/src/internalClusterTest/java/org/opensearch/indices/recovery/IndexRecoveryIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/indices/recovery/IndexRecoveryIT.java @@ -81,6 +81,7 @@ import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.concurrent.OpenSearchRejectedExecutionException; import org.opensearch.common.xcontent.XContentType; +import org.opensearch.gateway.ReplicaShardAllocatorIT; import org.opensearch.index.Index; import org.opensearch.index.IndexService; import org.opensearch.index.IndexSettings; @@ -100,7 +101,6 @@ import org.opensearch.indices.IndicesService; import org.opensearch.indices.NodeIndicesStats; import org.opensearch.indices.analysis.AnalysisModule; -import org.opensearch.indices.flush.SyncedFlushUtil; import org.opensearch.indices.recovery.RecoveryState.Stage; import org.opensearch.node.NodeClosedException; import org.opensearch.node.RecoverySettingsChunkSizePlugin; @@ -138,7 +138,6 @@ import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; import java.util.concurrent.Semaphore; @@ -148,7 +147,6 @@ import java.util.function.Consumer; import java.util.stream.Collectors; import java.util.stream.IntStream; -import java.util.stream.Stream; import java.util.stream.StreamSupport; import static java.util.Collections.singletonMap; @@ -403,7 +401,23 @@ public void testCancelNewShardRecoveryAndUsesExistingShardCopy() throws Exceptio final String nodeA = internalCluster().startNode(); logger.info("--> create index on node: {}", nodeA); - createAndPopulateIndex(INDEX_NAME, 1, SHARD_COUNT, REPLICA_COUNT).getShards()[0].getStats().getStore().size(); + createIndex( + INDEX_NAME, + Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1) + .put(IndexService.RETENTION_LEASE_SYNC_INTERVAL_SETTING.getKey(), "100ms") + .put(IndexService.GLOBAL_CHECKPOINT_SYNC_INTERVAL_SETTING.getKey(), "100ms") + .build() + ); + + int numDocs = randomIntBetween(10, 200); + final IndexRequestBuilder[] docs = new IndexRequestBuilder[numDocs]; + for (int i = 0; i < numDocs; i++) { + docs[i] = client().prepareIndex(INDEX_NAME, INDEX_TYPE) + .setSource("foo-int", randomInt(), "foo-string", randomAlphaOfLength(32), "foo-float", randomFloat()); + } + indexRandom(randomBoolean(), docs); logger.info("--> start node B"); // force a shard recovery from nodeA to nodeB @@ -425,8 +439,7 @@ public void testCancelNewShardRecoveryAndUsesExistingShardCopy() throws Exceptio logger.info("--> start node C"); final String nodeC = internalCluster().startNode(); - // do sync flush to gen sync id - assertThat(client().admin().indices().prepareSyncedFlush(INDEX_NAME).get().failedShards(), equalTo(0)); + ReplicaShardAllocatorIT.ensureActivePeerRecoveryRetentionLeasesAdvanced(INDEX_NAME); // hold peer recovery on phase 2 after nodeB down CountDownLatch phase1ReadyBlocked = new CountDownLatch(1); @@ -1524,93 +1537,6 @@ public void testOngoingRecoveryAndMasterFailOver() throws Exception { ensureGreen(indexName); } - public void testRecoveryFlushReplica() throws Exception { - internalCluster().ensureAtLeastNumDataNodes(3); - String indexName = "test-index"; - createIndex(indexName, Settings.builder().put("index.number_of_replicas", 0).put("index.number_of_shards", 1).build()); - int numDocs = randomIntBetween(0, 10); - indexRandom( - randomBoolean(), - false, - randomBoolean(), - IntStream.range(0, numDocs).mapToObj(n -> client().prepareIndex(indexName, "_doc").setSource("num", n)).collect(toList()) - ); - assertAcked( - client().admin().indices().prepareUpdateSettings(indexName).setSettings(Settings.builder().put("index.number_of_replicas", 1)) - ); - ensureGreen(indexName); - ShardId shardId = null; - for (ShardStats shardStats : client().admin().indices().prepareStats(indexName).get().getIndex(indexName).getShards()) { - shardId = shardStats.getShardRouting().shardId(); - if (shardStats.getShardRouting().primary() == false) { - assertThat(shardStats.getCommitStats().getNumDocs(), equalTo(numDocs)); - SequenceNumbers.CommitInfo commitInfo = SequenceNumbers.loadSeqNoInfoFromLuceneCommit( - shardStats.getCommitStats().getUserData().entrySet() - ); - assertThat(commitInfo.localCheckpoint, equalTo(shardStats.getSeqNoStats().getLocalCheckpoint())); - assertThat(commitInfo.maxSeqNo, equalTo(shardStats.getSeqNoStats().getMaxSeqNo())); - } - } - SyncedFlushUtil.attemptSyncedFlush(logger, internalCluster(), shardId); - assertBusy(() -> assertThat(client().admin().indices().prepareSyncedFlush(indexName).get().failedShards(), equalTo(0))); - assertAcked( - client().admin().indices().prepareUpdateSettings(indexName).setSettings(Settings.builder().put("index.number_of_replicas", 2)) - ); - ensureGreen(indexName); - // Recovery should keep syncId if no indexing activity on the primary after synced-flush. - Set syncIds = Stream.of(client().admin().indices().prepareStats(indexName).get().getIndex(indexName).getShards()) - .map(shardStats -> shardStats.getCommitStats().syncId()) - .collect(Collectors.toSet()); - assertThat(syncIds, hasSize(1)); - } - - public void testRecoveryUsingSyncedFlushWithoutRetentionLease() throws Exception { - internalCluster().ensureAtLeastNumDataNodes(2); - String indexName = "test-index"; - createIndex( - indexName, - Settings.builder() - .put("index.number_of_shards", 1) - .put("index.number_of_replicas", 1) - .put(UnassignedInfo.INDEX_DELAYED_NODE_LEFT_TIMEOUT_SETTING.getKey(), "24h") // do not reallocate the lost shard - .put(IndexSettings.INDEX_SOFT_DELETES_RETENTION_LEASE_PERIOD_SETTING.getKey(), "100ms") // expire leases quickly - .put(IndexService.RETENTION_LEASE_SYNC_INTERVAL_SETTING.getKey(), "100ms") // sync frequently - .build() - ); - int numDocs = randomIntBetween(0, 10); - indexRandom( - randomBoolean(), - false, - randomBoolean(), - IntStream.range(0, numDocs).mapToObj(n -> client().prepareIndex(indexName, "_doc").setSource("num", n)).collect(toList()) - ); - ensureGreen(indexName); - - final ShardId shardId = new ShardId(resolveIndex(indexName), 0); - assertThat(SyncedFlushUtil.attemptSyncedFlush(logger, internalCluster(), shardId).successfulShards(), equalTo(2)); - - final ClusterState clusterState = client().admin().cluster().prepareState().get().getState(); - final ShardRouting shardToResync = randomFrom(clusterState.routingTable().shardRoutingTable(shardId).activeShards()); - internalCluster().restartNode( - clusterState.nodes().get(shardToResync.currentNodeId()).getName(), - new InternalTestCluster.RestartCallback() { - @Override - public Settings onNodeStopped(String nodeName) throws Exception { - assertBusy( - () -> assertFalse( - client().admin().indices().prepareStats(indexName).get().getShards()[0].getRetentionLeaseStats() - .retentionLeases() - .contains(ReplicationTracker.getPeerRecoveryRetentionLeaseId(shardToResync)) - ) - ); - return super.onNodeStopped(nodeName); - } - } - ); - - ensureGreen(indexName); - } - public void testRecoverLocallyUpToGlobalCheckpoint() throws Exception { internalCluster().ensureAtLeastNumDataNodes(2); List nodes = randomSubsetOf( diff --git a/server/src/internalClusterTest/java/org/opensearch/indices/state/CloseIndexIT.java b/server/src/internalClusterTest/java/org/opensearch/indices/state/CloseIndexIT.java index db8f9ea360598..aebb891ae784b 100644 --- a/server/src/internalClusterTest/java/org/opensearch/indices/state/CloseIndexIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/indices/state/CloseIndexIT.java @@ -484,11 +484,7 @@ public void testRecoverExistingReplica() throws Exception { .collect(toList()) ); ensureGreen(indexName); - if (randomBoolean()) { - client().admin().indices().prepareFlush(indexName).get(); - } else { - client().admin().indices().prepareSyncedFlush(indexName).get(); - } + client().admin().indices().prepareFlush(indexName).get(); // index more documents while one shard copy is offline internalCluster().restartNode(dataNodes.get(1), new InternalTestCluster.RestartCallback() { @Override diff --git a/server/src/main/java/org/opensearch/action/ActionModule.java b/server/src/main/java/org/opensearch/action/ActionModule.java index c9b3360e92e87..8e31aa23d88cf 100644 --- a/server/src/main/java/org/opensearch/action/ActionModule.java +++ b/server/src/main/java/org/opensearch/action/ActionModule.java @@ -140,9 +140,7 @@ import org.opensearch.action.admin.indices.exists.types.TransportTypesExistsAction; import org.opensearch.action.admin.indices.exists.types.TypesExistsAction; import org.opensearch.action.admin.indices.flush.FlushAction; -import org.opensearch.action.admin.indices.flush.SyncedFlushAction; import org.opensearch.action.admin.indices.flush.TransportFlushAction; -import org.opensearch.action.admin.indices.flush.TransportSyncedFlushAction; import org.opensearch.action.admin.indices.forcemerge.ForceMergeAction; import org.opensearch.action.admin.indices.forcemerge.TransportForceMergeAction; import org.opensearch.action.admin.indices.get.GetIndexAction; @@ -589,7 +587,6 @@ public void reg actions.register(ValidateQueryAction.INSTANCE, TransportValidateQueryAction.class); actions.register(RefreshAction.INSTANCE, TransportRefreshAction.class); actions.register(FlushAction.INSTANCE, TransportFlushAction.class); - actions.register(SyncedFlushAction.INSTANCE, TransportSyncedFlushAction.class); actions.register(ForceMergeAction.INSTANCE, TransportForceMergeAction.class); actions.register(UpgradeAction.INSTANCE, TransportUpgradeAction.class); actions.register(UpgradeStatusAction.INSTANCE, TransportUpgradeStatusAction.class); diff --git a/server/src/main/java/org/opensearch/action/admin/indices/flush/SyncedFlushAction.java b/server/src/main/java/org/opensearch/action/admin/indices/flush/SyncedFlushAction.java deleted file mode 100644 index dbc138b2ee387..0000000000000 --- a/server/src/main/java/org/opensearch/action/admin/indices/flush/SyncedFlushAction.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -/* - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.action.admin.indices.flush; - -import org.opensearch.action.ActionType; - -public class SyncedFlushAction extends ActionType { - - public static final SyncedFlushAction INSTANCE = new SyncedFlushAction(); - public static final String NAME = "indices:admin/synced_flush"; - - private SyncedFlushAction() { - super(NAME, SyncedFlushResponse::new); - } -} diff --git a/server/src/main/java/org/opensearch/action/admin/indices/flush/SyncedFlushRequest.java b/server/src/main/java/org/opensearch/action/admin/indices/flush/SyncedFlushRequest.java deleted file mode 100644 index c3ed5a3c4599f..0000000000000 --- a/server/src/main/java/org/opensearch/action/admin/indices/flush/SyncedFlushRequest.java +++ /dev/null @@ -1,69 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -/* - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.action.admin.indices.flush; - -import org.opensearch.action.support.broadcast.BroadcastRequest; -import org.opensearch.common.io.stream.StreamInput; - -import java.io.IOException; -import java.util.Arrays; - -/** - * A synced flush request to sync flush one or more indices. The synced flush process of an index performs a flush - * and writes the same sync id to primary and all copies. - * - *

Best created with {@link org.opensearch.client.Requests#syncedFlushRequest(String...)}.

- * - * @see org.opensearch.client.Requests#flushRequest(String...) - * @see org.opensearch.client.IndicesAdminClient#syncedFlush(SyncedFlushRequest) - * @see SyncedFlushResponse - */ -public class SyncedFlushRequest extends BroadcastRequest { - - /** - * Constructs a new synced flush request against one or more indices. If nothing is provided, all indices will - * be sync flushed. - */ - public SyncedFlushRequest(String... indices) { - super(indices); - } - - public SyncedFlushRequest(StreamInput in) throws IOException { - super(in); - } - - @Override - public String toString() { - return "SyncedFlushRequest{" + "indices=" + Arrays.toString(indices) + "}"; - } -} diff --git a/server/src/main/java/org/opensearch/action/admin/indices/flush/SyncedFlushRequestBuilder.java b/server/src/main/java/org/opensearch/action/admin/indices/flush/SyncedFlushRequestBuilder.java deleted file mode 100644 index 48ac3d406655d..0000000000000 --- a/server/src/main/java/org/opensearch/action/admin/indices/flush/SyncedFlushRequestBuilder.java +++ /dev/null @@ -1,54 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -/* - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.action.admin.indices.flush; - -import org.opensearch.action.ActionRequestBuilder; -import org.opensearch.action.support.IndicesOptions; -import org.opensearch.client.OpenSearchClient; - -public class SyncedFlushRequestBuilder extends ActionRequestBuilder { - - public SyncedFlushRequestBuilder(OpenSearchClient client, SyncedFlushAction action) { - super(client, action, new SyncedFlushRequest()); - } - - public SyncedFlushRequestBuilder setIndices(String[] indices) { - super.request().indices(indices); - return this; - } - - public SyncedFlushRequestBuilder setIndicesOptions(IndicesOptions indicesOptions) { - super.request().indicesOptions(indicesOptions); - return this; - } -} diff --git a/server/src/main/java/org/opensearch/action/admin/indices/flush/SyncedFlushResponse.java b/server/src/main/java/org/opensearch/action/admin/indices/flush/SyncedFlushResponse.java deleted file mode 100644 index 95d8d0592187b..0000000000000 --- a/server/src/main/java/org/opensearch/action/admin/indices/flush/SyncedFlushResponse.java +++ /dev/null @@ -1,226 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -/* - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.action.admin.indices.flush; - -import org.opensearch.action.ActionResponse; -import org.opensearch.cluster.routing.ShardRouting; -import org.opensearch.common.io.stream.StreamInput; -import org.opensearch.common.io.stream.StreamOutput; -import org.opensearch.common.io.stream.Writeable; -import org.opensearch.common.util.iterable.Iterables; -import org.opensearch.common.xcontent.ToXContentFragment; -import org.opensearch.common.xcontent.XContentBuilder; -import org.opensearch.indices.flush.ShardsSyncedFlushResult; -import org.opensearch.indices.flush.SyncedFlushService; -import org.opensearch.rest.RestStatus; - -import java.io.IOException; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -import static java.util.Collections.unmodifiableMap; - -/** - * The result of performing a sync flush operation on all shards of multiple indices - */ -public class SyncedFlushResponse extends ActionResponse implements ToXContentFragment { - - private final Map> shardsResultPerIndex; - private final ShardCounts shardCounts; - - public SyncedFlushResponse(Map> shardsResultPerIndex) { - // shardsResultPerIndex is never modified after it is passed to this - // constructor so this is safe even though shardsResultPerIndex is a - // ConcurrentHashMap - this.shardsResultPerIndex = unmodifiableMap(shardsResultPerIndex); - this.shardCounts = calculateShardCounts(Iterables.flatten(shardsResultPerIndex.values())); - } - - public SyncedFlushResponse(StreamInput in) throws IOException { - super(in); - shardCounts = new ShardCounts(in); - Map> tmpShardsResultPerIndex = new HashMap<>(); - int numShardsResults = in.readInt(); - for (int i = 0; i < numShardsResults; i++) { - String index = in.readString(); - List shardsSyncedFlushResults = new ArrayList<>(); - int numShards = in.readInt(); - for (int j = 0; j < numShards; j++) { - shardsSyncedFlushResults.add(new ShardsSyncedFlushResult(in)); - } - tmpShardsResultPerIndex.put(index, shardsSyncedFlushResults); - } - shardsResultPerIndex = Collections.unmodifiableMap(tmpShardsResultPerIndex); - } - - /** - * total number shards, including replicas, both assigned and unassigned - */ - public int totalShards() { - return shardCounts.total; - } - - /** - * total number of shards for which the operation failed - */ - public int failedShards() { - return shardCounts.failed; - } - - /** - * total number of shards which were successfully sync-flushed - */ - public int successfulShards() { - return shardCounts.successful; - } - - public RestStatus restStatus() { - return failedShards() == 0 ? RestStatus.OK : RestStatus.CONFLICT; - } - - public Map> getShardsResultPerIndex() { - return shardsResultPerIndex; - } - - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(Fields._SHARDS); - shardCounts.toXContent(builder, params); - builder.endObject(); - for (Map.Entry> indexEntry : shardsResultPerIndex.entrySet()) { - List indexResult = indexEntry.getValue(); - builder.startObject(indexEntry.getKey()); - ShardCounts indexShardCounts = calculateShardCounts(indexResult); - indexShardCounts.toXContent(builder, params); - if (indexShardCounts.failed > 0) { - builder.startArray(Fields.FAILURES); - for (ShardsSyncedFlushResult shardResults : indexResult) { - if (shardResults.failed()) { - builder.startObject(); - builder.field(Fields.SHARD, shardResults.shardId().id()); - builder.field(Fields.REASON, shardResults.failureReason()); - builder.endObject(); - continue; - } - Map failedShards = shardResults.failedShards(); - for (Map.Entry shardEntry : failedShards.entrySet()) { - builder.startObject(); - builder.field(Fields.SHARD, shardResults.shardId().id()); - builder.field(Fields.REASON, shardEntry.getValue().failureReason()); - builder.field(Fields.ROUTING, shardEntry.getKey()); - builder.endObject(); - } - } - builder.endArray(); - } - builder.endObject(); - } - return builder; - } - - static ShardCounts calculateShardCounts(Iterable results) { - int total = 0, successful = 0, failed = 0; - for (ShardsSyncedFlushResult result : results) { - total += result.totalShards(); - successful += result.successfulShards(); - if (result.failed()) { - // treat all shard copies as failed - failed += result.totalShards(); - } else { - // some shards may have failed during the sync phase - failed += result.failedShards().size(); - } - } - return new ShardCounts(total, successful, failed); - } - - static final class ShardCounts implements ToXContentFragment, Writeable { - - public final int total; - public final int successful; - public final int failed; - - ShardCounts(int total, int successful, int failed) { - this.total = total; - this.successful = successful; - this.failed = failed; - } - - ShardCounts(StreamInput in) throws IOException { - total = in.readInt(); - successful = in.readInt(); - failed = in.readInt(); - } - - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.field(Fields.TOTAL, total); - builder.field(Fields.SUCCESSFUL, successful); - builder.field(Fields.FAILED, failed); - return builder; - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeInt(total); - out.writeInt(successful); - out.writeInt(failed); - } - } - - static final class Fields { - static final String _SHARDS = "_shards"; - static final String TOTAL = "total"; - static final String SUCCESSFUL = "successful"; - static final String FAILED = "failed"; - static final String FAILURES = "failures"; - static final String SHARD = "shard"; - static final String ROUTING = "routing"; - static final String REASON = "reason"; - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - shardCounts.writeTo(out); - out.writeInt(shardsResultPerIndex.size()); - for (Map.Entry> entry : shardsResultPerIndex.entrySet()) { - out.writeString(entry.getKey()); - out.writeInt(entry.getValue().size()); - for (ShardsSyncedFlushResult shardsSyncedFlushResult : entry.getValue()) { - shardsSyncedFlushResult.writeTo(out); - } - } - } -} diff --git a/server/src/main/java/org/opensearch/action/admin/indices/flush/TransportShardFlushAction.java b/server/src/main/java/org/opensearch/action/admin/indices/flush/TransportShardFlushAction.java index e267c5e224581..53e774306e746 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/flush/TransportShardFlushAction.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/flush/TransportShardFlushAction.java @@ -32,6 +32,7 @@ package org.opensearch.action.admin.indices.flush; +import org.opensearch.Version; import org.opensearch.action.ActionListener; import org.opensearch.action.support.ActionFilters; import org.opensearch.action.support.replication.ReplicationResponse; @@ -40,10 +41,16 @@ import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.inject.Inject; import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.common.io.stream.StreamOutput; import org.opensearch.common.settings.Settings; import org.opensearch.index.shard.IndexShard; +import org.opensearch.index.shard.ShardId; import org.opensearch.indices.IndicesService; +import org.opensearch.tasks.Task; import org.opensearch.threadpool.ThreadPool; +import org.opensearch.transport.TransportChannel; +import org.opensearch.transport.TransportRequest; +import org.opensearch.transport.TransportRequestHandler; import org.opensearch.transport.TransportService; import java.io.IOException; @@ -75,6 +82,12 @@ public TransportShardFlushAction( ShardFlushRequest::new, ThreadPool.Names.FLUSH ); + transportService.registerRequestHandler( + PRE_SYNCED_FLUSH_ACTION_NAME, + ThreadPool.Names.FLUSH, + PreShardSyncedFlushRequest::new, + new PreSyncedFlushTransportHandler(indicesService) + ); } @Override @@ -103,4 +116,43 @@ protected void shardOperationOnReplica(ShardFlushRequest request, IndexShard rep return new ReplicaResult(); }); } + + // TODO: Remove this transition in OpenSearch 3.0 + private static final String PRE_SYNCED_FLUSH_ACTION_NAME = "internal:indices/flush/synced/pre"; + + private static class PreShardSyncedFlushRequest extends TransportRequest { + private final ShardId shardId; + + private PreShardSyncedFlushRequest(StreamInput in) throws IOException { + super(in); + assert in.getVersion().before(Version.V_2_0_0) : "received pre_sync request from a new node"; + this.shardId = new ShardId(in); + } + + @Override + public String toString() { + return "PreShardSyncedFlushRequest{" + "shardId=" + shardId + '}'; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + assert false : "must not send pre_sync request from a new node"; + throw new UnsupportedOperationException(""); + } + } + + private static final class PreSyncedFlushTransportHandler implements TransportRequestHandler { + private final IndicesService indicesService; + + PreSyncedFlushTransportHandler(IndicesService indicesService) { + this.indicesService = indicesService; + } + + @Override + public void messageReceived(PreShardSyncedFlushRequest request, TransportChannel channel, Task task) { + IndexShard indexShard = indicesService.indexServiceSafe(request.shardId.getIndex()).getShard(request.shardId.id()); + indexShard.flush(new FlushRequest().force(false).waitIfOngoing(true)); + throw new UnsupportedOperationException("Synced flush was removed and a normal flush was performed instead."); + } + } } diff --git a/server/src/main/java/org/opensearch/action/admin/indices/flush/TransportSyncedFlushAction.java b/server/src/main/java/org/opensearch/action/admin/indices/flush/TransportSyncedFlushAction.java deleted file mode 100644 index 619c408a54c92..0000000000000 --- a/server/src/main/java/org/opensearch/action/admin/indices/flush/TransportSyncedFlushAction.java +++ /dev/null @@ -1,64 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -/* - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.action.admin.indices.flush; - -import org.opensearch.action.ActionListener; -import org.opensearch.action.support.ActionFilters; -import org.opensearch.action.support.HandledTransportAction; -import org.opensearch.common.inject.Inject; -import org.opensearch.indices.flush.SyncedFlushService; -import org.opensearch.tasks.Task; -import org.opensearch.transport.TransportService; - -/** - * Synced flush ActionType. - */ -public class TransportSyncedFlushAction extends HandledTransportAction { - - SyncedFlushService syncedFlushService; - - @Inject - public TransportSyncedFlushAction( - TransportService transportService, - ActionFilters actionFilters, - SyncedFlushService syncedFlushService - ) { - super(SyncedFlushAction.NAME, transportService, actionFilters, SyncedFlushRequest::new); - this.syncedFlushService = syncedFlushService; - } - - @Override - protected void doExecute(Task task, SyncedFlushRequest request, ActionListener listener) { - syncedFlushService.attemptSyncedFlush(request.indices(), request.indicesOptions(), listener); - } -} diff --git a/server/src/main/java/org/opensearch/client/IndicesAdminClient.java b/server/src/main/java/org/opensearch/client/IndicesAdminClient.java index 886ce5776eb20..7f51b8af19e4b 100644 --- a/server/src/main/java/org/opensearch/client/IndicesAdminClient.java +++ b/server/src/main/java/org/opensearch/client/IndicesAdminClient.java @@ -66,9 +66,6 @@ import org.opensearch.action.admin.indices.flush.FlushRequest; import org.opensearch.action.admin.indices.flush.FlushRequestBuilder; import org.opensearch.action.admin.indices.flush.FlushResponse; -import org.opensearch.action.admin.indices.flush.SyncedFlushRequest; -import org.opensearch.action.admin.indices.flush.SyncedFlushRequestBuilder; -import org.opensearch.action.admin.indices.flush.SyncedFlushResponse; import org.opensearch.action.admin.indices.forcemerge.ForceMergeRequest; import org.opensearch.action.admin.indices.forcemerge.ForceMergeRequestBuilder; import org.opensearch.action.admin.indices.forcemerge.ForceMergeResponse; @@ -433,29 +430,6 @@ public interface IndicesAdminClient extends OpenSearchClient { */ FlushRequestBuilder prepareFlush(String... indices); - /** - * Explicitly sync flush one or more indices (write sync id to shards for faster recovery). - * - * @param request The sync flush request - * @return A result future - * @see org.opensearch.client.Requests#syncedFlushRequest(String...) - */ - ActionFuture syncedFlush(SyncedFlushRequest request); - - /** - * Explicitly sync flush one or more indices (write sync id to shards for faster recovery). - * - * @param request The sync flush request - * @param listener A listener to be notified with a result - * @see org.opensearch.client.Requests#syncedFlushRequest(String...) - */ - void syncedFlush(SyncedFlushRequest request, ActionListener listener); - - /** - * Explicitly sync flush one or more indices (write sync id to shards for faster recovery). - */ - SyncedFlushRequestBuilder prepareSyncedFlush(String... indices); - /** * Explicitly force merge one or more indices into a the number of segments. * diff --git a/server/src/main/java/org/opensearch/client/Requests.java b/server/src/main/java/org/opensearch/client/Requests.java index b62f6ee7f7234..a7818e9ac136b 100644 --- a/server/src/main/java/org/opensearch/client/Requests.java +++ b/server/src/main/java/org/opensearch/client/Requests.java @@ -61,7 +61,6 @@ import org.opensearch.action.admin.indices.delete.DeleteIndexRequest; import org.opensearch.action.admin.indices.exists.indices.IndicesExistsRequest; import org.opensearch.action.admin.indices.flush.FlushRequest; -import org.opensearch.action.admin.indices.flush.SyncedFlushRequest; import org.opensearch.action.admin.indices.forcemerge.ForceMergeRequest; import org.opensearch.action.admin.indices.mapping.put.PutMappingRequest; import org.opensearch.action.admin.indices.open.OpenIndexRequest; @@ -274,17 +273,6 @@ public static FlushRequest flushRequest(String... indices) { return new FlushRequest(indices); } - /** - * Creates a synced flush indices request. - * - * @param indices The indices to sync flush. Use {@code null} or {@code _all} to execute against all indices - * @return The synced flush request - * @see org.opensearch.client.IndicesAdminClient#syncedFlush(SyncedFlushRequest) - */ - public static SyncedFlushRequest syncedFlushRequest(String... indices) { - return new SyncedFlushRequest(indices); - } - /** * Creates a force merge request. * diff --git a/server/src/main/java/org/opensearch/client/support/AbstractClient.java b/server/src/main/java/org/opensearch/client/support/AbstractClient.java index a796185e2145c..2e2c07a2433f4 100644 --- a/server/src/main/java/org/opensearch/client/support/AbstractClient.java +++ b/server/src/main/java/org/opensearch/client/support/AbstractClient.java @@ -200,10 +200,6 @@ import org.opensearch.action.admin.indices.flush.FlushRequest; import org.opensearch.action.admin.indices.flush.FlushRequestBuilder; import org.opensearch.action.admin.indices.flush.FlushResponse; -import org.opensearch.action.admin.indices.flush.SyncedFlushAction; -import org.opensearch.action.admin.indices.flush.SyncedFlushRequest; -import org.opensearch.action.admin.indices.flush.SyncedFlushRequestBuilder; -import org.opensearch.action.admin.indices.flush.SyncedFlushResponse; import org.opensearch.action.admin.indices.forcemerge.ForceMergeAction; import org.opensearch.action.admin.indices.forcemerge.ForceMergeRequest; import org.opensearch.action.admin.indices.forcemerge.ForceMergeRequestBuilder; @@ -1517,21 +1513,6 @@ public FlushRequestBuilder prepareFlush(String... indices) { return new FlushRequestBuilder(this, FlushAction.INSTANCE).setIndices(indices); } - @Override - public ActionFuture syncedFlush(SyncedFlushRequest request) { - return execute(SyncedFlushAction.INSTANCE, request); - } - - @Override - public void syncedFlush(SyncedFlushRequest request, ActionListener listener) { - execute(SyncedFlushAction.INSTANCE, request, listener); - } - - @Override - public SyncedFlushRequestBuilder prepareSyncedFlush(String... indices) { - return new SyncedFlushRequestBuilder(this, SyncedFlushAction.INSTANCE).setIndices(indices); - } - @Override public void getMappings(GetMappingsRequest request, ActionListener listener) { execute(GetMappingsAction.INSTANCE, request, listener); diff --git a/server/src/main/java/org/opensearch/index/CompositeIndexEventListener.java b/server/src/main/java/org/opensearch/index/CompositeIndexEventListener.java index 60c012d9e652a..79c97d1126e93 100644 --- a/server/src/main/java/org/opensearch/index/CompositeIndexEventListener.java +++ b/server/src/main/java/org/opensearch/index/CompositeIndexEventListener.java @@ -135,21 +135,6 @@ public void afterIndexShardClosed(ShardId shardId, @Nullable IndexShard indexSha } } - @Override - public void onShardInactive(IndexShard indexShard) { - for (IndexEventListener listener : listeners) { - try { - listener.onShardInactive(indexShard); - } catch (Exception e) { - logger.warn( - () -> new ParameterizedMessage("[{}] failed to invoke on shard inactive callback", indexShard.shardId().getId()), - e - ); - throw e; - } - } - } - @Override public void indexShardStateChanged( IndexShard indexShard, diff --git a/server/src/main/java/org/opensearch/index/engine/CommitStats.java b/server/src/main/java/org/opensearch/index/engine/CommitStats.java index cd9b2fe64a431..1220208d3a8f3 100644 --- a/server/src/main/java/org/opensearch/index/engine/CommitStats.java +++ b/server/src/main/java/org/opensearch/index/engine/CommitStats.java @@ -89,20 +89,6 @@ public String getId() { return id; } - /** - * A raw version of the commit id (see {@link SegmentInfos#getId()} - */ - public Engine.CommitId getRawCommitId() { - return new Engine.CommitId(Base64.getDecoder().decode(id)); - } - - /** - * The synced-flush id of the commit if existed. - */ - public String syncId() { - return userData.get(InternalEngine.SYNC_COMMIT_ID); - } - /** * Returns the number of documents in the in this commit */ diff --git a/server/src/main/java/org/opensearch/index/engine/Engine.java b/server/src/main/java/org/opensearch/index/engine/Engine.java index d4415e91e2fe8..3e92f6a2aef97 100644 --- a/server/src/main/java/org/opensearch/index/engine/Engine.java +++ b/server/src/main/java/org/opensearch/index/engine/Engine.java @@ -60,9 +60,6 @@ import org.opensearch.common.Nullable; import org.opensearch.common.bytes.BytesReference; import org.opensearch.common.collect.ImmutableOpenMap; -import org.opensearch.common.io.stream.StreamInput; -import org.opensearch.common.io.stream.StreamOutput; -import org.opensearch.common.io.stream.Writeable; import org.opensearch.common.lease.Releasable; import org.opensearch.common.lease.Releasables; import org.opensearch.common.logging.Loggers; @@ -96,7 +93,6 @@ import java.io.UncheckedIOException; import java.nio.file.NoSuchFileException; import java.util.Arrays; -import java.util.Base64; import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; @@ -121,7 +117,7 @@ public abstract class Engine implements Closeable { - public static final String SYNC_COMMIT_ID = "sync_id"; + public static final String SYNC_COMMIT_ID = "sync_id"; // TODO: remove sync_id in 3.0 public static final String HISTORY_UUID_KEY = "history_uuid"; public static final String FORCE_MERGE_UUID_KEY = "force_merge_uuid"; public static final String MIN_RETAINED_SEQNO = "min_retained_seq_no"; @@ -577,22 +573,6 @@ public static class NoOpResult extends Result { } - /** - * Attempts to do a special commit where the given syncID is put into the commit data. The attempt - * succeeds if there are not pending writes in lucene and the current point is equal to the expected one. - * - * @param syncId id of this sync - * @param expectedCommitId the expected value of - * @return true if the sync commit was made, false o.w. - */ - public abstract SyncedFlushResult syncFlush(String syncId, CommitId expectedCommitId) throws EngineException; - - public enum SyncedFlushResult { - SUCCESS, - COMMIT_MISMATCH, - PENDING_OPERATIONS - } - protected final GetResult getFromSearcher( Get get, BiFunction searcherFactory, @@ -1139,20 +1119,17 @@ public boolean refreshNeeded() { * @param force if true a lucene commit is executed even if no changes need to be committed. * @param waitIfOngoing if true this call will block until all currently running flushes have finished. * Otherwise this call will return without blocking. - * @return the commit Id for the resulting commit */ - public abstract CommitId flush(boolean force, boolean waitIfOngoing) throws EngineException; + public abstract void flush(boolean force, boolean waitIfOngoing) throws EngineException; /** * Flushes the state of the engine including the transaction log, clearing memory and persisting * documents in the lucene index to disk including a potentially heavy and durable fsync operation. * This operation is not going to block if another flush operation is currently running and won't write * a lucene commit if nothing needs to be committed. - * - * @return the commit Id for the resulting commit */ - public final CommitId flush() throws EngineException { - return flush(false, false); + public final void flush() throws EngineException { + flush(false, false); } /** @@ -1923,58 +1900,6 @@ private void awaitPendingClose() { } } - public static class CommitId implements Writeable { - - private final byte[] id; - - public CommitId(byte[] id) { - assert id != null; - this.id = Arrays.copyOf(id, id.length); - } - - /** - * Read from a stream. - */ - public CommitId(StreamInput in) throws IOException { - assert in != null; - this.id = in.readByteArray(); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeByteArray(id); - } - - @Override - public String toString() { - return Base64.getEncoder().encodeToString(id); - } - - public boolean idsEqual(byte[] id) { - return Arrays.equals(id, this.id); - } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - - CommitId commitId = (CommitId) o; - - return Arrays.equals(id, commitId.id); - - } - - @Override - public int hashCode() { - return Arrays.hashCode(id); - } - } - public static class IndexCommitRef implements Closeable { private final AtomicBoolean closed = new AtomicBoolean(); private final CheckedRunnable onClose; diff --git a/server/src/main/java/org/opensearch/index/engine/InternalEngine.java b/server/src/main/java/org/opensearch/index/engine/InternalEngine.java index 3c9c2d2d591f1..955ddcef4869d 100644 --- a/server/src/main/java/org/opensearch/index/engine/InternalEngine.java +++ b/server/src/main/java/org/opensearch/index/engine/InternalEngine.java @@ -1922,71 +1922,6 @@ public void writeIndexingBuffer() throws EngineException { refresh("write indexing buffer", SearcherScope.INTERNAL, false); } - @Override - public SyncedFlushResult syncFlush(String syncId, CommitId expectedCommitId) throws EngineException { - // best effort attempt before we acquire locks - ensureOpen(); - if (indexWriter.hasUncommittedChanges()) { - logger.trace("can't sync commit [{}]. have pending changes", syncId); - return SyncedFlushResult.PENDING_OPERATIONS; - } - if (expectedCommitId.idsEqual(lastCommittedSegmentInfos.getId()) == false) { - logger.trace("can't sync commit [{}]. current commit id is not equal to expected.", syncId); - return SyncedFlushResult.COMMIT_MISMATCH; - } - try (ReleasableLock lock = writeLock.acquire()) { - ensureOpen(); - ensureCanFlush(); - // lets do a refresh to make sure we shrink the version map. This refresh will be either a no-op (just shrink the version map) - // or we also have uncommitted changes and that causes this syncFlush to fail. - refresh("sync_flush", SearcherScope.INTERNAL, true); - if (indexWriter.hasUncommittedChanges()) { - logger.trace("can't sync commit [{}]. have pending changes", syncId); - return SyncedFlushResult.PENDING_OPERATIONS; - } - if (expectedCommitId.idsEqual(lastCommittedSegmentInfos.getId()) == false) { - logger.trace("can't sync commit [{}]. current commit id is not equal to expected.", syncId); - return SyncedFlushResult.COMMIT_MISMATCH; - } - logger.trace("starting sync commit [{}]", syncId); - commitIndexWriter(indexWriter, translog, syncId); - logger.debug("successfully sync committed. sync id [{}].", syncId); - lastCommittedSegmentInfos = store.readLastCommittedSegmentsInfo(); - return SyncedFlushResult.SUCCESS; - } catch (IOException ex) { - maybeFailEngine("sync commit", ex); - throw new EngineException(shardId, "failed to sync commit", ex); - } - } - - final boolean tryRenewSyncCommit() { - boolean renewed = false; - try (ReleasableLock lock = writeLock.acquire()) { - ensureOpen(); - ensureCanFlush(); - String syncId = lastCommittedSegmentInfos.getUserData().get(SYNC_COMMIT_ID); - long localCheckpointOfLastCommit = Long.parseLong(lastCommittedSegmentInfos.userData.get(SequenceNumbers.LOCAL_CHECKPOINT_KEY)); - if (syncId != null - && indexWriter.hasUncommittedChanges() - && translog.estimateTotalOperationsFromMinSeq(localCheckpointOfLastCommit + 1) == 0) { - logger.trace("start renewing sync commit [{}]", syncId); - commitIndexWriter(indexWriter, translog, syncId); - logger.debug("successfully sync committed. sync id [{}].", syncId); - lastCommittedSegmentInfos = store.readLastCommittedSegmentsInfo(); - renewed = true; - } - } catch (IOException ex) { - maybeFailEngine("renew sync commit", ex); - throw new EngineException(shardId, "failed to renew sync commit", ex); - } - if (renewed) { - // refresh outside of the write lock - // we have to refresh internal reader here to ensure we release unreferenced segments. - refresh("renew sync commit", SearcherScope.INTERNAL, true); - } - return renewed; - } - @Override public boolean shouldPeriodicallyFlush() { ensureOpen(); @@ -2026,7 +1961,7 @@ public boolean shouldPeriodicallyFlush() { } @Override - public CommitId flush(boolean force, boolean waitIfOngoing) throws EngineException { + public void flush(boolean force, boolean waitIfOngoing) throws EngineException { ensureOpen(); if (force && waitIfOngoing == false) { assert false : "wait_if_ongoing must be true for a force flush: force=" + force + " wait_if_ongoing=" + waitIfOngoing; @@ -2034,18 +1969,16 @@ public CommitId flush(boolean force, boolean waitIfOngoing) throws EngineExcepti "wait_if_ongoing must be true for a force flush: force=" + force + " wait_if_ongoing=" + waitIfOngoing ); } - final byte[] newCommitId; try (ReleasableLock lock = readLock.acquire()) { ensureOpen(); if (flushLock.tryLock() == false) { // if we can't get the lock right away we block if needed otherwise barf - if (waitIfOngoing) { - logger.trace("waiting for in-flight flush to finish"); - flushLock.lock(); - logger.trace("acquired flush lock after blocking"); - } else { - return new CommitId(lastCommittedSegmentInfos.getId()); + if (waitIfOngoing == false) { + return; } + logger.trace("waiting for in-flight flush to finish"); + flushLock.lock(); + logger.trace("acquired flush lock after blocking"); } else { logger.trace("acquired flush lock immediately"); } @@ -2065,7 +1998,7 @@ public CommitId flush(boolean force, boolean waitIfOngoing) throws EngineExcepti try { translog.rollGeneration(); logger.trace("starting commit for flush; commitTranslog=true"); - commitIndexWriter(indexWriter, translog, null); + commitIndexWriter(indexWriter, translog); logger.trace("finished commit for flush"); // a temporary debugging to investigate test failure - issue#32827. Remove when the issue is resolved @@ -2088,7 +2021,6 @@ public CommitId flush(boolean force, boolean waitIfOngoing) throws EngineExcepti refreshLastCommittedSegmentInfos(); } - newCommitId = lastCommittedSegmentInfos.getId(); } catch (FlushFailedEngineException ex) { maybeFailEngine("flush", ex); throw ex; @@ -2101,7 +2033,6 @@ public CommitId flush(boolean force, boolean waitIfOngoing) throws EngineExcepti if (engineConfig.isEnableGcDeletes()) { pruneDeletedTombstones(); } - return new CommitId(newCommitId); } private void refreshLastCommittedSegmentInfos() { @@ -2273,9 +2204,7 @@ public void forceMerge( this.forceMergeUUID = forceMergeUUID; } if (flush) { - if (tryRenewSyncCommit() == false) { - flush(false, true); - } + flush(false, true); } if (upgrade) { logger.info("finished segment upgrade"); @@ -2664,15 +2593,9 @@ public void onFailure(Exception e) { @Override protected void doRun() { - // if we have no pending merges and we are supposed to flush once merges have finished - // we try to renew a sync commit which is the case when we are having a big merge after we - // are inactive. If that didn't work we go and do a real flush which is ok since it only doesn't work - // if we either have records in the translog or if we don't have a sync ID at all... - // maybe even more important, we flush after all merges finish and we are inactive indexing-wise to + // if we have no pending merges and we are supposed to flush once merges have finished to // free up transient disk usage of the (presumably biggish) segments that were just merged - if (tryRenewSyncCommit() == false) { - flush(); - } + flush(); } }); } else if (merge.getTotalBytesSize() >= engineConfig.getIndexSettings().getFlushAfterMergeThresholdSize().getBytes()) { @@ -2709,10 +2632,8 @@ protected void doRun() throws Exception { * * @param writer the index writer to commit * @param translog the translog - * @param syncId the sync flush ID ({@code null} if not committing a synced flush) - * @throws IOException if an I/O exception occurs committing the specfied writer */ - protected void commitIndexWriter(final IndexWriter writer, final Translog translog, @Nullable final String syncId) throws IOException { + protected void commitIndexWriter(final IndexWriter writer, final Translog translog) throws IOException { ensureCanFlush(); try { final long localCheckpoint = localCheckpointTracker.getProcessedCheckpoint(); @@ -2729,9 +2650,6 @@ protected void commitIndexWriter(final IndexWriter writer, final Translog transl final Map commitData = new HashMap<>(7); commitData.put(Translog.TRANSLOG_UUID_KEY, translog.getTranslogUUID()); commitData.put(SequenceNumbers.LOCAL_CHECKPOINT_KEY, Long.toString(localCheckpoint)); - if (syncId != null) { - commitData.put(Engine.SYNC_COMMIT_ID, syncId); - } commitData.put(SequenceNumbers.MAX_SEQ_NO, Long.toString(localCheckpointTracker.getMaxSeqNo())); commitData.put(MAX_UNSAFE_AUTO_ID_TIMESTAMP_COMMIT_ID, Long.toString(maxUnsafeAutoIdTimestamp.get())); commitData.put(HISTORY_UUID_KEY, historyUUID); diff --git a/server/src/main/java/org/opensearch/index/engine/ReadOnlyEngine.java b/server/src/main/java/org/opensearch/index/engine/ReadOnlyEngine.java index 98ad64d2e2a71..b32618df7932c 100644 --- a/server/src/main/java/org/opensearch/index/engine/ReadOnlyEngine.java +++ b/server/src/main/java/org/opensearch/index/engine/ReadOnlyEngine.java @@ -429,15 +429,7 @@ public boolean shouldPeriodicallyFlush() { } @Override - public SyncedFlushResult syncFlush(String syncId, CommitId expectedCommitId) { - // we can't do synced flushes this would require an indexWriter which we don't have - throw new UnsupportedOperationException("syncedFlush is not supported on a read-only engine"); - } - - @Override - public CommitId flush(boolean force, boolean waitIfOngoing) throws EngineException { - return new CommitId(lastCommittedSegmentInfos.getId()); - } + public void flush(boolean force, boolean waitIfOngoing) throws EngineException {} @Override public void forceMerge( diff --git a/server/src/main/java/org/opensearch/index/shard/IndexEventListener.java b/server/src/main/java/org/opensearch/index/shard/IndexEventListener.java index 0f44ddbb5e8f7..69f283a53ca79 100644 --- a/server/src/main/java/org/opensearch/index/shard/IndexEventListener.java +++ b/server/src/main/java/org/opensearch/index/shard/IndexEventListener.java @@ -101,13 +101,6 @@ default void indexShardStateChanged( @Nullable String reason ) {} - /** - * Called when a shard is marked as inactive - * - * @param indexShard The shard that was marked inactive - */ - default void onShardInactive(IndexShard indexShard) {} - /** * Called before the index gets created. Note that this is also called * when the index is created on data nodes diff --git a/server/src/main/java/org/opensearch/index/shard/IndexShard.java b/server/src/main/java/org/opensearch/index/shard/IndexShard.java index 9217a2b67c179..b2f94f3d398ef 100644 --- a/server/src/main/java/org/opensearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/opensearch/index/shard/IndexShard.java @@ -1317,19 +1317,12 @@ public CompletionStats completionStats(String... fields) { return getEngine().completionStats(fields); } - public Engine.SyncedFlushResult syncFlush(String syncId, Engine.CommitId expectedCommitId) { - verifyNotClosed(); - logger.trace("trying to sync flush. sync id [{}]. expected commit id [{}]]", syncId, expectedCommitId); - return getEngine().syncFlush(syncId, expectedCommitId); - } - /** * Executes the given flush request against the engine. * * @param request the flush request - * @return the commit ID */ - public Engine.CommitId flush(FlushRequest request) { + public void flush(FlushRequest request) { final boolean waitIfOngoing = request.waitIfOngoing(); final boolean force = request.force(); logger.trace("flush with {}", request); @@ -1340,9 +1333,8 @@ public Engine.CommitId flush(FlushRequest request) { */ verifyNotClosed(); final long time = System.nanoTime(); - final Engine.CommitId commitId = getEngine().flush(force, waitIfOngoing); + getEngine().flush(force, waitIfOngoing); flushMetric.inc(System.nanoTime() - time); - return commitId; } /** @@ -1966,7 +1958,7 @@ private void innerOpenEngineAndTranslog(LongSupplier globalCheckpointSupplier) t onNewEngine(newEngine); currentEngineReference.set(newEngine); // We set active because we are now writing operations to the engine; this way, - // if we go idle after some time and become inactive, we still give sync'd flush a chance to run. + // we can flush if we go idle after some time and become inactive. active.set(true); } // time elapses after the engine is created above (pulling the config settings) until we set the engine reference, during @@ -2162,19 +2154,28 @@ public void addShardFailureCallback(Consumer onShardFailure) { /** * Called by {@link IndexingMemoryController} to check whether more than {@code inactiveTimeNS} has passed since the last - * indexing operation, and notify listeners that we are now inactive so e.g. sync'd flush can happen. + * indexing operation, so we can flush the index. */ - public void checkIdle(long inactiveTimeNS) { + public void flushOnIdle(long inactiveTimeNS) { Engine engineOrNull = getEngineOrNull(); if (engineOrNull != null && System.nanoTime() - engineOrNull.getLastWriteNanos() >= inactiveTimeNS) { boolean wasActive = active.getAndSet(false); if (wasActive) { - logger.debug("shard is now inactive"); - try { - indexEventListener.onShardInactive(this); - } catch (Exception e) { - logger.warn("failed to notify index event listener", e); - } + logger.debug("flushing shard on inactive"); + threadPool.executor(ThreadPool.Names.FLUSH).execute(new AbstractRunnable() { + @Override + public void onFailure(Exception e) { + if (state != IndexShardState.CLOSED) { + logger.warn("failed to flush shard on inactive", e); + } + } + + @Override + protected void doRun() { + flush(new FlushRequest().waitIfOngoing(false).force(false)); + periodicFlushMetric.inc(); + } + }); } } } diff --git a/server/src/main/java/org/opensearch/indices/IndexingMemoryController.java b/server/src/main/java/org/opensearch/indices/IndexingMemoryController.java index 6e74882fdac37..1917a224e5410 100644 --- a/server/src/main/java/org/opensearch/indices/IndexingMemoryController.java +++ b/server/src/main/java/org/opensearch/indices/IndexingMemoryController.java @@ -329,7 +329,7 @@ private void runUnlocked() { long totalBytesWriting = 0; for (IndexShard shard : availableShards()) { - // Give shard a chance to transition to inactive so sync'd flush can happen: + // Give shard a chance to transition to inactive so we can flush: checkIdle(shard, inactiveTime.nanos()); // How many bytes this shard is currently (async'd) moving from heap to disk: @@ -443,7 +443,7 @@ private void runUnlocked() { */ protected void checkIdle(IndexShard shard, long inactiveTimeNS) { try { - shard.checkIdle(inactiveTimeNS); + shard.flushOnIdle(inactiveTimeNS); } catch (AlreadyClosedException e) { logger.trace(() -> new ParameterizedMessage("ignore exception while checking if shard {} is inactive", shard.shardId()), e); } diff --git a/server/src/main/java/org/opensearch/indices/IndicesModule.java b/server/src/main/java/org/opensearch/indices/IndicesModule.java index 6f777f4833c6a..e685ea52aa5ca 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesModule.java +++ b/server/src/main/java/org/opensearch/indices/IndicesModule.java @@ -72,7 +72,6 @@ import org.opensearch.index.seqno.GlobalCheckpointSyncAction; import org.opensearch.index.shard.PrimaryReplicaSyncer; import org.opensearch.indices.cluster.IndicesClusterStateService; -import org.opensearch.indices.flush.SyncedFlushService; import org.opensearch.indices.mapper.MapperRegistry; import org.opensearch.indices.store.IndicesStore; import org.opensearch.indices.store.TransportNodesListShardStoreMetadata; @@ -270,7 +269,6 @@ private static Function> and( protected void configure() { bind(IndicesStore.class).asEagerSingleton(); bind(IndicesClusterStateService.class).asEagerSingleton(); - bind(SyncedFlushService.class).asEagerSingleton(); bind(TransportNodesListShardStoreMetadata.class).asEagerSingleton(); bind(GlobalCheckpointSyncAction.class).asEagerSingleton(); bind(TransportResyncReplicationAction.class).asEagerSingleton(); diff --git a/server/src/main/java/org/opensearch/indices/cluster/IndicesClusterStateService.java b/server/src/main/java/org/opensearch/indices/cluster/IndicesClusterStateService.java index ee0d76b4a9117..9031230092a97 100644 --- a/server/src/main/java/org/opensearch/indices/cluster/IndicesClusterStateService.java +++ b/server/src/main/java/org/opensearch/indices/cluster/IndicesClusterStateService.java @@ -76,7 +76,6 @@ import org.opensearch.index.shard.ShardId; import org.opensearch.index.shard.ShardNotFoundException; import org.opensearch.indices.IndicesService; -import org.opensearch.indices.flush.SyncedFlushService; import org.opensearch.indices.recovery.PeerRecoverySourceService; import org.opensearch.indices.recovery.PeerRecoveryTargetService; import org.opensearch.indices.recovery.RecoveryFailedException; @@ -144,7 +143,6 @@ public IndicesClusterStateService( final NodeMappingRefreshAction nodeMappingRefreshAction, final RepositoriesService repositoriesService, final SearchService searchService, - final SyncedFlushService syncedFlushService, final PeerRecoverySourceService peerRecoverySourceService, final SnapshotShardsService snapshotShardsService, final PrimaryReplicaSyncer primaryReplicaSyncer, @@ -161,7 +159,6 @@ public IndicesClusterStateService( nodeMappingRefreshAction, repositoriesService, searchService, - syncedFlushService, peerRecoverySourceService, snapshotShardsService, primaryReplicaSyncer, @@ -181,7 +178,6 @@ public IndicesClusterStateService( final NodeMappingRefreshAction nodeMappingRefreshAction, final RepositoriesService repositoriesService, final SearchService searchService, - final SyncedFlushService syncedFlushService, final PeerRecoverySourceService peerRecoverySourceService, final SnapshotShardsService snapshotShardsService, final PrimaryReplicaSyncer primaryReplicaSyncer, @@ -189,13 +185,7 @@ public IndicesClusterStateService( final RetentionLeaseSyncer retentionLeaseSyncer ) { this.settings = settings; - this.buildInIndexListener = Arrays.asList( - peerRecoverySourceService, - recoveryTargetService, - searchService, - syncedFlushService, - snapshotShardsService - ); + this.buildInIndexListener = Arrays.asList(peerRecoverySourceService, recoveryTargetService, searchService, snapshotShardsService); this.indicesService = indicesService; this.clusterService = clusterService; this.threadPool = threadPool; diff --git a/server/src/main/java/org/opensearch/indices/flush/ShardsSyncedFlushResult.java b/server/src/main/java/org/opensearch/indices/flush/ShardsSyncedFlushResult.java deleted file mode 100644 index 9bc5b3f7a7720..0000000000000 --- a/server/src/main/java/org/opensearch/indices/flush/ShardsSyncedFlushResult.java +++ /dev/null @@ -1,179 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -/* - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.indices.flush; - -import org.opensearch.cluster.routing.ShardRouting; -import org.opensearch.common.io.stream.StreamInput; -import org.opensearch.common.io.stream.StreamOutput; -import org.opensearch.common.io.stream.Writeable; -import org.opensearch.index.shard.ShardId; - -import java.io.IOException; -import java.util.HashMap; -import java.util.Map; - -import static java.util.Collections.emptyMap; -import static java.util.Collections.unmodifiableMap; - -/** - * Result for all copies of a shard - */ -public class ShardsSyncedFlushResult implements Writeable { - private String failureReason; - private Map shardResponses; - private String syncId; - private ShardId shardId; - // some shards may be unassigned, so we need this as state - private int totalShards; - - public ShardsSyncedFlushResult(StreamInput in) throws IOException { - failureReason = in.readOptionalString(); - int numResponses = in.readInt(); - shardResponses = new HashMap<>(); - for (int i = 0; i < numResponses; i++) { - ShardRouting shardRouting = new ShardRouting(in); - SyncedFlushService.ShardSyncedFlushResponse response = SyncedFlushService.ShardSyncedFlushResponse.readSyncedFlushResponse(in); - shardResponses.put(shardRouting, response); - } - syncId = in.readOptionalString(); - shardId = new ShardId(in); - totalShards = in.readInt(); - } - - public ShardId getShardId() { - return shardId; - } - - /** - * failure constructor - */ - public ShardsSyncedFlushResult(ShardId shardId, int totalShards, String failureReason) { - this.syncId = null; - this.failureReason = failureReason; - this.shardResponses = emptyMap(); - this.shardId = shardId; - this.totalShards = totalShards; - } - - /** - * success constructor - */ - public ShardsSyncedFlushResult( - ShardId shardId, - String syncId, - int totalShards, - Map shardResponses - ) { - this.failureReason = null; - this.shardResponses = unmodifiableMap(new HashMap<>(shardResponses)); - this.syncId = syncId; - this.totalShards = totalShards; - this.shardId = shardId; - } - - /** - * @return true if the operation failed before reaching step three of synced flush. {@link #failureReason()} can be used for - * more details - */ - public boolean failed() { - return failureReason != null; - } - - /** - * @return the reason for the failure if synced flush failed before step three of synced flush - */ - public String failureReason() { - return failureReason; - } - - public String syncId() { - return syncId; - } - - /** - * @return total number of shards for which a sync attempt was made - */ - public int totalShards() { - return totalShards; - } - - /** - * @return total number of successful shards - */ - public int successfulShards() { - int i = 0; - for (SyncedFlushService.ShardSyncedFlushResponse result : shardResponses.values()) { - if (result.success()) { - i++; - } - } - return i; - } - - /** - * @return an array of shard failures - */ - public Map failedShards() { - Map failures = new HashMap<>(); - for (Map.Entry result : shardResponses.entrySet()) { - if (result.getValue().success() == false) { - failures.put(result.getKey(), result.getValue()); - } - } - return failures; - } - - /** - * @return Individual responses for each shard copy with a detailed failure message if the copy failed to perform the synced flush. - * Empty if synced flush failed before step three. - */ - public Map shardResponses() { - return shardResponses; - } - - public ShardId shardId() { - return shardId; - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeOptionalString(failureReason); - out.writeInt(shardResponses.size()); - for (Map.Entry entry : shardResponses.entrySet()) { - entry.getKey().writeTo(out); - entry.getValue().writeTo(out); - } - out.writeOptionalString(syncId); - shardId.writeTo(out); - out.writeInt(totalShards); - } -} diff --git a/server/src/main/java/org/opensearch/indices/flush/SyncedFlushService.java b/server/src/main/java/org/opensearch/indices/flush/SyncedFlushService.java deleted file mode 100644 index 88c1fd03d9c54..0000000000000 --- a/server/src/main/java/org/opensearch/indices/flush/SyncedFlushService.java +++ /dev/null @@ -1,891 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -/* - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.indices.flush; - -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.apache.logging.log4j.message.ParameterizedMessage; -import org.opensearch.LegacyESVersion; -import org.opensearch.OpenSearchException; -import org.opensearch.action.ActionListener; -import org.opensearch.action.StepListener; -import org.opensearch.action.admin.indices.flush.FlushRequest; -import org.opensearch.action.admin.indices.flush.SyncedFlushResponse; -import org.opensearch.action.support.IndicesOptions; -import org.opensearch.cluster.ClusterState; -import org.opensearch.cluster.metadata.IndexMetadata; -import org.opensearch.cluster.metadata.IndexNameExpressionResolver; -import org.opensearch.cluster.node.DiscoveryNode; -import org.opensearch.cluster.routing.IndexShardRoutingTable; -import org.opensearch.cluster.routing.ShardRouting; -import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.Nullable; -import org.opensearch.common.Strings; -import org.opensearch.common.UUIDs; -import org.opensearch.common.inject.Inject; -import org.opensearch.common.io.stream.StreamInput; -import org.opensearch.common.io.stream.StreamOutput; -import org.opensearch.common.logging.DeprecationLogger; -import org.opensearch.common.util.concurrent.AbstractRunnable; -import org.opensearch.common.util.concurrent.ConcurrentCollections; -import org.opensearch.common.util.concurrent.CountDown; -import org.opensearch.index.Index; -import org.opensearch.index.IndexNotFoundException; -import org.opensearch.index.IndexService; -import org.opensearch.index.engine.CommitStats; -import org.opensearch.index.engine.Engine; -import org.opensearch.index.shard.IndexEventListener; -import org.opensearch.index.shard.IndexShard; -import org.opensearch.index.shard.IndexShardState; -import org.opensearch.index.shard.ShardId; -import org.opensearch.index.shard.ShardNotFoundException; -import org.opensearch.indices.IndexClosedException; -import org.opensearch.indices.IndicesService; -import org.opensearch.tasks.Task; -import org.opensearch.threadpool.ThreadPool; -import org.opensearch.transport.TransportChannel; -import org.opensearch.transport.TransportException; -import org.opensearch.transport.TransportRequest; -import org.opensearch.transport.TransportRequestHandler; -import org.opensearch.transport.TransportResponse; -import org.opensearch.transport.TransportResponseHandler; -import org.opensearch.transport.TransportService; - -import java.io.IOException; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.concurrent.ConcurrentMap; - -public class SyncedFlushService implements IndexEventListener { - - private static final Logger logger = LogManager.getLogger(SyncedFlushService.class); - - private static final DeprecationLogger DEPRECATION_LOGGER = DeprecationLogger.getLogger(logger.getName()); - - 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"; - - private final IndicesService indicesService; - private final ClusterService clusterService; - private final TransportService transportService; - private final IndexNameExpressionResolver indexNameExpressionResolver; - - @Inject - public SyncedFlushService( - IndicesService indicesService, - ClusterService clusterService, - TransportService transportService, - IndexNameExpressionResolver indexNameExpressionResolver - ) { - this.indicesService = indicesService; - this.clusterService = clusterService; - this.transportService = transportService; - this.indexNameExpressionResolver = indexNameExpressionResolver; - transportService.registerRequestHandler( - PRE_SYNCED_FLUSH_ACTION_NAME, - ThreadPool.Names.FLUSH, - PreShardSyncedFlushRequest::new, - new PreSyncedFlushTransportHandler() - ); - transportService.registerRequestHandler( - SYNCED_FLUSH_ACTION_NAME, - ThreadPool.Names.FLUSH, - ShardSyncedFlushRequest::new, - new SyncedFlushTransportHandler() - ); - transportService.registerRequestHandler( - IN_FLIGHT_OPS_ACTION_NAME, - ThreadPool.Names.SAME, - InFlightOpsRequest::new, - new InFlightOpCountTransportHandler() - ); - } - - @Override - public void onShardInactive(final IndexShard indexShard) { - // A normal flush has the same effect as a synced flush if all nodes are on 7.6 or later. - final boolean preferNormalFlush = clusterService.state().nodes().getMinNodeVersion().onOrAfter(LegacyESVersion.V_7_6_0); - if (preferNormalFlush) { - performNormalFlushOnInactive(indexShard); - } else if (indexShard.routingEntry().primary()) { - // we only want to call sync flush once, so only trigger it when we are on a primary - attemptSyncedFlush(indexShard.shardId(), new ActionListener() { - @Override - public void onResponse(ShardsSyncedFlushResult syncedFlushResult) { - logger.trace( - "{} sync flush on inactive shard returned successfully for sync_id: {}", - syncedFlushResult.getShardId(), - syncedFlushResult.syncId() - ); - } - - @Override - public void onFailure(Exception e) { - logger.debug(() -> new ParameterizedMessage("{} sync flush on inactive shard failed", indexShard.shardId()), e); - } - }); - } - } - - private void performNormalFlushOnInactive(IndexShard shard) { - logger.debug("flushing shard {} on inactive", shard.routingEntry()); - shard.getThreadPool().executor(ThreadPool.Names.FLUSH).execute(new AbstractRunnable() { - @Override - public void onFailure(Exception e) { - if (shard.state() != IndexShardState.CLOSED) { - logger.warn(new ParameterizedMessage("failed to flush shard {} on inactive", shard.routingEntry()), e); - } - } - - @Override - protected void doRun() { - shard.flush(new FlushRequest().force(false).waitIfOngoing(false)); - } - }); - } - - /** - * a utility method to perform a synced flush for all shards of multiple indices. - * see {@link #attemptSyncedFlush(ShardId, ActionListener)} - * for more details. - */ - public void attemptSyncedFlush( - final String[] aliasesOrIndices, - IndicesOptions indicesOptions, - final ActionListener listener - ) { - final ClusterState state = clusterService.state(); - if (state.nodes().getMinNodeVersion().onOrAfter(LegacyESVersion.V_7_6_0)) { - DEPRECATION_LOGGER.deprecate("synced_flush", SYNCED_FLUSH_DEPRECATION_MESSAGE); - } - final Index[] concreteIndices = indexNameExpressionResolver.concreteIndices(state, indicesOptions, aliasesOrIndices); - final Map> results = ConcurrentCollections.newConcurrentMap(); - int numberOfShards = 0; - for (Index index : concreteIndices) { - final IndexMetadata indexMetadata = state.metadata().getIndexSafe(index); - numberOfShards += indexMetadata.getNumberOfShards(); - results.put(index.getName(), Collections.synchronizedList(new ArrayList<>())); - - } - if (numberOfShards == 0) { - listener.onResponse(new SyncedFlushResponse(results)); - return; - } - final CountDown countDown = new CountDown(numberOfShards); - - for (final Index concreteIndex : concreteIndices) { - final String index = concreteIndex.getName(); - final IndexMetadata indexMetadata = state.metadata().getIndexSafe(concreteIndex); - final int indexNumberOfShards = indexMetadata.getNumberOfShards(); - for (int shard = 0; shard < indexNumberOfShards; shard++) { - final ShardId shardId = new ShardId(indexMetadata.getIndex(), shard); - innerAttemptSyncedFlush(shardId, state, new ActionListener() { - @Override - public void onResponse(ShardsSyncedFlushResult syncedFlushResult) { - results.get(index).add(syncedFlushResult); - if (countDown.countDown()) { - listener.onResponse(new SyncedFlushResponse(results)); - } - } - - @Override - public void onFailure(Exception e) { - logger.debug("{} unexpected error while executing synced flush", shardId); - final int totalShards = indexMetadata.getNumberOfReplicas() + 1; - results.get(index).add(new ShardsSyncedFlushResult(shardId, totalShards, e.getMessage())); - if (countDown.countDown()) { - listener.onResponse(new SyncedFlushResponse(results)); - } - } - }); - } - } - } - - /* - * Tries to flush all copies of a shard and write a sync id to it. - * After a synced flush two shard copies may only contain the same sync id if they contain the same documents. - * To ensure this, synced flush works in three steps: - * 1. Flush all shard copies and gather the commit ids for each copy after the flush - * 2. Ensure that there are no ongoing indexing operations on the primary - * 3. Perform an additional flush on each shard copy that writes the sync id - * - * Step 3 is only executed on a shard if - * a) the shard has no uncommitted changes since the last flush - * b) the last flush was the one executed in 1 (use the collected commit id to verify this) - * - * This alone is not enough to ensure that all copies contain the same documents. - * Without step 2 a sync id would be written for inconsistent copies in the following scenario: - * - * Write operation has completed on a primary and is being sent to replicas. The write request does not reach the - * replicas until sync flush is finished. - * Step 1 is executed. After the flush the commit points on primary contains a write operation that the replica does not have. - * Step 3 will be executed on primary and replica as well because there are no uncommitted changes on primary (the first flush - * committed them) and there are no uncommitted changes on the replica (the write operation has not reached the replica yet). - * - * Step 2 detects this scenario and fails the whole synced flush if a write operation is ongoing on the primary. - * Together with the conditions for step 3 (same commit id and no uncommitted changes) this guarantees that a snc id will only - * be written on a primary if no write operation was executed between step 1 and step 3 and sync id will only be written on - * the replica if it contains the same changes that the primary contains. - * - * Synced flush is a best effort operation. The sync id may be written on all, some or none of the copies. - **/ - public void attemptSyncedFlush(final ShardId shardId, final ActionListener actionListener) { - innerAttemptSyncedFlush(shardId, clusterService.state(), actionListener); - } - - private void innerAttemptSyncedFlush( - final ShardId shardId, - final ClusterState state, - final ActionListener actionListener - ) { - try { - final IndexShardRoutingTable shardRoutingTable = getShardRoutingTable(shardId, state); - final List activeShards = shardRoutingTable.activeShards(); - final int totalShards = shardRoutingTable.getSize(); - - if (activeShards.size() == 0) { - actionListener.onResponse(new ShardsSyncedFlushResult(shardId, totalShards, "no active shards")); - return; - } - - // 1. send pre-sync flushes to all replicas - final StepListener> presyncStep = new StepListener<>(); - sendPreSyncRequests(activeShards, state, shardId, presyncStep); - - // 2. fetch in flight operations - final StepListener inflightOpsStep = new StepListener<>(); - presyncStep.whenComplete(presyncResponses -> { - if (presyncResponses.isEmpty()) { - actionListener.onResponse(new ShardsSyncedFlushResult(shardId, totalShards, "all shards failed to commit on pre-sync")); - } else { - getInflightOpsCount(shardId, state, shardRoutingTable, inflightOpsStep); - } - }, actionListener::onFailure); - - // 3. now send the sync request to all the shards - inflightOpsStep.whenComplete(inFlightOpsResponse -> { - final Map presyncResponses = presyncStep.result(); - final int inflight = inFlightOpsResponse.opCount(); - assert inflight >= 0; - if (inflight != 0) { - actionListener.onResponse( - new ShardsSyncedFlushResult(shardId, totalShards, "[" + inflight + "] ongoing operations on primary") - ); - } else { - final String sharedSyncId = sharedExistingSyncId(presyncResponses); - if (sharedSyncId != null) { - assert presyncResponses.values() - .stream() - .allMatch(r -> r.existingSyncId.equals(sharedSyncId)) : "Not all shards have the same existing sync id [" - + sharedSyncId - + "], responses [" - + presyncResponses - + "]"; - reportSuccessWithExistingSyncId(shardId, sharedSyncId, activeShards, totalShards, presyncResponses, actionListener); - } else { - String syncId = UUIDs.randomBase64UUID(); - sendSyncRequests(syncId, activeShards, state, presyncResponses, shardId, totalShards, actionListener); - } - } - }, actionListener::onFailure); - } catch (Exception e) { - actionListener.onFailure(e); - } - } - - private String sharedExistingSyncId(Map preSyncedFlushResponses) { - String existingSyncId = null; - for (PreSyncedFlushResponse resp : preSyncedFlushResponses.values()) { - if (Strings.isNullOrEmpty(resp.existingSyncId)) { - return null; - } - if (existingSyncId == null) { - existingSyncId = resp.existingSyncId; - } - if (existingSyncId.equals(resp.existingSyncId) == false) { - return null; - } - } - return existingSyncId; - } - - private void reportSuccessWithExistingSyncId( - ShardId shardId, - String existingSyncId, - List shards, - int totalShards, - Map preSyncResponses, - ActionListener listener - ) { - final Map results = new HashMap<>(); - for (final ShardRouting shard : shards) { - if (preSyncResponses.containsKey(shard.currentNodeId())) { - results.put(shard, new ShardSyncedFlushResponse((String) null)); - } - } - listener.onResponse(new ShardsSyncedFlushResult(shardId, existingSyncId, totalShards, results)); - } - - final IndexShardRoutingTable getShardRoutingTable(final ShardId shardId, final ClusterState state) { - final IndexMetadata indexMetadata = state.getMetadata().index(shardId.getIndex()); - if (indexMetadata == null) { - throw new IndexNotFoundException(shardId.getIndexName()); - } else if (indexMetadata.getState() == IndexMetadata.State.CLOSE) { - throw new IndexClosedException(shardId.getIndex()); - } - final IndexShardRoutingTable shardRoutingTable = state.routingTable().index(indexMetadata.getIndex()).shard(shardId.id()); - if (shardRoutingTable == null) { - throw new ShardNotFoundException(shardId); - } - return shardRoutingTable; - } - - /** - * returns the number of in flight operations on primary. -1 upon error. - */ - protected void getInflightOpsCount( - final ShardId shardId, - ClusterState state, - IndexShardRoutingTable shardRoutingTable, - final ActionListener listener - ) { - try { - final ShardRouting primaryShard = shardRoutingTable.primaryShard(); - final DiscoveryNode primaryNode = state.nodes().get(primaryShard.currentNodeId()); - if (primaryNode == null) { - logger.trace("{} failed to resolve node for primary shard {}, skipping sync", shardId, primaryShard); - listener.onResponse(new InFlightOpsResponse(-1)); - return; - } - logger.trace("{} retrieving in flight operation count", shardId); - transportService.sendRequest( - primaryNode, - IN_FLIGHT_OPS_ACTION_NAME, - new InFlightOpsRequest(shardId), - new TransportResponseHandler() { - @Override - public InFlightOpsResponse read(StreamInput in) throws IOException { - return new InFlightOpsResponse(in); - } - - @Override - public void handleResponse(InFlightOpsResponse response) { - listener.onResponse(response); - } - - @Override - public void handleException(TransportException exp) { - logger.debug("{} unexpected error while retrieving in flight op count", shardId); - listener.onFailure(exp); - } - - @Override - public String executor() { - return ThreadPool.Names.SAME; - } - } - ); - } catch (Exception e) { - listener.onFailure(e); - } - } - - private int numDocsOnPrimary(List shards, Map preSyncResponses) { - for (ShardRouting shard : shards) { - if (shard.primary()) { - final PreSyncedFlushResponse resp = preSyncResponses.get(shard.currentNodeId()); - if (resp != null) { - return resp.numDocs; - } - } - } - return PreSyncedFlushResponse.UNKNOWN_NUM_DOCS; - } - - void sendSyncRequests( - final String syncId, - final List shards, - ClusterState state, - Map preSyncResponses, - final ShardId shardId, - final int totalShards, - final ActionListener listener - ) { - final CountDown countDown = new CountDown(shards.size()); - final Map results = ConcurrentCollections.newConcurrentMap(); - final int numDocsOnPrimary = numDocsOnPrimary(shards, preSyncResponses); - for (final ShardRouting shard : shards) { - final DiscoveryNode node = state.nodes().get(shard.currentNodeId()); - if (node == null) { - logger.trace("{} is assigned to an unknown node. skipping for sync id [{}]. shard routing {}", shardId, syncId, shard); - results.put(shard, new ShardSyncedFlushResponse("unknown node")); - countDownAndSendResponseIfDone(syncId, shards, shardId, totalShards, listener, countDown, results); - continue; - } - final PreSyncedFlushResponse preSyncedResponse = preSyncResponses.get(shard.currentNodeId()); - if (preSyncedResponse == null) { - logger.trace( - "{} can't resolve expected commit id for current node, skipping for sync id [{}]. shard routing {}", - shardId, - syncId, - shard - ); - results.put(shard, new ShardSyncedFlushResponse("no commit id from pre-sync flush")); - countDownAndSendResponseIfDone(syncId, shards, shardId, totalShards, listener, countDown, results); - continue; - } - if (preSyncedResponse.numDocs != numDocsOnPrimary - && preSyncedResponse.numDocs != PreSyncedFlushResponse.UNKNOWN_NUM_DOCS - && numDocsOnPrimary != PreSyncedFlushResponse.UNKNOWN_NUM_DOCS) { - logger.debug( - "{} can't issue sync id [{}] for replica [{}] with num docs [{}]; num docs on primary [{}]", - shardId, - syncId, - shard, - preSyncedResponse.numDocs, - numDocsOnPrimary - ); - results.put( - shard, - new ShardSyncedFlushResponse( - "ongoing indexing operations: " - + "num docs on replica [" - + preSyncedResponse.numDocs - + "]; num docs on primary [" - + numDocsOnPrimary - + "]" - ) - ); - countDownAndSendResponseIfDone(syncId, shards, shardId, totalShards, listener, countDown, results); - continue; - } - logger.trace("{} sending synced flush request to {}. sync id [{}].", shardId, shard, syncId); - ShardSyncedFlushRequest syncedFlushRequest = new ShardSyncedFlushRequest(shard.shardId(), syncId, preSyncedResponse.commitId); - transportService.sendRequest( - node, - SYNCED_FLUSH_ACTION_NAME, - syncedFlushRequest, - new TransportResponseHandler() { - @Override - public ShardSyncedFlushResponse read(StreamInput in) throws IOException { - return new ShardSyncedFlushResponse(in); - } - - @Override - public void handleResponse(ShardSyncedFlushResponse response) { - ShardSyncedFlushResponse existing = results.put(shard, response); - assert existing == null : "got two answers for node [" + node + "]"; - // count after the assert so we won't decrement twice in handleException - countDownAndSendResponseIfDone(syncId, shards, shardId, totalShards, listener, countDown, results); - } - - @Override - public void handleException(TransportException exp) { - logger.trace( - () -> new ParameterizedMessage("{} error while performing synced flush on [{}], skipping", shardId, shard), - exp - ); - results.put(shard, new ShardSyncedFlushResponse(exp.getMessage())); - countDownAndSendResponseIfDone(syncId, shards, shardId, totalShards, listener, countDown, results); - } - - @Override - public String executor() { - return ThreadPool.Names.SAME; - } - } - ); - } - - } - - private void countDownAndSendResponseIfDone( - String syncId, - List shards, - ShardId shardId, - int totalShards, - ActionListener listener, - CountDown countDown, - Map results - ) { - if (countDown.countDown()) { - assert results.size() == shards.size(); - listener.onResponse(new ShardsSyncedFlushResult(shardId, syncId, totalShards, results)); - } - } - - /** - * send presync requests to all started copies of the given shard - */ - void sendPreSyncRequests( - final List shards, - final ClusterState state, - final ShardId shardId, - final ActionListener> listener - ) { - final CountDown countDown = new CountDown(shards.size()); - final ConcurrentMap presyncResponses = ConcurrentCollections.newConcurrentMap(); - for (final ShardRouting shard : shards) { - logger.trace("{} sending pre-synced flush request to {}", shardId, shard); - final DiscoveryNode node = state.nodes().get(shard.currentNodeId()); - if (node == null) { - logger.trace("{} shard routing {} refers to an unknown node. skipping.", shardId, shard); - if (countDown.countDown()) { - listener.onResponse(presyncResponses); - } - continue; - } - transportService.sendRequest( - node, - PRE_SYNCED_FLUSH_ACTION_NAME, - new PreShardSyncedFlushRequest(shard.shardId()), - new TransportResponseHandler() { - @Override - public PreSyncedFlushResponse read(StreamInput in) throws IOException { - return new PreSyncedFlushResponse(in); - } - - @Override - public void handleResponse(PreSyncedFlushResponse response) { - PreSyncedFlushResponse existing = presyncResponses.putIfAbsent(node.getId(), response); - assert existing == null : "got two answers for node [" + node + "]"; - // count after the assert so we won't decrement twice in handleException - if (countDown.countDown()) { - listener.onResponse(presyncResponses); - } - } - - @Override - public void handleException(TransportException exp) { - logger.trace( - () -> new ParameterizedMessage("{} error while performing pre synced flush on [{}], skipping", shardId, shard), - exp - ); - if (countDown.countDown()) { - listener.onResponse(presyncResponses); - } - } - - @Override - public String executor() { - return ThreadPool.Names.SAME; - } - } - ); - } - } - - private PreSyncedFlushResponse performPreSyncedFlush(PreShardSyncedFlushRequest request) { - IndexShard indexShard = indicesService.indexServiceSafe(request.shardId().getIndex()).getShard(request.shardId().id()); - FlushRequest flushRequest = new FlushRequest().force(false).waitIfOngoing(true); - logger.trace("{} performing pre sync flush", request.shardId()); - indexShard.flush(flushRequest); - final CommitStats commitStats = indexShard.commitStats(); - final Engine.CommitId commitId = commitStats.getRawCommitId(); - logger.trace("{} pre sync flush done. commit id {}, num docs {}", request.shardId(), commitId, commitStats.getNumDocs()); - return new PreSyncedFlushResponse(commitId, commitStats.getNumDocs(), commitStats.syncId()); - } - - private ShardSyncedFlushResponse performSyncedFlush(ShardSyncedFlushRequest request) { - IndexService indexService = indicesService.indexServiceSafe(request.shardId().getIndex()); - IndexShard indexShard = indexService.getShard(request.shardId().id()); - logger.trace( - "{} performing sync flush. sync id [{}], expected commit id {}", - request.shardId(), - request.syncId(), - request.expectedCommitId() - ); - Engine.SyncedFlushResult result = indexShard.syncFlush(request.syncId(), request.expectedCommitId()); - logger.trace("{} sync flush done. sync id [{}], result [{}]", request.shardId(), request.syncId(), result); - switch (result) { - case SUCCESS: - return new ShardSyncedFlushResponse((String) null); - case COMMIT_MISMATCH: - return new ShardSyncedFlushResponse("commit has changed"); - case PENDING_OPERATIONS: - return new ShardSyncedFlushResponse("pending operations"); - default: - throw new OpenSearchException("unknown synced flush result [" + result + "]"); - } - } - - private InFlightOpsResponse performInFlightOps(InFlightOpsRequest request) { - IndexService indexService = indicesService.indexServiceSafe(request.shardId().getIndex()); - IndexShard indexShard = indexService.getShard(request.shardId().id()); - if (indexShard.routingEntry().primary() == false) { - throw new IllegalStateException("[" + request.shardId() + "] expected a primary shard"); - } - int opCount = indexShard.getActiveOperationsCount(); - return new InFlightOpsResponse(opCount == IndexShard.OPERATIONS_BLOCKED ? 0 : opCount); - } - - public static final class PreShardSyncedFlushRequest extends TransportRequest { - private ShardId shardId; - - public PreShardSyncedFlushRequest(StreamInput in) throws IOException { - super(in); - this.shardId = new ShardId(in); - } - - public PreShardSyncedFlushRequest(ShardId shardId) { - this.shardId = shardId; - } - - @Override - public String toString() { - return "PreShardSyncedFlushRequest{" + "shardId=" + shardId + '}'; - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - shardId.writeTo(out); - } - - public ShardId shardId() { - return shardId; - } - } - - /** - * Response for first step of synced flush (flush) for one shard copy - */ - static final class PreSyncedFlushResponse extends TransportResponse { - static final int UNKNOWN_NUM_DOCS = -1; - - Engine.CommitId commitId; - int numDocs; - @Nullable - String existingSyncId = null; - - PreSyncedFlushResponse(StreamInput in) throws IOException { - super(in); - commitId = new Engine.CommitId(in); - numDocs = in.readInt(); - existingSyncId = in.readOptionalString(); - } - - PreSyncedFlushResponse(Engine.CommitId commitId, int numDocs, String existingSyncId) { - this.commitId = commitId; - this.numDocs = numDocs; - this.existingSyncId = existingSyncId; - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - commitId.writeTo(out); - out.writeInt(numDocs); - out.writeOptionalString(existingSyncId); - } - } - - public static final class ShardSyncedFlushRequest extends TransportRequest { - - private String syncId; - private Engine.CommitId expectedCommitId; - private ShardId shardId; - - public ShardSyncedFlushRequest(StreamInput in) throws IOException { - super(in); - shardId = new ShardId(in); - expectedCommitId = new Engine.CommitId(in); - syncId = in.readString(); - } - - public ShardSyncedFlushRequest(ShardId shardId, String syncId, Engine.CommitId expectedCommitId) { - this.expectedCommitId = expectedCommitId; - this.shardId = shardId; - this.syncId = syncId; - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - shardId.writeTo(out); - expectedCommitId.writeTo(out); - out.writeString(syncId); - } - - public ShardId shardId() { - return shardId; - } - - public String syncId() { - return syncId; - } - - public Engine.CommitId expectedCommitId() { - return expectedCommitId; - } - - @Override - public String toString() { - return "ShardSyncedFlushRequest{" + "shardId=" + shardId + ",syncId='" + syncId + '\'' + '}'; - } - } - - /** - * Response for third step of synced flush (writing the sync id) for one shard copy - */ - public static final class ShardSyncedFlushResponse extends TransportResponse { - - /** - * a non null value indicates a failure to sync flush. null means success - */ - String failureReason; - - public ShardSyncedFlushResponse(StreamInput in) throws IOException { - super(in); - failureReason = in.readOptionalString(); - } - - public ShardSyncedFlushResponse(String failureReason) { - this.failureReason = failureReason; - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeOptionalString(failureReason); - } - - public boolean success() { - return failureReason == null; - } - - public String failureReason() { - return failureReason; - } - - @Override - public String toString() { - return "ShardSyncedFlushResponse{" + "success=" + success() + ", failureReason='" + failureReason + '\'' + '}'; - } - - public static ShardSyncedFlushResponse readSyncedFlushResponse(StreamInput in) throws IOException { - return new ShardSyncedFlushResponse(in); - } - } - - public static final class InFlightOpsRequest extends TransportRequest { - - private ShardId shardId; - - public InFlightOpsRequest(StreamInput in) throws IOException { - super(in); - shardId = new ShardId(in); - } - - public InFlightOpsRequest(ShardId shardId) { - this.shardId = shardId; - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - shardId.writeTo(out); - } - - public ShardId shardId() { - return shardId; - } - - @Override - public String toString() { - return "InFlightOpsRequest{" + "shardId=" + shardId + '}'; - } - } - - /** - * Response for second step of synced flush (check operations in flight) - */ - static final class InFlightOpsResponse extends TransportResponse { - - int opCount; - - InFlightOpsResponse(StreamInput in) throws IOException { - super(in); - opCount = in.readVInt(); - } - - InFlightOpsResponse(int opCount) { - assert opCount >= 0 : opCount; - this.opCount = opCount; - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeVInt(opCount); - } - - public int opCount() { - return opCount; - } - - @Override - public String toString() { - return "InFlightOpsResponse{" + "opCount=" + opCount + '}'; - } - } - - private final class PreSyncedFlushTransportHandler implements TransportRequestHandler { - - @Override - public void messageReceived(PreShardSyncedFlushRequest request, TransportChannel channel, Task task) throws Exception { - channel.sendResponse(performPreSyncedFlush(request)); - } - } - - private final class SyncedFlushTransportHandler implements TransportRequestHandler { - - @Override - public void messageReceived(ShardSyncedFlushRequest request, TransportChannel channel, Task task) throws Exception { - channel.sendResponse(performSyncedFlush(request)); - } - } - - private final class InFlightOpCountTransportHandler implements TransportRequestHandler { - - @Override - public void messageReceived(InFlightOpsRequest request, TransportChannel channel, Task task) throws Exception { - channel.sendResponse(performInFlightOps(request)); - } - } - -} diff --git a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestSyncedFlushAction.java b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestSyncedFlushAction.java index ce5b816428e5f..726fd69dc29c1 100644 --- a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestSyncedFlushAction.java +++ b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestSyncedFlushAction.java @@ -32,17 +32,20 @@ package org.opensearch.rest.action.admin.indices; -import org.opensearch.action.admin.indices.flush.SyncedFlushRequest; -import org.opensearch.action.admin.indices.flush.SyncedFlushResponse; +import org.opensearch.action.admin.indices.flush.FlushRequest; +import org.opensearch.action.admin.indices.flush.FlushResponse; import org.opensearch.action.support.IndicesOptions; import org.opensearch.client.node.NodeClient; import org.opensearch.common.Strings; +import org.opensearch.common.logging.DeprecationLogger; import org.opensearch.common.xcontent.XContentBuilder; import org.opensearch.rest.BaseRestHandler; import org.opensearch.rest.BytesRestResponse; +import org.opensearch.rest.RestChannel; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestResponse; -import org.opensearch.rest.action.RestBuilderListener; +import org.opensearch.rest.RestStatus; +import org.opensearch.rest.action.RestToXContentListener; import java.io.IOException; import java.util.List; @@ -54,6 +57,8 @@ public class RestSyncedFlushAction extends BaseRestHandler { + private static final DeprecationLogger DEPRECATION_LOGGER = DeprecationLogger.getLogger(RestSyncedFlushAction.class); + @Override public List routes() { return unmodifiableList( @@ -73,17 +78,37 @@ public String getName() { @Override public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { - IndicesOptions indicesOptions = IndicesOptions.fromRequest(request, IndicesOptions.lenientExpandOpen()); - SyncedFlushRequest syncedFlushRequest = new SyncedFlushRequest(Strings.splitStringByCommaToArray(request.param("index"))); - syncedFlushRequest.indicesOptions(indicesOptions); - return channel -> client.admin().indices().syncedFlush(syncedFlushRequest, new RestBuilderListener(channel) { - @Override - public RestResponse buildResponse(SyncedFlushResponse results, XContentBuilder builder) throws Exception { - builder.startObject(); - results.toXContent(builder, request); - builder.endObject(); - return new BytesRestResponse(results.restStatus(), builder); - } - }); + DEPRECATION_LOGGER.deprecate( + "synced_flush", + "Synced flush was removed and a normal flush was performed instead. This transition will be removed in a future version." + ); + final FlushRequest flushRequest = new FlushRequest(Strings.splitStringByCommaToArray(request.param("index"))); + flushRequest.indicesOptions(IndicesOptions.fromRequest(request, flushRequest.indicesOptions())); + return channel -> client.admin().indices().flush(flushRequest, new SimulateSyncedFlushResponseListener(channel)); + } + + static final class SimulateSyncedFlushResponseListener extends RestToXContentListener { + + SimulateSyncedFlushResponseListener(RestChannel channel) { + super(channel); + } + + @Override + public RestResponse buildResponse(FlushResponse flushResponse, XContentBuilder builder) throws Exception { + builder.startObject(); + buildSyncedFlushResponse(builder, flushResponse); + builder.endObject(); + final RestStatus restStatus = flushResponse.getFailedShards() == 0 ? RestStatus.OK : RestStatus.CONFLICT; + return new BytesRestResponse(restStatus, builder); + } + + private void buildSyncedFlushResponse(XContentBuilder builder, FlushResponse flushResponse) throws IOException { + builder.startObject("_shards"); + builder.field("total", flushResponse.getTotalShards()); + builder.field("successful", flushResponse.getSuccessfulShards()); + builder.field("failed", flushResponse.getFailedShards()); + // can't serialize the detail of each index as we don't have the shard count per index. + builder.endObject(); + } } } diff --git a/server/src/test/java/org/opensearch/action/admin/indices/close/TransportVerifyShardBeforeCloseActionTests.java b/server/src/test/java/org/opensearch/action/admin/indices/close/TransportVerifyShardBeforeCloseActionTests.java index 43d5a85094a36..72c7b5168fe15 100644 --- a/server/src/test/java/org/opensearch/action/admin/indices/close/TransportVerifyShardBeforeCloseActionTests.java +++ b/server/src/test/java/org/opensearch/action/admin/indices/close/TransportVerifyShardBeforeCloseActionTests.java @@ -53,7 +53,6 @@ import org.opensearch.cluster.routing.ShardRoutingState; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.Settings; -import org.opensearch.index.engine.Engine; import org.opensearch.common.unit.TimeValue; import org.opensearch.index.shard.IndexShard; import org.opensearch.index.shard.ReplicationGroup; @@ -78,6 +77,7 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; +import static org.mockito.Mockito.doNothing; import static org.opensearch.action.support.replication.ClusterStateCreationUtils.state; import static org.opensearch.test.ClusterServiceUtils.createClusterService; import static org.opensearch.test.ClusterServiceUtils.setState; @@ -194,8 +194,7 @@ private void executeOnPrimaryOrReplica(boolean phase1) throws Throwable { public void testShardIsFlushed() throws Throwable { final ArgumentCaptor flushRequest = ArgumentCaptor.forClass(FlushRequest.class); - when(indexShard.flush(flushRequest.capture())).thenReturn(new Engine.CommitId(new byte[0])); - + doNothing().when(indexShard).flush(flushRequest.capture()); executeOnPrimaryOrReplica(); verify(indexShard, times(1)).flush(any(FlushRequest.class)); assertThat(flushRequest.getValue().force(), is(true)); diff --git a/server/src/test/java/org/opensearch/action/admin/indices/flush/SyncedFlushUnitTests.java b/server/src/test/java/org/opensearch/action/admin/indices/flush/SyncedFlushUnitTests.java deleted file mode 100644 index a2f85228024ea..0000000000000 --- a/server/src/test/java/org/opensearch/action/admin/indices/flush/SyncedFlushUnitTests.java +++ /dev/null @@ -1,208 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -/* - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.action.admin.indices.flush; - -import com.carrotsearch.hppc.ObjectIntHashMap; -import com.carrotsearch.hppc.ObjectIntMap; -import org.opensearch.action.admin.indices.flush.SyncedFlushResponse.ShardCounts; -import org.opensearch.cluster.routing.ShardRouting; -import org.opensearch.cluster.routing.ShardRoutingState; -import org.opensearch.cluster.routing.TestShardRouting; -import org.opensearch.common.io.stream.BytesStreamOutput; -import org.opensearch.common.io.stream.StreamInput; -import org.opensearch.index.shard.ShardId; -import org.opensearch.indices.flush.ShardsSyncedFlushResult; -import org.opensearch.indices.flush.SyncedFlushService; -import org.opensearch.rest.RestStatus; -import org.opensearch.test.OpenSearchTestCase; - -import java.io.IOException; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -import static org.opensearch.test.XContentTestUtils.convertToMap; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.hasSize; - -public class SyncedFlushUnitTests extends OpenSearchTestCase { - - private static class TestPlan { - public SyncedFlushResponse.ShardCounts totalCounts; - public Map countsPerIndex = new HashMap<>(); - public ObjectIntMap expectedFailuresPerIndex = new ObjectIntHashMap<>(); - public SyncedFlushResponse result; - } - - public void testIndicesSyncedFlushResult() throws IOException { - final TestPlan testPlan = createTestPlan(); - assertThat(testPlan.result.totalShards(), equalTo(testPlan.totalCounts.total)); - assertThat(testPlan.result.successfulShards(), equalTo(testPlan.totalCounts.successful)); - assertThat(testPlan.result.failedShards(), equalTo(testPlan.totalCounts.failed)); - assertThat(testPlan.result.restStatus(), equalTo(testPlan.totalCounts.failed > 0 ? RestStatus.CONFLICT : RestStatus.OK)); - Map asMap = convertToMap(testPlan.result); - assertShardCount("_shards header", (Map) asMap.get("_shards"), testPlan.totalCounts); - - assertThat("unexpected number of indices", asMap.size(), equalTo(1 + testPlan.countsPerIndex.size())); // +1 for the shards header - for (String index : testPlan.countsPerIndex.keySet()) { - Map indexMap = (Map) asMap.get(index); - assertShardCount(index, indexMap, testPlan.countsPerIndex.get(index)); - List> failureList = (List>) indexMap.get("failures"); - final int expectedFailures = testPlan.expectedFailuresPerIndex.get(index); - if (expectedFailures == 0) { - assertNull(index + " has unexpected failures", failureList); - } else { - assertNotNull(index + " should have failures", failureList); - assertThat(failureList, hasSize(expectedFailures)); - } - } - } - - public void testResponseStreaming() throws IOException { - final TestPlan testPlan = createTestPlan(); - assertThat(testPlan.result.totalShards(), equalTo(testPlan.totalCounts.total)); - assertThat(testPlan.result.successfulShards(), equalTo(testPlan.totalCounts.successful)); - assertThat(testPlan.result.failedShards(), equalTo(testPlan.totalCounts.failed)); - assertThat(testPlan.result.restStatus(), equalTo(testPlan.totalCounts.failed > 0 ? RestStatus.CONFLICT : RestStatus.OK)); - BytesStreamOutput out = new BytesStreamOutput(); - testPlan.result.writeTo(out); - StreamInput in = out.bytes().streamInput(); - SyncedFlushResponse readResponse = new SyncedFlushResponse(in); - assertThat(readResponse.totalShards(), equalTo(testPlan.totalCounts.total)); - assertThat(readResponse.successfulShards(), equalTo(testPlan.totalCounts.successful)); - assertThat(readResponse.failedShards(), equalTo(testPlan.totalCounts.failed)); - assertThat(readResponse.restStatus(), equalTo(testPlan.totalCounts.failed > 0 ? RestStatus.CONFLICT : RestStatus.OK)); - assertThat(readResponse.getShardsResultPerIndex().size(), equalTo(testPlan.result.getShardsResultPerIndex().size())); - for (Map.Entry> entry : readResponse.getShardsResultPerIndex().entrySet()) { - List originalShardsResults = testPlan.result.getShardsResultPerIndex().get(entry.getKey()); - assertNotNull(originalShardsResults); - List readShardsResults = entry.getValue(); - assertThat(readShardsResults.size(), equalTo(originalShardsResults.size())); - for (int i = 0; i < readShardsResults.size(); i++) { - ShardsSyncedFlushResult originalShardResult = originalShardsResults.get(i); - ShardsSyncedFlushResult readShardResult = readShardsResults.get(i); - assertThat(originalShardResult.failureReason(), equalTo(readShardResult.failureReason())); - assertThat(originalShardResult.failed(), equalTo(readShardResult.failed())); - assertThat(originalShardResult.getShardId(), equalTo(readShardResult.getShardId())); - assertThat(originalShardResult.successfulShards(), equalTo(readShardResult.successfulShards())); - assertThat(originalShardResult.syncId(), equalTo(readShardResult.syncId())); - assertThat(originalShardResult.totalShards(), equalTo(readShardResult.totalShards())); - assertThat(originalShardResult.failedShards().size(), equalTo(readShardResult.failedShards().size())); - for (Map.Entry shardEntry : originalShardResult.failedShards() - .entrySet()) { - SyncedFlushService.ShardSyncedFlushResponse readShardResponse = readShardResult.failedShards().get(shardEntry.getKey()); - assertNotNull(readShardResponse); - SyncedFlushService.ShardSyncedFlushResponse originalShardResponse = shardEntry.getValue(); - assertThat(originalShardResponse.failureReason(), equalTo(readShardResponse.failureReason())); - assertThat(originalShardResponse.success(), equalTo(readShardResponse.success())); - } - assertThat(originalShardResult.shardResponses().size(), equalTo(readShardResult.shardResponses().size())); - for (Map.Entry shardEntry : originalShardResult.shardResponses() - .entrySet()) { - SyncedFlushService.ShardSyncedFlushResponse readShardResponse = readShardResult.shardResponses() - .get(shardEntry.getKey()); - assertNotNull(readShardResponse); - SyncedFlushService.ShardSyncedFlushResponse originalShardResponse = shardEntry.getValue(); - assertThat(originalShardResponse.failureReason(), equalTo(readShardResponse.failureReason())); - assertThat(originalShardResponse.success(), equalTo(readShardResponse.success())); - } - } - } - } - - private void assertShardCount(String name, Map header, ShardCounts expectedCounts) { - assertThat(name + " has unexpected total count", (Integer) header.get("total"), equalTo(expectedCounts.total)); - assertThat(name + " has unexpected successful count", (Integer) header.get("successful"), equalTo(expectedCounts.successful)); - assertThat(name + " has unexpected failed count", (Integer) header.get("failed"), equalTo(expectedCounts.failed)); - } - - protected TestPlan createTestPlan() { - final TestPlan testPlan = new TestPlan(); - final Map> indicesResults = new HashMap<>(); - final int indexCount = randomIntBetween(1, 10); - int totalShards = 0; - int totalSuccesful = 0; - int totalFailed = 0; - for (int i = 0; i < indexCount; i++) { - final String index = "index_" + i; - int shards = randomIntBetween(1, 4); - int replicas = randomIntBetween(0, 2); - int successful = 0; - int failed = 0; - int failures = 0; - List shardsResults = new ArrayList<>(); - for (int shard = 0; shard < shards; shard++) { - final ShardId shardId = new ShardId(index, "_na_", shard); - if (randomInt(5) < 2) { - // total shard failure - failed += replicas + 1; - failures++; - shardsResults.add(new ShardsSyncedFlushResult(shardId, replicas + 1, "simulated total failure")); - } else { - Map shardResponses = new HashMap<>(); - for (int copy = 0; copy < replicas + 1; copy++) { - final ShardRouting shardRouting = TestShardRouting.newShardRouting( - index, - shard, - "node_" + shardId + "_" + copy, - null, - copy == 0, - ShardRoutingState.STARTED - ); - if (randomInt(5) < 2) { - // shard copy failure - failed++; - failures++; - shardResponses.put(shardRouting, new SyncedFlushService.ShardSyncedFlushResponse("copy failure " + shardId)); - } else { - successful++; - shardResponses.put(shardRouting, new SyncedFlushService.ShardSyncedFlushResponse((String) null)); - } - } - shardsResults.add(new ShardsSyncedFlushResult(shardId, "_sync_id_" + shard, replicas + 1, shardResponses)); - } - } - indicesResults.put(index, shardsResults); - testPlan.countsPerIndex.put(index, new SyncedFlushResponse.ShardCounts(shards * (replicas + 1), successful, failed)); - testPlan.expectedFailuresPerIndex.put(index, failures); - totalFailed += failed; - totalShards += shards * (replicas + 1); - totalSuccesful += successful; - } - testPlan.result = new SyncedFlushResponse(indicesResults); - testPlan.totalCounts = new SyncedFlushResponse.ShardCounts(totalShards, totalSuccesful, totalFailed); - return testPlan; - } - -} diff --git a/server/src/test/java/org/opensearch/index/engine/InternalEngineTests.java b/server/src/test/java/org/opensearch/index/engine/InternalEngineTests.java index 85f325fbee25a..1296532b0ba0d 100644 --- a/server/src/test/java/org/opensearch/index/engine/InternalEngineTests.java +++ b/server/src/test/java/org/opensearch/index/engine/InternalEngineTests.java @@ -82,6 +82,7 @@ import org.apache.lucene.util.Bits; import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.FixedBitSet; +import org.apache.lucene.util.SetOnce; import org.opensearch.OpenSearchException; import org.opensearch.Version; import org.opensearch.action.ActionListener; @@ -115,6 +116,7 @@ import org.opensearch.common.util.BigArrays; import org.opensearch.common.util.concurrent.AbstractRunnable; import org.opensearch.common.util.concurrent.ConcurrentCollections; +import org.opensearch.common.util.concurrent.ReleasableLock; import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.internal.io.IOUtils; import org.opensearch.index.IndexSettings; @@ -165,7 +167,6 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; -import java.util.Base64; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; @@ -548,10 +549,9 @@ public long getProcessedCheckpoint() { : randomIntBetween(0, (int) localCheckpoint.get()) ); - final Engine.CommitId commitId = engine.flush(true, true); + engine.flush(true, true); CommitStats stats2 = engine.commitStats(); - assertThat(stats2.getRawCommitId(), equalTo(commitId)); assertThat(stats2.getGeneration(), greaterThan(stats1.getGeneration())); assertThat(stats2.getId(), notNullValue()); assertThat(stats2.getId(), not(equalTo(stats1.getId()))); @@ -660,9 +660,9 @@ public void testTranslogRecoveryDoesNotReplayIntoTranslog() throws IOException { recoveringEngine = new InternalEngine(initialEngine.config()) { @Override - protected void commitIndexWriter(IndexWriter writer, Translog translog, String syncId) throws IOException { + protected void commitIndexWriter(IndexWriter writer, Translog translog) throws IOException { committed.set(true); - super.commitIndexWriter(writer, translog, syncId); + super.commitIndexWriter(writer, translog); } }; assertThat(getTranslog(recoveringEngine).stats().getUncommittedOperations(), equalTo(docs)); @@ -1116,137 +1116,31 @@ public void testSyncTranslogConcurrently() throws Exception { checker.run(); } - public void testSyncedFlush() throws IOException { - try ( - Store store = createStore(); - Engine engine = createEngine(defaultSettings, store, createTempDir(), new LogByteSizeMergePolicy(), null) - ) { - final String syncId = randomUnicodeOfCodepointLengthBetween(10, 20); - ParsedDocument doc = testParsedDocument("1", null, testDocumentWithTextField(), B_1, null); - engine.index(indexForDoc(doc)); - Engine.CommitId commitID = engine.flush(); - assertThat(commitID, equalTo(new Engine.CommitId(store.readLastCommittedSegmentsInfo().getId()))); - byte[] wrongBytes = Base64.getDecoder().decode(commitID.toString()); - wrongBytes[0] = (byte) ~wrongBytes[0]; - Engine.CommitId wrongId = new Engine.CommitId(wrongBytes); - assertEquals( - "should fail to sync flush with wrong id (but no docs)", - engine.syncFlush(syncId + "1", wrongId), - Engine.SyncedFlushResult.COMMIT_MISMATCH - ); - engine.index(indexForDoc(doc)); - assertEquals( - "should fail to sync flush with right id but pending doc", - engine.syncFlush(syncId + "2", commitID), - Engine.SyncedFlushResult.PENDING_OPERATIONS - ); - commitID = engine.flush(); - assertEquals( - "should succeed to flush commit with right id and no pending doc", - engine.syncFlush(syncId, commitID), - Engine.SyncedFlushResult.SUCCESS - ); - assertEquals(store.readLastCommittedSegmentsInfo().getUserData().get(Engine.SYNC_COMMIT_ID), syncId); - assertEquals(engine.getLastCommittedSegmentInfos().getUserData().get(Engine.SYNC_COMMIT_ID), syncId); - } - } - - public void testRenewSyncFlush() throws Exception { - final int iters = randomIntBetween(2, 5); // run this a couple of times to get some coverage - for (int i = 0; i < iters; i++) { - try ( - Store store = createStore(); - InternalEngine engine = createEngine(config(defaultSettings, store, createTempDir(), new LogDocMergePolicy(), null)) - ) { - final String syncId = randomUnicodeOfCodepointLengthBetween(10, 20); - Engine.Index doc1 = indexForDoc(testParsedDocument("1", null, testDocumentWithTextField(), B_1, null)); - engine.index(doc1); - assertEquals(engine.getLastWriteNanos(), doc1.startTime()); - engine.flush(); - Engine.Index doc2 = indexForDoc(testParsedDocument("2", null, testDocumentWithTextField(), B_1, null)); - engine.index(doc2); - assertEquals(engine.getLastWriteNanos(), doc2.startTime()); - engine.flush(); - final boolean forceMergeFlushes = randomBoolean(); - final ParsedDocument parsedDoc3 = testParsedDocument("3", null, testDocumentWithTextField(), B_1, null); - if (forceMergeFlushes) { - engine.index( - new Engine.Index( - newUid(parsedDoc3), - parsedDoc3, - UNASSIGNED_SEQ_NO, - 0, - Versions.MATCH_ANY, - VersionType.INTERNAL, - Engine.Operation.Origin.PRIMARY, - System.nanoTime() - engine.engineConfig.getFlushMergesAfter().nanos(), - -1, - false, - UNASSIGNED_SEQ_NO, - 0 - ) - ); - } else { - engine.index(indexForDoc(parsedDoc3)); - } - Engine.CommitId commitID = engine.flush(); - assertEquals( - "should succeed to flush commit with right id and no pending doc", - engine.syncFlush(syncId, commitID), - Engine.SyncedFlushResult.SUCCESS - ); - assertEquals(3, engine.segments(false).size()); - - engine.forceMerge(forceMergeFlushes, 1, false, false, false, UUIDs.randomBase64UUID()); - if (forceMergeFlushes == false) { - engine.refresh("make all segments visible"); - assertEquals(4, engine.segments(false).size()); - assertEquals(store.readLastCommittedSegmentsInfo().getUserData().get(Engine.SYNC_COMMIT_ID), syncId); - assertEquals(engine.getLastCommittedSegmentInfos().getUserData().get(Engine.SYNC_COMMIT_ID), syncId); - assertTrue(engine.tryRenewSyncCommit()); - assertEquals(1, engine.segments(false).size()); - } else { - engine.refresh("test"); - assertBusy(() -> assertEquals(1, engine.segments(false).size())); - } - assertEquals(store.readLastCommittedSegmentsInfo().getUserData().get(Engine.SYNC_COMMIT_ID), syncId); - assertEquals(engine.getLastCommittedSegmentInfos().getUserData().get(Engine.SYNC_COMMIT_ID), syncId); - - if (randomBoolean()) { - Engine.Index doc4 = indexForDoc(testParsedDocument("4", null, testDocumentWithTextField(), B_1, null)); - engine.index(doc4); - assertEquals(engine.getLastWriteNanos(), doc4.startTime()); - } else { - Engine.Delete delete = new Engine.Delete(doc1.type(), doc1.id(), doc1.uid(), primaryTerm.get()); - engine.delete(delete); - assertEquals(engine.getLastWriteNanos(), delete.startTime()); - } - assertFalse(engine.tryRenewSyncCommit()); - // we might hit a concurrent flush from a finishing merge here - just wait if ongoing... - engine.flush(false, true); - assertNull(store.readLastCommittedSegmentsInfo().getUserData().get(Engine.SYNC_COMMIT_ID)); - assertNull(engine.getLastCommittedSegmentInfos().getUserData().get(Engine.SYNC_COMMIT_ID)); - } - } - } - public void testSyncedFlushSurvivesEngineRestart() throws IOException { final AtomicLong globalCheckpoint = new AtomicLong(SequenceNumbers.NO_OPS_PERFORMED); IOUtils.close(store, engine); + SetOnce indexWriterHolder = new SetOnce<>(); + IndexWriterFactory indexWriterFactory = (directory, iwc) -> { + indexWriterHolder.set(new IndexWriter(directory, iwc)); + return indexWriterHolder.get(); + }; store = createStore(); - engine = createEngine(store, primaryTranslogDir, globalCheckpoint::get); + engine = createEngine( + defaultSettings, + store, + primaryTranslogDir, + newMergePolicy(), + indexWriterFactory, + null, + globalCheckpoint::get + ); final String syncId = randomUnicodeOfCodepointLengthBetween(10, 20); ParsedDocument doc = testParsedDocument("1", null, testDocumentWithTextField(), new BytesArray("{}"), null); engine.index(indexForDoc(doc)); globalCheckpoint.set(0L); - final Engine.CommitId commitID = engine.flush(); - assertEquals( - "should succeed to flush commit with right id and no pending doc", - engine.syncFlush(syncId, commitID), - Engine.SyncedFlushResult.SUCCESS - ); + engine.flush(); + syncFlush(indexWriterHolder.get(), engine, syncId); assertEquals(store.readLastCommittedSegmentsInfo().getUserData().get(Engine.SYNC_COMMIT_ID), syncId); - assertEquals(engine.getLastCommittedSegmentInfos().getUserData().get(Engine.SYNC_COMMIT_ID), syncId); EngineConfig config = engine.config(); if (randomBoolean()) { engine.close(); @@ -1268,17 +1162,30 @@ public void testSyncedFlushSurvivesEngineRestart() throws IOException { } public void testSyncedFlushVanishesOnReplay() throws IOException { + IOUtils.close(store, engine); + SetOnce indexWriterHolder = new SetOnce<>(); + IndexWriterFactory indexWriterFactory = (directory, iwc) -> { + indexWriterHolder.set(new IndexWriter(directory, iwc)); + return indexWriterHolder.get(); + }; + store = createStore(); + final AtomicLong globalCheckpoint = new AtomicLong(SequenceNumbers.NO_OPS_PERFORMED); + engine = createEngine( + defaultSettings, + store, + primaryTranslogDir, + newMergePolicy(), + indexWriterFactory, + null, + globalCheckpoint::get + ); final String syncId = randomUnicodeOfCodepointLengthBetween(10, 20); ParsedDocument doc = testParsedDocument("1", null, testDocumentWithTextField(), new BytesArray("{}"), null); + globalCheckpoint.set(engine.getProcessedLocalCheckpoint()); engine.index(indexForDoc(doc)); - final Engine.CommitId commitID = engine.flush(); - assertEquals( - "should succeed to flush commit with right id and no pending doc", - engine.syncFlush(syncId, commitID), - Engine.SyncedFlushResult.SUCCESS - ); + engine.flush(); + syncFlush(indexWriterHolder.get(), engine, syncId); assertEquals(store.readLastCommittedSegmentsInfo().getUserData().get(Engine.SYNC_COMMIT_ID), syncId); - assertEquals(engine.getLastCommittedSegmentInfos().getUserData().get(Engine.SYNC_COMMIT_ID), syncId); doc = testParsedDocument("2", null, testDocumentWithTextField(), new BytesArray("{}"), null); engine.index(indexForDoc(doc)); EngineConfig config = engine.config(); @@ -1291,6 +1198,16 @@ public void testSyncedFlushVanishesOnReplay() throws IOException { ); } + void syncFlush(IndexWriter writer, InternalEngine engine, String syncId) throws IOException { + try (ReleasableLock ignored = engine.writeLock.acquire()) { + Map userData = new HashMap<>(); + writer.getLiveCommitData().forEach(e -> userData.put(e.getKey(), e.getValue())); + userData.put(Engine.SYNC_COMMIT_ID, syncId); + writer.setLiveCommitData(userData.entrySet()); + writer.commit(); + } + } + public void testVersioningNewCreate() throws IOException { ParsedDocument doc = testParsedDocument("1", null, testDocument(), B_1, null); Engine.Index create = new Engine.Index(newUid(doc), primaryTerm.get(), doc, Versions.MATCH_DELETED); @@ -3249,8 +3166,8 @@ public void testTranslogCleanUpPostCommitCrash() throws Exception { ) { @Override - protected void commitIndexWriter(IndexWriter writer, Translog translog, String syncId) throws IOException { - super.commitIndexWriter(writer, translog, syncId); + protected void commitIndexWriter(IndexWriter writer, Translog translog) throws IOException { + super.commitIndexWriter(writer, translog); if (throwErrorOnCommit.get()) { throw new RuntimeException("power's out"); } @@ -5588,14 +5505,14 @@ public void testKeepTranslogAfterGlobalCheckpoint() throws Exception { final AtomicLong lastSyncedGlobalCheckpointBeforeCommit = new AtomicLong(Translog.readGlobalCheckpoint(translogPath, translogUUID)); try (InternalEngine engine = new InternalEngine(engineConfig) { @Override - protected void commitIndexWriter(IndexWriter writer, Translog translog, String syncId) throws IOException { + protected void commitIndexWriter(IndexWriter writer, Translog translog) throws IOException { lastSyncedGlobalCheckpointBeforeCommit.set(Translog.readGlobalCheckpoint(translogPath, translogUUID)); // Advance the global checkpoint during the flush to create a lag between a persisted global checkpoint in the translog // (this value is visible to the deletion policy) and an in memory global checkpoint in the SequenceNumbersService. if (rarely()) { globalCheckpoint.set(randomLongBetween(globalCheckpoint.get(), getPersistedLocalCheckpoint())); } - super.commitIndexWriter(writer, translog, syncId); + super.commitIndexWriter(writer, translog); } }) { engine.recoverFromTranslog(translogHandler, Long.MAX_VALUE); diff --git a/server/src/test/java/org/opensearch/index/engine/ReadOnlyEngineTests.java b/server/src/test/java/org/opensearch/index/engine/ReadOnlyEngineTests.java index 956e136575a69..609e972b2c026 100644 --- a/server/src/test/java/org/opensearch/index/engine/ReadOnlyEngineTests.java +++ b/server/src/test/java/org/opensearch/index/engine/ReadOnlyEngineTests.java @@ -150,50 +150,6 @@ public void testReadOnlyEngine() throws Exception { } } - public void testFlushes() throws IOException { - IOUtils.close(engine, store); - Engine readOnlyEngine = null; - final AtomicLong globalCheckpoint = new AtomicLong(SequenceNumbers.NO_OPS_PERFORMED); - try (Store store = createStore()) { - EngineConfig config = config(defaultSettings, store, createTempDir(), newMergePolicy(), null, null, globalCheckpoint::get); - int numDocs = scaledRandomIntBetween(10, 1000); - try (InternalEngine engine = createEngine(config)) { - for (int i = 0; i < numDocs; i++) { - ParsedDocument doc = testParsedDocument(Integer.toString(i), null, testDocument(), new BytesArray("{}"), null); - engine.index( - new Engine.Index( - newUid(doc), - doc, - i, - primaryTerm.get(), - 1, - null, - Engine.Operation.Origin.REPLICA, - System.nanoTime(), - -1, - false, - SequenceNumbers.UNASSIGNED_SEQ_NO, - 0 - ) - ); - if (rarely()) { - engine.flush(); - } - engine.syncTranslog(); // advance persisted local checkpoint - globalCheckpoint.set(engine.getPersistedLocalCheckpoint()); - } - globalCheckpoint.set(engine.getPersistedLocalCheckpoint()); - engine.syncTranslog(); - engine.flushAndClose(); - readOnlyEngine = new ReadOnlyEngine(engine.engineConfig, null, null, true, Function.identity(), true); - Engine.CommitId flush = readOnlyEngine.flush(randomBoolean(), true); - assertEquals(flush, readOnlyEngine.flush(randomBoolean(), true)); - } finally { - IOUtils.close(readOnlyEngine); - } - } - } - public void testEnsureMaxSeqNoIsEqualToGlobalCheckpoint() throws IOException { IOUtils.close(engine, store); final AtomicLong globalCheckpoint = new AtomicLong(SequenceNumbers.NO_OPS_PERFORMED); @@ -263,7 +219,6 @@ public void testReadOnly() throws IOException { expectThrows(expectedException, () -> readOnlyEngine.index(null)); expectThrows(expectedException, () -> readOnlyEngine.delete(null)); expectThrows(expectedException, () -> readOnlyEngine.noOp(null)); - expectThrows(UnsupportedOperationException.class, () -> readOnlyEngine.syncFlush(null, null)); } } } diff --git a/server/src/test/java/org/opensearch/index/shard/IndexShardTests.java b/server/src/test/java/org/opensearch/index/shard/IndexShardTests.java index cdb2857c8582c..2575258b28968 100644 --- a/server/src/test/java/org/opensearch/index/shard/IndexShardTests.java +++ b/server/src/test/java/org/opensearch/index/shard/IndexShardTests.java @@ -103,7 +103,6 @@ import org.opensearch.index.engine.InternalEngine; import org.opensearch.index.engine.InternalEngineFactory; import org.opensearch.index.engine.ReadOnlyEngine; -import org.opensearch.index.engine.Segment; import org.opensearch.index.engine.SegmentsStats; import org.opensearch.index.fielddata.FieldDataStats; import org.opensearch.index.fielddata.IndexFieldData; @@ -3871,7 +3870,7 @@ public void testScheduledRefresh() throws Exception { indexDoc(primary, "_doc", "2", "{\"foo\" : \"bar\"}"); assertFalse(primary.scheduledRefresh()); assertTrue(primary.isSearchIdle()); - primary.checkIdle(0); + primary.flushOnIdle(0); assertTrue(primary.scheduledRefresh()); // make sure we refresh once the shard is inactive try (Engine.Searcher searcher = primary.acquireSearcher("test")) { assertEquals(3, searcher.getIndexReader().numDocs()); @@ -4083,92 +4082,6 @@ public void testSegmentMemoryTrackedWithRandomSearchers() throws Exception { assertThat(breaker.getUsed(), equalTo(0L)); } - public void testFlushOnInactive() throws Exception { - Settings settings = Settings.builder() - .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) - .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) - .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) - .build(); - IndexMetadata metadata = IndexMetadata.builder("test") - .putMapping("_doc", "{ \"properties\": { \"foo\": { \"type\": \"text\"}}}") - .settings(settings) - .primaryTerm(0, 1) - .build(); - ShardRouting shardRouting = TestShardRouting.newShardRouting( - new ShardId(metadata.getIndex(), 0), - "n1", - true, - ShardRoutingState.INITIALIZING, - RecoverySource.EmptyStoreRecoverySource.INSTANCE - ); - final ShardId shardId = shardRouting.shardId(); - final NodeEnvironment.NodePath nodePath = new NodeEnvironment.NodePath(createTempDir()); - ShardPath shardPath = new ShardPath(false, nodePath.resolve(shardId), nodePath.resolve(shardId), shardId); - AtomicBoolean markedInactive = new AtomicBoolean(); - AtomicReference primaryRef = new AtomicReference<>(); - IndexShard primary = newShard( - shardRouting, - shardPath, - metadata, - null, - null, - new InternalEngineFactory(), - new EngineConfigFactory(new IndexSettings(metadata, settings)), - () -> {}, - RetentionLeaseSyncer.EMPTY, - new IndexEventListener() { - @Override - public void onShardInactive(IndexShard indexShard) { - markedInactive.set(true); - primaryRef.get().flush(new FlushRequest()); - } - } - ); - primaryRef.set(primary); - recoverShardFromStore(primary); - for (int i = 0; i < 3; i++) { - indexDoc(primary, "_doc", "" + i, "{\"foo\" : \"" + randomAlphaOfLength(10) + "\"}"); - primary.refresh("test"); // produce segments - } - List segments = primary.segments(false); - Set names = new HashSet<>(); - for (Segment segment : segments) { - assertFalse(segment.committed); - assertTrue(segment.search); - names.add(segment.getName()); - } - assertEquals(3, segments.size()); - primary.flush(new FlushRequest()); - primary.forceMerge(new ForceMergeRequest().maxNumSegments(1).flush(false)); - primary.refresh("test"); - segments = primary.segments(false); - for (Segment segment : segments) { - if (names.contains(segment.getName())) { - assertTrue(segment.committed); - assertFalse(segment.search); - } else { - assertFalse(segment.committed); - assertTrue(segment.search); - } - } - assertEquals(4, segments.size()); - - assertFalse(markedInactive.get()); - assertBusy(() -> { - primary.checkIdle(0); - assertFalse(primary.isActive()); - }); - - assertTrue(markedInactive.get()); - segments = primary.segments(false); - assertEquals(1, segments.size()); - for (Segment segment : segments) { - assertTrue(segment.committed); - assertTrue(segment.search); - } - closeShards(primary); - } - public void testOnCloseStats() throws IOException { final IndexShard indexShard = newStartedShard(true); diff --git a/server/src/test/java/org/opensearch/indices/cluster/IndicesClusterStateServiceRandomUpdatesTests.java b/server/src/test/java/org/opensearch/indices/cluster/IndicesClusterStateServiceRandomUpdatesTests.java index 9206a15f6fa73..5574e3c911213 100644 --- a/server/src/test/java/org/opensearch/indices/cluster/IndicesClusterStateServiceRandomUpdatesTests.java +++ b/server/src/test/java/org/opensearch/indices/cluster/IndicesClusterStateServiceRandomUpdatesTests.java @@ -569,7 +569,6 @@ private IndicesClusterStateService createIndicesClusterStateService( null, null, null, - null, primaryReplicaSyncer, s -> {}, RetentionLeaseSyncer.EMPTY diff --git a/server/src/test/java/org/opensearch/indices/flush/SyncedFlushSingleNodeTests.java b/server/src/test/java/org/opensearch/indices/flush/SyncedFlushSingleNodeTests.java deleted file mode 100644 index 98305f198e6df..0000000000000 --- a/server/src/test/java/org/opensearch/indices/flush/SyncedFlushSingleNodeTests.java +++ /dev/null @@ -1,267 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -/* - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.indices.flush; - -import org.opensearch.action.support.PlainActionFuture; -import org.opensearch.cluster.ClusterState; -import org.opensearch.cluster.metadata.IndexMetadata; -import org.opensearch.cluster.routing.IndexShardRoutingTable; -import org.opensearch.cluster.routing.ShardRouting; -import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.UUIDs; -import org.opensearch.common.lease.Releasable; -import org.opensearch.common.settings.Settings; -import org.opensearch.common.xcontent.XContentType; -import org.opensearch.index.IndexService; -import org.opensearch.index.shard.IndexShard; -import org.opensearch.index.shard.ShardId; -import org.opensearch.index.shard.ShardNotFoundException; -import org.opensearch.indices.IndicesService; -import org.opensearch.test.OpenSearchSingleNodeTestCase; -import org.opensearch.threadpool.ThreadPool; - -import java.util.List; -import java.util.Map; - -import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; - -public class SyncedFlushSingleNodeTests extends OpenSearchSingleNodeTestCase { - - public void testModificationPreventsFlushing() throws InterruptedException { - createIndex("test"); - client().prepareIndex("test", "test", "1").setSource("{}", XContentType.JSON).get(); - IndexService test = getInstanceFromNode(IndicesService.class).indexService(resolveIndex("test")); - IndexShard shard = test.getShardOrNull(0); - - SyncedFlushService flushService = getInstanceFromNode(SyncedFlushService.class); - final ShardId shardId = shard.shardId(); - final ClusterState state = getInstanceFromNode(ClusterService.class).state(); - final IndexShardRoutingTable shardRoutingTable = flushService.getShardRoutingTable(shardId, state); - final List activeShards = shardRoutingTable.activeShards(); - assertEquals("exactly one active shard", 1, activeShards.size()); - Map preSyncedResponses = SyncedFlushUtil.sendPreSyncRequests( - flushService, - activeShards, - state, - shardId - ); - assertEquals("exactly one commit id", 1, preSyncedResponses.size()); - client().prepareIndex("test", "test", "2").setSource("{}", XContentType.JSON).get(); - String syncId = UUIDs.randomBase64UUID(); - SyncedFlushUtil.LatchedListener listener = new SyncedFlushUtil.LatchedListener<>(); - flushService.sendSyncRequests(syncId, activeShards, state, preSyncedResponses, shardId, shardRoutingTable.size(), listener); - listener.latch.await(); - assertNull(listener.error); - ShardsSyncedFlushResult syncedFlushResult = listener.result; - assertNotNull(syncedFlushResult); - assertEquals(0, syncedFlushResult.successfulShards()); - assertEquals(1, syncedFlushResult.totalShards()); - assertEquals(syncId, syncedFlushResult.syncId()); - assertNotNull(syncedFlushResult.shardResponses().get(activeShards.get(0))); - assertFalse(syncedFlushResult.shardResponses().get(activeShards.get(0)).success()); - assertEquals("pending operations", syncedFlushResult.shardResponses().get(activeShards.get(0)).failureReason()); - - // pull another commit and make sure we can't sync-flush with the old one - SyncedFlushUtil.sendPreSyncRequests(flushService, activeShards, state, shardId); - listener = new SyncedFlushUtil.LatchedListener<>(); - flushService.sendSyncRequests(syncId, activeShards, state, preSyncedResponses, shardId, shardRoutingTable.size(), listener); - listener.latch.await(); - assertNull(listener.error); - syncedFlushResult = listener.result; - assertNotNull(syncedFlushResult); - assertEquals(0, syncedFlushResult.successfulShards()); - assertEquals(1, syncedFlushResult.totalShards()); - assertEquals(syncId, syncedFlushResult.syncId()); - assertNotNull(syncedFlushResult.shardResponses().get(activeShards.get(0))); - assertFalse(syncedFlushResult.shardResponses().get(activeShards.get(0)).success()); - assertEquals("commit has changed", syncedFlushResult.shardResponses().get(activeShards.get(0)).failureReason()); - } - - public void testSingleShardSuccess() throws InterruptedException { - createIndex("test"); - client().prepareIndex("test", "test", "1").setSource("{}", XContentType.JSON).get(); - IndexService test = getInstanceFromNode(IndicesService.class).indexService(resolveIndex("test")); - IndexShard shard = test.getShardOrNull(0); - - SyncedFlushService flushService = getInstanceFromNode(SyncedFlushService.class); - final ShardId shardId = shard.shardId(); - SyncedFlushUtil.LatchedListener listener = new SyncedFlushUtil.LatchedListener<>(); - flushService.attemptSyncedFlush(shardId, listener); - listener.latch.await(); - assertNull(listener.error); - ShardsSyncedFlushResult syncedFlushResult = listener.result; - assertNotNull(syncedFlushResult); - assertEquals(1, syncedFlushResult.successfulShards()); - assertEquals(1, syncedFlushResult.totalShards()); - SyncedFlushService.ShardSyncedFlushResponse response = syncedFlushResult.shardResponses().values().iterator().next(); - assertTrue(response.success()); - } - - public void testSyncFailsIfOperationIsInFlight() throws Exception { - createIndex("test"); - client().prepareIndex("test", "test", "1").setSource("{}", XContentType.JSON).get(); - IndexService test = getInstanceFromNode(IndicesService.class).indexService(resolveIndex("test")); - IndexShard shard = test.getShardOrNull(0); - - // wait for the GCP sync spawned from the index request above to complete to avoid that request disturbing the check below - assertBusy(() -> { - assertEquals(0, shard.getLastSyncedGlobalCheckpoint()); - assertEquals(0, shard.getActiveOperationsCount()); - }); - - SyncedFlushService flushService = getInstanceFromNode(SyncedFlushService.class); - final ShardId shardId = shard.shardId(); - PlainActionFuture fut = new PlainActionFuture<>(); - shard.acquirePrimaryOperationPermit(fut, ThreadPool.Names.WRITE, ""); - try (Releasable operationLock = fut.get()) { - SyncedFlushUtil.LatchedListener listener = new SyncedFlushUtil.LatchedListener<>(); - flushService.attemptSyncedFlush(shardId, listener); - listener.latch.await(); - assertNull(listener.error); - ShardsSyncedFlushResult syncedFlushResult = listener.result; - assertNotNull(syncedFlushResult); - assertEquals(0, syncedFlushResult.successfulShards()); - assertNotEquals(0, syncedFlushResult.totalShards()); - assertEquals("[1] ongoing operations on primary", syncedFlushResult.failureReason()); - } - } - - public void testSyncFailsOnIndexClosedOrMissing() throws InterruptedException { - createIndex( - "test", - Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1).put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0).build() - ); - IndexService test = getInstanceFromNode(IndicesService.class).indexService(resolveIndex("test")); - final IndexShard shard = test.getShardOrNull(0); - assertNotNull(shard); - final ShardId shardId = shard.shardId(); - - final SyncedFlushService flushService = getInstanceFromNode(SyncedFlushService.class); - - SyncedFlushUtil.LatchedListener listener = new SyncedFlushUtil.LatchedListener(); - flushService.attemptSyncedFlush(new ShardId(shard.shardId().getIndex(), 1), listener); - listener.latch.await(); - assertNotNull(listener.error); - assertNull(listener.result); - assertEquals(ShardNotFoundException.class, listener.error.getClass()); - assertEquals("no such shard", listener.error.getMessage()); - - assertAcked(client().admin().indices().prepareClose("test")); - listener = new SyncedFlushUtil.LatchedListener(); - flushService.attemptSyncedFlush(shardId, listener); - listener.latch.await(); - assertNotNull(listener.error); - assertNull(listener.result); - assertEquals("closed", listener.error.getMessage()); - - listener = new SyncedFlushUtil.LatchedListener(); - flushService.attemptSyncedFlush(new ShardId("index not found", "_na_", 0), listener); - listener.latch.await(); - assertNotNull(listener.error); - assertNull(listener.result); - assertEquals("no such index [index not found]", listener.error.getMessage()); - } - - public void testFailAfterIntermediateCommit() throws InterruptedException { - createIndex("test"); - client().prepareIndex("test", "test", "1").setSource("{}", XContentType.JSON).get(); - IndexService test = getInstanceFromNode(IndicesService.class).indexService(resolveIndex("test")); - IndexShard shard = test.getShardOrNull(0); - - SyncedFlushService flushService = getInstanceFromNode(SyncedFlushService.class); - final ShardId shardId = shard.shardId(); - final ClusterState state = getInstanceFromNode(ClusterService.class).state(); - final IndexShardRoutingTable shardRoutingTable = flushService.getShardRoutingTable(shardId, state); - final List activeShards = shardRoutingTable.activeShards(); - assertEquals("exactly one active shard", 1, activeShards.size()); - Map preSyncedResponses = SyncedFlushUtil.sendPreSyncRequests( - flushService, - activeShards, - state, - shardId - ); - assertEquals("exactly one commit id", 1, preSyncedResponses.size()); - if (randomBoolean()) { - client().prepareIndex("test", "test", "2").setSource("{}", XContentType.JSON).get(); - } - client().admin().indices().prepareFlush("test").setForce(true).get(); - String syncId = UUIDs.randomBase64UUID(); - final SyncedFlushUtil.LatchedListener listener = new SyncedFlushUtil.LatchedListener<>(); - flushService.sendSyncRequests(syncId, activeShards, state, preSyncedResponses, shardId, shardRoutingTable.size(), listener); - listener.latch.await(); - assertNull(listener.error); - ShardsSyncedFlushResult syncedFlushResult = listener.result; - assertNotNull(syncedFlushResult); - assertEquals(0, syncedFlushResult.successfulShards()); - assertEquals(1, syncedFlushResult.totalShards()); - assertEquals(syncId, syncedFlushResult.syncId()); - assertNotNull(syncedFlushResult.shardResponses().get(activeShards.get(0))); - assertFalse(syncedFlushResult.shardResponses().get(activeShards.get(0)).success()); - assertEquals("commit has changed", syncedFlushResult.shardResponses().get(activeShards.get(0)).failureReason()); - } - - public void testFailWhenCommitIsMissing() throws InterruptedException { - createIndex("test"); - client().prepareIndex("test", "test", "1").setSource("{}", XContentType.JSON).get(); - IndexService test = getInstanceFromNode(IndicesService.class).indexService(resolveIndex("test")); - IndexShard shard = test.getShardOrNull(0); - - SyncedFlushService flushService = getInstanceFromNode(SyncedFlushService.class); - final ShardId shardId = shard.shardId(); - final ClusterState state = getInstanceFromNode(ClusterService.class).state(); - final IndexShardRoutingTable shardRoutingTable = flushService.getShardRoutingTable(shardId, state); - final List activeShards = shardRoutingTable.activeShards(); - assertEquals("exactly one active shard", 1, activeShards.size()); - Map preSyncedResponses = SyncedFlushUtil.sendPreSyncRequests( - flushService, - activeShards, - state, - shardId - ); - assertEquals("exactly one commit id", 1, preSyncedResponses.size()); - preSyncedResponses.clear(); // wipe it... - String syncId = UUIDs.randomBase64UUID(); - SyncedFlushUtil.LatchedListener listener = new SyncedFlushUtil.LatchedListener<>(); - flushService.sendSyncRequests(syncId, activeShards, state, preSyncedResponses, shardId, shardRoutingTable.size(), listener); - listener.latch.await(); - assertNull(listener.error); - ShardsSyncedFlushResult syncedFlushResult = listener.result; - assertNotNull(syncedFlushResult); - assertEquals(0, syncedFlushResult.successfulShards()); - assertEquals(1, syncedFlushResult.totalShards()); - assertEquals(syncId, syncedFlushResult.syncId()); - assertNotNull(syncedFlushResult.shardResponses().get(activeShards.get(0))); - assertFalse(syncedFlushResult.shardResponses().get(activeShards.get(0)).success()); - assertEquals("no commit id from pre-sync flush", syncedFlushResult.shardResponses().get(activeShards.get(0)).failureReason()); - } - -} diff --git a/server/src/test/java/org/opensearch/indices/flush/SyncedFlushUtil.java b/server/src/test/java/org/opensearch/indices/flush/SyncedFlushUtil.java deleted file mode 100644 index def9c0c15ced2..0000000000000 --- a/server/src/test/java/org/opensearch/indices/flush/SyncedFlushUtil.java +++ /dev/null @@ -1,126 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -/* - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.indices.flush; - -import org.apache.logging.log4j.Logger; -import org.opensearch.ExceptionsHelper; -import org.opensearch.action.ActionListener; -import org.opensearch.cluster.ClusterState; -import org.opensearch.cluster.routing.ShardRouting; -import org.opensearch.index.shard.ShardId; -import org.opensearch.test.InternalTestCluster; - -import java.util.List; -import java.util.Map; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.atomic.AtomicReference; - -import static org.opensearch.test.OpenSearchTestCase.assertBusy; - -/** Utils for SyncedFlush */ -public class SyncedFlushUtil { - - private SyncedFlushUtil() { - - } - - /** - * Blocking version of {@link SyncedFlushService#attemptSyncedFlush(ShardId, ActionListener)} - */ - public static ShardsSyncedFlushResult attemptSyncedFlush(Logger logger, InternalTestCluster cluster, ShardId shardId) throws Exception { - /* - * When the last indexing operation is completed, we will fire a global checkpoint sync. - * Since a global checkpoint sync request is a replication request, it will acquire an index - * shard permit on the primary when executing. If this happens at the same time while we are - * issuing the synced-flush, the synced-flush request will fail as it thinks there are - * in-flight operations. We can avoid such situation by continuing issuing another synced-flush - * if the synced-flush failed due to the ongoing operations on the primary. - */ - SyncedFlushService service = cluster.getInstance(SyncedFlushService.class); - AtomicReference> listenerHolder = new AtomicReference<>(); - assertBusy(() -> { - LatchedListener listener = new LatchedListener<>(); - listenerHolder.set(listener); - service.attemptSyncedFlush(shardId, listener); - listener.latch.await(); - if (listener.result != null - && listener.result.failureReason() != null - && listener.result.failureReason().contains("ongoing operations on primary")) { - throw new AssertionError(listener.result.failureReason()); // cause the assert busy to retry - } - }); - if (listenerHolder.get().error != null) { - throw ExceptionsHelper.convertToOpenSearchException(listenerHolder.get().error); - } - return listenerHolder.get().result; - } - - public static final class LatchedListener implements ActionListener { - public volatile T result; - public volatile Exception error; - public final CountDownLatch latch = new CountDownLatch(1); - - @Override - public void onResponse(T result) { - this.result = result; - latch.countDown(); - } - - @Override - public void onFailure(Exception e) { - error = e; - latch.countDown(); - } - } - - /** - * Blocking version of {@link SyncedFlushService#sendPreSyncRequests(List, ClusterState, ShardId, ActionListener)} - */ - public static Map sendPreSyncRequests( - SyncedFlushService service, - List activeShards, - ClusterState state, - ShardId shardId - ) { - LatchedListener> listener = new LatchedListener<>(); - service.sendPreSyncRequests(activeShards, state, shardId, listener); - try { - listener.latch.await(); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - if (listener.error != null) { - throw ExceptionsHelper.convertToOpenSearchException(listener.error); - } - return listener.result; - } -} diff --git a/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java b/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java index 9496054e80ad8..74da44af28e1d 100644 --- a/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java +++ b/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java @@ -178,7 +178,6 @@ import org.opensearch.indices.analysis.AnalysisModule; import org.opensearch.indices.breaker.NoneCircuitBreakerService; import org.opensearch.indices.cluster.IndicesClusterStateService; -import org.opensearch.indices.flush.SyncedFlushService; import org.opensearch.indices.mapper.MapperRegistry; import org.opensearch.indices.recovery.PeerRecoverySourceService; import org.opensearch.indices.recovery.PeerRecoveryTargetService; @@ -1835,7 +1834,6 @@ public void onFailure(final Exception e) { new NodeMappingRefreshAction(transportService, metadataMappingService), repositoriesService, mock(SearchService.class), - new SyncedFlushService(indicesService, clusterService, transportService, indexNameExpressionResolver), new PeerRecoverySourceService(transportService, indicesService, recoverySettings), snapshotShardsService, new PrimaryReplicaSyncer( diff --git a/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java index 6e1972f193948..f078db94c6441 100644 --- a/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java @@ -95,7 +95,6 @@ import org.opensearch.index.Index; import org.opensearch.index.IndexService; import org.opensearch.index.IndexingPressure; -import org.opensearch.index.engine.CommitStats; import org.opensearch.index.engine.DocIdSeqNoAndSource; import org.opensearch.index.engine.Engine; import org.opensearch.index.engine.EngineTestCase; @@ -135,7 +134,6 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -1252,48 +1250,11 @@ public void beforeIndexDeletion() throws Exception { // and not all docs have been purged after the test) and inherit from // ElasticsearchIntegrationTest must override beforeIndexDeletion() to avoid failures. assertNoPendingIndexOperations(); - // check that shards that have same sync id also contain same number of documents - assertSameSyncIdSameDocs(); assertAllPendingWriteLimitsReleased(); assertOpenTranslogReferences(); assertNoSnapshottedIndexCommit(); } - private void assertSameSyncIdSameDocs() { - Map docsOnShards = new HashMap<>(); - final Collection nodesAndClients = nodes.values(); - for (NodeAndClient nodeAndClient : nodesAndClients) { - IndicesService indexServices = getInstance(IndicesService.class, nodeAndClient.name); - for (IndexService indexService : indexServices) { - for (IndexShard indexShard : indexService) { - try { - CommitStats commitStats = indexShard.commitStats(); - String syncId = commitStats.getUserData().get(Engine.SYNC_COMMIT_ID); - if (syncId != null) { - long liveDocsOnShard = commitStats.getNumDocs(); - if (docsOnShards.get(syncId) != null) { - assertThat( - "sync id is equal but number of docs does not match on node " - + nodeAndClient.name - + ". expected " - + docsOnShards.get(syncId) - + " but got " - + liveDocsOnShard, - docsOnShards.get(syncId), - equalTo(liveDocsOnShard) - ); - } else { - docsOnShards.put(syncId, liveDocsOnShard); - } - } - } catch (AlreadyClosedException e) { - // the engine is closed or if the shard is recovering - } - } - } - } - } - private void assertAllPendingWriteLimitsReleased() throws Exception { assertBusy(() -> { for (NodeAndClient nodeAndClient : nodes.values()) { diff --git a/test/framework/src/main/java/org/opensearch/test/MockIndexEventListener.java b/test/framework/src/main/java/org/opensearch/test/MockIndexEventListener.java index a6aba5269ec91..ecdae6a0e8b64 100644 --- a/test/framework/src/main/java/org/opensearch/test/MockIndexEventListener.java +++ b/test/framework/src/main/java/org/opensearch/test/MockIndexEventListener.java @@ -135,11 +135,6 @@ public void indexShardStateChanged( delegate.indexShardStateChanged(indexShard, previousState, currentState, reason); } - @Override - public void onShardInactive(IndexShard indexShard) { - delegate.onShardInactive(indexShard); - } - @Override public void beforeIndexCreated(Index index, Settings indexSettings) { delegate.beforeIndexCreated(index, indexSettings); diff --git a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java index cdfd1b15a09f3..bb5268122af42 100644 --- a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java +++ b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java @@ -197,7 +197,6 @@ import java.util.function.Function; import java.util.stream.Collectors; -import static org.opensearch.client.Requests.syncedFlushRequest; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS; import static org.opensearch.common.unit.TimeValue.timeValueMillis; @@ -1674,20 +1673,11 @@ private void postIndexAsyncActions(String[] indices, List inFlig .setIndicesOptions(IndicesOptions.lenientExpandOpen()) .execute(new LatchedActionListener<>(newLatch(inFlightAsyncOperations))); } else if (maybeFlush && rarely()) { - if (randomBoolean()) { - client().admin() - .indices() - .prepareFlush(indices) - .setIndicesOptions(IndicesOptions.lenientExpandOpen()) - .execute(new LatchedActionListener<>(newLatch(inFlightAsyncOperations))); - } else { - client().admin() - .indices() - .syncedFlush( - syncedFlushRequest(indices).indicesOptions(IndicesOptions.lenientExpandOpen()), - new LatchedActionListener<>(newLatch(inFlightAsyncOperations)) - ); - } + client().admin() + .indices() + .prepareFlush(indices) + .setIndicesOptions(IndicesOptions.lenientExpandOpen()) + .execute(new LatchedActionListener<>(newLatch(inFlightAsyncOperations))); } else if (rarely()) { client().admin() .indices() diff --git a/test/framework/src/main/java/org/opensearch/test/rest/OpenSearchRestTestCase.java b/test/framework/src/main/java/org/opensearch/test/rest/OpenSearchRestTestCase.java index 6405d5177f389..79980ccc39272 100644 --- a/test/framework/src/main/java/org/opensearch/test/rest/OpenSearchRestTestCase.java +++ b/test/framework/src/main/java/org/opensearch/test/rest/OpenSearchRestTestCase.java @@ -71,7 +71,6 @@ import org.opensearch.core.internal.io.IOUtils; import org.opensearch.index.IndexSettings; import org.opensearch.index.seqno.ReplicationTracker; -import org.opensearch.indices.flush.SyncedFlushService; import org.opensearch.rest.RestStatus; import org.opensearch.snapshots.SnapshotState; import org.opensearch.test.OpenSearchTestCase; @@ -101,6 +100,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.TreeSet; import java.util.concurrent.ConcurrentHashMap; @@ -1269,13 +1269,27 @@ protected static Version minimumNodeVersion() throws IOException { return minVersion; } - protected static void performSyncedFlush(String indexName, boolean retryOnConflict) throws Exception { + protected void syncedFlush(String indexName, boolean retryOnConflict) throws Exception { final Request request = new Request("POST", indexName + "/_flush/synced"); - final List expectedWarnings = Collections.singletonList(SyncedFlushService.SYNCED_FLUSH_DEPRECATION_MESSAGE); final Builder options = RequestOptions.DEFAULT.toBuilder(); - options.setWarningsHandler(warnings -> warnings.isEmpty() == false && warnings.equals(expectedWarnings) == false); + // 8.0 kept in warning message for legacy purposes TODO: changge to 3.0 + final List warningMessage = Arrays.asList( + "Synced flush is deprecated and will be removed in 8.0. Use flush at _/flush or /{index}/_flush instead." + ); + final List expectedWarnings = Arrays.asList( + "Synced flush was removed and a normal flush was performed instead. This transition will be removed in a future version." + ); + if (nodeVersions.stream().allMatch(version -> version.onOrAfter(Version.V_2_0_0))) { + options.setWarningsHandler(warnings -> warnings.isEmpty() == false && warnings.equals(expectedWarnings) == false); + } else if (nodeVersions.stream().anyMatch(version -> version.onOrAfter(LegacyESVersion.V_7_6_0))) { + options.setWarningsHandler( + warnings -> warnings.isEmpty() == false + && warnings.equals(expectedWarnings) == false + && warnings.equals(warningMessage) == false + ); + } request.setOptions(options); - // We have to spin a synced-flush request because we fire the global checkpoint sync for the last write operation. + // We have to spin synced-flush requests here because we fire the global checkpoint sync for the last write operation. // A synced-flush request considers the global checkpoint sync as an going operation because it acquires a shard permit. assertBusy(() -> { try { @@ -1291,6 +1305,26 @@ protected static void performSyncedFlush(String indexName, boolean retryOnConfli } } }); + // ensure the global checkpoint is synced; otherwise we might trim the commit with syncId + ensureGlobalCheckpointSynced(indexName); + } + + @SuppressWarnings("unchecked") + private void ensureGlobalCheckpointSynced(String index) throws Exception { + assertBusy(() -> { + Map stats = entityAsMap(client().performRequest(new Request("GET", index + "/_stats?level=shards"))); + List> shardStats = (List>) XContentMapValues.extractValue("indices." + index + ".shards.0", stats); + shardStats.stream() + .map(shard -> (Map) XContentMapValues.extractValue("seq_no", shard)) + .filter(Objects::nonNull) + .forEach(seqNoStat -> { + long globalCheckpoint = ((Number) XContentMapValues.extractValue("global_checkpoint", seqNoStat)).longValue(); + long localCheckpoint = ((Number) XContentMapValues.extractValue("local_checkpoint", seqNoStat)).longValue(); + long maxSeqNo = ((Number) XContentMapValues.extractValue("max_seq_no", seqNoStat)).longValue(); + assertThat(shardStats.toString(), localCheckpoint, equalTo(maxSeqNo)); + assertThat(shardStats.toString(), globalCheckpoint, equalTo(maxSeqNo)); + }); + }, 60, TimeUnit.SECONDS); } static final Pattern CREATE_INDEX_MULTIPLE_MATCHING_TEMPLATES = Pattern.compile(