Sai Hemanth Gantasala has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19378 )

Change subject: IMPALA-11808: Add support for reload event in catalogD
......................................................................


Patch Set 9:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/19378/8/fe/src/compat-apache-hive-3/java/org/apache/impala/compat/MetastoreShim.java
File 
fe/src/compat-apache-hive-3/java/org/apache/impala/compat/MetastoreShim.java:

http://gerrit.cloudera.org:8080/#/c/19378/8/fe/src/compat-apache-hive-3/java/org/apache/impala/compat/MetastoreShim.java@429
PS8, Line 429:
             :    */
             :   @VisibleForTesting
             :   public static List<Long> fireReloadEventHelper(MetaStoreClient 
msClient,
             :       boolean isRefresh, List<String> partVals, String dbName, 
String tableName,
             :       Map<String, String> selfEventParams) throws TException {
             :     throw new UnsupportedOperationException("Reload event is not 
supported.");
             :   }
             :
             :   /**
> These comments are stale now.
Ack


http://gerrit.cloudera.org:8080/#/c/19378/8/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/19378/8/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@541
PS8, Line 541:
> nit: "getFieldsFromReloadEvent" sounds better since it returns the results
Ack


http://gerrit.cloudera.org:8080/#/c/19378/8/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/19378/8/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@6417
PS8, Line 6417:       
MetastoreShim.fireReloadEventHelper(catalog_.getMetaStoreClient(),
> Just realized that the returned event ids are ignored. Does the self-event
Self-events are detected using the catalog service id and catalog version. 
Event id will not be used at all. Initially, I thought we can use event id but 
later, so that is why event ids are present here but I'm not using this. Will 
remove it now.


http://gerrit.cloudera.org:8080/#/c/19378/8/tests/custom_cluster/test_events_custom_configs.py
File tests/custom_cluster/test_events_custom_configs.py:

http://gerrit.cloudera.org:8080/#/c/19378/8/tests/custom_cluster/test_events_custom_configs.py@244
PS8, Line 244:         .format(unique_database, test_reload_table))
> To avoid the test become flaky, we need to wait after this line until the e
Ack


http://gerrit.cloudera.org:8080/#/c/19378/8/tests/custom_cluster/test_events_custom_configs.py@246
PS8, Line 246:     def check_self_events(query):
> Can we add an optional argument for the partition values (default to None)
There is no partition info in the notification event. we'll have to deserialize 
the notification message to obtain this value. So I don't think this is 
possible in python code



--
To view, visit http://gerrit.cloudera.org:8080/19378
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic62d58837d356dc2113f3c0904228ac9de484136
Gerrit-Change-Number: 19378
Gerrit-PatchSet: 9
Gerrit-Owner: Sai Hemanth Gantasala <[email protected]>
Gerrit-Reviewer: Abhishek Rawat <[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: Thu, 05 Jan 2023 03:12:13 +0000
Gerrit-HasComments: Yes

Reply via email to