hudi-agent commented on code in PR #19432:
URL: https://github.com/apache/hudi/pull/19432#discussion_r3688846203
##########
hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/sink/StreamWriteOperatorCoordinator.java:
##########
@@ -254,8 +254,12 @@ public void start() throws Exception {
}
restoreEvents(Long.MAX_VALUE);
} catch (Throwable throwable) {
- log.error("Failed to start operator coordinator.", throwable);
- context.failJob(throwable);
+ // Rethrow instead of context.failJob(): failJob triggers an in-graph
global failover that
+ // keeps this same coordinator instance alive without calling start()
again, leaving the
+ // half-initialized null fields (executor, writeClient, metaClient ...)
to be reused and NPE
+ // later. Rethrowing surfaces the failure as a JobMaster start failure
so the partially
+ // initialized instance is discarded rather than kept serving.
+ throw new HoodieException("Failed to start operator coordinator.",
throwable);
Review Comment:
🤖 Building on the cleanup point already raised here: if start() fails at
`restoreEvents(...)` after `initClientIds(conf)` ran, `clientIds` is now
discarded without `close()`. `ClientIds.start()` spawns a
`ScheduledExecutorService` that keeps renewing the heartbeat file. On the retry
with a fresh coordinator, `initClientIds` sees the old heartbeat still alive
(not expired) and allocates a *new* client id, so failed attempts can
accumulate orphan heartbeat threads and leak client ids in multi-writer mode.
Could the catch clean up `clientIds` (and the other partially-created
resources) before rethrowing? @danny0405 does this match your understanding of
the client-id lifecycle on failover?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
