Skip to content

Commit

Permalink
Merge branch 'main' into bump-spullara
Browse files Browse the repository at this point in the history
Signed-off-by: Craig Perkins <[email protected]>
  • Loading branch information
cwperks authored Apr 22, 2024
2 parents fcf4be0 + cf58fab commit b838c5f
Show file tree
Hide file tree
Showing 15 changed files with 121 additions and 72 deletions.
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,19 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Bump `org.apache.commons:commonscodec` from 1.15 to 1.16.1 ([#12627](https://github.com/opensearch-project/OpenSearch/pull/12627))
- Bump `org.apache.commons:commonslang` from 3.13.0 to 3.14.0 ([#12627](https://github.com/opensearch-project/OpenSearch/pull/12627))
- Bump Apache Tika from 2.6.0 to 2.9.2 ([#12627](https://github.com/opensearch-project/OpenSearch/pull/12627))
- Bump `com.gradle.enterprise` from 3.16.2 to 3.17.1 ([#13116](https://github.com/opensearch-project/OpenSearch/pull/13116), [#13191](https://github.com/opensearch-project/OpenSearch/pull/13191))
- Bump `com.gradle.enterprise` from 3.16.2 to 3.17.2 ([#13116](https://github.com/opensearch-project/OpenSearch/pull/13116), [#13191](https://github.com/opensearch-project/OpenSearch/pull/13191), [#13327](https://github.com/opensearch-project/OpenSearch/pull/13327))
- Bump `gradle/wrapper-validation-action` from 2 to 3 ([#13192](https://github.com/opensearch-project/OpenSearch/pull/13192))
- Bump joda from 2.12.2 to 2.12.7 ([#13193](https://github.com/opensearch-project/OpenSearch/pull/13193))
- Bump bouncycastle from 1.77 to 1.78 ([#13243](https://github.com/opensearch-project/OpenSearch/pull/13243))
- Update google dependencies in repository-gcs and discovery-gce ([#13213](https://github.com/opensearch-project/OpenSearch/pull/13213))
- Bump `com.google.apis:google-api-services-compute` from v1-rev235-1.25.0 to v1-rev20240407-2.0.0 ([#13333](https://github.com/opensearch-project/OpenSearch/pull/13333))
- Bump `com.github.spullara.mustache.java:compiler` from 0.9.10 to 0.9.11 ([#13329](https://github.com/opensearch-project/OpenSearch/pull/13329))

### Changed
- [BWC and API enforcement] Enforcing the presence of API annotations at build time ([#12872](https://github.com/opensearch-project/OpenSearch/pull/12872))
- Improve built-in secure transports support ([#12907](https://github.com/opensearch-project/OpenSearch/pull/12907))
- Update links to documentation in rest-api-spec ([#13043](https://github.com/opensearch-project/OpenSearch/pull/13043))
- Ignoring unavailable shards during search request execution with ignore_available parameter ([#13298](https://github.com/opensearch-project/OpenSearch/pull/13298))
- Refactoring globMatch using simpleMatchWithNormalizedStrings from Regex ([#13104](https://github.com/opensearch-project/OpenSearch/pull/13104))
- [BWC and API enforcement] Reconsider the breaking changes check policy to detect breaking changes against released versions ([#13292](https://github.com/opensearch-project/OpenSearch/pull/13292))

Expand All @@ -61,6 +63,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Enabled mockTelemetryPlugin for IT and fixed OOM issues ([#13054](https://github.com/opensearch-project/OpenSearch/pull/13054))
- Fix implement mark() and markSupported() in class FilterStreamInput ([#13098](https://github.com/opensearch-project/OpenSearch/pull/13098))
- Fix snapshot _status API to return correct status for partial snapshots ([#12812](https://github.com/opensearch-project/OpenSearch/pull/12812))
- Improve the error messages for _stats with closed indices ([#13012](https://github.com/opensearch-project/OpenSearch/pull/13012))
- Ignore BaseRestHandler unconsumed content check as it's always consumed. ([#13290](https://github.com/opensearch-project/OpenSearch/pull/13290))

### Security
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ public void testIndicesOptions() {
request.indicesOptions(IndicesOptions.fromParameters("closed", null, null, "false", SearchRequest.DEFAULT_INDICES_OPTIONS));
response = client().execute(RankEvalAction.INSTANCE, request).actionGet();
assertEquals(1, response.getFailures().size());
assertThat(response.getFailures().get("amsterdam_query"), instanceOf(IndexClosedException.class));
assertThat(response.getFailures().get("amsterdam_query"), instanceOf(IllegalArgumentException.class));

// test allow_no_indices
request = new RankEvalRequest(task, new String[] { "bad*" });
Expand Down
2 changes: 1 addition & 1 deletion plugins/discovery-gce/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ opensearchplugin {
}

dependencies {
api "com.google.apis:google-api-services-compute:v1-rev235-1.25.0"
api "com.google.apis:google-api-services-compute:v1-rev20240407-2.0.0"
api "com.google.api-client:google-api-client:1.35.2"
api "com.google.oauth-client:google-oauth-client:1.35.0"
api "com.google.http-client:google-http-client:${versions.google_http_client}"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
edf93bc92c9b87fee51aa6c3545b565e58075c05

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,17 @@
*/

import org.opensearch.Version;
import org.opensearch.action.admin.cluster.health.ClusterHealthRequest;
import org.opensearch.action.admin.cluster.health.ClusterHealthResponse;
import org.opensearch.action.admin.indices.settings.get.GetSettingsResponse;
import org.opensearch.action.admin.indices.shrink.ResizeType;
import org.opensearch.action.admin.indices.stats.IndicesStatsResponse;
import org.opensearch.client.Requests;
import org.opensearch.cluster.routing.allocation.decider.EnableAllocationDecider;
import org.opensearch.common.settings.Settings;
import org.opensearch.core.xcontent.MediaTypeRegistry;
import org.opensearch.index.query.TermsQueryBuilder;
import org.opensearch.indices.recovery.RecoverySettings;
import org.opensearch.remotestore.RemoteStoreBaseIntegTestCase;
import org.opensearch.test.VersionUtils;

Expand Down Expand Up @@ -156,7 +160,11 @@ public void testCreateCloneIndexFailure() throws ExecutionException, Interrupted
client().admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(Settings.builder().put(EnableAllocationDecider.CLUSTER_ROUTING_REBALANCE_ENABLE_SETTING.getKey(), "none"))
.setTransientSettings(
Settings.builder()
.put(EnableAllocationDecider.CLUSTER_ROUTING_REBALANCE_ENABLE_SETTING.getKey(), "none")
.put(RecoverySettings.INDICES_INTERNAL_REMOTE_UPLOAD_TIMEOUT.getKey(), "10s")
)
.get();
try {
setFailRate(REPOSITORY_NAME, 100);
Expand All @@ -168,9 +176,14 @@ public void testCreateCloneIndexFailure() throws ExecutionException, Interrupted
.setWaitForActiveShards(0)
.setSettings(Settings.builder().put("index.number_of_replicas", 0).putNull("index.blocks.write").build())
.get();

Thread.sleep(2000);
ensureYellow("target");
// waiting more than waitForRemoteStoreSync's sleep time of 30 sec to deterministically fail
Thread.sleep(40000);
ensureRed("target");
ClusterHealthRequest healthRequest = Requests.clusterHealthRequest()
.waitForNoRelocatingShards(true)
.waitForNoInitializingShards(true);
ClusterHealthResponse actionGet = client().admin().cluster().health(healthRequest).actionGet();
assertEquals(actionGet.getUnassignedShards(), numPrimaryShards);

} catch (ExecutionException | InterruptedException e) {
throw new RuntimeException(e);
Expand All @@ -182,11 +195,12 @@ public void testCreateCloneIndexFailure() throws ExecutionException, Interrupted
.cluster()
.prepareUpdateSettings()
.setTransientSettings(
Settings.builder().put(EnableAllocationDecider.CLUSTER_ROUTING_REBALANCE_ENABLE_SETTING.getKey(), (String) null)
Settings.builder()
.put(EnableAllocationDecider.CLUSTER_ROUTING_REBALANCE_ENABLE_SETTING.getKey(), (String) null)
.put(RecoverySettings.INDICES_INTERNAL_REMOTE_UPLOAD_TIMEOUT.getKey(), (String) null)
)
.get();
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,22 @@

package org.opensearch.remotemigration;

import com.carrotsearch.randomizedtesting.generators.RandomNumbers;

import org.opensearch.action.DocWriteResponse;
import org.opensearch.action.admin.cluster.health.ClusterHealthRequest;
import org.opensearch.action.admin.cluster.health.ClusterHealthResponse;
import org.opensearch.action.admin.cluster.repositories.get.GetRepositoriesRequest;
import org.opensearch.action.admin.cluster.repositories.get.GetRepositoriesResponse;
import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest;
import org.opensearch.action.delete.DeleteResponse;
import org.opensearch.action.index.IndexResponse;
import org.opensearch.client.Client;
import org.opensearch.client.Requests;
import org.opensearch.cluster.routing.allocation.command.MoveAllocationCommand;
import org.opensearch.common.Priority;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.unit.TimeValue;
import org.opensearch.index.query.QueryBuilders;
import org.opensearch.indices.recovery.RecoverySettings;
import org.opensearch.plugins.Plugin;
import org.opensearch.test.OpenSearchIntegTestCase;
import org.opensearch.test.hamcrest.OpenSearchAssertions;
Expand Down Expand Up @@ -162,12 +163,12 @@ public void testMixedModeRelocation_RemoteSeedingFail() throws Exception {
String remoteNode = internalCluster().startNode();
internalCluster().validateClusterFormed();

// assert repo gets registered
GetRepositoriesRequest gr = new GetRepositoriesRequest(new String[] { REPOSITORY_NAME });
GetRepositoriesResponse getRepositoriesResponse = client.admin().cluster().getRepositories(gr).actionGet();
assertEquals(1, getRepositoriesResponse.repositories().size());

setFailRate(REPOSITORY_NAME, 100);
client().admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(Settings.builder().put(RecoverySettings.INDICES_INTERNAL_REMOTE_UPLOAD_TIMEOUT.getKey(), "10s"))
.get();

logger.info("--> relocating from {} to {} ", docRepNode, remoteNode);
client().admin().cluster().prepareReroute().add(new MoveAllocationCommand("test", 0, docRepNode, remoteNode)).execute().actionGet();
Expand All @@ -181,29 +182,23 @@ public void testMixedModeRelocation_RemoteSeedingFail() throws Exception {
.actionGet();

assertTrue(clusterHealthResponse.getRelocatingShards() == 1);
setFailRate(REPOSITORY_NAME, 0);
Thread.sleep(RandomNumbers.randomIntBetween(random(), 0, 2000));
clusterHealthResponse = client().admin()
.cluster()
.prepareHealth()
.setTimeout(TimeValue.timeValueSeconds(45))
.setWaitForEvents(Priority.LANGUID)
.setWaitForNoRelocatingShards(true)
.execute()
.actionGet();
assertTrue(clusterHealthResponse.getRelocatingShards() == 0);
logger.info("--> remote to remote relocation complete");
// waiting more than waitForRemoteStoreSync's sleep time of 30 sec to deterministically fail
Thread.sleep(40000);

ClusterHealthRequest healthRequest = Requests.clusterHealthRequest()
.waitForNoRelocatingShards(true)
.waitForNoInitializingShards(true);
ClusterHealthResponse actionGet = client().admin().cluster().health(healthRequest).actionGet();
assertEquals(actionGet.getRelocatingShards(), 0);
assertEquals(docRepNode, primaryNodeName("test"));

finished.set(true);
indexingThread.join();
refresh("test");
OpenSearchAssertions.assertHitCount(client().prepareSearch("test").setTrackTotalHits(true).get(), numAutoGenDocs.get());
OpenSearchAssertions.assertHitCount(
client().prepareSearch("test")
.setTrackTotalHits(true)// extra paranoia ;)
.setQuery(QueryBuilders.termQuery("auto", true))
.get(),
numAutoGenDocs.get()
);
client().admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(Settings.builder().put(RecoverySettings.INDICES_INTERNAL_REMOTE_UPLOAD_TIMEOUT.getKey(), (String) null))
.get();
}

private static Thread getIndexingThread(AtomicBoolean finished, AtomicInteger numAutoGenDocs) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -425,8 +425,10 @@ public final void executeNextPhase(SearchPhase currentPhase, SearchPhase nextPha
currentPhase.getName()
);
}
onPhaseFailure(currentPhase, "Partial shards failure (" + discrepancy + " shards unavailable)", null);
return;
if (!request.indicesOptions().ignoreUnavailable()) {
onPhaseFailure(currentPhase, "Partial shards failure (" + discrepancy + " shards unavailable)", null);
return;
}
}
}
if (logger.isTraceEnabled()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,13 @@ private void checkSystemIndexAccess(Context context, Metadata metadata, Set<Inde
private static boolean shouldTrackConcreteIndex(Context context, IndicesOptions options, IndexMetadata index) {
if (index.getState() == IndexMetadata.State.CLOSE) {
if (options.forbidClosedIndices() && options.ignoreUnavailable() == false) {
throw new IndexClosedException(index.getIndex());
if (options.expandWildcardsClosed() == true && options.getExpandWildcards().size() == 1) {
throw new IllegalArgumentException(
"To expand [" + index.getState() + "] wildcard, please set forbid_closed_indices to `false`"
);
} else {
throw new IndexClosedException(index.getIndex());
}
} else {
return options.forbidClosedIndices() == false && addIndex(index, context);
}
Expand Down
12 changes: 9 additions & 3 deletions server/src/main/java/org/opensearch/index/shard/IndexShard.java
Original file line number Diff line number Diff line change
Expand Up @@ -2118,15 +2118,16 @@ public boolean isRemoteSegmentStoreInSync() {
return false;
}

public void waitForRemoteStoreSync() {
public void waitForRemoteStoreSync() throws IOException {
waitForRemoteStoreSync(() -> {});
}

/*
Blocks the calling thread, waiting for the remote store to get synced till internal Remote Upload Timeout
Calls onProgress on seeing an increased file count on remote
Throws IOException if the remote store is not synced within the timeout
*/
public void waitForRemoteStoreSync(Runnable onProgress) {
public void waitForRemoteStoreSync(Runnable onProgress) throws IOException {
assert indexSettings.isAssignedOnRemoteNode();
RemoteSegmentStoreDirectory directory = getRemoteDirectory();
int segmentUploadeCount = 0;
Expand All @@ -2138,7 +2139,7 @@ public void waitForRemoteStoreSync(Runnable onProgress) {
while (System.nanoTime() - startNanos < getRecoverySettings().internalRemoteUploadTimeout().nanos()) {
try {
if (isRemoteSegmentStoreInSync()) {
break;
return;
} else {
if (directory.getSegmentsUploadedToRemoteStore().size() > segmentUploadeCount) {
onProgress.run();
Expand All @@ -2156,6 +2157,11 @@ public void waitForRemoteStoreSync(Runnable onProgress) {
return;
}
}
throw new IOException(
"Failed to upload to remote segment store within remote upload timeout of "
+ getRecoverySettings().internalRemoteUploadTimeout().getMinutes()
+ " minutes"
);
}

public void preRecovery() {
Expand Down
15 changes: 0 additions & 15 deletions server/src/main/java/org/opensearch/index/shard/StoreRecovery.java
Original file line number Diff line number Diff line change
Expand Up @@ -193,13 +193,6 @@ void recoverFromLocalShards(
indexShard.getEngine().forceMerge(false, -1, false, false, false, UUIDs.randomBase64UUID());
if (indexShard.isRemoteTranslogEnabled() && indexShard.shardRouting.primary()) {
indexShard.waitForRemoteStoreSync();
if (indexShard.isRemoteSegmentStoreInSync() == false) {
throw new IndexShardRecoveryException(
indexShard.shardId(),
"failed to upload to remote",
new IOException("Failed to upload to remote segment store")
);
}
}
return true;
} catch (IOException ex) {
Expand Down Expand Up @@ -436,10 +429,6 @@ void recoverFromSnapshotAndRemoteStore(
indexShard.finalizeRecovery();
if (indexShard.isRemoteTranslogEnabled() && indexShard.shardRouting.primary()) {
indexShard.waitForRemoteStoreSync();
if (indexShard.isRemoteSegmentStoreInSync() == false) {
listener.onFailure(new IndexShardRestoreFailedException(shardId, "Failed to upload to remote segment store"));
return;
}
}
indexShard.postRecovery("restore done");

Expand Down Expand Up @@ -722,10 +711,6 @@ private void restore(
indexShard.finalizeRecovery();
if (indexShard.isRemoteTranslogEnabled() && indexShard.shardRouting.primary()) {
indexShard.waitForRemoteStoreSync();
if (indexShard.isRemoteSegmentStoreInSync() == false) {
listener.onFailure(new IndexShardRestoreFailedException(shardId, "Failed to upload to remote segment store"));
return;
}
}
indexShard.postRecovery("restore done");
listener.onResponse(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.EnumSet;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
Expand Down Expand Up @@ -442,6 +443,24 @@ public void testShardNotAvailableWithDisallowPartialFailures() {
assertEquals(0, searchPhaseExecutionException.getSuppressed().length);
}

public void testShardNotAvailableWithIgnoreUnavailable() {
SearchRequest searchRequest = new SearchRequest().allowPartialSearchResults(false)
.indicesOptions(new IndicesOptions(EnumSet.of(IndicesOptions.Option.IGNORE_UNAVAILABLE), IndicesOptions.WildcardStates.NONE));
AtomicReference<Exception> exception = new AtomicReference<>();
ActionListener<SearchResponse> listener = ActionListener.wrap(response -> {}, exception::set);
int numShards = randomIntBetween(2, 10);
ArraySearchPhaseResults<SearchPhaseResult> phaseResults = new ArraySearchPhaseResults<>(numShards);
AbstractSearchAsyncAction<SearchPhaseResult> action = createAction(searchRequest, phaseResults, listener, false, new AtomicLong());
// skip one to avoid the "all shards failed" failure.
SearchShardIterator skipIterator = new SearchShardIterator(null, null, Collections.emptyList(), null);
skipIterator.resetAndSkip();
action.skipShard(skipIterator);

// Validate no exception is thrown
action.executeNextPhase(action, createFetchSearchPhase());
action.sendSearchResponse(InternalSearchResponse.empty(), phaseResults.results);
}

private static ArraySearchPhaseResults<SearchPhaseResult> phaseResults(
Set<ShardSearchContextId> contextIds,
List<Tuple<String, String>> nodeLookups,
Expand Down
Loading

0 comments on commit b838c5f

Please sign in to comment.