From a2806849624d1c75e1b3eddaf945e67ba2cf18df Mon Sep 17 00:00:00 2001 From: bbilger Date: Sat, 30 Dec 2023 23:13:19 +0100 Subject: [PATCH] few more adjustments according to PR feeback 1. use WriteableTileArchive#bytesWritten in summmary as well 2. call WriteableTileArchive#init in a safer manner ..and a few more adjustments --- .../com/onthegomap/planetiler/Planetiler.java | 2 +- .../planetiler/archive/TileArchiveConfig.java | 2 +- .../planetiler/archive/TileArchiveWriter.java | 6 ++--- .../files/WriteableFilesArchive.java | 2 +- .../planetiler/stats/PrometheusStats.java | 10 +++++++- .../onthegomap/planetiler/stats/Stats.java | 19 ++++++++++++++- .../archive/TileArchiveConfigTest.java | 24 +++++++++---------- .../planetiler/pmtiles/PmtilesTest.java | 3 +-- 8 files changed, 46 insertions(+), 22 deletions(-) diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/Planetiler.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/Planetiler.java index c7dc889c56..b2503d76e3 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/Planetiler.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/Planetiler.java @@ -756,7 +756,7 @@ public void run() throws Exception { stats.monitorFile("nodes", nodeDbPath); stats.monitorFile("features", featureDbPath); stats.monitorFile("multipolygons", multipolygonPath); - stats.monitorFile("archive", output.getLocalPath()); + stats.monitorFile("archive", output.getLocalPath(), archive::bytesWritten); for (Stage stage : stages) { stage.task.run(); diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/archive/TileArchiveConfig.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/archive/TileArchiveConfig.java index b701786c9e..0e10afa7f5 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/archive/TileArchiveConfig.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/archive/TileArchiveConfig.java @@ -171,7 +171,7 @@ public Path getLocalBasePath() { */ public void delete() { if (scheme == Scheme.FILE) { - FileUtils.delete(getLocalPath()); + FileUtils.delete(getLocalBasePath()); } } diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/archive/TileArchiveWriter.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/archive/TileArchiveWriter.java index da7fac8707..4843147426 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/archive/TileArchiveWriter.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/archive/TileArchiveWriter.java @@ -150,6 +150,9 @@ public static void writeOutput(FeatureGroup features, WriteableTileArchive outpu .addBuffer("reader_queue", queueSize) .sinkTo("encode", processThreads, writer::tileEncoderSink); + // ensure to initialize the archive BEFORE starting to write any tiles + output.initialize(); + // the tile writer will wait on the result of each batch to ensure tiles are written in order WorkerPipeline writeBranch = pipeline.readFromQueue(writerQueue) .sinkTo("write", tileWriteThreads, writer::tileWriter); @@ -339,9 +342,6 @@ private void tileEncoderSink(Iterable prev) throws IOException { private void tileWriter(Iterable tileBatches) throws ExecutionException, InterruptedException { final boolean firstTileWriter = firstTileWriterTracker.compareAndExchange(true, false); - if (firstTileWriter) { - archive.initialize(); - } var f = NumberFormat.getNumberInstance(Locale.getDefault()); f.setMaximumFractionDigits(5); diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/files/WriteableFilesArchive.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/files/WriteableFilesArchive.java index f61ddb45d2..ee4d70c00f 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/files/WriteableFilesArchive.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/files/WriteableFilesArchive.java @@ -38,7 +38,7 @@ * 
  * --output=/path/to/tiles/ --files_tile_scheme={z}/{x}/{y}.pbf --files_metadata_path=/some/other/path/metadata.json
  * --output=/path/to/tiles/{z}/{x}/{y}.pbf
- * --output=/path/to/tiles?format=files&tile_scheme={z}/{x}/{y}.pbf
+ * --output=/path/to/tiles?format=files&tile_scheme={z}/{x}/{y}.pbf
  *
