Skip to content

Fix metrics lost after metric service restart and conflicting metric series - #18748

Merged
JackieTien97 merged 3 commits into
apache:masterfrom
JackieTien97:fix/metrics-lost-after-restart
Sep 29, 2026
Merged

JackieTien97 merged 3 commits into
apache:masterfrom
JackieTien97:fix/metrics-lost-after-restart

Conversation

@JackieTien97

Copy link
Copy Markdown
Contributor

Description

Changing dn_metric_level with SET CONFIGURATION + LOAD CONFIGURATION, or calling restartService() of the metric service over JMX, runs MetricService.restartService(): it drops all metrics, then unbinds and binds every metric set again. Many metric sets could not survive this, so their metric families disappeared until the node restarted, e.g. all thread_pool_* series (#18681 fixed only client_manager). Some other metric series were never exported or reported wrong values, because two owners registered the same series, or the same metric name with different tag keys.

Metrics lost after the metric service restarts

The metric sets followed three broken patterns:

  • unbindFrom dropped the registrations that bindTo binds from, so nothing was bound again. Now unbindFrom only removes the metrics and keeps the registrations, which are removed when their owner deregisters: ThreadPoolMetrics, the pipe metric sets on the DataNode and the ConfigNode, PipeEventCommitMetrics, the subscription queue metrics, and the IoTConsensusV2 sync lag, which is now released when the server stops instead of when its metrics unbind. Thread pools are also unregistered by instance, so a new pool with the same name is kept when the old one shuts down.
  • Metric objects cached by their users were replaced by the restart. Ratis creates its metrics once and keeps using them, so the Ratis metric registry bridge now keeps its counters, timers and gauges, and points its proxies to the new metrics when binding. The pipe sink compression timers are fetched on each use.
  • Metrics created only by one-shot paths are created again when binding, with the recorded values: TsFileMetrics (file_global_*, file_level_*), the per-table schema gauges, the WritingMetrics thresholds, WAL queue size and active memtable / time partition counts, the load TsFile memory, the active loading counters, the system disk metrics (also when the level was OFF before), and the JVM GC pause timers. JvmGcMetrics also stops the GC listeners of the previous binding, which kept running and reset the timers.

Besides, a metric set failing to rebind no longer stops the others, CacheMetrics (partition cache hits) is registered at all, and RPCServiceThriftHandlerMetrics no longer has two instances owning the same series.

The ConfigNode starts the metric service after the consensus layer, so its Ratis metrics (ConfigRegion ...) were missing from the start for the same reason. They are exported now.

Metric series with conflicting owners

  • The metric manager requires the same tag keys for a metric name, and silently returns a no-op metric otherwise.
    • The per-table device number gauges added a table tag to schema_engine / schema_region, so they were never exported. They now have their own names schema_engine_table and schema_region_table, with the same tags.
    • The pipe-based and consensus-based subscription queues share three metric names, but only the latter have the region tag ([Subscription] Fix consensus subscription metrics across Regions #18277), so whichever registered later lost its metrics. The pipe-based queues now have the region tag too, with an empty value as they are not bound to a region. Renaming the metrics of the consensus-based queues instead would drop the dashboard compatibility that [Subscription] Fix consensus subscription metrics across Regions #18277 kept on purpose.
  • mem{name="database_<db>"} was registered by each data region of the database, and mem{name="chunkMetaData_<db>"} by each TsFileProcessor, so the gauge reported one of them and disappeared when any of them was closed. They now sum up all data regions / processors of the database, and the last one removes the gauge.
  • The IoTConsensus iot_send_log stage timers of a region were created and removed by the dispatcher thread of each peer: a peer leaving removed the timers the others still used, and after a restart only one peer recorded into the exported timers. The peers of a region now share the timers, and the last one to unbind removes them. Making the timers per peer instead would change the name tag.
  • WritingMetrics recorded the active memtable count under region="<N>", while it creates and removes the counter under region="DataRegion[<N>]", so each region had two series and one of them leaked when the region was deleted.
  • JvmGcMonitorMetrics (jvm_gc_accumulated_time_percentage) left the latest sample out of the observation window, and kept the ring buffer indices of the previous binding, which counted the samples before a restart again. As the latest sample now counts, the first one is taken after a whole interval (3 s instead of 50 ms), so the percentage is never computed over a tiny window.

Compatibility

  • New metric names schema_engine_table and schema_region_table. These gauges were never exported before.
  • subscription_uncommitted_event_count, subscription_current_commit_id and subscription_event_transfer of pipe-based queues get region="". Prometheus treats an empty label like a missing one; the IoTDB reporter paths of these metrics get a region= node.
  • active_memtable_count is only exported with region="DataRegion[<N>]".
  • mem{name="database_<db>"} and mem{name="chunkMetaData_<db>"} report the totals of the database.
  • The names and labels of the other metrics are unchanged.

Validation

  • 99 JUnit tests in 28 classes pass, 15 of the classes are new. They use a real MetricService, as a mocked one cannot show the tag key conflicts.
  • The new tests fail on the unmodified code, each with the symptom it targets, e.g. thread pool, pipe, Ratis and file metrics missing after restartService(), the table gauge not registered, 2 of 3 subscription queues exported, 2 counters for one region, 1 of 2 timer records counted, 200 instead of 300 for the database memory. JvmGcMonitorMetricsTest and the load TsFile memory and cache cases of DataNodeMetricsRestartTest call methods added by this PR, so they cannot run on the old code.
  • The full English reactor build, the Chinese-locale test compilation and Spotless pass.
  • Manually on a 1C1D standalone (Windows) reporting to Prometheus at level ALL, with tree model data, table model data and a pipe: scraped, set dn_metric_level to IMPORTANT and back to ALL with LOAD CONFIGURATION ON LOCAL, and scraped again, twice.
    • DataNode: 0 of the 548 metric families of the first scrape were missing afterwards (master: 234 of 545 after one round, 236 after two). Only series created on first use were missing until their next use, e.g. per-statement latencies.
    • ConfigNode: 254 families, including 164 ConfigRegion Ratis series (master: 112 families, no Ratis series).
    • The per-table gauges were exported, and the sum of mem{name="database_*"} equaled the sum of data_region_mem_cost (4255 = 4255; master: 2770 vs 4255).
    • A temporary check logging every metric rejected for mismatched tag keys reported none during the run.
  • Not covered end to end: the subscription queue metrics need consumers, and the IoTConsensus timers need replicas. Unit tests cover them.
mvn test -pl iotdb-core/node-commons,iotdb-core/consensus,iotdb-core/confignode,iotdb-core/datanode -am -Dtest=ThreadPoolMetricsTest,MetricServiceRestartTest,JvmGcMonitorMetricsTest,PipeEventCommitMetricsTest,ClientManagerMetricsTest,MetricReporterSwitchTest,IoTDBThreadPoolFactoryTest,MetricManagerLifecycleTest,PrometheusReporterTest,RatisMetricSetTest,IoTConsensusV2ServerMetricsTest,RatisConsensusTest,LogDispatcherThreadMetricsTest,PipeConfigNodeMetricsRestartTest,PipeMetricsRestartTest,PipeSchemaRegionListenerMetricsTest,PipeSchemaRegionSinkMetricsTest,TsFileMetricsTest,DataNodeMetricsRestartTest,SchemaMemMetricTableTest,SubscriptionMetricsRestartTest,ConsensusSubscriptionPrefetchingQueueMetricsTest,WritingMetricsTest,MetricServiceTest,DatabaseMemMetricsTest,ActiveLoadListeningDirConfigTest,ActiveLoadDirScannerTest,ActiveLoadTsFileLoaderTest -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false
mvn test-compile -DskipTests
mvn test-compile -P with-zh-locale -DskipTests

This PR has:

  • been self-reviewed.
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths.
  • been tested on a 1C1D standalone.

Key changed/added classes in this PR
  • MetricService, ThreadPoolMetrics, PipeEventCommitMetrics, JvmGcMonitorMetrics, Metric
  • RatisMetricSet, MetricRegistryManager, IoTDBMetricRegistry, CounterProxy, TimerProxy
  • IoTConsensusV2ServerImpl, IoTConsensusV2ServerMetrics, LogDispatcherThreadMetrics
  • The pipe metric sets under org.apache.iotdb.db.pipe.metric and org.apache.iotdb.confignode.manager.pipe.metric
  • SubscriptionPrefetchingQueueMetrics, ConsensusSubscriptionPrefetchingQueueMetrics
  • TsFileMetrics, WritingMetrics, SchemaEngineMemMetric, SchemaRegionMemMetric, DataRegionMetrics, TsFileProcessorInfoMetrics
  • LoadTsFileMemMetricSet, ActiveLoadingFilesMetricsSet, ActiveLoadingFilesNumberMetricsSet
  • RPCServiceThriftHandlerMetrics, CacheMetrics
  • SystemMetrics, JvmGcMetrics

🤖 Generated with Claude Code

…series

Changing dn_metric_level through a hot reload, or restarting the metric
service over JMX, unbinds and rebinds every metric set. Many metric sets
dropped their registrations when unbinding, kept metric objects that the
restart replaced, or created their metrics only once, so their metrics
disappeared for good after the restart, e.g. the thread pool, pipe, file
and Ratis metrics.

- Keep the registrations when unbinding, bind them again, and remove them
  only when the owner deregisters: thread pools, pipe metrics on the
  DataNode and ConfigNode, pipe event commit, subscription queues and
  IoTConsensusV2 sync lag.
- Refresh the cached metric objects when binding: the Ratis metric
  registry bridge and the pipe sink compression timers.
- Create again the metrics created by one-shot paths and restore the
  recorded values: file metrics, per-table schema gauges, writing
  thresholds and active counts, load TsFile memory, active loading
  counters, system disk metrics and JVM GC pause timers. Stop the GC
  listeners of the previous binding.
- Keep binding the other metric sets when one fails, register the
  partition cache metrics, and give the RPC connection and decoding
  metrics a single owner each.

Some metrics were also never exported or reported wrong values:

- A metric name must keep the same tag keys, otherwise the metrics
  registered later are silently dropped. Move the per-table device number
  gauges to their own names, and add the region tag to the pipe-based
  subscription queue metrics, which share their names with the
  consensus-based queues.
- The database memory and chunk metadata memory gauges sum up all data
  regions and TsFile processors of the database, instead of each of them
  replacing and removing the shared gauge.
- The IoTConsensus stage timers of a region are shared by the dispatcher
  threads of all its peers, so only the last one to unbind removes them.
- Record the active memtable count under the region tag of the counter
  created with the region.
- Count the latest sample in the GC time window, and reset the ring
  buffer when the monitor binds again.
@JackieTien97

Copy link
Copy Markdown
Contributor Author

Reviewed commit 68302d6112902eccf3db6c66ea4604267eb21c5d. I found two P2 issues that should be addressed before merging:

  1. [P2] Concurrent pipe deregistration can abort rebinding of the entire metric set

    PipeProcessorMetrics.java:109–126

    The new unbindFrom() passes a snapshot of task IDs directly to removeMetrics(). If a task closes after that snapshot is taken, PipeProcessorSubtask.close() can deregister it and remove it from processorMap before the loop reaches its ID. removeRate() then reads null and dereferences processor.getPipeName().

    Since MetricService.restartService() wraps both unbindFrom() and bindTo() in the same try, this exception also skips rebinding for the entire metric set. Other running pipes retain their registrations but lose their exported processor metrics until another successful rebind.

    I reproduced this with latches: pause the first snapshotted task during unbinding, deregister the second task, then resume. Direct unbinding throws NullPointerException; an actual restartService() leaves 0 processor tablet-rate series for the remaining task instead of 1. The same direct-unbind test passes with the parent implementation, whose deregister() path checks for already-removed IDs. This comparison isolates the newly introduced exception; the parent still has the original restart-loss issue.

    Please coordinate binding/unbinding with registration/removal, or otherwise make the object lookup and removal safe against concurrent deregistration. The other pipe/subscription metric sets changed to this snapshot-and-remove pattern deserve the same check.

  2. [P2] Startup binds recovered dispatchers twice and overcounts timer owners

    LogDispatcherThreadMetrics.java:101–103

    REGION_TO_STAGE_TIMER_OWNERS is incremented on every bindStageTimer() call, but bindings are not always separated by an unbind. AbstractMetricService.addMetricSet() binds immediately, even before the service starts, and startService() binds all registered sets again.

    This matches DataNode startup: active() restores the consensus layer before setUpMetricService(), and the recovered IoTConsensus dispatchers register their metric sets in their constructors. Each recovered dispatcher is therefore counted twice. Removing all peers or deleting the region decrements only once per dispatcher, leaving both iot_send_log stage timers exported and phantom entries in the static owner map.

    Reproduction: register two dispatcher metric sets before startService(), start the service, then remove both. The assertion that the last owner removes the timer fails on this PR and passes with the parent version of LogDispatcherThreadMetrics. The existing test starts the service before adding either dispatcher, so it misses this path.

    Please make ownership registration idempotent, for example by tracking owner instances, while still refreshing the cached timers on every bind.

Validation: a clean reactor test build of the 28 selected classes listed in the PR description passed all 99 tests. Three additional targeted tests reproduced the findings above. The parent comparisons replaced only the affected production classes (and the old pipe message constants), using the same JVM, test classes, and dependency classpath; they were not full parent-revision builds. No distributed end-to-end test was run for this review.

- A pipe or subscription registration may be removed while the metric
  service restart unbinds it, which failed the unbinding and skipped
  binding the whole metric set again. Synchronize binding and unbinding
  with registration and deregistration in the metric sets that unbind a
  snapshot of their registrations.
- The dispatchers recovered at startup register their metrics before the
  metric service starts, which binds them again. Track the owners of the
  shared IoTConsensus stage timers instead of counting the bindings, so
  that the last owner still removes the timers.
@JackieTien97
JackieTien97 merged commit b2cc8b5 into apache:master Sep 29, 2026
38 of 39 checks passed
@JackieTien97
JackieTien97 deleted the fix/metrics-lost-after-restart branch September 29, 2026 05:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant