From 1772af27f7c393082c9fb30476b57efb640cacc3 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Thu, 12 Jun 2025 14:54:50 +0200 Subject: [PATCH 1/2] When a primary shard uses the read-only engine, it always returns a RefreshResult.NO_REFRESH for refreshes. Since #93600 we added an extra roundtrip to hook unpromotable shard refresh logic. This hook is always executed, even if there are no unpromotable shards, but the UnpromotableShardRefreshRequest would fail if the primary shard returns a RefreshResult.NO_REFRESH result. Fix to be backported to several versions as it's annoying. Closes #129036 Backport of #129176 for 9.0.3 --- .../src/main/resources/changelog-schema.json | 1 + docs/changelog/129176.yaml | 6 ++++ .../refresh/TransportShardRefreshAction.java | 12 +++---- ...ansportUnpromotableShardRefreshAction.java | 11 ++++--- .../UnpromotableShardRefreshRequest.java | 4 ++- .../index/engine/ReadOnlyEngine.java | 2 +- .../FrozenSearchableSnapshotsIntegTests.java | 32 ++++++++++++++++++ .../SearchableSnapshotsIntegTests.java | 33 +++++++++++++++++++ 8 files changed, 86 insertions(+), 15 deletions(-) create mode 100644 docs/changelog/129176.yaml diff --git a/build-tools-internal/src/main/resources/changelog-schema.json b/build-tools-internal/src/main/resources/changelog-schema.json index 7229571fc8bf4..24189a3c50106 100644 --- a/build-tools-internal/src/main/resources/changelog-schema.json +++ b/build-tools-internal/src/main/resources/changelog-schema.json @@ -86,6 +86,7 @@ "Rollup", "SQL", "Search", + "Searchable Snapshots", "Security", "Snapshot/Restore", "Stats", 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 6b77a39c32ffe..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 @@ -125,17 +125,13 @@ public void onPrimaryOperationComplete( IndexShardRoutingTable indexShardRoutingTable, ActionListener listener ) { - assert replicaRequest.primaryRefreshResult.refreshed() : "primary has not refreshed"; - UnpromotableShardRefreshRequest unpromotableReplicaRequest = new UnpromotableShardRefreshRequest( - indexShardRoutingTable, - replicaRequest.primaryRefreshResult.primaryTerm(), - replicaRequest.primaryRefreshResult.generation(), - false - ); + var primaryTerm = replicaRequest.primaryRefreshResult.primaryTerm(); + var generation = replicaRequest.primaryRefreshResult.generation(); + transportService.sendRequest( transportService.getLocalNode(), TransportUnpromotableShardRefreshAction.NAME, - unpromotableReplicaRequest, + new UnpromotableShardRefreshRequest(indexShardRoutingTable, primaryTerm, generation, false), new ActionListenerResponseHandler<>(listener.safeMap(r -> null), in -> ActionResponse.Empty.INSTANCE, refreshExecutor) ); } 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..435b4293ad0ca 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 @@ -154,12 +154,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 da60f9a84c3ba..655b78233660b 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 @@ -70,6 +70,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.arrayWithSize; @@ -536,6 +537,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 c207ea1fde1ea..e31ca34113922 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; @@ -1274,6 +1276,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, From 3fc67ffa576e85c56f4cbfc748aa556149427669 Mon Sep 17 00:00:00 2001 From: tlrx Date: Thu, 12 Jun 2025 15:26:49 +0200 Subject: [PATCH 2/2] fix import --- .../indices/refresh/TransportUnpromotableShardRefreshAction.java | 1 + 1 file changed, 1 insertion(+) 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 435b4293ad0ca..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;