Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/22367 )
Change subject: IMPALA-13654: Tolerate missing data files of Iceberg tables ...................................................................... Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/22367/4/fe/src/main/java/org/apache/impala/catalog/IcebergContentFileStore.java File fe/src/main/java/org/apache/impala/catalog/IcebergContentFileStore.java: http://gerrit.cloudera.org:8080/#/c/22367/4/fe/src/main/java/org/apache/impala/catalog/IcebergContentFileStore.java@232 PS4, Line 232: missingFiles_.add(contentFile.path().toString()); > If I understand you correctly, the messages would have to be serialized so Not exactly. Before this patch, table loading failed, and the table was unusable in any statement, moreover we didn't provide enough context in the error messages when the statements failed. Now we continue table loading, but also collect the missing files. So later in the Coordinator we can abort any queries during planning if they need the missing files. Other statements should work. Table information is added to the exception texts in IcebergScanPlanner. http://gerrit.cloudera.org:8080/#/c/22367/4/fe/src/main/java/org/apache/impala/catalog/IcebergFileMetadataLoader.java File fe/src/main/java/org/apache/impala/catalog/IcebergFileMetadataLoader.java: http://gerrit.cloudera.org:8080/#/c/22367/4/fe/src/main/java/org/apache/impala/catalog/IcebergFileMetadataLoader.java@183 PS4, Line 183: LOG.warn(String.format("Missing Iceberg content file: %s", > Does the exception already include the path? Ideally it should. FileNotFoundException and permission exceptions should include the path. But I think we can also get e.g. SocketException here in which case it could be useful to have the path as well, so if the user searches for the path in the logs they will find it. Hopefully it's not causing too much noise in the generic case. But I can also add extra catches, like: catch (FileNotFoundException) { ... omit file path } catch (IOException e) { ... include file path } -- To view, visit http://gerrit.cloudera.org:8080/22367 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If753619d8ee1b30f018e90157ff7bdbe5d7f1525 Gerrit-Change-Number: 22367 Gerrit-PatchSet: 4 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Fri, 07 Feb 2025 18:29:32 +0000 Gerrit-HasComments: Yes
