thomasmueller commented on code in PR #3112:
URL: https://github.com/apache/jackrabbit-oak/pull/3112#discussion_r3957333599
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdate.java:
##########
@@ -1364,29 +1390,50 @@ ExecutionStats getExecutionStats() {
return execStats;
}
+ void close() {
+ execStats.close();
+ }
+
class ExecutionStats {
public static final String INDEXER_COUNT = "INDEXER_COUNT";
public static final String INDEXER_NODE_COUNT =
"INDEXER_NODE_COUNT";
+ public static final String LAST_INDEXED_TIME = "LAST_INDEXED_TIME";
private final MeterStats indexerExecutionCountMeter;
private final MeterStats indexedNodeCountMeter;
private final TimerStats indexerTimer;
private final HistogramStats indexedNodePerCycleHisto;
- private final CounterStats lastIndexedTime;
+ /**
+ * LAST_INDEXED_TIME exposed as a gauge evaluated on every scrape.
When the
+ * durable-metric feature is enabled (default) it returns the
persisted
+ * ":async/<lane>-LastIndexedTo" timestamp, so the metric is
accurate for
+ * every lane and every cluster member, survives restarts and
stays correct
+ * while a lane is failing. See OAK-10260.
+ */
+ private final GaugeStats<Long> lastIndexedTimeGauge;
Review Comment:
Is it possible to retain the CounterStats (_not_ switch to GaugeStats)?
Switching could cause compatibility issues.
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdate.java:
##########
@@ -1364,29 +1390,50 @@ ExecutionStats getExecutionStats() {
return execStats;
}
+ void close() {
+ execStats.close();
Review Comment:
If we don't pass the whiteboard to the ExecutionStats constructor (see
below), then this is not needed.
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdate.java:
##########
@@ -486,6 +511,7 @@ public void close() {
} else {
log.info("[{}] Closed", name);
}
+ indexStats.close();
Review Comment:
This seems unrelated. Is it needed?
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdate.java:
##########
@@ -1364,29 +1390,50 @@ ExecutionStats getExecutionStats() {
return execStats;
}
+ void close() {
+ execStats.close();
+ }
+
class ExecutionStats {
public static final String INDEXER_COUNT = "INDEXER_COUNT";
public static final String INDEXER_NODE_COUNT =
"INDEXER_NODE_COUNT";
+ public static final String LAST_INDEXED_TIME = "LAST_INDEXED_TIME";
private final MeterStats indexerExecutionCountMeter;
private final MeterStats indexedNodeCountMeter;
private final TimerStats indexerTimer;
private final HistogramStats indexedNodePerCycleHisto;
- private final CounterStats lastIndexedTime;
+ /**
+ * LAST_INDEXED_TIME exposed as a gauge evaluated on every scrape.
When the
+ * durable-metric feature is enabled (default) it returns the
persisted
+ * ":async/<lane>-LastIndexedTo" timestamp, so the metric is
accurate for
+ * every lane and every cluster member, survives restarts and
stays correct
+ * while a lane is failing. See OAK-10260.
+ */
+ private final GaugeStats<Long> lastIndexedTimeGauge;
+ /**
+ * Legacy in-memory value (wall-clock of the last successful
cycle) retained as
+ * the fallback returned by the gauge when the durable-metric
feature is disabled.
+ */
+ private final AtomicLong legacyLastIndexedTimeMillis = new
AtomicLong();
+ @Nullable
+ private final Feature durableMetricFeature;
private StatisticsProvider statisticsProvider;
private final String[] names = {"Executions", "Nodes"};
private final String name;
private CompositeType consolidatedType;
- public ExecutionStats(String name, StatisticsProvider
statsProvider) {
+ public ExecutionStats(String name, StatisticsProvider
statsProvider, @Nullable Whiteboard whiteboard) {
Review Comment:
Instead of adding a Whiteboard as a parameter (and then internally use the
Feature at runtime), I think it's slightly better to just pass a boolean
parameter "durableMetric". This mean when switching the feature toggle, there
is no effect in the running system, but that's fine: the feature toggle is then
only read at initialization, which is enough.
The main advantage is that this approach avoids having to add "close()"
everywhere.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
