diff --git a/docs/changelog/129176.yaml b/docs/changelog/129176.yaml new file mode 100644 index 0000000000000..668cfb68b9f89 --- /dev/null +++ b/docs/changelog/129176.yaml @@ -0,0 +1,6 @@ +pr: 129176 +summary: Adjust unpromotable shard refresh request validation to allow `RefreshResult.NO_REFRESH` +area: Searchable Snapshots +type: bug +issues: + - 129036 diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/refresh/TransportShardRefreshAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/refresh/TransportShardRefreshAction.java index 2286f64648185..3dc3e19dcb979 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/refresh/TransportShardRefreshAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/refresh/TransportShardRefreshAction.java @@ -23,7 +23,6 @@ import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.index.engine.Engine; import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.indices.IndicesService; import org.elasticsearch.injection.guice.Inject; @@ -127,10 +126,7 @@ public void onPrimaryOperationComplete( ActionListener listener ) { var primaryTerm = replicaRequest.primaryRefreshResult.primaryTerm(); - assert Engine.UNKNOWN_PRIMARY_TERM < primaryTerm : primaryTerm; - var generation = replicaRequest.primaryRefreshResult.generation(); - assert Engine.RefreshResult.UNKNOWN_GENERATION < generation : generation; transportService.sendRequest( transportService.getLocalNode(), diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/refresh/TransportUnpromotableShardRefreshAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/refresh/TransportUnpromotableShardRefreshAction.java index dd4fbedad98a6..283774ed4a439 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/refresh/TransportUnpromotableShardRefreshAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/refresh/TransportUnpromotableShardRefreshAction.java @@ -22,6 +22,7 @@ import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.core.TimeValue; +import org.elasticsearch.index.engine.Engine; import org.elasticsearch.indices.IndicesService; import org.elasticsearch.injection.guice.Inject; import org.elasticsearch.node.NodeClosedException; @@ -154,12 +155,13 @@ protected void unpromotableShardOperation( return; } + var primaryTerm = request.getPrimaryTerm(); + assert Engine.UNKNOWN_PRIMARY_TERM < primaryTerm : primaryTerm; + var segmentGeneration = request.getSegmentGeneration(); + assert Engine.RefreshResult.UNKNOWN_GENERATION < segmentGeneration : segmentGeneration; + ActionListener.run(responseListener, listener -> { - shard.waitForPrimaryTermAndGeneration( - request.getPrimaryTerm(), - request.getSegmentGeneration(), - listener.map(l -> ActionResponse.Empty.INSTANCE) - ); + shard.waitForPrimaryTermAndGeneration(primaryTerm, segmentGeneration, listener.map(l -> ActionResponse.Empty.INSTANCE)); }); } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/refresh/UnpromotableShardRefreshRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/refresh/UnpromotableShardRefreshRequest.java index 07b2bc1fcf7c1..af6aac3c7fa53 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/refresh/UnpromotableShardRefreshRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/refresh/UnpromotableShardRefreshRequest.java @@ -63,7 +63,9 @@ public UnpromotableShardRefreshRequest(StreamInput in) throws IOException { @Override public ActionRequestValidationException validate() { ActionRequestValidationException validationException = super.validate(); - if (segmentGeneration == Engine.RefreshResult.UNKNOWN_GENERATION) { + if (segmentGeneration == Engine.RefreshResult.UNKNOWN_GENERATION && primaryTerm == Engine.UNKNOWN_PRIMARY_TERM) { + // read-only primary shards (like searchable snapshot shard) return Engine.RefreshResult.NO_REFRESH during refresh + } else if (segmentGeneration == Engine.RefreshResult.UNKNOWN_GENERATION) { validationException = addValidationError("segment generation is unknown", validationException); } return validationException; diff --git a/server/src/main/java/org/elasticsearch/index/engine/ReadOnlyEngine.java b/server/src/main/java/org/elasticsearch/index/engine/ReadOnlyEngine.java index 63a4696ddb08e..11ba440a3d6ea 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/ReadOnlyEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/ReadOnlyEngine.java @@ -446,7 +446,7 @@ public RefreshResult refresh(String source) { @Override public void maybeRefresh(String source, ActionListener listener) throws EngineException { - listener.onResponse(RefreshResult.NO_REFRESH); + ActionListener.completeWith(listener, () -> refresh(source)); } @Override diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/FrozenSearchableSnapshotsIntegTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/FrozenSearchableSnapshotsIntegTests.java index 8f6329be0ccda..5c86ca4649194 100644 --- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/FrozenSearchableSnapshotsIntegTests.java +++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/FrozenSearchableSnapshotsIntegTests.java @@ -67,6 +67,7 @@ import static org.elasticsearch.search.aggregations.AggregationBuilders.dateHistogram; import static org.elasticsearch.snapshots.SearchableSnapshotsSettings.SEARCHABLE_SNAPSHOT_STORE_TYPE; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailuresAndResponse; import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_RECOVERY_STATE_FACTORY_KEY; import static org.hamcrest.Matchers.containsString; @@ -521,6 +522,37 @@ public void testRequestCacheOnFrozen() throws Exception { } } + public void testRefreshPartiallyMountedIndex() throws Exception { + internalCluster().ensureAtLeastNumDataNodes(2); + + final var index = "index"; + createIndex(index, 1, 0); + populateIndex(index, 1_000); + + final var repository = "repository"; + createRepository(repository, FsRepository.TYPE, Settings.builder().put("location", randomRepoPath())); + + final var snapshot = "repository"; + createFullSnapshot(repository, snapshot); + + assertAcked(indicesAdmin().prepareDelete(index)); + + final var partialIndex = "partial-" + index; + mountSnapshot( + repository, + snapshot, + index, + partialIndex, + Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, randomInt(1)).build(), + MountSearchableSnapshotRequest.Storage.SHARED_CACHE + ); + ensureGreen(partialIndex); + + // before the fix this would have failed + var refreshResult = indicesAdmin().prepareRefresh(partialIndex).execute().actionGet(); + assertNoFailures(refreshResult); + } + public void testTierPreferenceCannotBeRemovedForFrozenIndex() throws Exception { final String fsRepoName = randomAlphaOfLength(10); final String indexName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT); diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsIntegTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsIntegTests.java index 22c4fc17a1911..16543a30238b3 100644 --- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsIntegTests.java +++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsIntegTests.java @@ -51,6 +51,7 @@ import org.elasticsearch.index.shard.IndexLongFieldRange; import org.elasticsearch.indices.IndicesService; import org.elasticsearch.repositories.RepositoryData; +import org.elasticsearch.repositories.fs.FsRepository; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.search.SearchResponseUtils; import org.elasticsearch.snapshots.SnapshotId; @@ -89,6 +90,7 @@ import static org.elasticsearch.snapshots.SearchableSnapshotsSettings.SEARCHABLE_SNAPSHOT_STORE_TYPE; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures; import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_RECOVERY_STATE_FACTORY_KEY; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; @@ -1280,6 +1282,37 @@ public void onFailure(Exception e) { } } + public void testRefreshFullyMountedIndex() throws Exception { + internalCluster().ensureAtLeastNumDataNodes(2); + + final var index = "index"; + createIndex(index, 1, 0); + populateIndex(index, 1_000); + + final var repository = "repository"; + createRepository(repository, FsRepository.TYPE, Settings.builder().put("location", randomRepoPath())); + + final var snapshot = "repository"; + createFullSnapshot(repository, snapshot); + + assertAcked(indicesAdmin().prepareDelete(index)); + + final var fullIndex = "full-" + index; + mountSnapshot( + repository, + snapshot, + index, + fullIndex, + Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, randomInt(1)).build(), + MountSearchableSnapshotRequest.Storage.FULL_COPY + ); + ensureGreen(fullIndex); + + // before the fix this would have failed + var refreshResult = indicesAdmin().prepareRefresh(fullIndex).execute().actionGet(); + assertNoFailures(refreshResult); + } + private TaskInfo getTaskForActionFromMaster(String action) { ListTasksResponse response = client().execute( TransportListTasksAction.TYPE,