GGraziadei commented on code in PR #9153:
URL: https://github.com/apache/storm/pull/9153#discussion_r4417338907
##########
storm-server/src/main/java/org/apache/storm/daemon/supervisor/timer/SupervisorHeartbeat.java:
##########
@@ -162,7 +164,17 @@ public void run() {
Map<String, Object> validatedNumaMap =
SupervisorUtils.getNumaMap(conf);
Map<String, SupervisorInfo> supervisorInfoList =
buildSupervisorInfo(conf, supervisor, validatedNumaMap);
for (Map.Entry<String, SupervisorInfo> supervisorInfoEntry :
supervisorInfoList.entrySet()) {
-
stormClusterState.supervisorHeartbeat(supervisorInfoEntry.getKey(),
supervisorInfoEntry.getValue());
+ try {
+
stormClusterState.supervisorHeartbeat(supervisorInfoEntry.getKey(),
supervisorInfoEntry.getValue());
+ } catch (Exception e) {
+ if (!Utils.exceptionCauseIsInstanceOf(KeeperException.class,
e)) {
Review Comment:
`KeeperException` is broader than a transient failure. It also covers
persistent errors such as `NoAuthException` or `InvalidACLException` (e.g.
wrong ZooKeeper ACLs or credentials), which this catch would now swallow on
every cycle. In that case the supervisor stays up but never registers its node,
so Nimbus doesn't see it and the only signal is a WARN every few seconds. This
also changes startup: `Supervisor.launch()` runs this heartbeat synchronously
(`hb.run()`) before scheduling it, so a supervisor with a broken zk setup no
longer fails at launch.
Could you narrow the check to connection-level failures? For example:
```suggestion
if (!isTransientZkFailure(e)) {
```
```java
private static boolean isTransientZkFailure(Throwable t) {
return
Utils.exceptionCauseIsInstanceOf(KeeperException.ConnectionLossException.class,
t)
||
Utils.exceptionCauseIsInstanceOf(KeeperException.SessionExpiredException.class,
t)
||
Utils.exceptionCauseIsInstanceOf(KeeperException.OperationTimeoutException.class,
t);
}
```
`ConnectionLossException` also covers Curator's
`CuratorConnectionLossException`, which extends it.
--
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]
