Skip to content

Commit f033fa1

Browse files
Fix SearchableSnapshotsIntegTests.testCreateAndRestoreSearchableSnapshot (#65343) (#65350)
The recovery stats assertions in this test ran without any waiting for the recoveries to actually finish. The fact that they ran after the concurrent searches checks generally meant that they would pass (because of searches warming caches + general relative slowness of searches) but there is no hard guarantees this will work reliably as the pre-fetch threads which will update the recovery state might still be slow to do so randomly, causing the assertions to trip. closes #65302
1 parent 53c64d5 commit f033fa1

File tree

1 file changed

+26
-23
lines changed

1 file changed

+26
-23
lines changed

x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsIntegTests.java

Lines changed: 26 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
import java.util.concurrent.BrokenBarrierException;
6666
import java.util.concurrent.CountDownLatch;
6767
import java.util.concurrent.CyclicBarrier;
68+
import java.util.concurrent.TimeUnit;
6869
import java.util.stream.Collectors;
6970
import java.util.stream.IntStream;
7071
import java.util.stream.Stream;
@@ -710,32 +711,34 @@ private void assertTotalHits(String indexName, TotalHits originalAllHits, TotalH
710711
}
711712
}
712713

713-
private void assertRecoveryStats(String indexName, boolean preWarmEnabled) {
714+
private void assertRecoveryStats(String indexName, boolean preWarmEnabled) throws Exception {
714715
int shardCount = getNumShards(indexName).totalNumShards;
715-
final RecoveryResponse recoveryResponse = client().admin().indices().prepareRecoveries(indexName).get();
716-
assertThat(recoveryResponse.toString(), recoveryResponse.shardRecoveryStates().get(indexName).size(), equalTo(shardCount));
717-
718-
for (List<RecoveryState> recoveryStates : recoveryResponse.shardRecoveryStates().values()) {
719-
for (RecoveryState recoveryState : recoveryStates) {
720-
ByteSizeValue cacheSize = getCacheSizeForNode(recoveryState.getTargetNode().getName());
721-
boolean unboundedCache = cacheSize.equals(new ByteSizeValue(Long.MAX_VALUE, ByteSizeUnit.BYTES));
722-
RecoveryState.Index index = recoveryState.getIndex();
723-
assertThat(
724-
Strings.toString(recoveryState, true, true),
725-
index.recoveredFileCount(),
726-
preWarmEnabled && unboundedCache ? equalTo(index.totalRecoverFiles()) : greaterThanOrEqualTo(0)
727-
);
716+
assertBusy(() -> {
717+
final RecoveryResponse recoveryResponse = client().admin().indices().prepareRecoveries(indexName).get();
718+
assertThat(recoveryResponse.toString(), recoveryResponse.shardRecoveryStates().get(indexName).size(), equalTo(shardCount));
719+
720+
for (List<RecoveryState> recoveryStates : recoveryResponse.shardRecoveryStates().values()) {
721+
for (RecoveryState recoveryState : recoveryStates) {
722+
ByteSizeValue cacheSize = getCacheSizeForNode(recoveryState.getTargetNode().getName());
723+
boolean unboundedCache = cacheSize.equals(new ByteSizeValue(Long.MAX_VALUE, ByteSizeUnit.BYTES));
724+
RecoveryState.Index index = recoveryState.getIndex();
725+
assertThat(
726+
Strings.toString(recoveryState, true, true),
727+
index.recoveredFileCount(),
728+
preWarmEnabled && unboundedCache ? equalTo(index.totalRecoverFiles()) : greaterThanOrEqualTo(0)
729+
);
728730

729-
// Since the cache size is variable, the pre-warm phase might fail as some of the files can be evicted
730-
// while a part is pre-fetched, in that case the recovery state stage is left as FINALIZE.
731-
assertThat(
732-
recoveryState.getStage(),
733-
unboundedCache
734-
? equalTo(RecoveryState.Stage.DONE)
735-
: anyOf(equalTo(RecoveryState.Stage.DONE), equalTo(RecoveryState.Stage.FINALIZE))
736-
);
731+
// Since the cache size is variable, the pre-warm phase might fail as some of the files can be evicted
732+
// while a part is pre-fetched, in that case the recovery state stage is left as FINALIZE.
733+
assertThat(
734+
recoveryState.getStage(),
735+
unboundedCache
736+
? equalTo(RecoveryState.Stage.DONE)
737+
: anyOf(equalTo(RecoveryState.Stage.DONE), equalTo(RecoveryState.Stage.FINALIZE))
738+
);
739+
}
737740
}
738-
}
741+
}, 30L, TimeUnit.SECONDS);
739742
}
740743

741744
private void assertSearchableSnapshotStats(String indexName, boolean cacheEnabled, List<String> nonCachedExtensions) {

0 commit comments

Comments
 (0)