You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@celeborn.apache.org by "yew1eb (via GitHub)" <gi...@apache.org> on 2026/09/06 06:21:59 UTC

[PR] [CELEBORN-2454] Add config celeborn.master.ha.ratis.raft.server.close.threshold and docs for Ratis JvmPauseMonitor [celeborn]

yew1eb opened a new pull request, #3839:
URL: https://github.com/apache/celeborn/pull/3839

   ### What changes were proposed in this pull request?
   
   
   
   ### Why are the changes needed?
   
   
   
   ### Does this PR resolve a correctness bug?
   
   <!-- Check if yes. The `correctness` label will be added/removed automatically. -->
   - [ ] Yes
   
   ### Does this PR introduce _any_ user-facing change?
   
   <!-- Check if yes. -->
   - [ ] Yes
   
   
   ### How was this patch tested?
   
   


-- 
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: issues-unsubscribe@celeborn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [CELEBORN-2454] Add config celeborn.master.ha.ratis.raft.server.close.threshold and docs for Ratis JvmPauseMonitor [celeborn]

Posted by "zaynt4606 (via GitHub)" <gi...@apache.org>.
zaynt4606 commented on PR #3839:
URL: https://github.com/apache/celeborn/pull/3839#issuecomment-6095694403

   **Merge Summary:**
   - merged into main https://github.com/apache/celeborn/commit/cfaac4e0
   - merged into branch-0.7 https://github.com/apache/celeborn/commit/ff1944b9
   
   *Posted by `merge_pr.py`*


-- 
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: issues-unsubscribe@celeborn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [CELEBORN-2454] Add config celeborn.master.ha.ratis.raft.server.close.threshold and docs for Ratis JvmPauseMonitor [celeborn]

