Alter table when necessary

3 files changed, 57 insertions, 26 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java
index 8fcba452d26..3a01e2c7287 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java
@@ -72,10 +72,10 @@ public class Autoscaler {
         if  (measurementsPerNode < minimumMeasurementsPerNode(clusterSpec))
             return Advice.none("Collecting more data before making new scaling decisions" +
                                ": Has " + measurementsPerNode + " data points per node" +
-                               "(all: " + clusterTimeseries.measurementCount +
+                               " (all: " + clusterTimeseries.measurementCount +
                                ", without stale: " + clusterTimeseries.measurementCountWithoutStale +
                                ", without out of service: " + clusterTimeseries.measurementCountWithoutStaleOutOfService +
-                               ", without unstable: " + clusterTimeseries.measurementCountWithoutStaleOutOfServiceUnstable);
+                               ", without unstable: " + clusterTimeseries.measurementCountWithoutStaleOutOfServiceUnstable + ")");
 
         int nodesMeasured = clusterTimeseries.nodesMeasured();
         if (nodesMeasured != clusterNodes.size())
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/QuestMetricsDb.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/QuestMetricsDb.java
index ad989d5abfd..0b3775a683f 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/QuestMetricsDb.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/QuestMetricsDb.java
@@ -45,7 +45,7 @@ import java.util.stream.Collectors;
 public class QuestMetricsDb extends AbstractComponent implements MetricsDb {
 
     private static final Logger log = Logger.getLogger(QuestMetricsDb.class.getName());
-    private static final String tableName = "metrics";
+    private static final String table = "metrics";
 
     private final Clock clock;
     private final String dataDir;
@@ -69,7 +69,7 @@ public class QuestMetricsDb extends AbstractComponent implements MetricsDb {
     }
 
     private void initializeDb() {
-        IOUtils.createDirectory(dataDir + "/" + tableName);
+        IOUtils.createDirectory(dataDir + "/" + table);
 
         // silence Questdb's custom logging system
         IOUtils.writeFile(new File(dataDir, "quest-log.conf"), new byte[0]);
@@ -78,19 +78,19 @@ public class QuestMetricsDb extends AbstractComponent implements MetricsDb {
 
         CairoConfiguration configuration = new DefaultCairoConfiguration(dataDir);
         engine = new CairoEngine(configuration);
-        ensureExists(tableName);
+        ensureExists(table);
     }
 
     @Override
     public void add(Collection<Pair<String, MetricSnapshot>> snapshots) {
-        try (TableWriter writer = engine.getWriter(newContext().getCairoSecurityContext(), tableName)) {
+        try (TableWriter writer = engine.getWriter(newContext().getCairoSecurityContext(), table)) {
             add(snapshots, writer);
         }
         catch (CairoException e) {
             if (e.getMessage().contains("Cannot read offset")) {
                 // This error seems non-recoverable
                 repair(e);
-                try (TableWriter writer = engine.getWriter(newContext().getCairoSecurityContext(), tableName)) {
+                try (TableWriter writer = engine.getWriter(newContext().getCairoSecurityContext(), table)) {
                     add(snapshots, writer);
                 }
             }
@@ -136,7 +136,7 @@ public class QuestMetricsDb extends AbstractComponent implements MetricsDb {
         SqlExecutionContext context = newContext();
         int partitions = 0;
         try (SqlCompiler compiler = new SqlCompiler(engine)) {
-            File tableRoot = new File(dataDir, tableName);
+            File tableRoot = new File(dataDir, table);
             List<String> removeList = new ArrayList<>();
             for (String dirEntry : tableRoot.list()) {
                 File partitionDir = new File(tableRoot, dirEntry);
@@ -151,7 +151,7 @@ public class QuestMetricsDb extends AbstractComponent implements MetricsDb {
             }
             // Remove unless all partitions are old: Removing all partitions "will be supported in the future"
             if ( removeList.size() < partitions && ! removeList.isEmpty())
-                compiler.compile("alter table " + tableName + " drop partition " +
+                compiler.compile("alter table " + table + " drop partition " +
                                  removeList.stream().map(dir -> "'" + dir + "'").collect(Collectors.joining(",")),
                                  context);
         }
@@ -180,23 +180,47 @@ public class QuestMetricsDb extends AbstractComponent implements MetricsDb {
         initializeDb();
     }
 
-    private void ensureExists(String tableName) {
+    private void ensureExists(String table) {
         SqlExecutionContext context = newContext();
-        if (0 == engine.getStatus(context.getCairoSecurityContext(), new Path(), tableName)) return;
-
         try (SqlCompiler compiler = new SqlCompiler(engine)) {
-            compiler.compile("create table " + tableName +
-                             " (hostname string, at timestamp, cpu_util float, mem_total_util float, disk_util float," +
-                             "  application_generation long, inService boolean, stable boolean)" +
-                             " timestamp(at)" +
-                             "PARTITION BY DAY;",
-                             context);
-            // We should do this if we get a version where selecting on stringhs work embedded, see below
-            // compiler.compile("alter table " + tableName + " alter column hostname add index", context);
+            if (0 == engine.getStatus(context.getCairoSecurityContext(), new Path(), table)) {
+                ensureColumnExists("inService", "boolean", table, compiler, context); // TODO: Remove after December 2020
+                ensureColumnExists("stable", "boolean", table, compiler, context); // TODO: Remove after December 2020
+            }
+            else {
+                compiler.compile("create table " + table +
+                                 " (hostname string, at timestamp, cpu_util float, mem_total_util float, disk_util float," +
+                                 "  application_generation long, inService boolean, stable boolean)" +
+                                 " timestamp(at)" +
+                                 "PARTITION BY DAY;",
+                                 context);
+                // We should do this if we get a version where selecting on strings work embedded, see below
+                // compiler.compile("alter table " + tableName + " alter column hostname add index", context);
+            }
+
         }
         catch (SqlException e) {
-            throw new IllegalStateException("Could not create Quest db table '" + tableName + "'", e);
+            throw new IllegalStateException("Could not create Quest db table '" + table + "'", e);
+        }
+    }
+
+    private void ensureColumnExists(String column, String columnType,
+                                    String table, SqlCompiler compiler, SqlExecutionContext context) throws SqlException {
+        if (columnNamesOf(table, compiler, context).contains(column)) return;
+        compiler.compile("alter table " + table + " add column " + column + " " + columnType, context);
+    }
+
+    private List<String> columnNamesOf(String tableName, SqlCompiler compiler, SqlExecutionContext context) throws SqlException {
+        List<String> columns = new ArrayList<>();
+        try (RecordCursorFactory factory = compiler.compile("show columns from " + tableName, context).getRecordCursorFactory()) {
+            try (RecordCursor cursor = factory.getCursor(context)) {
+                Record record = cursor.getRecord();
+                while (cursor.hasNext()) {
+                    columns.add(record.getStr(0).toString());
+                }
+            }
         }
+        return columns;
     }
 
     private long adjustIfRecent(long timestamp, long highestTimestampAdded) {
@@ -217,7 +241,7 @@ public class QuestMetricsDb extends AbstractComponent implements MetricsDb {
         DateTimeFormatter formatter = DateTimeFormatter.ISO_DATE_TIME.withZone(ZoneId.of("UTC"));
         String from = formatter.format(startTime).substring(0, 19) + ".000000Z";
         String to = formatter.format(clock.instant()).substring(0, 19) + ".000000Z";
-        String sql = "select * from " + tableName + " where at in('" + from + "', '" + to + "');";
+        String sql = "select * from " + table + " where at in('" + from + "', '" + to + "');";
 
         // WHERE clauses does not work:
         // String sql = "select * from " + tableName + " where hostname in('host1', 'host2', 'host3');";
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/QuestMetricsDbTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/QuestMetricsDbTest.java
index b2c9da4d22c..ba60a6a2207 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/QuestMetricsDbTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/QuestMetricsDbTest.java
@@ -117,7 +117,7 @@ public class QuestMetricsDbTest {
     /** To manually test that we can read existing data */
     @Ignore
     @Test
-    public void testReadingExistingData() {
+    public void testReadingAndAppendingToExistingData() {
         String dataDir = "data/QuestMetricsDbExistingData";
         if ( ! new File(dataDir).exists()) {
             System.out.println("No existing data to check");
@@ -125,14 +125,21 @@ public class QuestMetricsDbTest {
         }
         IOUtils.createDirectory(dataDir + "/metrics");
         ManualClock clock = new ManualClock("2020-10-01T00:00:00");
-        clock.advance(Duration.ofSeconds(10)); // Adjust to end time of data written
+        clock.advance(Duration.ofSeconds(9)); // Adjust to last data written
         QuestMetricsDb db = new QuestMetricsDb(dataDir, clock);
 
-        List<NodeTimeseries> timeseries = db.getNodeTimeseries(clock.instant().minus(Duration.ofSeconds(10)), Set.of("host1"));
+        List<NodeTimeseries> timeseries = db.getNodeTimeseries(clock.instant().minus(Duration.ofSeconds(9)), Set.of("host1"));
         assertFalse("Could read existing data", timeseries.isEmpty());
         assertEquals(10, timeseries.get(0).size());
 
-        System.out.println("Existing data:");
+        System.out.println("Existing data read:");
+        for (var snapshot : timeseries.get(0).asList())
+            System.out.println("  " + snapshot);
+
+        clock.advance(Duration.ofSeconds(1));
+        db.add(timeseries(2, Duration.ofSeconds(1), clock, "host1"));
+        System.out.println("New data written and read:");
+        timeseries = db.getNodeTimeseries(clock.instant().minus(Duration.ofSeconds(2)), Set.of("host1"));
         for (var snapshot : timeseries.get(0).asList())
             System.out.println("  " + snapshot);
     }