Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/22571 )
Change subject: IMPALA-13684: Improve waitForHmsEvent() to only wait for related events ...................................................................... Patch Set 11: (5 comments) To reduce the patch size, split coordinator side changes to a separate patch: http://gerrit.cloudera.org:8080/22743 http://gerrit.cloudera.org:8080/#/c/22571/10/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/22571/10/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@4363 PS10, Line 4363: LOG.warn("Failed to check and expand views. Query might see stale metadata", e); > stale Done http://gerrit.cloudera.org:8080/#/c/22571/10/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@4452 PS10, Line 4452: // It's a cheap operation since there are no partition/file metadata. > Should we set reason to "Reference from view <viewname>"? I thought the reason string is not used since it's not used in View#load(): https://github.com/apache/impala/blob/d7b5819f906c19281fdf9594ecf4616f6c9f92a0/fe/src/main/java/org/apache/impala/catalog/View.java#L87 Later I realized it's used here: https://github.com/apache/impala/blob/4c549d79f2648c5ed906deef42c8d05bb747d8b0/fe/src/main/java/org/apache/impala/catalog/TableLoader.java#L80-L81 Updated this to "expand latest view to check HMS events on underlying tables". The log already has the view name, e.g. I0407 19:27:51.064941 115108 TableLoader.java:81] Loading metadata for: test_hms_event_sync_with_view_64cdc1a6.foo_view (expand latest view to check HMS events on underlying tables) http://gerrit.cloudera.org:8080/#/c/22571/10/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@4483 PS10, Line 4483: LOG.warn("Unable to expand view {}.{} since it's modified concurrently", > Vague message. If this is expected to be a view we should be more concise a There are some corner cases that the view could be dropped concurrently (tbl == null) or catalogd fails to load it (tbl becomes IncompleteTable). They will lead to query failures which will log the real cause. So just use warning here. http://gerrit.cloudera.org:8080/#/c/22571/10/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@4523 PS10, Line 4523: LOG.warn("Illegal table name found in view {}: {}", view.getFullName(), strs); > Why only a warning? I'll revisit this to see how to create such a view. http://gerrit.cloudera.org:8080/#/c/22571/8/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/22571/8/fe/src/main/java/org/apache/impala/service/Frontend.java@2250 PS8, Line 2250: bject(); : dbDesc.setType(TCatalogObjectType.DATABASE) > CreateTableAsSelectStmt and CreateTableLikeStmt extend CreateTableStmt so t Sorry that CreateTableAsSelectStmt and CreateTableLikeStmt don't extend CreateTableStmt actually. We don't need these two checks. Thanks for catching this! -- To view, visit http://gerrit.cloudera.org:8080/22571 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic033b7e197cd19505653c3ff80c4857cc474bcfc Gerrit-Change-Number: 22571 Gerrit-PatchSet: 11 Gerrit-Owner: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Anonymous Coward <k.venureddy2...@gmail.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com> Gerrit-Reviewer: Kurt Deschler <kdesc...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Sai Hemanth Gantasala <saihema...@cloudera.com> Gerrit-Comment-Date: Mon, 07 Apr 2025 11:31:24 +0000 Gerrit-HasComments: Yes
