You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org> on 2020/04/17 01:15:36 UTC

[Impala-ASF-CR] IMPALA-9663: Fix for NPE in fire listener events 

Vihang Karajgaonkar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15745


Change subject: IMPALA-9663: Fix for NPE in fire listener events
......................................................................

IMPALA-9663: Fix for NPE in fire listener events

IMPALA-8632 introduced a improvement to Events processor which issues a
new HMS API call to fire INSERT events when table and partitions are
inserted into. However, if there is no new events generated HMS does
not set the eventIds field in the response. This fix adds a check to
not fire the listener events in such cases. Also, adds a check to make
sure that eventIds are set in the response.

Testing:
1. Added a test sql in test_event_processing.py which was failing with
a NPE before the fix.

Change-Id: Ibfcc5acd598fb0354a5a8288df7c495359f9e53d
---
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/custom_cluster/test_event_processing.py
3 files changed, 29 insertions(+), 6 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/15745/2
-- 
To view, visit http://gerrit.cloudera.org:8080/15745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibfcc5acd598fb0354a5a8288df7c495359f9e53d
Gerrit-Change-Number: 15745
Gerrit-PatchSet: 2
Gerrit-Owner: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Xiaomeng Zhang <xi...@cloudera.com>

[Impala-ASF-CR] IMPALA-9663: Fix for NPE in fire listener events

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15745 )

Change subject: IMPALA-9663: Fix for NPE in fire listener events
......................................................................


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5665/ DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfcc5acd598fb0354a5a8288df7c495359f9e53d
Gerrit-Change-Number: 15745
Gerrit-PatchSet: 2
Gerrit-Owner: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xiaomeng Zhang <xi...@cloudera.com>
Gerrit-Comment-Date: Fri, 17 Apr 2020 01:16:25 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9663: Fix for NPE in fire listener events

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15745 )

Change subject: IMPALA-9663: Fix for NPE in fire listener events
......................................................................


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfcc5acd598fb0354a5a8288df7c495359f9e53d
Gerrit-Change-Number: 15745
Gerrit-PatchSet: 2
Gerrit-Owner: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Xiaomeng Zhang <xi...@cloudera.com>
Gerrit-Comment-Date: Fri, 17 Apr 2020 05:49:53 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9663: Fix for NPE in fire listener events

Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15745 )

Change subject: IMPALA-9663: Fix for NPE in fire listener events
......................................................................


Patch Set 2:

Thanks.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfcc5acd598fb0354a5a8288df7c495359f9e53d
Gerrit-Change-Number: 15745
Gerrit-PatchSet: 2
Gerrit-Owner: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Xiaomeng Zhang <xi...@cloudera.com>
Gerrit-Comment-Date: Fri, 17 Apr 2020 01:30:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9663: Fix for NPE in fire listener events

Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15745 )

Change subject: IMPALA-9663: Fix for NPE in fire listener events
......................................................................

IMPALA-9663: Fix for NPE in fire listener events

IMPALA-8632 introduced a improvement to Events processor which issues a
new HMS API call to fire INSERT events when table and partitions are
inserted into. However, if there is no new events generated HMS does
not set the eventIds field in the response. This fix adds a check to
not fire the listener events in such cases. Also, adds a check to make
sure that eventIds are set in the response.

Testing:
1. Added a test sql in test_event_processing.py which was failing with
a NPE before the fix.

Change-Id: Ibfcc5acd598fb0354a5a8288df7c495359f9e53d
Reviewed-on: http://gerrit.cloudera.org:8080/15745
Reviewed-by: Anurag Mantripragada <an...@cloudera.com>
Reviewed-by: Xiaomeng Zhang <xi...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Quanlong Huang <hu...@gmail.com>
---
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/custom_cluster/test_event_processing.py
3 files changed, 29 insertions(+), 6 deletions(-)

Approvals:
  Anurag Mantripragada: Looks good to me, but someone else must approve
  Xiaomeng Zhang: Looks good to me, but someone else must approve
  Impala Public Jenkins: Verified
  Quanlong Huang: Looks good to me, approved

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibfcc5acd598fb0354a5a8288df7c495359f9e53d
Gerrit-Change-Number: 15745
Gerrit-PatchSet: 3
Gerrit-Owner: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Xiaomeng Zhang <xi...@cloudera.com>

[Impala-ASF-CR] IMPALA-9663: Fix for NPE in fire listener events

Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15745 )

Change subject: IMPALA-9663: Fix for NPE in fire listener events
......................................................................


Patch Set 2: Code-Review+1

(1 comment)

Thanks for the change. Looks good to me.

http://gerrit.cloudera.org:8080/#/c/15745/2/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/15745/2/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@1062
PS2, Line 1062: Collections.EMPTY_LIST;
Is this same as Collections.emptyList();?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfcc5acd598fb0354a5a8288df7c495359f9e53d
Gerrit-Change-Number: 15745
Gerrit-PatchSet: 2
Gerrit-Owner: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xiaomeng Zhang <xi...@cloudera.com>
Gerrit-Comment-Date: Fri, 17 Apr 2020 01:21:30 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9663: Fix for NPE in fire listener events

Posted by "Xiaomeng Zhang (Code Review)" <ge...@cloudera.org>.
Xiaomeng Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15745 )

Change subject: IMPALA-9663: Fix for NPE in fire listener events
......................................................................


Patch Set 2: Code-Review+1

Thanks Vihang, LGTM


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfcc5acd598fb0354a5a8288df7c495359f9e53d
Gerrit-Change-Number: 15745
Gerrit-PatchSet: 2
Gerrit-Owner: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Xiaomeng Zhang <xi...@cloudera.com>
Gerrit-Comment-Date: Fri, 17 Apr 2020 01:35:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9663: Fix for NPE in fire listener events

Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15745 )

Change subject: IMPALA-9663: Fix for NPE in fire listener events
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15745/2/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/15745/2/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@1062
PS2, Line 1062: Collections.EMPTY_LIST;
> Is this same as Collections.emptyList();?
yes, its the same. Collections.emptyList() returns Collections.EMPTY_LIST internally with the addition that it typecasts it to the expected type.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfcc5acd598fb0354a5a8288df7c495359f9e53d
Gerrit-Change-Number: 15745
Gerrit-PatchSet: 2
Gerrit-Owner: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Xiaomeng Zhang <xi...@cloudera.com>
Gerrit-Comment-Date: Fri, 17 Apr 2020 01:29:24 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9663: Fix for NPE in fire listener events

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15745 )

Change subject: IMPALA-9663: Fix for NPE in fire listener events
......................................................................


Patch Set 2: Code-Review+2

(1 comment)

Carry on the +1s to +2.

http://gerrit.cloudera.org:8080/#/c/15745/2/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/15745/2/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@1032
PS2, Line 1032: Atleast
nit: At least



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfcc5acd598fb0354a5a8288df7c495359f9e53d
Gerrit-Change-Number: 15745
Gerrit-PatchSet: 2
Gerrit-Owner: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Xiaomeng Zhang <xi...@cloudera.com>
Gerrit-Comment-Date: Fri, 17 Apr 2020 06:08:28 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9663: Fix for NPE in fire listener events

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15745 )

Change subject: IMPALA-9663: Fix for NPE in fire listener events
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/5814/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfcc5acd598fb0354a5a8288df7c495359f9e53d
Gerrit-Change-Number: 15745
Gerrit-PatchSet: 2
Gerrit-Owner: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Xiaomeng Zhang <xi...@cloudera.com>
Gerrit-Comment-Date: Fri, 17 Apr 2020 01:56:18 +0000
Gerrit-HasComments: No