smengcl opened a new pull request, #10988: URL: https://github.com/apache/ozone/pull/10988
Generated-by: Claude Code (Opus 4.8) ## What changes were proposed in this pull request? `DatanodeStorageMetrics` (added in HDDS-13128) has two defects, both fixed here. **Defect 1 (deadlock).** `getMetrics()` reads storage totals via `MutableVolumeSet.getStorageReport()`, which takes the volume-set read lock. The metrics sampler timer calls `getMetrics()` while holding the global `DefaultMetricsSystem` monitor (every HDDS service registers `PrometheusMetricsSink` by default, so the timer samples all sources). Meanwhile a volume-failure handler (`MutableVolumeSet.failVolume()`) holds the volume-set write lock and then calls `VolumeIOStats.unregister()`, which needs the same monitor. The two lock orders are opposite, so when a sample tick lands inside a `failVolume` critical section the two threads deadlock. The sampler then holds the metrics monitor forever and every metrics register, unregister, or sample across the process blocks. On a datanode this hangs the metrics thread and the Prometheus endpoint when a data volume fails; in a mini-cluster (where SCM, OM, and datanodes share one metrics system per JVM) it freezes the whole cluster. The fix adds `MutableVolumeSet.getStorageReportSnapshot()`, a lock-free read from the existing `ConcurrentHashMap`s (the same weakly-consistent guarantee that `getVolumesList()` already provides), and `getMetrics()` uses it. The locking `getStorageReport()` is unchanged for the node-report path. `DatanodeStorageMetrics` was the only sampled metrics source that reached into the shared volume-set lock (`VolumeInfoMetrics.getMetrics()` reads only its own volume), so removing this edge breaks the cycle. **Defect 2 (source leak, mini-cluster and test scope).** The source was registered under a constant name (uniquified to `-N` in mini-cluster mode) but unregistered by the base name, so every datanode past the first leaked its source and pinned a shut-down datanode's `MutableVolumeSet`. In mini-cluster mode the source is now registered and unregistered under a per-datanode name so the two are symmetric. Production keeps the plain `DatanodeStorageMetrics` name (one instance per JVM) so JMX and Prometheus metric names are unchanged. This deadlock is also the root cause of the recent master integration-job 90 minute timeouts: an intermittent, silent hang of a long-running test with a random cross-subsystem victim, no slowdown on runs that miss the race, and a sharp onset at the first integration coverage of HDDS-13128. ## What is the link to the Apache JIRA https://issues.apache.org/jira/browse/HDDS-16119 ## How was this patch tested? New and updated unit and integration tests, all run locally: * `TestVolumeSet#testStorageReportSnapshotDoesNotBlockOnWriteLock`: holds the volume-set write lock and asserts the snapshot returns promptly from another thread while the locking `getStorageReport()` times out. * `TestDatanodeStorageMetrics#testNoSourceLeakInMiniClusterMode`: in mini-cluster mode, asserts create then unregister leaves no residual source. * `TestDatanodeStorageMetricsIntegration`: updated to look up the per-datanode source name; passes on a real single-datanode `MiniOzoneCluster`. `mvn -pl :hdds-container-service test -Dtest=TestDatanodeStorageMetrics,TestVolumeSet` and the integration test pass, and `checkstyle.sh` is clean on both changed modules. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
