Xiaomeng Zhang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15648 )

Change subject: IMPALA-8632: Add support for self-event detection for insert 
events
......................................................................


Patch Set 4:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/15648/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:

http://gerrit.cloudera.org:8080/#/c/15648/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@286
PS3, Line 286:    *
> guess this will be removed once we bump up the CDP_BUILD right?
Yes. It will be removed after I have hive jar updated.


http://gerrit.cloudera.org:8080/#/c/15648/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@849
PS3, Line 849: versionNumber == -1
> Why would this be a case?
As we check -1 for version number. I am checking -1 for eventId as well. Is it 
guaranteed that eventId will not be less than 0?


http://gerrit.cloudera.org:8080/#/c/15648/3/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java:

http://gerrit.cloudera.org:8080/#/c/15648/3/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@897
PS3, Line 897:     Preconditions.checkState(inFlightEvents_.size(false) == 0);
> do we need a similar check for inFlightEvents_.size(false)?
Done


http://gerrit.cloudera.org:8080/#/c/15648/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/15648/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@4411
PS3, Line 4411: ram isInsertOverwrite indicates if the operation was an inse
> I think this check should be ((!catalog_.isEventProcessingActive() && isIns
Done


http://gerrit.cloudera.org:8080/#/c/15648/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@4466
PS3, Line 4466: sMapBeforeInsert.entrySet().iterator().next();
> I think we need to do this via MetastoreShim since otherwise the response w
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7873fbb2c159343690f93b9d120f6b425b983dcf
Gerrit-Change-Number: 15648
Gerrit-PatchSet: 4
Gerrit-Owner: Xiaomeng Zhang <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]>
Gerrit-Reviewer: Xiaomeng Zhang <[email protected]>
Gerrit-Comment-Date: Tue, 07 Apr 2020 21:41:18 +0000
Gerrit-HasComments: Yes

Reply via email to