Fixing DatabaseNodeServiceIT testNonGzippedDatabase and testGzippedDa…

…tabase race condition (elastic#115463) Co-authored-by: Joe Gallo <joegallo@gmail.com>
masseyke · Oct 25, 2024 · 656ce78 · 656ce78
1 parent 7097946
commit 656ce78
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 14 deletions.
diff --git a/...ip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/DatabaseNodeServiceIT.java b/...ip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/DatabaseNodeServiceIT.java
@@ -46,15 +46,21 @@ public class DatabaseNodeServiceIT extends AbstractGeoIpIT {
     public void testNonGzippedDatabase() throws Exception {
         String databaseType = "GeoLite2-Country";
         String databaseFileName = databaseType + ".mmdb";
-        // making the dabase name unique so we know we're not using another one:
+        // making the database name unique so we know we're not using another one:
         String databaseName = randomAlphaOfLength(20) + "-" + databaseFileName;
         byte[] mmdbBytes = getBytesForFile(databaseFileName);
         final DatabaseNodeService databaseNodeService = internalCluster().getInstance(DatabaseNodeService.class);
         assertNull(databaseNodeService.getDatabase(databaseName));
         int numChunks = indexData(databaseName, mmdbBytes);
-        retrieveDatabase(databaseNodeService, databaseName, mmdbBytes, numChunks);
-        assertBusy(() -> assertNotNull(databaseNodeService.getDatabase(databaseName)));
-        assertValidDatabase(databaseNodeService, databaseName, databaseType);
+        /*
+         * If DatabaseNodeService::checkDatabases runs it will sometimes (rarely) remove the database we are using in this test while we
+         * are trying to assert things about it. So if it does then we 'just' try again.
+         */
+        assertBusy(() -> {
+            retrieveDatabase(databaseNodeService, databaseName, mmdbBytes, numChunks);
+            assertNotNull(databaseNodeService.getDatabase(databaseName));
+            assertValidDatabase(databaseNodeService, databaseName, databaseType);
+        });
     }
 
     /*
@@ -64,16 +70,22 @@ public void testNonGzippedDatabase() throws Exception {
     public void testGzippedDatabase() throws Exception {
         String databaseType = "GeoLite2-Country";
         String databaseFileName = databaseType + ".mmdb";
-        // making the dabase name unique so we know we're not using another one:
+        // making the database name unique so we know we're not using another one:
         String databaseName = randomAlphaOfLength(20) + "-" + databaseFileName;
         byte[] mmdbBytes = getBytesForFile(databaseFileName);
         byte[] gzipBytes = gzipFileBytes(databaseName, mmdbBytes);
         final DatabaseNodeService databaseNodeService = internalCluster().getInstance(DatabaseNodeService.class);
         assertNull(databaseNodeService.getDatabase(databaseName));
         int numChunks = indexData(databaseName, gzipBytes);
-        retrieveDatabase(databaseNodeService, databaseName, gzipBytes, numChunks);
-        assertBusy(() -> assertNotNull(databaseNodeService.getDatabase(databaseName)));
-        assertValidDatabase(databaseNodeService, databaseName, databaseType);
+        /*
+         * If DatabaseNodeService::checkDatabases runs it will sometimes (rarely) remove the database we are using in this test while we
+         * are trying to assert things about it. So if it does then we 'just' try again.
+         */
+        assertBusy(() -> {
+            retrieveDatabase(databaseNodeService, databaseName, gzipBytes, numChunks);
+            assertNotNull(databaseNodeService.getDatabase(databaseName));
+            assertValidDatabase(databaseNodeService, databaseName, databaseType);
+        });
     }
 
     /*

diff --git a/muted-tests.yml b/muted-tests.yml
@@ -327,9 +327,6 @@ tests:
 - class: org.elasticsearch.xpack.inference.TextEmbeddingCrudIT
   method: testPutE5Small_withPlatformAgnosticVariant
   issue: https://github.com/elastic/elasticsearch/issues/113983
-- class: org.elasticsearch.ingest.geoip.DatabaseNodeServiceIT
-  method: testGzippedDatabase
-  issue: https://github.com/elastic/elasticsearch/issues/113752
 - class: org.elasticsearch.xpack.rank.rrf.RRFRankClientYamlTestSuiteIT
   method: test {yaml=rrf/800_rrf_with_text_similarity_reranker_retriever/explain using rrf retriever and text-similarity}
   issue: https://github.com/elastic/elasticsearch/issues/114757
@@ -346,9 +343,6 @@ tests:
 - class: org.elasticsearch.xpack.security.operator.OperatorPrivilegesIT
   method: testEveryActionIsEitherOperatorOnlyOrNonOperator
   issue: https://github.com/elastic/elasticsearch/issues/102992
-- class: org.elasticsearch.ingest.geoip.DatabaseNodeServiceIT
-  method: testNonGzippedDatabase
-  issue: https://github.com/elastic/elasticsearch/issues/113821
 - class: org.elasticsearch.xpack.inference.DefaultEndPointsIT
   method: testInferDeploysDefaultE5
   issue: https://github.com/elastic/elasticsearch/issues/115361