Fix client manager metrics and JMX lifecycle races - #18681
Merged
JackieTien97 merged 4 commits intoSep 21, 2026
Merged
JackieTien97 merged 4 commits into
JackieTien97 merged 4 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
tag.name="client_manager_num_active"andtag.type="first"in addition to the metric name and type. Consumers using exact legacy ObjectNames must update their queries.Validation
414d7bae82(a null-reporter NPE and a notification sent to the wrong reporter).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 -DskipTestsThe 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:
Key changed/added classes