Skip to content

Commit

Permalink
more PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
bbilger committed Jan 2, 2024
1 parent a280684 commit 57843b0
Show file tree
Hide file tree
Showing 10 changed files with 225 additions and 68 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,6 @@ private long tilesEmitted() {

private void finishArchive() {
archive.finish(tileArchiveMetadata.withLayerStats(layerAttrStats.getTileStats()));
archive.printStats();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ default void initialize() {}
*/
default void finish(TileArchiveMetadata tileArchiveMetadata) {}

default void printStats() {}

long bytesWritten();

interface TileWriter extends Closeable {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import com.onthegomap.planetiler.config.Arguments;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Objects;
import java.util.Optional;

public final class FilesArchiveUtils {
Expand Down Expand Up @@ -45,29 +46,48 @@ static TileSchemeEncoding tilesSchemeEncoding(Arguments options, Path basePath,
}

static BasePathWithTileSchemeEncoding basePathWithTileSchemeEncoding(Arguments options, Path basePath) {
final String basePathStr = basePath.toString();
final int curlyIndex = basePathStr.indexOf('{');
if (curlyIndex >= 0) {
final Path newBasePath = Paths.get(basePathStr.substring(0, curlyIndex));
return new BasePathWithTileSchemeEncoding(
newBasePath,
tilesSchemeEncoding(options, newBasePath, basePathStr.substring(curlyIndex))
);
} else {
return new BasePathWithTileSchemeEncoding(
basePath,
tilesSchemeEncoding(options, basePath, Path.of(Z_TEMPLATE, X_TEMPLATE, Y_TEMPLATE + ".pbf").toString()));
}

final SplitShortcutPath split = SplitShortcutPath.split(basePath);

final String tileScheme = Objects
.requireNonNullElse(split.tileSchemePart(), Path.of(Z_TEMPLATE, X_TEMPLATE, Y_TEMPLATE + ".pbf")).toString();

return new BasePathWithTileSchemeEncoding(
split.basePart(),
tilesSchemeEncoding(options, split.basePart(), tileScheme)
);
}

public static Path cleanBasePath(Path basePath) {
final String basePathStr = basePath.toString();
final int curlyIndex = basePathStr.indexOf('{');
if (curlyIndex >= 0) {
return Paths.get(basePathStr.substring(0, curlyIndex));
}
return basePath;
return SplitShortcutPath.split(basePath).basePart();
}

record BasePathWithTileSchemeEncoding(Path basePath, TileSchemeEncoding tileSchemeEncoding) {}

private record SplitShortcutPath(Path basePart, Path tileSchemePart) {
public static SplitShortcutPath split(Path basePath) {
Path basePart = Objects.requireNonNullElse(basePath.getRoot(), Paths.get(""));
Path tileSchemePart = null;

boolean remainingIsTileScheme = false;
for (int i = 0; i < basePath.getNameCount(); i++) {
final Path part = basePath.getName(i);
if (!remainingIsTileScheme && part.toString().contains("{")) {
remainingIsTileScheme = true;
}
if (remainingIsTileScheme) {
tileSchemePart = tileSchemePart == null ? part : tileSchemePart.resolve(part);
} else {
basePart = basePart.resolve(part);
}
}

if (tileSchemePart == null) {
// just in case: use the "original" basePath in case no tile scheme is included, but basePart _should_ be identical
return new SplitShortcutPath(basePath, null);
} else {
return new SplitShortcutPath(basePart, tileSchemePart);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.io.File;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Function;
import java.util.regex.Matcher;
Expand Down Expand Up @@ -161,4 +162,22 @@ private static String validate(String tileScheme) {
return tileScheme;
}

@Override
public boolean equals(Object o) {
return this == o || (o instanceof TileSchemeEncoding that && Objects.equals(tileScheme, that.tileScheme) &&
Objects.equals(basePath, that.basePath));
}

@Override
public int hashCode() {
return Objects.hash(tileScheme, basePath);
}

@Override
public String toString() {
return "TileSchemeEncoding[" +
"tileScheme='" + tileScheme + '\'' +
", basePath=" + basePath +
']';
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package com.onthegomap.planetiler.stats;

import com.onthegomap.planetiler.util.FileUtils;
import com.onthegomap.planetiler.util.MemoryEstimator;
import io.prometheus.client.Collector;
import io.prometheus.client.CollectorRegistry;
Expand Down Expand Up @@ -49,8 +48,7 @@ class PrometheusStats implements Stats {
private PushGateway pg;
private ScheduledExecutorService executor;
private final String job;
private final Map<String, Path> filesToMonitor = new ConcurrentSkipListMap<>();
private final Map<String, LongSupplier> sizesOfFilesToMonitor = new ConcurrentSkipListMap<>();
private final Map<String, MonitoredFile> filesToMonitor = new ConcurrentSkipListMap<>();
private final Map<String, Long> dataErrorCounters = new ConcurrentHashMap<>();
private final Map<String, MemoryEstimator.HasEstimate> heapObjectsToMonitor = new ConcurrentSkipListMap<>();

Expand Down Expand Up @@ -172,15 +170,10 @@ public Timers timers() {
}

@Override
public Map<String, Path> monitoredFiles() {
public Map<String, MonitoredFile> monitoredFiles() {
return filesToMonitor;
}

@Override
public Map<String, LongSupplier> monitoredFileSizes() {
return sizesOfFilesToMonitor;
}

@Override
public void monitorInMemoryObject(String name, MemoryEstimator.HasEstimate object) {
heapObjectsToMonitor.put(name, object);
Expand Down Expand Up @@ -254,11 +247,11 @@ private class FileSizeCollector extends Collector {
@Override
public List<MetricFamilySamples> collect() {
List<Collector.MetricFamilySamples> results = new ArrayList<>();
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();
for (var entry : filesToMonitor.entrySet()) {
String name = sanitizeMetricName(entry.getKey());
MonitoredFile monitoredFile = entry.getValue();
Path path = monitoredFile.path();
long size = monitoredFile.sizeProvider().getAsLong();
results.add(new GaugeMetricFamily(BASE + "file_" + name + "_size_bytes", "Size of " + name + " in bytes",
size));
if (Files.exists(path)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ default void printSummary() {
timers().printSummary();
logger.info("-".repeat(40));
for (var entry : monitoredFiles().entrySet()) {
var sizeSupplier = monitoredFileSizes().getOrDefault(entry.getKey(), () -> FileUtils.size(entry.getValue()));
long size = sizeSupplier.getAsLong();
long size = entry.getValue().sizeProvider().getAsLong();
if (size > 0) {
logger.info("\t{}\t{}B", entry.getKey(), format.storage(size, false));
}
Expand Down Expand Up @@ -119,9 +118,7 @@ default Timers.Finishable startStage(String name, boolean log) {
Timers timers();

/** Returns all the files being monitored. */
Map<String, Path> monitoredFiles();

Map<String, LongSupplier> monitoredFileSizes();
Map<String, MonitoredFile> monitoredFiles();

/** Adds a stat that will track the size of a file or directory located at {@code path}. */
default void monitorFile(String name, Path path) {
Expand All @@ -130,10 +127,7 @@ default void monitorFile(String name, Path path) {

default void monitorFile(String name, Path path, LongSupplier sizeProvider) {
if (path != null) {
monitoredFiles().put(name, path);
}
if (sizeProvider != null) {
monitoredFileSizes().put(name, sizeProvider);
monitoredFiles().put(name, new MonitoredFile(path, sizeProvider));
}
}

Expand Down Expand Up @@ -199,8 +193,7 @@ class InMemory implements Stats {
private InMemory() {}

private final Timers timers = new Timers();
private final Map<String, Path> monitoredFiles = new ConcurrentSkipListMap<>();
private final Map<String, LongSupplier> monitoredFileSizes = new ConcurrentSkipListMap<>();
private final Map<String, MonitoredFile> monitoredFiles = new ConcurrentSkipListMap<>();
private final Map<String, Long> dataErrors = new ConcurrentHashMap<>();

@Override
Expand All @@ -212,15 +205,10 @@ public Timers timers() {
}

@Override
public Map<String, Path> monitoredFiles() {
public Map<String, MonitoredFile> monitoredFiles() {
return monitoredFiles;
}

@Override
public Map<String, LongSupplier> monitoredFileSizes() {
return monitoredFileSizes;
}

@Override
public void monitorInMemoryObject(String name, MemoryEstimator.HasEstimate object) {}

Expand Down Expand Up @@ -259,4 +247,11 @@ public void close() {

}
}

record MonitoredFile(Path path, LongSupplier sizeProvider) {
public MonitoredFile(Path path, LongSupplier sizeProvider) {
this.path = path;
this.sizeProvider = sizeProvider != null ? sizeProvider : () -> FileUtils.size(path);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ abstract class WriteableStreamArchive implements WriteableTileArchive {

private WriteableStreamArchive(OutputStreamSupplier outputStreamFactory, StreamArchiveConfig config) {
this.outputStreamFactory =
i -> new CountingOutputStream(outputStreamFactory.newOutputStream(i), bytesWritten::incBy);
i -> new CountingOutputStream(outputStreamFactory.newOutputStream(i), bytesWritten.counterForThread()::incBy);
this.config = config;

this.primaryOutputStream = this.outputStreamFactory.newOutputStream(0);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
package com.onthegomap.planetiler.files;

import static org.junit.jupiter.api.Assertions.assertEquals;

import com.onthegomap.planetiler.config.Arguments;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Map;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;

class FilesArchiveUtilsTest {

@ParameterizedTest
@CsvSource(textBlock = """
{z}/{x}/{y}.pbf , , {z}/{x}/{y}.pbf
, , {z}/{x}/{y}.pbf
{x}/{y}/{z}.pbf , , {x}/{y}/{z}.pbf
tiles/{z}/{x}/{y}.pbf , tiles, {z}/{x}/{y}.pbf
tiles/z{z}/{x}/{y}.pbf , tiles, z{z}/{x}/{y}.pbf
z{z}/x{x}/y{y}.pbf , , z{z}/x{x}/y{y}.pbf
tiles/tile-{z}-{x}-{y}.pbf, tiles, tile-{z}-{x}-{y}.pbf
/a , /a , {z}/{x}/{y}.pbf
/ , / , {z}/{x}/{y}.pbf
"""
)
void testBasePathWithTileSchemeEncoding(String shortcutBase, String actualBase, String tileScheme,
@TempDir Path tempDir) {

final Path shortcutBasePath = makePath(shortcutBase, tempDir);
final Path actualBasePath = makePath(actualBase, tempDir);

assertEquals(
new FilesArchiveUtils.BasePathWithTileSchemeEncoding(
actualBasePath,
new TileSchemeEncoding(
Paths.get(tileScheme).toString(),
actualBasePath
)
),
FilesArchiveUtils.basePathWithTileSchemeEncoding(Arguments.of(), shortcutBasePath)
);
}

@Test
void testBasePathWithTileSchemeEncodingPrefersArgOverShortcut() {
final Path basePath = Paths.get("");
final Path schemeShortcutPath = Paths.get("{x}", "{y}", "{z}.pbf");
final Path schemeArgumentPath = Paths.get("x{x}", "y{y}", "z{z}.pbf");
final Path shortcutPath = basePath.resolve(schemeShortcutPath);
assertEquals(
new FilesArchiveUtils.BasePathWithTileSchemeEncoding(
basePath,
new TileSchemeEncoding(
schemeShortcutPath.toString(),
basePath
)
),
FilesArchiveUtils.basePathWithTileSchemeEncoding(Arguments.of(), shortcutPath)
);
assertEquals(
new FilesArchiveUtils.BasePathWithTileSchemeEncoding(
basePath,
new TileSchemeEncoding(
schemeArgumentPath.toString(),
basePath
)
),
FilesArchiveUtils.basePathWithTileSchemeEncoding(
Arguments.of(Map.of(FilesArchiveUtils.OPTION_TILE_SCHEME, schemeArgumentPath.toString())), shortcutPath)
);
}

@ParameterizedTest
@CsvSource(textBlock = """
{z}/{x}/{y}.pbf ,
,
{x}/{y}/{z}.pbf ,
tiles/{z}/{x}/{y}.pbf , tiles
tiles/z{z}/{x}/{y}.pbf , tiles
z{z}/x{x}/y{y}.pbf ,
tiles/tile-{z}-{x}-{y}.pbf, tiles
/a , /a
/ , /
"""
)
void testCleanBasePath(String shortcutBase, String actualBase, @TempDir Path tempDir) {

assertEquals(
makePath(actualBase, tempDir),
FilesArchiveUtils.cleanBasePath(makePath(shortcutBase, tempDir))
);
}


private static Path makePath(String in, @TempDir Path tempDir) {
if (in == null) {
return Paths.get("");
}
if (in.startsWith("/")) {
return tempDir.resolve(in.substring(1));
}
return Paths.get(in);
}
}
Loading

0 comments on commit 57843b0

Please sign in to comment.