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 7: (10 comments) http://gerrit.cloudera.org:8080/#/c/22571/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/22571/6//COMMIT_MSG@12 PS6, Line 12: by query option sync_hms_events_wait_time_s. Currently, when this option > Currently, when this option is enabled, Done http://gerrit.cloudera.org:8080/#/c/22571/6//COMMIT_MSG@15 PS6, Line 15: > change "improves this" to "reduces waiting" Done http://gerrit.cloudera.org:8080/#/c/22571/6//COMMIT_MSG@16 PS6, Line 16: This patch reduces waiting by only checking related events and wait > change "the last event of them" to "the last related event" Done http://gerrit.cloudera.org:8080/#/c/22571/6//COMMIT_MSG@17 PS6, Line 17: until the last related event has been processed. In the ideal case, if > they query Done http://gerrit.cloudera.org:8080/#/c/22571/6//COMMIT_MSG@19 PS6, Line 19: wait. > Related pending events are determined as follows: Done http://gerrit.cloudera.org:8080/#/c/22571/7/fe/src/main/java/org/apache/impala/analysis/SingleTableStmt.java File fe/src/main/java/org/apache/impala/analysis/SingleTableStmt.java: http://gerrit.cloudera.org:8080/#/c/22571/7/fe/src/main/java/org/apache/impala/analysis/SingleTableStmt.java@1 PS7, Line 1: package org.apache.impala.analysis; > Need Apache license header here? Yeah, missing this. http://gerrit.cloudera.org:8080/#/c/22571/7/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java: http://gerrit.cloudera.org:8080/#/c/22571/7/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1578 PS7, Line 1578: waitForSyncUpToFreshMetadata > nit: I think waitForMostRecentMetadata sounds better. Done http://gerrit.cloudera.org:8080/#/c/22571/7/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1769 PS7, Line 1769: // TODO: expand views I need to think more about this. Without expanding the views we might end up using stale metadata on the underlying tables. We can expand views here(catalogd) or on coordinator side. Doing this on catalogd side requires adding query parsing functionality here. But if catalogd can parse the view statement, it can actually parse the original query so coordinator don't need to parse it and send the db/table names. We need to modify TWaitForHmsEventRequest to send the original query string. Doing this on coordinator side is similar to how we trigger metadata loading in the legacy catalog mode - coordinator sends prioritizedLoad requests for the table names and expands views and then send another set of missing table names. We might be able to reuse some codes there. We need to modify TWaitForHmsEventResponse to send back the event id when the query starts. Then coordinator can send that event id with new table names so catalogd don't need to check new events after that. http://gerrit.cloudera.org:8080/#/c/22571/7/tests/authorization/test_ranger.py File tests/authorization/test_ranger.py: http://gerrit.cloudera.org:8080/#/c/22571/7/tests/authorization/test_ranger.py@1983 PS7, Line 1983: --hms_event_polling_interval_s=5 > Instead of setting long polling interval, can you pause/resume EP using com It's more convenient to use a long polling internval here. Using pause & resume commands we have to submit the query asynchronously and then resume EP and check the query results. http://gerrit.cloudera.org:8080/#/c/22571/7/tests/common/impala_test_suite.py File tests/common/impala_test_suite.py: http://gerrit.cloudera.org:8080/#/c/22571/7/tests/common/impala_test_suite.py@1516 PS7, Line 1516: def run_with_hms_sync(cls, client, timeout_s=10, strict=True): > This is unused? Oops, this can be removed. -- 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: 7 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: Wed, 26 Mar 2025 12:53:03 +0000 Gerrit-HasComments: Yes
