Repository navigation
Fix concurrent JVM thread state metric refresh - #18950
Conversation
|
|
||
| // VisibleForTesting | ||
| static long getThreadStateCount(ThreadMXBean threadBean, Thread.State state) { | ||
| static synchronized long getThreadStateCount(ThreadMXBean threadBean, Thread.State state) { |
There was a problem hiding this comment.
Synchronize the entire cache refresh and lookup because the shared EnumMap is mutated in place. This class-level lock makes the read atomic with respect to clear/compute, preventing a concurrent gauge collection from observing a transient null value and unboxing it as a long. The 10-second sampling behavior is unchanged; the targeted JUnit regression passes in the reactor test run.
| // Pause a refresh after clearing the cache, then read the same state concurrently. Both calls | ||
| // must return the complete sample rather than a temporarily absent entry. | ||
| @Test | ||
| public void testConcurrentReadDuringRefresh() throws Exception { |
There was a problem hiding this comment.
This regression pauses the first refresh after the cache is cleared and starts a concurrent gauge read. The expected count verifies that the reader waits for the refresh and never observes the intermediate empty EnumMap; the missing-state case above separately verifies the documented zero default. The original implementation fails this test with expected 1 but actual 0, while the synchronized implementation passes.
Caideyipi
left a comment
Caideyipi
left a comment
There was a problem hiding this comment.
Reviewed the implementation and regression tests. No blocking issues found; formatting check is clean.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #18950 +/- ##
============================================
+ Coverage 46.03% 47.06% +1.02%
- Complexity 712 737 +25
============================================
Files 5509 5539 +30
Lines 400878 405207 +4329
Branches 52375 53028 +653
============================================
+ Hits 184536 190701 +6165
+ Misses 216342 214506 -1836 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Description
Concurrent JVM thread-state gauge reads can overlap with clearing and rebuilding the shared
EnumMap. Its non-atomicgetOrDefaultchecks can returnnullduring repopulation, causing an NPE when the count is unboxed.Synchronize the complete refresh-and-read operation with a class-level lock, matching the static cache's scope. Missing states continue to return zero, and the sampling interval remains unchanged.
Add regression coverage for missing states, null thread information, cached sampling, and a concurrent read while a refresh is paused after clearing the cache.
Validation
mvn spotless:apply -pl iotdb-core/metrics/interface -am -Drat.skip=truemvn test -pl iotdb-core/metrics/interface -am -Dtest=JvmThreadMetricsTest -Dsurefire.failIfNoSpecifiedTests=false -Drat.skip=true: 2 tests passed; reactor build succeeded, including Checkstyle and Spotless checks.This PR has:
Key changed/added classes
JvmThreadMetricsJvmThreadMetricsTest