diff options
author | Harald Musum <musum@verizonmedia.com> | 2019-11-28 12:41:55 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-11-28 12:41:55 +0100 |
commit | d3f582ff72b2330ba9d290a3b6afe445872db510 (patch) | |
tree | 1aaf2068d1a9fc4680b6b9752f09e0d0f00f65b3 /node-admin | |
parent | 933346058b47094a5eb3115bb163f0f3fc639c62 (diff) | |
parent | 9da9004e15651c6c3c2718a5008849dbd165dc65 (diff) |
Merge pull request #11440 from vespa-engine/freva/prevent-double-archive
Prevent double container archive
Diffstat (limited to 'node-admin')
2 files changed, 27 insertions, 16 deletions
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java index e9dd8a859cc..2969611e729 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java @@ -15,8 +15,8 @@ import com.yahoo.vespa.hosted.node.admin.task.util.process.Terminal; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.time.Clock; import java.time.Duration; -import java.time.Instant; import java.time.ZoneOffset; import java.time.format.DateTimeFormatter; import java.util.Collections; @@ -42,6 +42,7 @@ public class StorageMaintainer { private final Terminal terminal; private final CoredumpHandler coredumpHandler; private final Path archiveContainerStoragePath; + private final Clock clock; // We cache disk usage to avoid doing expensive disk operations so often private final Cache<Path, Long> diskUsage = CacheBuilder.newBuilder() @@ -50,9 +51,14 @@ public class StorageMaintainer { .build(); public StorageMaintainer(Terminal terminal, CoredumpHandler coredumpHandler, Path archiveContainerStoragePath) { + this(terminal, coredumpHandler, archiveContainerStoragePath, Clock.systemUTC()); + } + + public StorageMaintainer(Terminal terminal, CoredumpHandler coredumpHandler, Path archiveContainerStoragePath, Clock clock) { this.terminal = terminal; this.coredumpHandler = coredumpHandler; this.archiveContainerStoragePath = archiveContainerStoragePath; + this.clock = clock; } public Optional<Long> getDiskUsageFor(NodeAgentContext context) { @@ -154,12 +160,14 @@ public class StorageMaintainer { */ public void archiveNodeStorage(NodeAgentContext context) { Path logsDirInContainer = context.pathInNodeUnderVespaHome("logs"); - Path containerLogsOnHost = context.pathOnHostFromPathInNode(logsDirInContainer); Path containerLogsInArchiveDir = archiveContainerStoragePath - .resolve(context.containerName().asString() + "_" + DATE_TIME_FORMATTER.format(Instant.now()) + logsDirInContainer); + .resolve(context.containerName().asString() + "_" + DATE_TIME_FORMATTER.format(clock.instant()) + logsDirInContainer); + UnixPath containerLogsOnHost = new UnixPath(context.pathOnHostFromPathInNode(logsDirInContainer)); - new UnixPath(containerLogsInArchiveDir).createParents(); - new UnixPath(containerLogsOnHost).moveIfExists(containerLogsInArchiveDir); + if (containerLogsOnHost.exists()) { + new UnixPath(containerLogsInArchiveDir).createParents(); + containerLogsOnHost.moveIfExists(containerLogsInArchiveDir); + } new UnixPath(context.pathOnHostFromPathInNode("/")).deleteRecursively(); } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java index 2a670ba8322..50a8da63b96 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java @@ -1,7 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.maintenance; -import com.google.common.collect.ImmutableSet; +import com.yahoo.test.ManualClock; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImpl; import com.yahoo.vespa.hosted.node.admin.task.util.file.FileFinder; @@ -17,15 +17,14 @@ import java.nio.file.FileSystem; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.Arrays; -import java.util.HashSet; +import java.time.Duration; +import java.time.Instant; import java.util.List; import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; /** * @author dybis @@ -81,11 +80,15 @@ public class StorageMaintainerTest { .stream() .map(FileFinder.FileAttributes::filename) .collect(Collectors.toSet()); - assertEquals(ImmutableSet.of("container-archive", "container-1", "container-2"), containerStorageRootContentsBeforeArchive); + assertEquals(Set.of("container-archive", "container-1", "container-2"), containerStorageRootContentsBeforeArchive); // Archive container-1 - StorageMaintainer storageMaintainer = new StorageMaintainer(null, null, pathToArchiveDir); + ManualClock clock = new ManualClock(Instant.ofEpochSecond(1234567890)); + StorageMaintainer storageMaintainer = new StorageMaintainer(null, null, pathToArchiveDir, clock); + storageMaintainer.archiveNodeStorage(context1); + + clock.advance(Duration.ofSeconds(3)); storageMaintainer.archiveNodeStorage(context1); // container-1 should be gone from container-storage @@ -94,18 +97,18 @@ public class StorageMaintainerTest { .stream() .map(FileFinder.FileAttributes::filename) .collect(Collectors.toSet()); - assertEquals(ImmutableSet.of("container-archive", "container-2"), containerStorageRootContentsAfterArchive); + assertEquals(Set.of("container-archive", "container-2"), containerStorageRootContentsAfterArchive); // container archive directory should contain exactly 1 directory - the one we just archived List<FileFinder.FileAttributes> containerArchiveContentsAfterArchive = FileFinder.from(pathToArchiveDir).maxDepth(1).list(); assertEquals(1, containerArchiveContentsAfterArchive.size()); Path archivedContainerStoragePath = containerArchiveContentsAfterArchive.get(0).path(); - assertTrue(archivedContainerStoragePath.getFileName().toString().matches("container-1_[0-9]{14}")); + assertEquals("container-1_20090213233130", archivedContainerStoragePath.getFileName().toString()); Set<String> archivedContainerStorageContents = FileFinder.files(archivedContainerStoragePath) .stream() .map(fileAttributes -> archivedContainerStoragePath.relativize(fileAttributes.path()).toString()) .collect(Collectors.toSet()); - assertEquals(ImmutableSet.of("opt/vespa/logs/vespa/vespa.log", "opt/vespa/logs/vespa/zookeeper.log"), archivedContainerStorageContents); + assertEquals(Set.of("opt/vespa/logs/vespa/vespa.log", "opt/vespa/logs/vespa/zookeeper.log"), archivedContainerStorageContents); } private NodeAgentContext createNodeAgentContextAndContainerStorage(FileSystem fileSystem, String containerName) throws IOException { @@ -128,11 +131,11 @@ public class StorageMaintainerTest { .stream() .map(fileAttributes -> containerRootOnHost.relativize(fileAttributes.path()).toString()) .collect(Collectors.toSet()); - Set<String> expectedContents = new HashSet<>(Arrays.asList( + Set<String> expectedContents = Set.of( "etc/something/conf", "opt/vespa/logs/vespa/vespa.log", "opt/vespa/logs/vespa/zookeeper.log", - "opt/vespa/var/db/some-file")); + "opt/vespa/var/db/some-file"); assertEquals(expectedContents, actualContents); return context; } |