diff --git a/build-tools-internal/src/main/resources/changelog-schema.json b/build-tools-internal/src/main/resources/changelog-schema.json index 451701d74d690..1d9d34a890c8d 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 cb667400240f0..54430b8abd894 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 @@ -118,17 +118,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 6c24ec2d17604..67e8cd8a82a00 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 @@ -16,6 +16,7 @@ import org.elasticsearch.cluster.action.shard.ShardStateAction; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.index.engine.Engine; import org.elasticsearch.indices.IndicesService; import org.elasticsearch.injection.guice.Inject; import org.elasticsearch.tasks.Task; @@ -73,12 +74,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 f0629bee5f72f..8f66891b26f88 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 @@ -47,7 +47,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 31934c1606446..25032accb4535 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/ReadOnlyEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/ReadOnlyEngine.java @@ -445,7 +445,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 67d9d7a82acf3..11b63c0f440ff 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 @@ -71,6 +71,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; @@ -542,6 +543,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 86120196492c9..90d2f0bc1b6b5 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,