diff options
2 files changed, 52 insertions, 53 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 2c627e3f6e9..a849cef31ff 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 @@ -1,23 +1,21 @@ // 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.cache.Cache; import com.google.common.cache.CacheBuilder; -import com.google.common.cache.CacheLoader; -import com.google.common.cache.LoadingCache; import com.yahoo.config.provision.NodeType; -import com.yahoo.io.IOUtils; import com.yahoo.log.LogLevel; import com.yahoo.vespa.hosted.dockerapi.Container; +import com.yahoo.vespa.hosted.node.admin.component.TaskContext; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations; 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; +import com.yahoo.vespa.hosted.node.admin.task.util.process.Terminal; import com.yahoo.vespa.hosted.node.admin.util.SecretAgentCheckConfig; import com.yahoo.vespa.hosted.node.admin.maintenance.coredump.CoredumpHandler; -import java.io.IOException; -import java.io.InputStreamReader; import java.nio.file.Files; import java.nio.file.Path; import java.time.Duration; @@ -31,7 +29,6 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.logging.Logger; import java.util.regex.Pattern; @@ -48,21 +45,19 @@ public class StorageMaintainer { private static final DateTimeFormatter DATE_TIME_FORMATTER = DateTimeFormatter .ofPattern("yyyyMMddHHmmss").withZone(ZoneOffset.UTC); + private final Terminal terminal; private final DockerOperations dockerOperations; private final CoredumpHandler coredumpHandler; private final Path archiveContainerStoragePath; // We cache disk usage to avoid doing expensive disk operations so often - private LoadingCache<Path, Long> diskUsage = CacheBuilder.newBuilder() + private final Cache<Path, Long> diskUsage = CacheBuilder.newBuilder() .maximumSize(100) .expireAfterWrite(5, TimeUnit.MINUTES) - .build(new CacheLoader<Path, Long>() { - public Long load(Path containerDir) throws IOException, InterruptedException { - return getDiskUsedInBytes(containerDir); - } - }); + .build(); - public StorageMaintainer(DockerOperations dockerOperations, CoredumpHandler coredumpHandler, Path archiveContainerStoragePath) { + public StorageMaintainer(Terminal terminal, DockerOperations dockerOperations, CoredumpHandler coredumpHandler, Path archiveContainerStoragePath) { + this.terminal = terminal; this.dockerOperations = dockerOperations; this.coredumpHandler = coredumpHandler; this.archiveContainerStoragePath = archiveContainerStoragePath; @@ -186,37 +181,37 @@ public class StorageMaintainer { } public Optional<Long> getDiskUsageFor(NodeAgentContext context) { - Path containerDir = context.pathOnHostFromPathInNode("/"); try { - return Optional.of(getDiskUsageFor(containerDir)); + Path path = context.pathOnHostFromPathInNode("/"); + + Long cachedDiskUsage = diskUsage.getIfPresent(path); + if (cachedDiskUsage != null) return Optional.of(cachedDiskUsage); + + long diskUsageBytes = getDiskUsedInBytes(context, path); + diskUsage.put(path, diskUsageBytes); + return Optional.of(diskUsageBytes); } catch (Exception e) { - context.log(logger, LogLevel.WARNING, "Problems during disk usage calculations in " + containerDir.toAbsolutePath(), e); + context.log(logger, LogLevel.WARNING, "Failed to get disk usage", e); return Optional.empty(); } } - long getDiskUsageFor(Path containerDir) throws ExecutionException { - return diskUsage.get(containerDir); - } - // Public for testing - long getDiskUsedInBytes(Path path) throws IOException, InterruptedException { + long getDiskUsedInBytes(TaskContext context, Path path) { if (!Files.exists(path)) return 0; - Process duCommand = new ProcessBuilder().command("du", "-xsk", path.toString()).start(); - if (!duCommand.waitFor(60, TimeUnit.SECONDS)) { - duCommand.destroy(); - duCommand.waitFor(); - throw new RuntimeException("Disk usage command timed out, aborting."); - } - String output = IOUtils.readAll(new InputStreamReader(duCommand.getInputStream())); + String output = terminal.newCommandLine(context) + .add("du", "-xsk", path.toString()) + .setTimeout(Duration.ofSeconds(60)) + .executeSilently() + .getOutput(); + String[] results = output.split("\t"); if (results.length != 2) { throw new RuntimeException("Result from disk usage command not as expected: " + output); } - long diskUsageKB = Long.valueOf(results[0]); - return diskUsageKB * 1024; + return 1024 * Long.valueOf(results[0]); } 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 81a0c4a5e20..d276bad9521 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 @@ -13,30 +13,30 @@ 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; import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; +import com.yahoo.vespa.hosted.node.admin.task.util.process.TestTerminal; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.test.file.TestFileSystem; -import org.junit.Rule; +import org.junit.After; import org.junit.Test; import org.junit.experimental.runners.Enclosed; -import org.junit.rules.TemporaryFolder; import org.junit.runner.RunWith; import java.io.IOException; 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.util.List; +import java.util.Optional; import java.util.Set; -import java.util.concurrent.ExecutionException; import java.util.stream.Collectors; import java.util.stream.Stream; import static com.yahoo.vespa.hosted.node.admin.task.util.file.IOExceptionUtil.uncheck; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; /** @@ -47,7 +47,7 @@ public class StorageMaintainerTest { private static final DockerOperations docker = mock(DockerOperations.class); public static class SecretAgentCheckTests { - private final StorageMaintainer storageMaintainer = new StorageMaintainer(docker, null, null); + private final StorageMaintainer storageMaintainer = new StorageMaintainer(null, docker, null, null); @Test public void tenant() { @@ -190,30 +190,34 @@ public class StorageMaintainerTest { } public static class DiskUsageTests { - @Rule - public TemporaryFolder folder = new TemporaryFolder(); + + private final TestTerminal terminal = new TestTerminal(); @Test - public void testDiskUsed() throws IOException, ExecutionException { - StorageMaintainer storageMaintainer = new StorageMaintainer(docker, null, null); - int writeSize = 10000; - Files.write(folder.newFile().toPath(), new byte[writeSize]); - - long usedBytes = storageMaintainer.getDiskUsageFor(folder.getRoot().toPath()); - if (usedBytes * 4 < writeSize || usedBytes > writeSize * 4) - fail("Used bytes is " + usedBytes + ", but wrote " + writeSize + " bytes, not even close."); - - // Write another file, since disk usage is cached it should not change - Files.write(folder.newFile().toPath(), new byte[writeSize]); - assertEquals(usedBytes, storageMaintainer.getDiskUsageFor(folder.getRoot().toPath())); + public void testDiskUsed() throws IOException { + StorageMaintainer storageMaintainer = new StorageMaintainer(terminal, docker, null, null); + FileSystem fileSystem = TestFileSystem.create(); + NodeAgentContext context = new NodeAgentContextImpl.Builder("host-1.domain.tld").fileSystem(fileSystem).build(); + Files.createDirectories(context.pathOnHostFromPathInNode("/")); + + terminal.expectCommand("du -xsk /home/docker/host-1 2>&1", 0, "321\t/home/docker/host-1/"); + assertEquals(Optional.of(328_704L), storageMaintainer.getDiskUsageFor(context)); + + // Value should still be cached, no new execution against the terminal + assertEquals(Optional.of(328_704L), storageMaintainer.getDiskUsageFor(context)); } @Test - public void testNonExistingDiskUsed() throws IOException, InterruptedException { - StorageMaintainer storageMaintainer = new StorageMaintainer(docker, null, null); - long usedBytes = storageMaintainer.getDiskUsedInBytes(folder.getRoot().toPath().resolve("doesn't exist")); + public void testNonExistingDiskUsed() { + StorageMaintainer storageMaintainer = new StorageMaintainer(terminal, docker, null, null); + long usedBytes = storageMaintainer.getDiskUsedInBytes(null, Paths.get("/fake/path")); assertEquals(0L, usedBytes); } + + @After + public void after() { + terminal.verifyAllCommandsExecuted(); + } } public static class ArchiveContainerDataTests { @@ -237,7 +241,7 @@ public class StorageMaintainerTest { // Archive container-1 - StorageMaintainer storageMaintainer = new StorageMaintainer(docker, null, pathToArchiveDir); + StorageMaintainer storageMaintainer = new StorageMaintainer(null, docker, null, pathToArchiveDir); storageMaintainer.archiveNodeStorage(context1); // container-1 should be gone from container-storage |