cshuo commented on code in PR #20133:
URL: https://github.com/apache/hudi/pull/20133#discussion_r4140215743
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieAppendHandle.java:
##########
@@ -401,7 +402,10 @@ protected boolean writeRecord(HoodieRecord<T>
hoodieRecord) {
return true;
} catch (Exception e) {
log.error("Error writing record {}", hoodieRecord, e);
- if (!config.getIgnoreWriteFailed() || ExceptionUtil.isCausedBy(e,
HoodieEarlyConflictDetectionException.class)) {
+ // A failed log flush affects the entire buffered batch, not just the
current record.
+ if (!config.getIgnoreWriteFailed()
+ || ExceptionUtil.isCausedBy(e, HoodieAppendException.class)
Review Comment:
An append for native writer can trigger an internal row-group flush, so an
IOException does not necessarily affect only the current record, and a
subsequent successful write would not establish that the buffered batch is
intact. However, appendRecord also includes record conversion, so making all
its failures fatal would conflate record-level errors with writer failures.
This is a pre-existing gap that needs a separate follow-up to establish that
boundary and revisit the mock-based test. This PR remains scoped to explicit
flushAppend failures.
--
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]
