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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
