Skip to content

Fix client manager metrics and JMX lifecycle races - #18681

Merged
JackieTien97 merged 4 commits into
apache:masterfrom
JackieTien97:fix/client-manager-metrics-lifecycle
Sep 21, 2026
Merged

JackieTien97 merged 4 commits into
apache:masterfrom
JackieTien97:fix/client-manager-metrics-lifecycle

Conversation

@JackieTien97

@JackieTien97 JackieTien97 commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Description

Stopping or reloading the metric service clears the original client pool registry, causing in-flight gauges to throw NullPointerException and preventing existing pools from being rebound. Pool closure also needs to unregister metrics without deleting other pools' JMX MBeans or interrupting resource cleanup.

This PR preserves live pool registrations across metric unbind/bind and makes each gauge read its pool directly. Closed pools are unregistered by instance identity, and AutoGauge reads its weak reference once per sample.

JMX identity and lifecycle

JMX ObjectNames now include every metric tag, using escaped tag.<key> properties and quoted literal values. The reporter tracks the metric instance owning each MBean, so delayed unregistration cannot remove a replacement. It ignores registration callbacks for stopped reporters and superseded registry entries, and restores current metrics when restarted.

Metric creation/replacement, removal, and registry reset share an internal lifecycle lock. Removal uses the atomically removed instance instead of a containsKey/get sequence. Reporter callbacks run after the registry lock is released and use the captured reporter reference, including when hot reload clears or replaces the current binding. Static notification helpers prevent accidental access to the mutable binding. Existing metric lookups, gauge sampling, and client borrow/return operations do not acquire this lock.

Nonfatal runtime failures during client metric cleanup are logged without failing ClientManager.close() or masking the original pool-close exception. The new warning is provided in both English and Chinese.

Compatibility

  • Tagged JMX ObjectNames change. For example, a client-pool metric now includes tag.name="client_manager_num_active" and tag.type="first" in addition to the metric name and type. Consumers using exact legacy ObjectNames must update their queries.
  • Ordinary untagged metric names retain their existing layout. Special metric names are quoted before construction so the metric type is preserved.
  • JMX now registers each distinct labeled series instead of collapsing them into one MBean per name/type. This increases MBean count and memory usage when JMX is enabled; client pools expose eight MBeans per registered pool.
  • Prometheus metric names and labels remain unchanged.

Validation

  • 35 selected JUnit tests pass, including the existing ClientManagerTest.
  • Coverage includes multi-pool and per-gauge JMX isolation, tag escaping and order independence, replacement ownership, delayed callbacks, reporter stop/start, atomic removal versus core reset, concurrent creation/removal, access to existing metrics during lifecycle changes, repeated pool cleanup, close-error isolation, and reporter hot switching during gauge replacement.
  • The original review probes were reproduced and fixed. The two reporter-switch regressions pass on the current sources and both fail when AbstractMetricManager is restored to 414d7bae82 (a null-reporter NPE and a notification sent to the wrong reporter).
  • The full English reactor build and Chinese-locale test compilation pass, along with Spotless and whitespace checks.
mvn clean test -pl iotdb-core/node-commons -am -Dtest=MetricReporterSwitchTest,MetricManagerLifecycleTest,IoTDBMetricObjNameFactoryTest,IoTDBJmxReporterTest,ClientManagerMetricsTest,IoTDBAutoGaugeTest,ClientManagerTest -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false
mvn test-compile -DskipTests
mvn test-compile -P with-zh-locale -DskipTests

The tests use real Commons Pool instances, metric managers, and MBean servers, with latches controlling concurrency. Full cluster integration tests and performance benchmarks were not run.


This PR has:

  • been self-reviewed, including concurrent reads, writes, callbacks, and lifecycle transitions.
  • documented the JMX naming compatibility change.
  • added regression unit tests and Apache license headers for new source files.
  • kept new operator-facing messages in English/Chinese locale parity.
Key changed/added classes
  • ClientManagerMetrics and ClientManager
  • AbstractMetricManager and IoTDBAutoGauge
  • IoTDBJmxReporter, ObjectNameFactory, and IoTDBMetricObjNameFactory
  • ClientManagerMetricsTest, MetricReporterSwitchTest, MetricManagerLifecycleTest, IoTDBJmxReporterTest, IoTDBMetricObjNameFactoryTest, and IoTDBAutoGaugeTest

@JackieTien97 JackieTien97 changed the title Fix client manager metric lifecycle and concurrent collection Fix client manager metrics and JMX lifecycle races Sep 20, 2026
@JackieTien97
JackieTien97 merged commit 61f2014 into apache:master Sep 21, 2026
42 of 43 checks passed
@JackieTien97
JackieTien97 deleted the fix/client-manager-metrics-lifecycle branch September 21, 2026 01:05
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