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

Change subject: IMPALA-11822: Optimize the Refresh/Invalidate event processing 
by skipping unnecessary events
......................................................................


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/19484/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java:

http://gerrit.cloudera.org:8080/#/c/19484/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@2442
PS1, Line 2442:     private int eventTime_;
> nit: need a blank line after this
Ack


http://gerrit.cloudera.org:8080/#/c/19484/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@2514
PS1, Line 2514: catalog_.getPartitionMapFromRefreshMap(dbName_, tblName_
> nit: we can extract the code of getting the map value
Ack


http://gerrit.cloudera.org:8080/#/c/19484/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@2516
PS1, Line 2516:
> Is the eventTime generated by HMS? If there are time skew between the HMS n
Yeah, the event time is generated by HMS. This variable stores the latest 
timestamp of catalogD node when there is a refresh event, the HMS timestamp is 
generated much later. So I don't think you can skip refresh events by any 
chance.

In HMS, the timestamp generation is in a private method of the 
DbNotificationListener class. The timestamp is unix epoch, so we don't need to 
compute the time difference. WHat do you think?


http://gerrit.cloudera.org:8080/#/c/19484/1/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/19484/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@6362
PS1, Line 6362:       }
> Should we update 'refreshMetadataMap' for normal refresh cases?
Yeah, I did cover the normal refresh cases as well. Did I miss anything?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I905957683a96c3ea01ab4bf043d6658ce37b7574
Gerrit-Change-Number: 19484
Gerrit-PatchSet: 2
Gerrit-Owner: Sai Hemanth Gantasala <[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: Wed, 22 Feb 2023 16:22:11 +0000
Gerrit-HasComments: Yes

Reply via email to