Kurt Deschler 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 3: (9 comments) Nice work! We should aim to make this the default behavior once event waiting and handling are optimized for common cases. http://gerrit.cloudera.org:8080/#/c/22571/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java: http://gerrit.cloudera.org:8080/#/c/22571/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@1112 PS3, Line 1112: eventRequest.setTableNames(metaDataFilter.getTableNames()); Looks like NotificationRequest handles null tableNames arg and you don't need conditional code at this level to handle that. http://gerrit.cloudera.org:8080/#/c/22571/3/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/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1506 PS3, Line 1506: public static List<String> TABLE_LIST_EVENT_TYPES = Lists.newArrayList( Can these be declared final? http://gerrit.cloudera.org:8080/#/c/22571/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1635 PS3, Line 1635: * Found the min required event id that should be synced to avoid the query using Change Found to Find http://gerrit.cloudera.org:8080/#/c/22571/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1649 PS3, Line 1649: List<String> dbNames = new ArrayList<>(); declare final. Can this be Set instead of List to remove duplicates? http://gerrit.cloudera.org:8080/#/c/22571/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1690 PS3, Line 1690: List<String> eventTypes = wantTableList ? declare final http://gerrit.cloudera.org:8080/#/c/22571/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1692 PS3, Line 1692: NotificationFilter filter = e -> dbName.equalsIgnoreCase(e.getDbName()) Can filter be declared final static? http://gerrit.cloudera.org:8080/#/c/22571/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1772 PS3, Line 1772: Set<String> tblNames = new HashSet<>(db2Tables.get(dbName)); declare final http://gerrit.cloudera.org:8080/#/c/22571/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1773 PS3, Line 1773: NotificationFilter filter = e -> dbName.equalsIgnoreCase(e.getDbName()) Can filter be declared final static? http://gerrit.cloudera.org:8080/#/c/22571/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1801 PS3, Line 1801: NotificationFilter filter = e -> MetastoreShim.isDefaultCatalog(e.getCatName()) Can filter be declared final static? -- 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: 3 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: 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: Fri, 14 Mar 2025 15:35:45 +0000 Gerrit-HasComments: Yes
