Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21665 )

Change subject: IMPALA-12865: enable_reload_events breaks 
enable_skipping_older_events by pushing lastRefreshEventId too high
......................................................................


Patch Set 5: Code-Review+1

(3 comments)

Thanks for the test, looks good in general, added few ideas.

http://gerrit.cloudera.org:8080/#/c/21665/5/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/21665/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1298
PS5, Line 1298: GET_PARTITIONS_DELAY
A separate debug action could be added for this, e.g. IS_OLDER_EVENT_CHECK_DELAY


http://gerrit.cloudera.org:8080/#/c/21665/3/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/21665/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7176
PS3, Line 7176: l.getFullName());
> Yeah, if we cannot get the latest event id from HMS, then it is possible fo
Isn't it still possible for this to cause issues if we use eventIds.get(0)? 
Setting setLastRefreshEventId could lead to unnecessary REFRESH but would 
ensure correctness.


http://gerrit.cloudera.org:8080/#/c/21665/5/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/21665/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7164
PS5, Line 7164: GET_PARTITIONS_DELAY
It looks a bit a strange that the name of the is GET_PARTITIONS_DELAY but it is 
run before fireReloadEventHelper. Maybe FIRE_RELOAD_EVENT_DELAY would be 
clearer?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I90039da77ec561c5aede44456f88c6650582815b
Gerrit-Change-Number: 21665
Gerrit-PatchSet: 5
Gerrit-Owner: Sai Hemanth Gantasala <[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, 07 Nov 2024 15:24:07 +0000
Gerrit-HasComments: Yes

Reply via email to