Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21478 )
Change subject: IMPALA-13120: Load failed table without need for manual invalidate ...................................................................... Patch Set 1: (8 comments) http://gerrit.cloudera.org:8080/#/c/21478/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21478/1//COMMIT_MSG@24 PS1, Line 24: Could you add a summary on the solution? http://gerrit.cloudera.org:8080/#/c/21478/1/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java File fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java: http://gerrit.cloudera.org:8080/#/c/21478/1/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java@169 PS1, Line 169: loadTables nit: Is this actually "unloadedTables" ? Could you add a comment on the relationship between this and 'missingTbls' ? http://gerrit.cloudera.org:8080/#/c/21478/1/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java@349 PS1, Line 349: missingTbls.addAll(getMissingTables(catalog, viewTbls, loadTables)); nit: add brackets {} http://gerrit.cloudera.org:8080/#/c/21478/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: http://gerrit.cloudera.org:8080/#/c/21478/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2470 PS1, Line 2470: !(tbl instanceof IncompleteTable) Could you add a comment on why do we need this? Is there an IncompleteTable that isLoaded() returns true? EDIT: I realized IncompleteTable.isLoaded() returns (cause_ != null). However, after this change, all IncompleteTable will be reloaded again regardless what the cause_ is. Can we just do so for recoverable errors? Errors like unsupported column types are unrecoverable. E.g. if I create a table with UNION column types in Hive and use it in Impala: hive> CREATE TABLE employee ( id INT, name STRING, salary DOUBLE, additional_info UNIONTYPE<STRING, INT, DOUBLE>); impala> invalidate metadata employee; impala> describe employee; ERROR: AnalysisException: Could not load table default.employee from catalog CAUSED BY: TableLoadingException: Could not load table default.employee from catalog CAUSED BY: TException: TGetPartialCatalogObjectResponse(status:TStatus(status_code:GENERAL, error_msgs:[TableLoadingException: Unsupported type 'uniontype<string,int,double>' in column 'additional_info' of table 'employee']), lookup_status:OK) It'd be better to keep the original behavior for such unrecoverable errors. http://gerrit.cloudera.org:8080/#/c/21478/1/fe/src/main/java/org/apache/impala/catalog/FeIncompleteTable.java File fe/src/main/java/org/apache/impala/catalog/FeIncompleteTable.java: http://gerrit.cloudera.org:8080/#/c/21478/1/fe/src/main/java/org/apache/impala/catalog/FeIncompleteTable.java@32 PS1, Line 32: } nit: format http://gerrit.cloudera.org:8080/#/c/21478/1/fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java File fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java: http://gerrit.cloudera.org:8080/#/c/21478/1/fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java@91 PS1, Line 91: isLoadFailed nit: can we emphasize the error is recoverable, e.g. isLoadFailedByRecoverableError()? I think HMS restart is just one cause of such failures. Restarts of other services like NameNode, Kudu Master, HBase Master could also lead to recoverable errors. http://gerrit.cloudera.org:8080/#/c/21478/1/tests/custom_cluster/test_catalog_hms_failures.py File tests/custom_cluster/test_catalog_hms_failures.py: http://gerrit.cloudera.org:8080/#/c/21478/1/tests/custom_cluster/test_catalog_hms_failures.py@88 PS1, Line 88: @pytest.mark.execute_serially Let's also test on local-catalog mode, i.e. adding parameters like @CustomClusterTestSuite.with_args( impalad_args='--use_local_catalog', catalogd_args='--catalog_topic_mode=minimal') http://gerrit.cloudera.org:8080/#/c/21478/1/tests/custom_cluster/test_catalog_hms_failures.py@91 PS1, Line 91: self.run_stmt_in_hive("create table {0} (i int)".format(table)) : EventProcessorUtils.wait_for_event_processing(self) We can create the table using Impala. The table is unloaded after the creation. -- To view, visit http://gerrit.cloudera.org:8080/21478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia882fdd865ef716351be7f1eaf203a9fb04c1c15 Gerrit-Change-Number: 21478 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]> Gerrit-Comment-Date: Mon, 10 Jun 2024 02:38:38 +0000 Gerrit-HasComments: Yes