Posted by "zaynt4606 (via GitHub)" <gi...@apache.org>.
zaynt4606 commented on PR #3839:
URL: https://github.com/apache/celeborn/pull/3839#issuecomment-6095729000

   Thanks @yew1eb for the contribution!
   
   Merged to main(v1.0.0) in [cfaac4e076e1d8eb23b147a485c0171ea1efb837](https://github.com/apache/celeborn/commit/cfaac4e076e1d8eb23b147a485c0171ea1efb837) and branch-0.7(v0.7.1) in [ff1944b9606ace9e5d461f6d9e7a2558838fe3aa](https://github.com/apache/celeborn/commit/ff1944b9606ace9e5d461f6d9e7a2558838fe3aa).
   


-- 
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: issues-unsubscribe@celeborn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [CELEBORN-2454] Add config celeborn.master.ha.ratis.raft.server.close.threshold and docs for Ratis JvmPauseMonitor [celeborn]

Posted by "zaynt4606 (via GitHub)" <gi...@apache.org>.
zaynt4606 closed pull request #3839: [CELEBORN-2454] Add config celeborn.master.ha.ratis.raft.server.close.threshold and docs for Ratis JvmPauseMonitor
URL: https://github.com/apache/celeborn/pull/3839


-- 
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: issues-unsubscribe@celeborn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [CELEBORN-2454] Add config celeborn.master.ha.ratis.raft.server.close.threshold and docs for Ratis JvmPauseMonitor [celeborn]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3839:
URL: https://github.com/apache/celeborn/pull/3839#issuecomment-5886746893

   This PR is stale because it has been open 20 days with no activity. Remove stale label or comment or this will be closed in 10 days.


-- 
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: issues-unsubscribe@celeborn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [CELEBORN-2454] Add config celeborn.master.ha.ratis.raft.server.close.threshold and docs for Ratis JvmPauseMonitor [celeborn]

Posted by "yew1eb (via GitHub)" <gi...@apache.org>.
yew1eb commented on code in PR #3839:
URL: https://github.com/apache/celeborn/pull/3839#discussion_r4236694162


##########
docs/deploy.md:
##########
@@ -112,6 +112,12 @@ celeborn.worker.replicate.fastFail.duration 240s
 celeborn.worker.monitor.disk.enabled false
 ```
 
+Note: In an HA cluster, each master runs a Ratis JvmPauseMonitor. If the JVM or the host is paused
+longer than `celeborn.master.ha.ratis.raft.server.close.threshold` (default 60s), the local raft
+server is closed and the master leaves the raft group; the process must be restarted to recover.
+Avoid operations that suspend the JVM for a long time on production masters, e.g. `jmap -F` or
+heap dumps, and prefer low-intrusion diagnostics like `jcmd <pid> GC.class_histogram`.

Review Comment:
   Fixed in c979047ce. deploy.md no longer recommends `jcmd GC.class_histogram` as low-intrusion: it now states the command forces a full GC by default (the `all=true` form skips the forced GC but still walks the whole heap) and recommends `jstat -gcutil <pid> <interval>` for GC observation instead.



##########
master/src/main/java/org/apache/celeborn/service/deploy/master/clustermeta/ha/HARaftServer.java:
##########
@@ -374,6 +374,10 @@ private RaftProperties newRaftProperties(CelebornConf conf, RpcType rpc) {
         TimeDuration.valueOf(conf.haMasterRatisRpcRequestTimeout(), TimeUnit.SECONDS);
     RaftServerConfigKeys.Rpc.setRequestTimeout(properties, serverRequestTimeout);
 
+    // Set the threshold for JvmPauseMonitor to close the local raft server
+    RaftServerConfigKeys.setCloseThreshold(

Review Comment:
   Fixed in c979047ce. The dedicated config is now re-applied after the `haRatisCustomConfigs()` passthrough loop, so it takes precedence when explicitly set, and a warning is logged when both keys are configured. When only the passthrough is set it still applies, so existing deployments keep their behavior. The precedence is documented in the config doc (and regenerated ha.md).



-- 
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: issues-unsubscribe@celeborn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [CELEBORN-2454] Add config celeborn.master.ha.ratis.raft.server.close.threshold and docs for Ratis JvmPauseMonitor [celeborn]

Posted by "zaynt4606 (via GitHub)" <gi...@apache.org>.
zaynt4606 commented on code in PR #3839:
URL: https://github.com/apache/celeborn/pull/3839#discussion_r4236300992


##########
master/src/main/java/org/apache/celeborn/service/deploy/master/clustermeta/ha/HARaftServer.java:
##########
@@ -374,6 +374,10 @@ private RaftProperties newRaftProperties(CelebornConf conf, RpcType rpc) {
         TimeDuration.valueOf(conf.haMasterRatisRpcRequestTimeout(), TimeUnit.SECONDS);
     RaftServerConfigKeys.Rpc.setRequestTimeout(properties, serverRequestTimeout);
 
+    // Set the threshold for JvmPauseMonitor to close the local raft server
+    RaftServerConfigKeys.setCloseThreshold(

Review Comment:
   The existing `celeborn.ratis.raft.server.close.threshold` setting is applied by `haRatisCustomConfigs()` later in this method, so it silently overrides this new setting. For example, leaving the old key at `60s` and setting the new key to `120s` still produces `60s`. Please define and document the precedence, or warn when both keys are set.



##########
docs/deploy.md:
##########
@@ -112,6 +112,12 @@ celeborn.worker.replicate.fastFail.duration 240s
 celeborn.worker.monitor.disk.enabled false
 ```
 
+Note: In an HA cluster, each master runs a Ratis JvmPauseMonitor. If the JVM or the host is paused
+longer than `celeborn.master.ha.ratis.raft.server.close.threshold` (default 60s), the local raft
+server is closed and the master leaves the raft group; the process must be restarted to recover.
+Avoid operations that suspend the JVM for a long time on production masters, e.g. `jmap -F` or
+heap dumps, and prefer low-intrusion diagnostics like `jcmd <pid> GC.class_histogram`.

Review Comment:
   `GC.class_histogram` should not be described as low-intrusion here. [Oracle's JDK 17 documentation](https://docs.oracle.com/en/java/javase/17/docs/specs/man/jcmd.html) rates it high impact, and [OpenJDK 17](https://github.com/openjdk/jdk/blob/jdk-17%2B35/src/hotspot/share/services/diagnosticCommand.cpp#L511-L523) requests a full GC by default. On a large master heap, this command can cause a pause that exceeds the new 60s threshold and closes Ratis. Please replace this recommendation with a lower-impact diagnostic, or explicitly warn that heap inspection may disrupt an HA master.



-- 
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: issues-unsubscribe@celeborn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org