Merge pull request #16663 from vespa-engine/freva/sync-on-dirty

Sync up to 100 files when in dirty, including unrotated ones

4 files changed, 43 insertions, 20 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 6049cc31b68..d52ad7529c5 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
@@ -79,7 +79,7 @@ public class StorageMaintainer {
         this.archiveContainerStoragePath = archiveContainerStoragePath;
     }
 
-    public boolean syncLogs(NodeAgentContext context) {
+    public boolean syncLogs(NodeAgentContext context, boolean throttle) {
         Optional<URI> archiveUri = context.node().archiveUri();
         if (archiveUri.isEmpty()) return false;
 
@@ -87,10 +87,10 @@ public class StorageMaintainer {
                 .maxDepth(2)
                 .stream()
                 .sorted(Comparator.comparing(FileFinder.FileAttributes::lastModifiedTime))
-                .flatMap(fa -> SyncFileInfo.forLogFile(archiveUri.get(), fa.path()).stream())
+                .flatMap(fa -> SyncFileInfo.forLogFile(archiveUri.get(), fa.path(), throttle).stream())
                 .collect(Collectors.toList());
 
-        return syncClient.sync(context, syncFileInfos, 1);
+        return syncClient.sync(context, syncFileInfos, throttle ? 1 : 100);
     }
 
     public Optional<DiskSize> diskUsageFor(NodeAgentContext context) {
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/sync/SyncFileInfo.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/sync/SyncFileInfo.java
index 55ead4ad737..cf7f54ae0f5 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/sync/SyncFileInfo.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/sync/SyncFileInfo.java
@@ -35,16 +35,19 @@ public class SyncFileInfo {
         return uploadCompression;
     }
 
-    public static Optional<SyncFileInfo> forLogFile(URI uri, Path logFile) {
+    public static Optional<SyncFileInfo> forLogFile(URI uri, Path logFile, boolean rotatedOnly) {
         String filename = logFile.getFileName().toString();
-        Compression compression = Compression.NONE;
+        Compression compression;
         String dir = null;
 
-        if (filename.startsWith("vespa.log-")) {
+        if ((!rotatedOnly && filename.equals("vespa.log")) || filename.startsWith("vespa.log-")) {
             dir = "logs/vespa/";
             compression = Compression.ZSTD;
-        } else if (filename.endsWith(".zst")) {
-            if (filename.startsWith("JsonAccessLog.") || filename.startsWith("access"))
+        } else {
+            compression = filename.endsWith(".zst") ? Compression.NONE : Compression.ZSTD;
+            if (rotatedOnly && compression != Compression.NONE)
+                dir = null;
+            else if (filename.startsWith("JsonAccessLog.") || filename.startsWith("access"))
                 dir = "logs/access/";
             else if (filename.startsWith("ConnectionLog."))
                 dir = "logs/connection/";
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
index 4bc6fae4e56..8f9e3a46613 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
@@ -452,6 +452,7 @@ public class NodeAgentImpl implements NodeAgent {
                 stopServicesIfNeeded(context);
                 break;
             case active:
+                storageMaintainer.syncLogs(context, true);
                 storageMaintainer.cleanDiskIfFull(context);
                 storageMaintainer.handleCoreDumpsForContainer(context, container);
 
@@ -499,7 +500,6 @@ public class NodeAgentImpl implements NodeAgent {
                     orchestrator.resume(context.hostname().value());
                     suspendedInOrchestrator = false;
                 }
-                storageMaintainer.syncLogs(context);
                 break;
             case provisioned:
                 nodeRepository.setNodeState(context.hostname().value(), NodeState.dirty);
@@ -508,6 +508,7 @@ public class NodeAgentImpl implements NodeAgent {
                 removeContainerIfNeededUpdateContainerState(context, container);
                 context.log(logger, "State is " + node.state() + ", will delete application storage and mark node as ready");
                 credentialsMaintainers.forEach(maintainer -> maintainer.clearCredentials(context));
+                storageMaintainer.syncLogs(context, false);
                 storageMaintainer.archiveNodeStorage(context);
                 updateNodeRepoWithCurrentAttributes(context);
                 nodeRepository.setNodeState(context.hostname().value(), NodeState.ready);
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/sync/SyncFileInfoTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/sync/SyncFileInfoTest.java
index 57f3e2011bd..8fe39bf7a9c 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/sync/SyncFileInfoTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/sync/SyncFileInfoTest.java
@@ -31,21 +31,40 @@ public class SyncFileInfoTest {
     private static final Path vespaLogPath2 = fileSystem.getPath("/opt/vespa/logs/vespa.log-2021-02-12");
 
     @Test
-    public void log_files_test() {
-        assertForLogFile(accessLogPath1, null, null);
-        assertForLogFile(accessLogPath2, "s3://vespa-data-bucket/vespa/music/main/h432a/logs/access/access.log.20210212.zst", NONE);
-        assertForLogFile(accessLogPath3, "s3://vespa-data-bucket/vespa/music/main/h432a/logs/access/access-json.log.20210213.zst", NONE);
-        assertForLogFile(accessLogPath4, "s3://vespa-data-bucket/vespa/music/main/h432a/logs/access/JsonAccessLog.default.20210214.zst", NONE);
+    public void access_logs() {
+        assertForLogFile(accessLogPath1, null, null, true);
+        assertForLogFile(accessLogPath1, "s3://vespa-data-bucket/vespa/music/main/h432a/logs/access/access.log.20210211.zst", ZSTD, false);
 
-        assertForLogFile(connectionLogPath1, null, null);
-        assertForLogFile(connectionLogPath2, "s3://vespa-data-bucket/vespa/music/main/h432a/logs/connection/ConnectionLog.default.20210212.zst", NONE);
+        assertForLogFile(accessLogPath2, "s3://vespa-data-bucket/vespa/music/main/h432a/logs/access/access.log.20210212.zst", NONE, true);
+        assertForLogFile(accessLogPath2, "s3://vespa-data-bucket/vespa/music/main/h432a/logs/access/access.log.20210212.zst", NONE, false);
 
-        assertForLogFile(vespaLogPath1, null, null);
-        assertForLogFile(vespaLogPath2, "s3://vespa-data-bucket/vespa/music/main/h432a/logs/vespa/vespa.log-2021-02-12.zst", ZSTD);
+        assertForLogFile(accessLogPath3, "s3://vespa-data-bucket/vespa/music/main/h432a/logs/access/access-json.log.20210213.zst", NONE, true);
+        assertForLogFile(accessLogPath3, "s3://vespa-data-bucket/vespa/music/main/h432a/logs/access/access-json.log.20210213.zst", NONE, false);
+
+        assertForLogFile(accessLogPath4, "s3://vespa-data-bucket/vespa/music/main/h432a/logs/access/JsonAccessLog.default.20210214.zst", NONE, true);
+        assertForLogFile(accessLogPath4, "s3://vespa-data-bucket/vespa/music/main/h432a/logs/access/JsonAccessLog.default.20210214.zst", NONE, false);
+    }
+
+    @Test
+    public void connection_logs() {
+        assertForLogFile(connectionLogPath1, null, null, true);
+        assertForLogFile(connectionLogPath1, "s3://vespa-data-bucket/vespa/music/main/h432a/logs/connection/ConnectionLog.default.20210210.zst", ZSTD, false);
+
+        assertForLogFile(connectionLogPath2, "s3://vespa-data-bucket/vespa/music/main/h432a/logs/connection/ConnectionLog.default.20210212.zst", NONE, true);
+        assertForLogFile(connectionLogPath2, "s3://vespa-data-bucket/vespa/music/main/h432a/logs/connection/ConnectionLog.default.20210212.zst", NONE, false);
+    }
+
+    @Test
+    public void vespa_logs() {
+        assertForLogFile(vespaLogPath1, null, null, true);
+        assertForLogFile(vespaLogPath1, "s3://vespa-data-bucket/vespa/music/main/h432a/logs/vespa/vespa.log.zst", ZSTD, false);
+
+        assertForLogFile(vespaLogPath2, "s3://vespa-data-bucket/vespa/music/main/h432a/logs/vespa/vespa.log-2021-02-12.zst", ZSTD, true);
+        assertForLogFile(vespaLogPath2, "s3://vespa-data-bucket/vespa/music/main/h432a/logs/vespa/vespa.log-2021-02-12.zst", ZSTD, false);
     }
 
-    private static void assertForLogFile(Path srcPath, String destination, SyncFileInfo.Compression compression) {
-        Optional<SyncFileInfo> sfi = SyncFileInfo.forLogFile(nodeArchiveUri, srcPath);
+    private static void assertForLogFile(Path srcPath, String destination, SyncFileInfo.Compression compression, boolean rotatedOnly) {
+        Optional<SyncFileInfo> sfi = SyncFileInfo.forLogFile(nodeArchiveUri, srcPath, rotatedOnly);
         assertEquals(destination, sfi.map(SyncFileInfo::destination).map(URI::toString).orElse(null));
         assertEquals(compression, sfi.map(SyncFileInfo::uploadCompression).orElse(null));
     }