From 403e0485180e98208a9cd10f52e3d262e2deb3ec Mon Sep 17 00:00:00 2001 From: Przemko Robakowski Date: Tue, 9 Mar 2021 11:36:04 +0100 Subject: [PATCH 1/4] Clean up databases after geoip integration tests --- .../ingest/geoip/AbstractGeoIpIT.java | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/modules/ingest-geoip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/AbstractGeoIpIT.java b/modules/ingest-geoip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/AbstractGeoIpIT.java index c3f3bd16bb293..9493e6f3d1614 100644 --- a/modules/ingest-geoip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/AbstractGeoIpIT.java +++ b/modules/ingest-geoip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/AbstractGeoIpIT.java @@ -10,9 +10,11 @@ import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.env.Environment; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.StreamsUtils; +import org.junit.After; import java.io.ByteArrayInputStream; import java.io.IOException; @@ -25,6 +27,7 @@ import java.util.List; public abstract class AbstractGeoIpIT extends ESIntegTestCase { + @Override protected Collection> nodePlugins() { return Arrays.asList(IngestGeoIpPlugin.class, IngestGeoIpSettingsPlugin.class); @@ -61,4 +64,22 @@ public List> getSettings() { return Collections.singletonList(Setting.simpleString("ingest.geoip.database_path", Setting.Property.NodeScope)); } } + + @After + public void cleanDatabases() throws Exception { + super.tearDown(); + Path path = internalCluster().getInstance(Environment.class).tmpFile(); + Path databases = path.resolve("geoip-databases"); + if (Files.exists(databases)) { + Files.walk(databases) + .filter(Files::isRegularFile) + .forEach(path1 -> { + try { + Files.delete(path1); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + }); + } + } } From f0c6c93c0a030ae3bb8a97842059952c962efcaa Mon Sep 17 00:00:00 2001 From: Przemko Robakowski Date: Tue, 9 Mar 2021 13:10:16 +0100 Subject: [PATCH 2/4] fix cleanup in DatabaseRegistry --- .../ingest/geoip/AbstractGeoIpIT.java | 21 ------- .../ingest/geoip/DatabaseRegistry.java | 61 ++++++++++++------- 2 files changed, 40 insertions(+), 42 deletions(-) diff --git a/modules/ingest-geoip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/AbstractGeoIpIT.java b/modules/ingest-geoip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/AbstractGeoIpIT.java index 9493e6f3d1614..c3f3bd16bb293 100644 --- a/modules/ingest-geoip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/AbstractGeoIpIT.java +++ b/modules/ingest-geoip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/AbstractGeoIpIT.java @@ -10,11 +10,9 @@ import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.env.Environment; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.StreamsUtils; -import org.junit.After; import java.io.ByteArrayInputStream; import java.io.IOException; @@ -27,7 +25,6 @@ import java.util.List; public abstract class AbstractGeoIpIT extends ESIntegTestCase { - @Override protected Collection> nodePlugins() { return Arrays.asList(IngestGeoIpPlugin.class, IngestGeoIpSettingsPlugin.class); @@ -64,22 +61,4 @@ public List> getSettings() { return Collections.singletonList(Setting.simpleString("ingest.geoip.database_path", Setting.Property.NodeScope)); } } - - @After - public void cleanDatabases() throws Exception { - super.tearDown(); - Path path = internalCluster().getInstance(Environment.class).tmpFile(); - Path databases = path.resolve("geoip-databases"); - if (Files.exists(databases)) { - Files.walk(databases) - .filter(Files::isRegularFile) - .forEach(path1 -> { - try { - Files.delete(path1); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - }); - } - } } diff --git a/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/DatabaseRegistry.java b/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/DatabaseRegistry.java index b8e40422e0b28..ed70da2d6bb32 100644 --- a/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/DatabaseRegistry.java +++ b/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/DatabaseRegistry.java @@ -32,12 +32,14 @@ import java.io.Closeable; import java.io.IOException; -import java.io.UncheckedIOException; import java.nio.file.FileAlreadyExistsException; +import java.nio.file.FileVisitResult; +import java.nio.file.FileVisitor; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.nio.file.StandardOpenOption; +import java.nio.file.attribute.BasicFileAttributes; import java.security.MessageDigest; import java.util.ArrayList; import java.util.Collection; @@ -52,20 +54,20 @@ /** * A component that is responsible for making the databases maintained by {@link GeoIpDownloader} * available for ingest processors. - * + *

* Also provided a lookup mechanism for geoip processors with fallback to {@link LocalDatabases}. * All databases are downloaded into a geoip tmp directory, which is created at node startup. - * + *

* The following high level steps are executed after each cluster state update: * 1) Check which databases are available in {@link GeoIpTaskState}, - * which is part of the geoip downloader persistent task. + * which is part of the geoip downloader persistent task. * 2) For each database check whether the databases have changed - * by comparing the local and remote md5 hash or are locally missing. + * by comparing the local and remote md5 hash or are locally missing. * 3) For each database identified in step 2 start downloading the database - * chunks. Each chunks is appended to a tmp file (inside geoip tmp dir) and - * after all chunks have been downloaded, the database is uncompressed and - * renamed to the final filename.After this the database is loaded and - * if there is an old instance of this database then that is closed. + * chunks. Each chunks is appended to a tmp file (inside geoip tmp dir) and + * after all chunks have been downloaded, the database is uncompressed and + * renamed to the final filename.After this the database is loaded and + * if there is an old instance of this database then that is closed. * 4) Cleanup locally loaded databases that are no longer mentioned in {@link GeoIpTaskState}. */ final class DatabaseRegistry implements Closeable { @@ -104,18 +106,35 @@ final class DatabaseRegistry implements Closeable { public void initialize(ResourceWatcherService resourceWatcher, IngestService ingestService) throws IOException { localDatabases.initialize(resourceWatcher); - if (Files.exists(geoipTmpDirectory)) { - Files.walk(geoipTmpDirectory) - .filter(Files::isRegularFile) - .peek(path -> LOGGER.info("deleting stale file [{}]", path)) - .forEach(path -> { - try { - Files.delete(path); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - }); - } else { + Files.walkFileTree(geoipTmpDirectory, new FileVisitor<>() { + @Override + public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) { + return FileVisitResult.CONTINUE; + } + + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) { + try { + LOGGER.info("deleting stale file [{}]", file); + Files.deleteIfExists(file); + } catch (IOException e) { + LOGGER.warn("can't delete stale file [" + file + "]", e); + } + return FileVisitResult.CONTINUE; + } + + @Override + public FileVisitResult visitFileFailed(Path file, IOException e) { + LOGGER.warn("can't delete stale file [" + file + "]", e); + return FileVisitResult.CONTINUE; + } + + @Override + public FileVisitResult postVisitDirectory(Path dir, IOException exc) { + return FileVisitResult.CONTINUE; + } + }); + if (Files.exists(geoipTmpDirectory) == false) { Files.createDirectory(geoipTmpDirectory); } LOGGER.info("initialized database registry, using geoip-databases directory [{}]", geoipTmpDirectory); From 012391b6eed2651c3fb7e68da64e97acf32a9753 Mon Sep 17 00:00:00 2001 From: Przemko Robakowski Date: Tue, 9 Mar 2021 13:15:51 +0100 Subject: [PATCH 3/4] NSF is safe to skip --- .../org/elasticsearch/ingest/geoip/DatabaseRegistry.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/DatabaseRegistry.java b/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/DatabaseRegistry.java index ed70da2d6bb32..f603bbc545fb3 100644 --- a/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/DatabaseRegistry.java +++ b/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/DatabaseRegistry.java @@ -36,6 +36,7 @@ import java.nio.file.FileVisitResult; import java.nio.file.FileVisitor; import java.nio.file.Files; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.nio.file.StandardOpenOption; @@ -125,7 +126,9 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) { @Override public FileVisitResult visitFileFailed(Path file, IOException e) { - LOGGER.warn("can't delete stale file [" + file + "]", e); + if(e instanceof NoSuchFileException == false) { + LOGGER.warn("can't delete stale file [" + file + "]", e); + } return FileVisitResult.CONTINUE; } From 52cfd971cc5eac4e432b5c0bfb7e36903fe328fa Mon Sep 17 00:00:00 2001 From: Przemko Robakowski Date: Tue, 9 Mar 2021 13:17:29 +0100 Subject: [PATCH 4/4] whitespaces --- .../ingest/geoip/DatabaseRegistry.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/DatabaseRegistry.java b/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/DatabaseRegistry.java index f603bbc545fb3..48133f0f5f2be 100644 --- a/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/DatabaseRegistry.java +++ b/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/DatabaseRegistry.java @@ -55,20 +55,20 @@ /** * A component that is responsible for making the databases maintained by {@link GeoIpDownloader} * available for ingest processors. - *

+ * * Also provided a lookup mechanism for geoip processors with fallback to {@link LocalDatabases}. * All databases are downloaded into a geoip tmp directory, which is created at node startup. - *

+ * * The following high level steps are executed after each cluster state update: * 1) Check which databases are available in {@link GeoIpTaskState}, - * which is part of the geoip downloader persistent task. + * which is part of the geoip downloader persistent task. * 2) For each database check whether the databases have changed - * by comparing the local and remote md5 hash or are locally missing. + * by comparing the local and remote md5 hash or are locally missing. * 3) For each database identified in step 2 start downloading the database - * chunks. Each chunks is appended to a tmp file (inside geoip tmp dir) and - * after all chunks have been downloaded, the database is uncompressed and - * renamed to the final filename.After this the database is loaded and - * if there is an old instance of this database then that is closed. + * chunks. Each chunks is appended to a tmp file (inside geoip tmp dir) and + * after all chunks have been downloaded, the database is uncompressed and + * renamed to the final filename.After this the database is loaded and + * if there is an old instance of this database then that is closed. * 4) Cleanup locally loaded databases that are no longer mentioned in {@link GeoIpTaskState}. */ final class DatabaseRegistry implements Closeable {