Skip to content

Commit 9056012

Browse files
nielsbaumanelasticsearchmachine
andauthored
[8.19] Fix GeoIpDownloaderIT.testInvalidTimestamp (elastic#137663) (elastic#137846)
* Fix `GeoIpDownloaderIT.testInvalidTimestamp` (elastic#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 elastic#128284 (comment) for a detailed investigation. Closes elastic#128284 (cherry picked from commit 2774bf4) # Conflicts: # muted-tests.yml * [CI] Auto commit changes from spotless --------- Co-authored-by: elasticsearchmachine <[email protected]>
1 parent ba77a50 commit 9056012

File tree

1 file changed

+52
-15
lines changed

1 file changed

+52
-15
lines changed

modules/ingest-geoip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/GeoIpDownloaderIT.java

Lines changed: 52 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,23 @@
1616
import org.elasticsearch.action.ingest.SimulatePipelineRequest;
1717
import org.elasticsearch.action.ingest.SimulatePipelineResponse;
1818
import org.elasticsearch.client.internal.Client;
19+
import org.elasticsearch.cluster.metadata.Metadata;
1920
import org.elasticsearch.cluster.node.DiscoveryNode;
2021
import org.elasticsearch.common.bytes.BytesReference;
22+
import org.elasticsearch.common.compress.CompressedXContent;
2123
import org.elasticsearch.common.settings.Setting;
2224
import org.elasticsearch.common.settings.Settings;
2325
import org.elasticsearch.core.IOUtils;
2426
import org.elasticsearch.core.SuppressForbidden;
2527
import org.elasticsearch.core.TimeValue;
2628
import org.elasticsearch.env.Environment;
29+
import org.elasticsearch.index.IndexMode;
30+
import org.elasticsearch.index.IndexSettingProvider;
2731
import org.elasticsearch.index.IndexSettings;
2832
import org.elasticsearch.index.query.BoolQueryBuilder;
2933
import org.elasticsearch.index.query.MatchQueryBuilder;
3034
import org.elasticsearch.index.query.RangeQueryBuilder;
35+
import org.elasticsearch.indices.IndicesRequestCache;
3136
import org.elasticsearch.ingest.AbstractProcessor;
3237
import org.elasticsearch.ingest.IngestDocument;
3338
import org.elasticsearch.ingest.Processor;
@@ -53,6 +58,7 @@
5358
import java.nio.file.Files;
5459
import java.nio.file.Path;
5560
import java.nio.file.StandardCopyOption;
61+
import java.time.Instant;
5662
import java.util.ArrayList;
5763
import java.util.Collection;
5864
import java.util.HashMap;
@@ -70,6 +76,7 @@
7076
import static org.elasticsearch.ingest.ConfigurationUtils.readStringProperty;
7177
import static org.elasticsearch.ingest.geoip.GeoIpTestUtils.copyDefaultDatabases;
7278
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
79+
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoSearchHits;
7380
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertResponse;
7481
import static org.hamcrest.Matchers.allOf;
7582
import static org.hamcrest.Matchers.anEmptyMap;
@@ -79,7 +86,6 @@
7986
import static org.hamcrest.Matchers.equalTo;
8087
import static org.hamcrest.Matchers.hasEntry;
8188
import static org.hamcrest.Matchers.hasItem;
82-
import static org.hamcrest.Matchers.hasItems;
8389
import static org.hamcrest.Matchers.hasKey;
8490
import static org.hamcrest.Matchers.is;
8591
import static org.hamcrest.Matchers.not;
@@ -95,7 +101,8 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
95101
ReindexPlugin.class,
96102
IngestGeoIpPlugin.class,
97103
GeoIpProcessorNonIngestNodeIT.IngestGeoIpSettingsPlugin.class,
98-
NonGeoProcessorsPlugin.class
104+
NonGeoProcessorsPlugin.class,
105+
GeoIpIndexSettingProviderPlugin.class
99106
);
100107
}
101108

