danny0405 commented on issue #19664: URL: https://github.com/apache/hudi/issues/19664#issuecomment-5350638650
I validated the failure path against the current Hudi code and Flink 2.1.1's `BinaryInMemorySortBuffer.write()` implementation. The report is correct: on an `EOFException`, Flink returns `false` before updating `currentDataBufferOffset`, although the paged output may already have advanced; Hudi can then retry that same buffer when a different bucket is selected as the largest bucket. The PR 1 approach looks right. A few implementation details are important: * Distinguish “could not create a new buffer” from “an existing buffer's `writeRow` returned false.” In the latter case, make the bucket unavailable for any further write immediately. * If the diverged bucket has committed records, flush only those indexed records and then dispose it. If it has no committed records, skip `flushBucket` (the current method rejects an empty bucket) and just dispose it. * Put disposal/page return on an exception-safe path. If flushing the committed records or the one fresh-buffer retry throws/fails, the failed buffer must still be disposed before propagating the error. * A focused `RowDataBucket` test should verify that a second write after `write() == false` is rejected, and recovery tests should verify page restoration in addition to record count and variable-length values. Keeping owner-aware preemption in PR 2 is a good separation. Calling back into the writer from `MemorySegmentPool.nextSegment()` introduces a re-entrant flush while serialization is in progress, so that path deserves the separate owner/progress/cleanup tests proposed here. -- 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]
