mihaibercea commented on code in PR #3112:
URL: https://github.com/apache/jackrabbit-oak/pull/3112#discussion_r3959123343
##########
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:
I reviewed both keeping a counter and converting to a gauge.
The correct approach, for this particular metric, would be to change it to a
gauge and to always read it from the property. This will make sure that the
value is always accurate (and fresh).
The risk, in this situation, would be on JMX, that it might fail when
reading the value, in the case where it is hardcoded to expect a counter.
- Doing an LLM review, I get this: "Every single consumer in this codebase
reads this metric via Prometheus - zero JMX hardcoded attribute reads."
- Prometheus — "no compatibility risk"
Dropwizard's Counter and Gauge both export as Prometheus gauge type. The
Dropwizard→Prometheus bridge treats a Dropwizard Counter as a gauge because
Dropwizard counters can go up and down (they're general-purpose, not
monotonically-increasing). So:
# Before (CounterStats)
# TYPE oak_fulltext_async_LAST_INDEXED_TIME gauge
oak_fulltext_async_LAST_INDEXED_TIME 1788789168378
# After (GaugeStats)
# TYPE oak_fulltext_async_LAST_INDEXED_TIME gauge
oak_fulltext_async_LAST_INDEXED_TIME 1788789168378
======
Implementing a fix to keep the counter, would involve extra work to always
refresh the value:
runWhenPermitted() ← runs EVERY scheduled tick, on EVERY member
┌───────────────────────────────────────────────────────────┐
│ ★ NEW: if (durableMetric) │
│ counter.set( read :async/async-LastIndexedTo ) │ ← push
durable → counter
│ │
│ if (paused) return; │
│ if (lease held by someone else) { failed(); return; } │ ← standby
bails HERE,
│ ... │ but
the push above
│ updateIndex(...) → doneOneCycle() │ already
ran ✓
└───────────────────────────────────────────────────────────┘
====
Should I keep the current proposal to change it to a gauge?
--
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]