@@ -188,6 +195,11 @@ public void testInvalidTimestamp() throws Exception {
188195
}
189196
}
190197
});
198+
// We wait for the deletion of the database chunks in the .geoip_databases index. Since the search request cache is disabled by
199+
// GeoIpIndexSettingProvider, we can be sure that all nodes are unable to fetch the deleted chunks from the cache.
200+
logger.info("---> waiting for database chunks to be deleted");
201+
assertBusy(() -> assertNoSearchHits(prepareSearch(GeoIpDownloader.DATABASES_INDEX).setRequestCache(false)));
202+
logger.info("---> database chunks deleted");
191203
putGeoIpPipeline();
192204
assertBusy(() -> {
193205
SimulateDocumentBaseResult result = simulatePipeline();
@@ -206,19 +218,12 @@ public void testInvalidTimestamp() throws Exception {
206218
assertFalse(result.getIngestDocument().hasField("ip-asn"));
207219
assertFalse(result.getIngestDocument().hasField("ip-country"));
208220
});
209-
updateClusterSettings(Settings.builder().putNull("ingest.geoip.database_validity"));
210-
assertBusy(() -> {
211-
for (Path geoIpTmpDir : geoIpTmpDirs) {
212-
try (Stream<Path> files = Files.list(geoIpTmpDir)) {
213-
Set<String> names = files.map(f -> f.getFileName().toString()).collect(Collectors.toSet());
214-
assertThat(
215-
names,
216-
hasItems("GeoLite2-ASN.mmdb", "GeoLite2-City.mmdb", "GeoLite2-Country.mmdb", "MyCustomGeoLite2-City.mmdb")
217-
);
218-
}
219-
}
220-
});
221-
awaitAllNodesDownloadedDatabases();
221+
// Ideally, we would reset the validity setting to its default value (30d) here and verify that the databases are downloaded again.
222+
// However, this expiration logic assumes that the md5 hashes of the databases change over time, as it deletes all database chunks
223+
// from the .geoip_databases index. If the hashes do not change, no new database chunks are indexed and so the ingest nodes fail
224+
// to download the databases as there are no chunks to download.
225+
// 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.
226+
// 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.
222227
}
223228

224229
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/92888")
@@ -871,4 +876,36 @@ public String getType() {
871876
return procMap;
872877
}
873878
}
879+
880+
/**
881+
* A simple plugin that provides the {@link GeoIpIndexSettingProvider}.
882+
*/
883+
public static final class GeoIpIndexSettingProviderPlugin extends Plugin {
884+
@Override
885+
public Collection<IndexSettingProvider> getAdditionalIndexSettingProviders(IndexSettingProvider.Parameters parameters) {
886+
return List.of(new GeoIpIndexSettingProvider());
887+
}
888+
}
889+
890+
/**
891+
* An index setting provider that disables the request cache for the `.geoip_databases` index.
892+
* Since `.geoip_databases` is a system index, we can't configure this setting using the API or index templates.
893+
*/
894+
public static final class GeoIpIndexSettingProvider implements IndexSettingProvider {
895+
@Override
896+
public Settings getAdditionalIndexSettings(
897+
String indexName,
898+
String dataStreamName,
899+
IndexMode templateIndexMode,
900+
Metadata metadata,
901+
Instant resolvedAt,
902+
Settings indexTemplateAndCreateRequestSettings,
903+
List<CompressedXContent> combinedTemplateMappings
904+
) {
905+
if (GeoIpDownloader.GEOIP_DOWNLOADER.equals(indexName)) {
906+
return Settings.builder().put(IndicesRequestCache.INDEX_CACHE_REQUEST_ENABLED_SETTING.getKey(), false).build();
907+
}
908+
return Settings.EMPTY;
909+
}
910+
}
874911
}

0 commit comments

Comments
 (0)