Skip to content

Commit d52ce6c

Browse files
Fix SearchableSnapshotsIntegTests.testCreateAndRestoreSearchableSnapshot (elastic#65343)
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 elastic#65302
1 parent b0cea04 commit d52ce6c

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
@@ -64,6 +64,7 @@
6464
import java.util.concurrent.BrokenBarrierException;
6565
import java.util.concurrent.CountDownLatch;
6666
import java.util.concurrent.CyclicBarrier;
67+
import java.util.concurrent.TimeUnit;
6768
import java.util.stream.Collectors;
6869
import java.util.stream.IntStream;
6970
import java.util.stream.Stream;
@@ -700,32 +701,34 @@ private void assertTotalHits(String indexName, TotalHits originalAllHits, TotalH
700701
}
701702
}
702703

703-
private void assertRecoveryStats(String indexName, boolean preWarmEnabled) {
704+
private void assertRecoveryStats(String indexName, boolean preWarmEnabled) throws Exception {
704705
int shardCount = getNumShards(indexName).totalNumShards;
705-
final RecoveryResponse recoveryResponse = client().admin().indices().prepareRecoveries(indexName).get();
706-
assertThat(recoveryResponse.toString(), recoveryResponse.shardRecoveryStates().get(indexName).size(), equalTo(shardCount));
707-
708-
for (List<RecoveryState> recoveryStates : recoveryResponse.shardRecoveryStates().values()) {
709-
for (RecoveryState recoveryState : recoveryStates) {
710-
ByteSizeValue cacheSize = getCacheSizeForNode(recoveryState.getTargetNode().getName());
711-
boolean unboundedCache = cacheSize.equals(new ByteSizeValue(Long.MAX_VALUE, ByteSizeUnit.BYTES));
712-
RecoveryState.Index index = recoveryState.getIndex();
713-
assertThat(
714-
Strings.toString(recoveryState, true, true),
715-
index.recoveredFileCount(),
716-
preWarmEnabled && unboundedCache ? equalTo(index.totalRecoverFiles()) : greaterThanOrEqualTo(0)
717-
);
706+
assertBusy(() -> {
707+
final RecoveryResponse recoveryResponse = client().admin().indices().prepareRecoveries(indexName).get();
708+
assertThat(recoveryResponse.toString(), recoveryResponse.shardRecoveryStates().get(indexName).size(), equalTo(shardCount));
709+
710+
for (List<RecoveryState> recoveryStates : recoveryResponse.shardRecoveryStates().values()) {
711+
for (RecoveryState recoveryState : recoveryStates) {
712+
ByteSizeValue cacheSize = getCacheSizeForNode(recoveryState.getTargetNode().getName());
713+
boolean unboundedCache = cacheSize.equals(new ByteSizeValue(Long.MAX_VALUE, ByteSizeUnit.BYTES));
714+
RecoveryState.Index index = recoveryState.getIndex();
715+
assertThat(
716+
Strings.toString(recoveryState, true, true),
717+
index.recoveredFileCount(),
718+
preWarmEnabled && unboundedCache ? equalTo(index.totalRecoverFiles()) : greaterThanOrEqualTo(0)
719+
);
718720

719-
// Since the cache size is variable, the pre-warm phase might fail as some of the files can be evicted
720-
// while a part is pre-fetched, in that case the recovery state stage is left as FINALIZE.
721-
assertThat(
722-
recoveryState.getStage(),
723-
unboundedCache
724-
? equalTo(RecoveryState.Stage.DONE)
725-
: anyOf(equalTo(RecoveryState.Stage.DONE), equalTo(RecoveryState.Stage.FINALIZE))
726-
);
721+
// Since the cache size is variable, the pre-warm phase might fail as some of the files can be evicted
722+
// while a part is pre-fetched, in that case the recovery state stage is left as FINALIZE.
723+
assertThat(
724+
recoveryState.getStage(),
725+
unboundedCache
726+
? equalTo(RecoveryState.Stage.DONE)
727+
: anyOf(equalTo(RecoveryState.Stage.DONE), equalTo(RecoveryState.Stage.FINALIZE))
728+
);
729+
}
727730
}
728-
}
731+
}, 30L, TimeUnit.SECONDS);
729732
}
730733

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

0 commit comments

Comments
 (0)