voonhous commented on code in PR #18837:
URL: https://github.com/apache/hudi/pull/18837#discussion_r3340070269
##########
hudi-trino-plugin/src/main/java/io/trino/plugin/hudi/HudiPageSource.java:
##########
@@ -115,8 +126,32 @@ public long getMemoryUsage()
public void close()
throws IOException
{
- fileGroupReader.close();
- pageSource.close();
+ IOException closeException = null;
+
+ recordIterator.close();
Review Comment:
Addressed in b4d97d5. `close()` now uses a Guava `Closer`, registered LIFO
so the original close order is preserved (recordIterator -> fileGroupReader ->
pageSource). Every resource is closed even when an earlier `close()` throws,
and failures are aggregated via `addSuppressed` and the first is rethrown.
##########
hudi-trino-plugin/src/main/java/io/trino/plugin/hudi/HudiPageSource.java:
##########
@@ -115,8 +126,32 @@ public long getMemoryUsage()
public void close()
throws IOException
{
- fileGroupReader.close();
- pageSource.close();
+ IOException closeException = null;
+
+ recordIterator.close();
Review Comment:
Addressed in b4d97d5. `close()` now uses a Guava `Closer`, registered LIFO
so the original close order is preserved (recordIterator -> fileGroupReader ->
pageSource). Every resource is closed even when an earlier `close()` throws,
and failures are aggregated via `addSuppressed` and the first is rethrown.
##########
hudi-trino-plugin/src/main/java/io/trino/plugin/hudi/HudiPageSource.java:
##########
@@ -51,11 +54,29 @@ public HudiPageSource(
{
this.pageSource = pageSource;
this.fileGroupReader = fileGroupReader;
- this.initFileGroupReader();
this.readerContext = readerContext;
this.columnHandles = columnHandles;
Review Comment:
Addressed in b4d97d5. The constructor catch is widened from `IOException` to
`Throwable`, so the cleanup ladder (`fileGroupReader.close()` +
`pageSource.close()`) also runs when `getClosableIterator()` fails with an
unchecked exception (`HoodieIOException`, NPE/IAE). The original exception type
is preserved via `throwIfUnchecked(e)` before the fallback `RuntimeException`
wrap.
--
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]
