Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20486 )

Change subject: IMPALA-12356: Alter partition event from hive is treated as 
self event
......................................................................


Patch Set 10:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/20486/10//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20486/10//COMMIT_MSG@7
PS10, Line 7: IMPALA-12356: Alter partition event from hive is treated as self 
event
I think we need a more specific title. Not all the ALTER_PARTITION events are 
treated as self events.


http://gerrit.cloudera.org:8080/#/c/20486/10/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/20486/10/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@a2049
PS10, Line 2049:
> Yes. self event detection is common for both transactional and non-transact
I was confused on why the bug of IMPALA-10502 won't occur after reverting this. 
Later I realized the new mechanism still exists so this is just added back as 
the first guard so it works.

However, we tend to remove the old mechanism of self-event detection in the 
future. It might be better to fix the issue in the root source, i.e. not adding 
the catalog version into the inFlightEvents list when INSERT creates new 
partitions. I left more details in the JIRA comment: 
https://issues.apache.org/jira/browse/IMPALA-12356?focusedCommentId=17795150&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17795150

We can continue the discussion there.


http://gerrit.cloudera.org:8080/#/c/20486/10/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/20486/10/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@4563
PS10, Line 4563: in
nit: could you add a space before "in" by the way?


http://gerrit.cloudera.org:8080/#/c/20486/10/fe/src/test/java/org/apache/impala/catalog/metastore/CatalogHmsSyncToLatestEventIdTest.java
File 
fe/src/test/java/org/apache/impala/catalog/metastore/CatalogHmsSyncToLatestEventIdTest.java:

http://gerrit.cloudera.org:8080/#/c/20486/10/fe/src/test/java/org/apache/impala/catalog/metastore/CatalogHmsSyncToLatestEventIdTest.java@165
PS10, Line 165:         catalog_.setMetastoreEventProcessor(eventsProcessor_);
> Events generated from previous tests are not processed with eventsProcessor
Ack



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23c2affa3fe32c0b3843bff5e4c0018dce9060d3
Gerrit-Change-Number: 20486
Gerrit-PatchSet: 10
Gerrit-Owner: Anonymous Coward <k.venureddy2...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k.venureddy2...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <saihema...@cloudera.com>
Gerrit-Comment-Date: Mon, 11 Dec 2023 03:44:00 +0000
Gerrit-HasComments: Yes

Reply via email to