Browse Source

Re-check node cache stats before failing (#112688)

Closes #112384
Nick Tindall 1 year ago
parent
commit
02859bad33

+ 0 - 3
muted-tests.yml

@@ -132,9 +132,6 @@ tests:
 - class: org.elasticsearch.xpack.ml.integration.MlJobIT
   method: testMultiIndexDelete
   issue: https://github.com/elastic/elasticsearch/issues/112381
-- class: org.elasticsearch.xpack.searchablesnapshots.cache.shared.NodesCachesStatsIntegTests
-  method: testNodesCachesStats
-  issue: https://github.com/elastic/elasticsearch/issues/112384
 - class: org.elasticsearch.action.admin.cluster.stats.CCSTelemetrySnapshotTests
   method: testToXContent
   issue: https://github.com/elastic/elasticsearch/issues/112325

+ 28 - 24
x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/shared/NodesCachesStatsIntegTests.java

@@ -136,30 +136,34 @@ public class NodesCachesStatsIntegTests extends BaseFrozenSearchableSnapshotsInt
             .collect(toSet())
             .toArray(String[]::new);
 
-        final NodesCachesStatsResponse response = client().execute(
-            TransportSearchableSnapshotsNodeCachesStatsAction.TYPE,
-            new NodesRequest(dataNodesWithFrozenShards)
-        ).actionGet();
-        assertThat(
-            response.getNodes().stream().map(r -> r.getNode().getId()).collect(Collectors.toList()),
-            containsInAnyOrder(dataNodesWithFrozenShards)
-        );
-        assertThat(response.hasFailures(), equalTo(false));
-
-        for (NodeCachesStatsResponse nodeCachesStats : response.getNodes()) {
-            if (nodeCachesStats.getNumRegions() > 0) {
-                assertThat(nodeCachesStats.getWrites(), greaterThan(0L));
-                assertThat(nodeCachesStats.getBytesWritten(), greaterThan(0L));
-                assertThat(nodeCachesStats.getReads(), greaterThan(0L));
-                assertThat(nodeCachesStats.getBytesRead(), greaterThan(0L));
-                assertThat(nodeCachesStats.getEvictions(), greaterThan(0L));
-            } else {
-                assertThat(nodeCachesStats.getWrites(), equalTo(0L));
-                assertThat(nodeCachesStats.getBytesWritten(), equalTo(0L));
-                assertThat(nodeCachesStats.getReads(), equalTo(0L));
-                assertThat(nodeCachesStats.getBytesRead(), equalTo(0L));
-                assertThat(nodeCachesStats.getEvictions(), equalTo(0L));
+        // We've seen `getWrites` inexplicably return zero. `assertBusy` to test the theory of it being due
+        // to contention on the `LongAdder` at `SharedBlobCacheService#writeCount`.
+        assertBusy(() -> {
+            final NodesCachesStatsResponse response = client().execute(
+                TransportSearchableSnapshotsNodeCachesStatsAction.TYPE,
+                new NodesRequest(dataNodesWithFrozenShards)
+            ).actionGet();
+            assertThat(
+                response.getNodes().stream().map(r -> r.getNode().getId()).collect(Collectors.toList()),
+                containsInAnyOrder(dataNodesWithFrozenShards)
+            );
+            assertThat(response.hasFailures(), equalTo(false));
+
+            for (NodeCachesStatsResponse nodeCachesStats : response.getNodes()) {
+                if (nodeCachesStats.getNumRegions() > 0) {
+                    assertThat(nodeCachesStats.getWrites(), greaterThan(0L));
+                    assertThat(nodeCachesStats.getBytesWritten(), greaterThan(0L));
+                    assertThat(nodeCachesStats.getReads(), greaterThan(0L));
+                    assertThat(nodeCachesStats.getBytesRead(), greaterThan(0L));
+                    assertThat(nodeCachesStats.getEvictions(), greaterThan(0L));
+                } else {
+                    assertThat(nodeCachesStats.getWrites(), equalTo(0L));
+                    assertThat(nodeCachesStats.getBytesWritten(), equalTo(0L));
+                    assertThat(nodeCachesStats.getReads(), equalTo(0L));
+                    assertThat(nodeCachesStats.getBytesRead(), equalTo(0L));
+                    assertThat(nodeCachesStats.getEvictions(), equalTo(0L));
+                }
             }
-        }
+        });
     }
 }