hudi-agent commented on code in PR #19503:
URL: https://github.com/apache/hudi/pull/19503#discussion_r3709035365
##########
hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/table/lookup/HoodieLookupFunction.java:
##########
@@ -155,15 +155,18 @@ private void checkCacheReload() throws IOException {
try {
long count = 0;
GenericRowData reuse = new GenericRowData(rowType.getFieldCount());
- partitionReader.open();
- RowData row;
- while ((row = partitionReader.read(reuse)) != null) {
- count++;
- RowData rowData = serializer.copy(row);
- RowData key = extractLookupKey(rowData);
- cache.addRow(key, rowData);
+ try {
+ partitionReader.open();
+ RowData row;
+ while ((row = partitionReader.read(reuse)) != null) {
+ count++;
+ RowData rowData = serializer.copy(row);
+ RowData key = extractLookupKey(rowData);
+ cache.addRow(key, rowData);
+ }
Review Comment:
🤖 With the new `finally { partitionReader.close(); }`, if the read loop
throws and `close()` also throws (which it now can, since it rethrows
format/rich close failures), the finally's exception replaces the original read
failure — so the outer catch logs/wraps the close exception instead of the true
root cause. Given the care taken elsewhere in this PR to preserve causes via
`addSuppressed`, would it be worth doing the same here (e.g. try-with-resources
or catch-and-suppress) so the reload's real failure isn't masked in the retry
log at line 180?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
