Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16545 )
Change subject: IMPALA-10215: Implement INSERT INTO for non-partitioned Iceberg tables (Parquet) ...................................................................... Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/16545/7/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java: http://gerrit.cloudera.org:8080/#/c/16545/7/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java@81 PS7, Line 81: Exception > Catching every exception is potentially an overkill, as it may hide bugs th It's added because we might get a NullPointerException, or a CannotObtainBlockLengthException from Iceberg. At least we've seen these two so far. They both might just disappear on retries. On the other hand, we shouldn't retry loading the table in the case of NoSuchTableException, as we would just get the same exception repeatedly. So the idea was to handle all exceptions as retriable errors (except NoSuchTableException). If loadTable() succeeds eventually, then the error was intermittent, but we still logged it. If the error doesn't disappear then we just throw a TableLoadingException and we can look at the logs to investigate the error. But if you want I can change the catch(Exception e) to catch(NullPointerException | CannotObtainBlockLengthException e). Alternatively, when 'attempt == MAX_ATTEMPTS - 1', I can just wrap the exception into a TableLoadingException and re-throw it. Which option do you prefer? I'm leaning towards catching all exceptions, but re-throwing on last attempt. This way all temporary errors would be ignored (but logged), but we'd still throw on real bugs. -- To view, visit http://gerrit.cloudera.org:8080/16545 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5690fb6c2cc51f0033fa26caf8597c80a11bcd8e Gerrit-Change-Number: 16545 Gerrit-PatchSet: 7 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: wangsheng <[email protected]> Gerrit-Comment-Date: Mon, 26 Oct 2020 09:32:32 +0000 Gerrit-HasComments: Yes
