Skip to content

[ISSUE #9701] Synchronize metrics shutdown to prevent JVM crashes during broker shutdown - #9702

Merged
RongtongJin merged 2 commits into
apache:developfrom
guyinyou:dev_fix_broker_metrics_shutdown
Sep 15, 2025
Merged

[ISSUE #9701] Synchronize metrics shutdown to prevent JVM crashes during broker shutdown#9702
RongtongJin merged 2 commits into
apache:developfrom
guyinyou:dev_fix_broker_metrics_shutdown

Conversation

@guyinyou

Copy link
Copy Markdown
Contributor
  • Change async shutdown to sync blocking wait in BrokerMetricsManager
  • Ensure proper shutdown order to avoid race conditions
  • Prevent accessing dependencies after they are shutdown
  • Use join() with timeout to wait for CompletableFuture completion
  • Apply fix to all metrics exporter types (OTLP_GRPC, PROM, LOG)

Which Issue(s) This PR Fixes

Fixes #9701

Brief Description

How Did You Test This Change?

@guyinyou
guyinyou force-pushed the dev_fix_broker_metrics_shutdown branch from 6584ed7 to 867f778 Compare September 15, 2025 03:24
- Change async shutdown to sync blocking wait in BrokerMetricsManager
- Ensure proper shutdown order to avoid race conditions
- Prevent accessing dependencies after they are shutdown
- Use join() with timeout to wait for CompletableFuture completion
- Apply fix to all metrics exporter types (OTLP_GRPC, PROM, LOG)
@guyinyou
guyinyou force-pushed the dev_fix_broker_metrics_shutdown branch from 867f778 to 06d0d36 Compare September 15, 2025 03:26
fuyou001
fuyou001 previously approved these changes Sep 15, 2025

@fuyou001 fuyou001 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@guyinyou
guyinyou force-pushed the dev_fix_broker_metrics_shutdown branch from 66272d1 to ea721a2 Compare September 15, 2025 04:23
@codecov-commenter

codecov-commenter commented Sep 15, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 55.88235% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.10%. Comparing base (4ae0294) to head (ea721a2).
⚠️ Report is 184 commits behind head on develop.

Files with missing lines Patch % Lines
.../rocketmq/broker/metrics/BrokerMetricsManager.java 55.88% 26 Missing and 4 partials ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #9702      +/-   ##
=============================================
- Coverage      48.24%   48.10%   -0.14%     
+ Complexity     12150    12110      -40     
=============================================
  Files           1313     1313              
  Lines          93081    93093      +12     
  Branches       11890    11902      +12     
=============================================
- Hits           44906    44785     -121     
- Misses         42653    42765     +112     
- Partials        5522     5543      +21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@RongtongJin
RongtongJin merged commit 02f413d into apache:develop Sep 15, 2025
15 of 18 checks passed
@RongtongJin RongtongJin added the ha label Sep 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Enhancement] Synchronize metrics shutdown to prevent JVM crashes during broker shutdown

4 participants