* * @see ReadableFilesArchive diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/stats/PrometheusStats.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/stats/PrometheusStats.java index c3b42a81a6..71143a543f 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/stats/PrometheusStats.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/stats/PrometheusStats.java @@ -50,6 +50,7 @@ class PrometheusStats implements Stats { private ScheduledExecutorService executor; private final String job; private final Map filesToMonitor = new ConcurrentSkipListMap<>(); + private final Map sizesOfFilesToMonitor = new ConcurrentSkipListMap<>(); private final Map dataErrorCounters = new ConcurrentHashMap<>(); private final Map heapObjectsToMonitor = new ConcurrentSkipListMap<>(); @@ -175,6 +176,11 @@ public Map monitoredFiles() { return filesToMonitor; } + @Override + public Map monitoredFileSizes() { + return sizesOfFilesToMonitor; + } + @Override public void monitorInMemoryObject(String name, MemoryEstimator.HasEstimate object) { heapObjectsToMonitor.put(name, object); @@ -251,8 +257,10 @@ public List collect() { for (var file : filesToMonitor.entrySet()) { String name = sanitizeMetricName(file.getKey()); Path path = file.getValue(); + var sizeSupplier = monitoredFileSizes().getOrDefault(file.getKey(), () -> FileUtils.size(path)); + long size = sizeSupplier.getAsLong(); results.add(new GaugeMetricFamily(BASE + "file_" + name + "_size_bytes", "Size of " + name + " in bytes", - FileUtils.size(path))); + size)); if (Files.exists(path)) { try { FileStore fileStore = Files.getFileStore(path); diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/stats/Stats.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/stats/Stats.java index fe37346ad8..d978620c02 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/stats/Stats.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/stats/Stats.java @@ -60,7 +60,8 @@ default void printSummary() { timers().printSummary(); logger.info("-".repeat(40)); for (var entry : monitoredFiles().entrySet()) { - long size = FileUtils.size(entry.getValue()); + var sizeSupplier = monitoredFileSizes().getOrDefault(entry.getKey(), () -> FileUtils.size(entry.getValue())); + long size = sizeSupplier.getAsLong(); if (size > 0) { logger.info("\t{}\t{}B", entry.getKey(), format.storage(size, false)); } @@ -120,13 +121,23 @@ default Timers.Finishable startStage(String name, boolean log) { /** Returns all the files being monitored. */ Map monitoredFiles(); + Map monitoredFileSizes(); + /** Adds a stat that will track the size of a file or directory located at {@code path}. */ default void monitorFile(String name, Path path) { + monitorFile(name, path, null); + } + + default void monitorFile(String name, Path path, LongSupplier sizeProvider) { if (path != null) { monitoredFiles().put(name, path); } + if (sizeProvider != null) { + monitoredFileSizes().put(name, sizeProvider); + } } + /** Adds a stat that will track the estimated in-memory size of {@code object}. */ void monitorInMemoryObject(String name, MemoryEstimator.HasEstimate object); @@ -189,6 +200,7 @@ private InMemory() {} private final Timers timers = new Timers(); private final Map monitoredFiles = new ConcurrentSkipListMap<>(); + private final Map monitoredFileSizes = new ConcurrentSkipListMap<>(); private final Map dataErrors = new ConcurrentHashMap<>(); @Override @@ -204,6 +216,11 @@ public Map monitoredFiles() { return monitoredFiles; } + @Override + public Map monitoredFileSizes() { + return monitoredFileSizes; + } + @Override public void monitorInMemoryObject(String name, MemoryEstimator.HasEstimate object) {} diff --git a/planetiler-core/src/test/java/com/onthegomap/planetiler/archive/TileArchiveConfigTest.java b/planetiler-core/src/test/java/com/onthegomap/planetiler/archive/TileArchiveConfigTest.java index 4907fd7d40..398e55f4bd 100644 --- a/planetiler-core/src/test/java/com/onthegomap/planetiler/archive/TileArchiveConfigTest.java +++ b/planetiler-core/src/test/java/com/onthegomap/planetiler/archive/TileArchiveConfigTest.java @@ -115,23 +115,23 @@ void testPathMapping(String path, TileArchiveConfig.Format format) { @ParameterizedTest @CsvSource({ - "/a/output.mbtiles,/a/output.mbtiles", - "/a/tiles/{x}/{y}/{z}.pbf,/a/tiles", - "/a/tiles/{x}/{y}/{z}.pbf?format=proto,/a/tiles/{x}/{y}/{z}.pbf" + "a/output.mbtiles,a/output.mbtiles", + "a/tiles/{x}/{y}/{z}.pbf,a/tiles", + "a/tiles/{x}/{y}/{z}.pbf?format=proto,a/tiles/{x}/{y}/{z}.pbf" }) - void testLocalBasePath(String path, Path localBasePath) { - final var config = TileArchiveConfig.from(path); - assertEquals(localBasePath, config.getLocalBasePath()); + void testLocalBasePath(String path, Path localBasePath, @TempDir Path tempDir) { + final var config = TileArchiveConfig.from(tempDir.toString() + "/" + path); + assertEquals(tempDir.resolve(localBasePath), config.getLocalBasePath()); } @ParameterizedTest @CsvSource({ - "/a/output.mbtiles,/a/output.mbtiles", - "/a/tiles/{x}/{y}/{z}.pbf,/a/tiles/{x}/{y}/{z}.pbf", - "/a/tiles/{x}/{y}/{z}.pbf?format=proto,/a/tiles/{x}/{y}/{z}.pbf" + "a/output.mbtiles,a/output.mbtiles", + "a/tiles/{x}/{y}/{z}.pbf,a/tiles/{x}/{y}/{z}.pbf", + "a/tiles/{x}/{y}/{z}.pbf?format=proto,a/tiles/{x}/{y}/{z}.pbf" }) - void testLocalPath(String path, Path localPath) { - final var config = TileArchiveConfig.from(path); - assertEquals(localPath, config.getLocalPath()); + void testLocalPath(String path, Path localPath, @TempDir Path tempDir) { + final var config = TileArchiveConfig.from(tempDir.toString() + "/" + path); + assertEquals(tempDir.resolve(localPath), config.getLocalPath()); } } diff --git a/planetiler-core/src/test/java/com/onthegomap/planetiler/pmtiles/PmtilesTest.java b/planetiler-core/src/test/java/com/onthegomap/planetiler/pmtiles/PmtilesTest.java index b253753846..5720f85a16 100644 --- a/planetiler-core/src/test/java/com/onthegomap/planetiler/pmtiles/PmtilesTest.java +++ b/planetiler-core/src/test/java/com/onthegomap/planetiler/pmtiles/PmtilesTest.java @@ -221,8 +221,7 @@ void testRoundtripMetadata() throws IOException { "baselayer", TileArchiveMetadata.MVT_FORMAT, new Envelope(1.1, 2.2, 3.3, 4.4), - new CoordinateXY(5.5, 6.6), - 7d, + new Coordinate(5.5, 6.6, 7d), 8, 9, TileArchiveMetadata.TileArchiveMetadataJson.create(