hudi-agent commented on code in PR #18743:
URL: https://github.com/apache/hudi/pull/18743#discussion_r3247269676
##########
hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/sink/common/AbstractStreamWriteFunction.java:
##########
@@ -176,6 +176,15 @@ public void endInput() {
this.inputEnded = true;
}
+ @Override
+ public void close() throws Exception {
+ super.close();
+ if (this.writeClient != null) {
+ this.writeClient.close();
+ }
+ super.close();
+ }
Review Comment:
🤖 `super.close()` is invoked twice here (once before `writeClient.close()`
and once after). Could you remove the duplicate trailing call? Depending on the
parent's implementation, double-invoking could trigger errors or repeat side
effects during teardown.
<sub><i>- AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/sink/common/AbstractStreamWriteFunction.java:
##########
@@ -176,6 +176,13 @@ public void endInput() {
this.inputEnded = true;
}
+ @Override
+ public void close() throws Exception {
+ if (this.writeClient != null) {
Review Comment:
🤖 The current diff already calls `super.close()` at line 313, so the
chaining concern looks addressed. The try/finally point still has merit though
— if `writeClient.close()` throws, `super.close()` would be skipped. Worth
noting that the sibling `StreamWriteFunction.close()` already uses this
try/finally pattern for `memorySegmentPool`, so mirroring it here would be
consistent.
--
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]
