From d79a53fe0d9d2f2d4b399e44e13495cc837614e5 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Thu, 1 Jul 2021 12:57:31 +0200 Subject: Use closed boolean and cleanup --- .../hosted/provision/autoscale/QuestMetricsDb.java | 53 ++++++++++------------ .../maintenance/NodeMetricsDbMaintainer.java | 3 -- 2 files changed, 24 insertions(+), 32 deletions(-) (limited to 'node-repository') 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 b90b8e24f91..dbfe90b5a5b 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 @@ -10,7 +10,6 @@ import com.yahoo.config.provision.ClusterSpec; import com.yahoo.io.IOUtils; import com.yahoo.vespa.defaults.Defaults; import com.yahoo.yolean.concurrent.ConcurrentResourcePool; -import com.yahoo.yolean.concurrent.ResourceFactory; import io.questdb.cairo.CairoEngine; import io.questdb.cairo.CairoException; import io.questdb.cairo.DefaultCairoConfiguration; @@ -37,8 +36,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -59,9 +57,10 @@ public class QuestMetricsDb extends AbstractComponent implements MetricsDb { private final Clock clock; private final String dataDir; - private final AtomicReference engine = new AtomicReference<>(); + private final CairoEngine engine; private final ConcurrentResourcePool sqlCompilerPool; - private final AtomicInteger nullRecords = new AtomicInteger(); + private final AtomicBoolean closed = new AtomicBoolean(false); + @Inject public QuestMetricsDb() { @@ -82,18 +81,19 @@ public class QuestMetricsDb extends AbstractComponent implements MetricsDb { System.setProperty("out", logConfig); this.dataDir = dataDir; - engine.set(new CairoEngine(new DefaultCairoConfiguration(dataDir))); - sqlCompilerPool = new ConcurrentResourcePool<>(new ResourceFactory<>() { - @Override - public SqlCompiler create() { - return new SqlCompiler(engine.get()); - } - }); + engine = new CairoEngine(new DefaultCairoConfiguration(dataDir)); + sqlCompilerPool = new ConcurrentResourcePool<>(() -> new SqlCompiler(engine())); nodeTable = new Table(dataDir, "metrics", clock); clusterTable = new Table(dataDir, "clusterMetrics", clock); ensureTablesExist(); } + private CairoEngine engine() { + if (closed.get()) + throw new IllegalStateException("Attempted to access QuestDb after calling close"); + return engine; + } + @Override public Clock clock() { return clock; } @@ -190,11 +190,8 @@ public class QuestMetricsDb extends AbstractComponent implements MetricsDb { } } - public int getNullRecordsCount() { return nullRecords.get(); } - @Override public void gc() { - nullRecords.set(0); nodeTable.gc(); clusterTable.gc(); } @@ -204,12 +201,14 @@ public class QuestMetricsDb extends AbstractComponent implements MetricsDb { @Override public void close() { - CairoEngine myEngine = engine.getAndSet(null); - for (SqlCompiler sqlCompiler : sqlCompilerPool) { - sqlCompiler.close(); - } - if (myEngine != null) { - myEngine.close(); + if (closed.get()) return; + closed.set(true); + synchronized (nodeTable.writeLock) { + synchronized (clusterTable.writeLock) { + for (SqlCompiler sqlCompiler : sqlCompilerPool) + sqlCompiler.close(); + engine.close(); + } } } @@ -235,7 +234,7 @@ public class QuestMetricsDb extends AbstractComponent implements MetricsDb { private void ensureClusterTableIsUpdated() { try { - if (0 == engine.get().getStatus(newContext().getCairoSecurityContext(), new Path(), clusterTable.name)) { + if (0 == engine().getStatus(newContext().getCairoSecurityContext(), new Path(), clusterTable.name)) { // Example: clusterTable.ensureColumnExists("write_rate", "float"); } } catch (Exception e) { @@ -290,10 +289,6 @@ public class QuestMetricsDb extends AbstractComponent implements MetricsDb { try (RecordCursor cursor = factory.getCursor(context)) { Record record = cursor.getRecord(); while (cursor.hasNext()) { - if (record == null || record.getStr(0) == null) { // Observed to happen. QuestDb bug? - nullRecords.incrementAndGet(); - continue; - } String hostname = record.getStr(0).toString(); if (hostnames.isEmpty() || hostnames.contains(hostname)) { snapshots.put(hostname, @@ -345,7 +340,7 @@ public class QuestMetricsDb extends AbstractComponent implements MetricsDb { } private SqlExecutionContext newContext() { - return new SqlExecutionContextImpl(engine.get(), 1); + return new SqlExecutionContextImpl(engine(), 1); } /** A questDb table */ @@ -367,11 +362,11 @@ public class QuestMetricsDb extends AbstractComponent implements MetricsDb { } boolean exists() { - return 0 == engine.get().getStatus(newContext().getCairoSecurityContext(), new Path(), name); + return 0 == engine().getStatus(newContext().getCairoSecurityContext(), new Path(), name); } TableWriter getWriter() { - return engine.get().getWriter(newContext().getCairoSecurityContext(), name); + return engine().getWriter(newContext().getCairoSecurityContext(), name); } void gc() { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainer.java index d671900d08c..4d16c90c002 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainer.java @@ -53,9 +53,6 @@ public class NodeMetricsDbMaintainer extends NodeRepositoryMaintainer { Thread.sleep(pauseMs); } - if (nodeRepository().metricsDb().getNullRecordsCount() > 0) - log.warning(nodeRepository().metricsDb().getNullRecordsCount() + " records returned null"); - nodeRepository().metricsDb().gc(); return asSuccessFactor(attempts, failures.get()); -- cgit v1.2.3