From f4f36872afa8e2bc8d0b21cd2c1a266836ee4343 Mon Sep 17 00:00:00 2001 From: Niels Bauman <33722607+nielsbauman@users.noreply.github.com> Date: Mon, 10 Nov 2025 19:03:26 +0100 Subject: [PATCH 1/2] Fix `GeoIpDownloaderIT.testInvalidTimestamp` (#137663) The GeoIP expiration logic assumes that the md5 hashes of the databases change over time, as it deletes all database chunks from the .geoip_databases index. If the hashes do not change, no new database chunks are indexed and so the ingest nodes fail to download the databases as there are no chunks to download. In this test suite, we don't have a way to change the database files served by the fixture, and thus change their md5 hashes. Since we assume that in a real-world scenario the database files change over time, we are ok with skipping this part of the test. See https://github.com/elastic/elasticsearch/issues/128284#issuecomment-3496286781 for a detailed investigation. Closes #128284 (cherry picked from commit 2774bf4680ce0dc0435e1318fe41c72f52407e13) # Conflicts: # muted-tests.yml --- .../ingest/geoip/GeoIpDownloaderIT.java | 68 +++++++++++++++---- 1 file changed, 53 insertions(+), 15 deletions(-) diff --git a/modules/ingest-geoip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/GeoIpDownloaderIT.java b/modules/ingest-geoip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/GeoIpDownloaderIT.java index c1699ebf487ff..2b03c1e1817b5 100644 --- a/modules/ingest-geoip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/GeoIpDownloaderIT.java +++ b/modules/ingest-geoip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/GeoIpDownloaderIT.java @@ -16,18 +16,24 @@ import org.elasticsearch.action.ingest.SimulatePipelineRequest; import org.elasticsearch.action.ingest.SimulatePipelineResponse; import org.elasticsearch.client.internal.Client; +import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.IOUtils; import org.elasticsearch.core.SuppressForbidden; import org.elasticsearch.core.TimeValue; import org.elasticsearch.env.Environment; +import org.elasticsearch.index.IndexMode; +import org.elasticsearch.index.IndexSettingProvider; import org.elasticsearch.index.IndexSettings; +import org.elasticsearch.index.IndexVersion; import org.elasticsearch.index.query.BoolQueryBuilder; import org.elasticsearch.index.query.MatchQueryBuilder; import org.elasticsearch.index.query.RangeQueryBuilder; +import org.elasticsearch.indices.IndicesRequestCache; import org.elasticsearch.ingest.AbstractProcessor; import org.elasticsearch.ingest.IngestDocument; import org.elasticsearch.ingest.Processor; @@ -53,6 +59,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; +import java.time.Instant; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; @@ -70,6 +77,7 @@ import static org.elasticsearch.ingest.ConfigurationUtils.readStringProperty; import static org.elasticsearch.ingest.geoip.GeoIpTestUtils.copyDefaultDatabases; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoSearchHits; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertResponse; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.anEmptyMap; @@ -79,7 +87,6 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasEntry; import static org.hamcrest.Matchers.hasItem; -import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; @@ -95,7 +102,8 @@ protected Collection> nodePlugins() { ReindexPlugin.class, IngestGeoIpPlugin.class, GeoIpProcessorNonIngestNodeIT.IngestGeoIpSettingsPlugin.class, - NonGeoProcessorsPlugin.class + NonGeoProcessorsPlugin.class, + GeoIpIndexSettingProviderPlugin.class ); } @@ -188,6 +196,11 @@ public void testInvalidTimestamp() throws Exception { } } }); + // We wait for the deletion of the database chunks in the .geoip_databases index. Since the search request cache is disabled by + // GeoIpIndexSettingProvider, we can be sure that all nodes are unable to fetch the deleted chunks from the cache. + logger.info("---> waiting for database chunks to be deleted"); + assertBusy(() -> assertNoSearchHits(prepareSearch(GeoIpDownloader.DATABASES_INDEX).setRequestCache(false))); + logger.info("---> database chunks deleted"); putGeoIpPipeline(); assertBusy(() -> { SimulateDocumentBaseResult result = simulatePipeline(); @@ -206,19 +219,12 @@ public void testInvalidTimestamp() throws Exception { assertFalse(result.getIngestDocument().hasField("ip-asn")); assertFalse(result.getIngestDocument().hasField("ip-country")); }); - updateClusterSettings(Settings.builder().putNull("ingest.geoip.database_validity")); - assertBusy(() -> { - for (Path geoIpTmpDir : geoIpTmpDirs) { - try (Stream files = Files.list(geoIpTmpDir)) { - Set names = files.map(f -> f.getFileName().toString()).collect(Collectors.toSet()); - assertThat( - names, - hasItems("GeoLite2-ASN.mmdb", "GeoLite2-City.mmdb", "GeoLite2-Country.mmdb", "MyCustomGeoLite2-City.mmdb") - ); - } - } - }); - awaitAllNodesDownloadedDatabases(); + // Ideally, we would reset the validity setting to its default value (30d) here and verify that the databases are downloaded again. + // However, this expiration logic assumes that the md5 hashes of the databases change over time, as it deletes all database chunks + // from the .geoip_databases index. If the hashes do not change, no new database chunks are indexed and so the ingest nodes fail + // to download the databases as there are no chunks to download. + // In this test suite, we don't have a way to change the database files served by the fixture, and thus change their md5 hashes. + // Since we assume that in a real world scenario the database files change over time, we are ok with skipping this part of the test. } @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/92888") @@ -871,4 +877,36 @@ public String getType() { return procMap; } } + + /** + * A simple plugin that provides the {@link GeoIpIndexSettingProvider}. + */ + public static final class GeoIpIndexSettingProviderPlugin extends Plugin { + @Override + public Collection getAdditionalIndexSettingProviders(IndexSettingProvider.Parameters parameters) { + return List.of(new GeoIpIndexSettingProvider()); + } + } + + /** + * An index setting provider that disables the request cache for the `.geoip_databases` index. + * Since `.geoip_databases` is a system index, we can't configure this setting using the API or index templates. + */ + public static final class GeoIpIndexSettingProvider implements IndexSettingProvider { + @Override + public Settings getAdditionalIndexSettings( + String indexName, + String dataStreamName, + IndexMode templateIndexMode, + Metadata metadata, + Instant resolvedAt, + Settings indexTemplateAndCreateRequestSettings, + List combinedTemplateMappings + ) { + if (GeoIpDownloader.GEOIP_DOWNLOADER.equals(indexName)) { + return Settings.builder().put(IndicesRequestCache.INDEX_CACHE_REQUEST_ENABLED_SETTING.getKey(), false).build(); + } + return Settings.EMPTY; + } + } } From 7dce51004ea1159fde0bf31a4d780cb57c09e989 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Mon, 10 Nov 2025 18:27:26 +0000 Subject: [PATCH 2/2] [CI] Auto commit changes from spotless --- .../java/org/elasticsearch/ingest/geoip/GeoIpDownloaderIT.java | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/ingest-geoip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/GeoIpDownloaderIT.java b/modules/ingest-geoip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/GeoIpDownloaderIT.java index 2b03c1e1817b5..2e392f9198071 100644 --- a/modules/ingest-geoip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/GeoIpDownloaderIT.java +++ b/modules/ingest-geoip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/GeoIpDownloaderIT.java @@ -29,7 +29,6 @@ import org.elasticsearch.index.IndexMode; import org.elasticsearch.index.IndexSettingProvider; import org.elasticsearch.index.IndexSettings; -import org.elasticsearch.index.IndexVersion; import org.elasticsearch.index.query.BoolQueryBuilder; import org.elasticsearch.index.query.MatchQueryBuilder; import org.elasticsearch.index.query.RangeQueryBuilder;