voonhous commented on code in PR #18776:
URL: https://github.com/apache/hudi/pull/18776#discussion_r4059225736
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieWriteMergeHandle.java:
##########
@@ -485,10 +483,30 @@ public List<WriteStatus> close() {
return Collections.singletonList(writeStatus);
} catch (IOException e) {
+ closeFileWriterQuietly(e);
throw new HoodieUpsertException("Failed to close UpdateHandle", e);
+ } catch (RuntimeException e) {
+ closeFileWriterQuietly(e);
+ throw e;
+ } finally {
+ keyToNewRecords = null;
+ writtenRecordKeys = null;
Review Comment:
**minor:** On the failure path `keyToNewRecords` is set to null here without
being closed, so an `ExternalSpillableMap`'s spill files stay until the JVM
shutdown hook (`DiskMap`). The new close-failure branches also have no test;
`TestSortedAndChangeLogMergeHandles` only covers the happy path (:83, :225).
Not blocking, but could the failure branch `closeSuppressing` the map, with a
test there where `fileWriter.close()` throws asserting the original exception,
a single writer close, and a no-op second `close()`?
--
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]
