diff options
author | Ola Aunrønning <olaa@verizonmedia.com> | 2019-06-05 17:57:54 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-06-05 17:57:54 +0200 |
commit | a53e3f72b08eb8ba70f493634b0f7bc3275bd30a (patch) | |
tree | 2daf0f74cb53608b6a3b903cce68b6daf1eaee2e | |
parent | 3e1ef49b358ef027311d1d44d846695ea46125b8 (diff) | |
parent | 2319fdb072fa4c81cae432b7a21849b3e2695b7e (diff) |
Merge pull request #9696 from vespa-engine/revert-9414-olaa/seeded-coredump-id
Revert "Merge pull request #9414"
2 files changed, 35 insertions, 36 deletions
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java index 60f4d532784..95964ec8e7f 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java @@ -3,8 +3,6 @@ package com.yahoo.vespa.hosted.node.admin.maintenance.coredump; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableMap; -import com.yahoo.component.Version; -import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; import com.yahoo.vespa.hosted.node.admin.task.util.file.FileFinder; import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; @@ -48,6 +46,7 @@ public class CoredumpHandler { private final CoredumpReporter coredumpReporter; private final Path crashPatchInContainer; private final Path doneCoredumpsPath; + private final Supplier<String> coredumpIdSupplier; /** * @param crashPathInContainer path inside the container where core dump are dumped @@ -55,17 +54,21 @@ public class CoredumpHandler { */ public CoredumpHandler(Terminal terminal, CoreCollector coreCollector, CoredumpReporter coredumpReporter, Path crashPathInContainer, Path doneCoredumpsPath) { + this(terminal, coreCollector, coredumpReporter, crashPathInContainer, doneCoredumpsPath, () -> UUID.randomUUID().toString()); + } + + CoredumpHandler(Terminal terminal, CoreCollector coreCollector, CoredumpReporter coredumpReporter, + Path crashPathInContainer, Path doneCoredumpsPath, Supplier<String> coredumpIdSupplier) { this.terminal = terminal; this.coreCollector = coreCollector; this.coredumpReporter = coredumpReporter; this.crashPatchInContainer = crashPathInContainer; this.doneCoredumpsPath = doneCoredumpsPath; + this.coredumpIdSupplier = coredumpIdSupplier; } public void converge(NodeAgentContext context, Supplier<Map<String, Object>> nodeAttributesSupplier) { - - UUID coredumpId = generateCoredumpId(context); Path containerCrashPathOnHost = context.pathOnHostFromPathInNode(crashPatchInContainer); Path containerProcessingPathOnHost = containerCrashPathOnHost.resolve(PROCESSING_DIRECTORY_NAME); @@ -76,16 +79,16 @@ public class CoredumpHandler { .deleteRecursively(); // Check if we have already started to process a core dump or we can enqueue a new core one - getCoredumpToProcess(coredumpId, containerCrashPathOnHost, containerProcessingPathOnHost) + getCoredumpToProcess(containerCrashPathOnHost, containerProcessingPathOnHost) .ifPresent(path -> processAndReportSingleCoredump(context, path, nodeAttributesSupplier)); } /** @return path to directory inside processing directory that contains a core dump file to process */ - Optional<Path> getCoredumpToProcess(UUID coredumpId, Path containerCrashPathOnHost, Path containerProcessingPathOnHost) { + Optional<Path> getCoredumpToProcess(Path containerCrashPathOnHost, Path containerProcessingPathOnHost) { return FileFinder.directories(containerProcessingPathOnHost).stream() .map(FileFinder.FileAttributes::path) .findAny() - .or(() -> enqueueCoredump(coredumpId, containerCrashPathOnHost, containerProcessingPathOnHost)); + .or(() -> enqueueCoredump(containerCrashPathOnHost, containerProcessingPathOnHost)); } /** @@ -94,7 +97,7 @@ public class CoredumpHandler { * * @return path to directory inside processing directory which contains the enqueued core dump file */ - Optional<Path> enqueueCoredump(UUID coredumpID, Path containerCrashPathOnHost, Path containerProcessingPathOnHost) { + Optional<Path> enqueueCoredump(Path containerCrashPathOnHost, Path containerProcessingPathOnHost) { return FileFinder.files(containerCrashPathOnHost) .match(nameStartsWith(".").negate()) .maxDepth(1) @@ -104,7 +107,7 @@ public class CoredumpHandler { .map(coredumpPath -> { UnixPath coredumpInProcessingDirectory = new UnixPath( containerProcessingPathOnHost - .resolve(coredumpID.toString()) + .resolve(coredumpIdSupplier.get()) .resolve(COREDUMP_FILENAME_PREFIX + coredumpPath.getFileName())); coredumpInProcessingDirectory.createParents(); return uncheck(() -> Files.move(coredumpPath, coredumpInProcessingDirectory.toPath())).getParent(); @@ -171,12 +174,4 @@ public class CoredumpHandler { .orElseThrow(() -> new IllegalStateException( "No coredump file found in processing directory " + coredumpProccessingDirectory)); } - - private UUID generateCoredumpId(NodeAgentContext context) { - NodeSpec nodeSpec = context.node(); - String hostname = nodeSpec.getParentHostname().orElse(nodeSpec.getHostname()); - String version = nodeSpec.getVespaVersion().orElse(Version.emptyVersion).toFullString(); - byte[] seed = (hostname + version).getBytes(); - return UUID.nameUUIDFromBytes(seed); - } } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java index fa5ed169fbc..6d3a4dbb553 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java @@ -24,7 +24,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.UUID; +import java.util.function.Supplier; import java.util.stream.Collectors; import static com.yahoo.yolean.Exceptions.uncheck; @@ -53,8 +53,9 @@ public class CoredumpHandlerTest { private final CoreCollector coreCollector = mock(CoreCollector.class); private final CoredumpReporter coredumpReporter = mock(CoredumpReporter.class); @SuppressWarnings("unchecked") + private final Supplier<String> coredumpIdSupplier = mock(Supplier.class); private final CoredumpHandler coredumpHandler = new CoredumpHandler(terminal, coreCollector, coredumpReporter, - crashPathInContainer, doneCoredumpsPath); + crashPathInContainer, doneCoredumpsPath, coredumpIdSupplier); @Test @@ -66,9 +67,7 @@ public class CoredumpHandlerTest { Files.setLastModifiedTime(Files.createFile(crashPathOnHost.resolve(".bash.core.431")), FileTime.from(Instant.now())); assertFolderContents(crashPathOnHost, ".bash.core.431"); - UUID coredumpId = UUID.randomUUID(); - - Optional<Path> enqueuedPath = coredumpHandler.enqueueCoredump(coredumpId, crashPathOnHost, processingDir); + Optional<Path> enqueuedPath = coredumpHandler.enqueueCoredump(crashPathOnHost, processingDir); assertEquals(Optional.empty(), enqueuedPath); // bash.core.431 finished writing... and 2 more have since been written @@ -76,27 +75,30 @@ public class CoredumpHandlerTest { Files.setLastModifiedTime(Files.createFile(crashPathOnHost.resolve("vespa-proton.core.119")), FileTime.from(Instant.now().minus(Duration.ofMinutes(10)))); Files.setLastModifiedTime(Files.createFile(crashPathOnHost.resolve("vespa-slobrok.core.673")), FileTime.from(Instant.now().minus(Duration.ofMinutes(5)))); - enqueuedPath = coredumpHandler.enqueueCoredump(coredumpId, crashPathOnHost, processingDir); - assertEquals(Optional.of(processingDir.resolve(coredumpId.toString())), enqueuedPath); + when(coredumpIdSupplier.get()).thenReturn("id-123").thenReturn("id-321"); + enqueuedPath = coredumpHandler.enqueueCoredump(crashPathOnHost, processingDir); + assertEquals(Optional.of(processingDir.resolve("id-123")), enqueuedPath); assertFolderContents(crashPathOnHost, "bash.core.431", "vespa-slobrok.core.673"); - assertFolderContents(processingDir, coredumpId.toString()); - assertFolderContents(processingDir.resolve(coredumpId.toString()), "dump_vespa-proton.core.119"); + assertFolderContents(processingDir, "id-123"); + assertFolderContents(processingDir.resolve("id-123"), "dump_vespa-proton.core.119"); + verify(coredumpIdSupplier, times(1)).get(); // Enqueue another - enqueuedPath = coredumpHandler.enqueueCoredump(coredumpId, crashPathOnHost, processingDir); - assertEquals(Optional.of(processingDir.resolve(coredumpId.toString())), enqueuedPath); + enqueuedPath = coredumpHandler.enqueueCoredump(crashPathOnHost, processingDir); + assertEquals(Optional.of(processingDir.resolve("id-321")), enqueuedPath); assertFolderContents(crashPathOnHost, "bash.core.431"); - assertFolderContents(processingDir, coredumpId.toString()); - assertFolderContents(processingDir.resolve(coredumpId.toString()), "dump_vespa-proton.core.119", "dump_vespa-slobrok.core.673"); + assertFolderContents(processingDir, "id-123", "id-321"); + assertFolderContents(processingDir.resolve("id-321"), "dump_vespa-slobrok.core.673"); + verify(coredumpIdSupplier, times(2)).get(); } @Test public void coredump_to_process_test() throws IOException { final Path crashPathOnHost = fileSystem.getPath("/home/docker/container-1/some/crash/path"); final Path processingDir = fileSystem.getPath("/home/docker/container-1/some/other/processing"); - UUID coredumpId = UUID.randomUUID(); + // Initially there are no core dumps - Optional<Path> enqueuedPath = coredumpHandler.enqueueCoredump(coredumpId, crashPathOnHost, processingDir); + Optional<Path> enqueuedPath = coredumpHandler.enqueueCoredump(crashPathOnHost, processingDir); assertEquals(Optional.empty(), enqueuedPath); // 3 core dumps occur @@ -105,12 +107,14 @@ public class CoredumpHandlerTest { Files.setLastModifiedTime(Files.createFile(crashPathOnHost.resolve("vespa-proton.core.119")), FileTime.from(Instant.now().minus(Duration.ofMinutes(10)))); Files.setLastModifiedTime(Files.createFile(crashPathOnHost.resolve("vespa-slobrok.core.673")), FileTime.from(Instant.now().minus(Duration.ofMinutes(5)))); - enqueuedPath = coredumpHandler.getCoredumpToProcess(coredumpId, crashPathOnHost, processingDir); - assertEquals(Optional.of(processingDir.resolve(coredumpId.toString())), enqueuedPath); + when(coredumpIdSupplier.get()).thenReturn("id-123"); + enqueuedPath = coredumpHandler.getCoredumpToProcess(crashPathOnHost, processingDir); + assertEquals(Optional.of(processingDir.resolve("id-123")), enqueuedPath); // Running this again wont enqueue new core dumps as we are still processing the one enqueued previously - enqueuedPath = coredumpHandler.getCoredumpToProcess(coredumpId, crashPathOnHost, processingDir); - assertEquals(Optional.of(processingDir.resolve(coredumpId.toString())), enqueuedPath); + enqueuedPath = coredumpHandler.getCoredumpToProcess(crashPathOnHost, processingDir); + assertEquals(Optional.of(processingDir.resolve("id-123")), enqueuedPath); + verify(coredumpIdSupplier, times(1)).get(); } @Test |