GGraziadei commented on PR #9153: URL: https://github.com/apache/storm/pull/9153#issuecomment-5969888143
Thanks @jkrauss82 for the report and fix, and @reiabreu for the review. I would leave the retry behaviour of the other Curator clients unchanged in this PR. - The supervisor crashes because a `ConnectionLoss` escapes on the `HBTimer` thread and the uncaught exception brings down the process. Catching it around the ZK write and letting the next heartbeat cycle retry seems enough to fix this specific issue. - `newCurator()` is used by all `ClientZookeeper` clients (Nimbus, supervisors and workers), the blobstore and Trident's `TransactionalState`. Changing the retry policy there could make operations that currently fail after the configured backoff block the calling thread for up to `storm.zookeeper.session.timeout` (20s) on each retry. I don't think we should introduce that change across the board based on one supervisor incident. - The exception happens 603 ms after the socket is closed, and just 21 ms after `SUSPENDED`. With the default settings, the first backoff in `StormBoundedExponentialBackoffRetry` is at least 1000 ms, so it looks like the retry loop didn't even get through its first sleep. This makes me wonder whether the retry policy was actually consulted, or whether it rejected the retry straight away. If that's the case, `ConnectionAwareRetryPolicy` wouldn't have made a difference here. The `catch` is what fixes the failure we're seeing. I'd keep this PR focused on the `SupervisorHeartbeat` change and handle `ConnectionAwareRetryPolicy` separately, with tests against an actual ZooKeeper failover, including the Nimbus paths. @jkrauss82, could you share the full stack trace for the `ConnectionLoss`? It would help us understand exactly where it comes from and clarify the last point. Happy to reconsider if the stack trace shows something different. What do you think? -- 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]
