[email protected] 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 4: (10 comments) Reworked on 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? Added 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: ableName t > nit: Is this actually "unloadedTables" ? Could you add a comment on the rel Added comments http://gerrit.cloudera.org:8080/#/c/21478/1/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java@349 PS1, Line 349: try { > nit: add brackets {} Done http://gerrit.cloudera.org:8080/#/c/21478/4/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/4/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java@332 PS4, Line 332: tablesToLoad.get(tblName) == tbl > Is there a scenario where tablesToLoad.get(tblName) != tbl && condition in isLoadFailedByRecoverableError() returns true if the load has failed due to recoverable error(i.e., HMS unreachable error). If the load has failed in this try again, isLoadFailedByRecoverableError() returns true and table in the db changes(i.e.,tablesToLoad.get(tblName) != tbl) since the catalogd updates the table in db after load. Note: StmtMetadataLoader#getMissingTables is called in loop from StmtMetadataLoader#loadTables(Set<TableName>) Have added comments in code now. http://gerrit.cloudera.org:8080/#/c/21478/4/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java@332 PS4, Line 332: get( > Should this be getOrDefault(tblName, null) to avoid unexpected null pointer We do not get null pointer exception since get() returns null when map do not contain mapping to the key. 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: f no validWriteIdList is provided > Could you add a comment on why do we need this? Is there an IncompleteTable Done. Have modified it to reload if it is incomplete table with recoverable error. 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: return false; > nit: format Done 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: > nit: can we emphasize the error is recoverable, e.g. isLoadFailedByRecovera Done 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 Added test with local catalog mode http://gerrit.cloudera.org:8080/#/c/21478/1/tests/custom_cluster/test_catalog_hms_failures.py@91 PS1, Line 91: catalogd_args='--catalog_topic_mode=minimal') : def test_local_catalog_load_with_hms_state_change(sel > We can create the table using Impala. The table is unloaded after the creat Done -- 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: 4 Gerrit-Owner: Anonymous Coward <[email protected]> Gerrit-Reviewer: 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, 24 Jun 2024 04:37:05 +0000 Gerrit-HasComments: Yes
