You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org> on 2019/09/24 22:58:16 UTC

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

Anurag Mantripragada has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14296


Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................

IMPALA-8968: Alter database events on dropped database should not put
events processor in error state.

This change is two-fold:
1. If an alter database event is received on database that does not
   exist, the event can be safely ignored. The events processor should
   only go into an error state if updateDb() fails.

2. This change also adds catalog service identifiers to create/drop
   function operations as Impala generates alter database events
   with these operations and they should be detected as self-events
   and ignored.

Testing:
Add tests to verify both the above changes to
MetastoreEventsProcessorTest.

Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
4 files changed, 116 insertions(+), 3 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/14296/1
-- 
To view, visit http://gerrit.cloudera.org:8080/14296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 1
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

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

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................


Patch Set 7:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 7
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sun, 29 Sep 2019 05:25:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

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

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................


Patch Set 7:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 7
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 28 Sep 2019 19:21:06 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/14296 )

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................

IMPALA-8968: Alter database events on dropped database should not put
events processor in error state.

This change is two-fold:
1. If an alter database event is received on database that does not
   exist, the event can be safely ignored. The events processor should
   only go into an error state if updateDb() fails.

2. This change also adds catalog service identifiers to create/drop
   function operations as Impala generates alter database events
   with these operations and they should be detected as self-events
   and ignored.

Testing:
Add tests to verify both the above changes to
MetastoreEventsProcessorTest.

Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
4 files changed, 111 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/14296/3
-- 
To view, visit http://gerrit.cloudera.org:8080/14296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 3
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

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

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................


Patch Set 7: Code-Review+2

(1 comment)

Thanks for fixing this!

http://gerrit.cloudera.org:8080/#/c/14296/6/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/14296/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1313
PS6, Line 1313:           catalog_.addVersionsForInflightEvents(db, newCatalogVersion);
> The ALTER_DATABASE event is generated by the applyAlterDatabase() function 
Thanks for the explanation!



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 7
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 27 Sep 2019 22:35:49 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

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

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/4657/ : 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/14296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 4
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 26 Sep 2019 20:20:46 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

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

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14296/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/14296/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2217
PS3, Line 2217: 
              :    */
> remove?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 6
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 27 Sep 2019 18:13:19 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

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

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................


Patch Set 2:

(4 comments)

Bunch of nits.

http://gerrit.cloudera.org:8080/#/c/14296/2/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/14296/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2220
PS2, Line 2220: CatalogException
Any reason this is CatalogException instead of DatabaseNotFound...?

While we are here, how about handling the DatabaseNotFoundException here in this method and not throw anything? (since we call the method updateDbIfExists anyway)


http://gerrit.cloudera.org:8080/#/c/14296/2/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/14296/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1228
PS2, Line 1228: OKay
nit: Okay


http://gerrit.cloudera.org:8080/#/c/14296/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1235
PS2, Line 1235:       } catch (DatabaseNotFoundException e) {
Consider moving the exception handling to updateDbIfExists?


http://gerrit.cloudera.org:8080/#/c/14296/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
File fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java:

http://gerrit.cloudera.org:8080/#/c/14296/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@2336
PS2, Line 2336: dropScalarFunctionFromImapala
Typo, impala



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 2
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 25 Sep 2019 21:44:29 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

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

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................


Patch Set 7: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 7
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sun, 29 Sep 2019 09:38:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

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

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................


Patch Set 4: Code-Review+1

(1 comment)

Quanlong can you +2 it if you have no further comments?

http://gerrit.cloudera.org:8080/#/c/14296/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/14296/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2217
PS3, Line 2217:  Throws
              :    * DatabaseNotFoundException if db was removed before update starts.
remove?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 4
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 27 Sep 2019 17:00:58 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

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

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14296/6/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/14296/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1313
PS6, Line 1313:           catalog_.addVersionsForInflightEvents(db, newCatalogVersion);
> Could you explain why we don't need this for persistent java functions? In 
The ALTER_DATABASE event is generated by the applyAlterDatabase() function above. For persistent Java functions, we call hms api for createFunction() which will not create an alter database function.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 6
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 27 Sep 2019 22:27:01 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/14296 )

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................

IMPALA-8968: Alter database events on dropped database should not put
events processor in error state.

This change is two-fold:
1. If an alter database event is received on database that does not
   exist, the event can be safely ignored. The events processor should
   only go into an error state if updateDb() fails.

2. This change also adds catalog service identifiers to create/drop
   function operations as Impala generates alter database events
   with these operations and they should be detected as self-events
   and ignored.

Testing:
Add tests to verify both the above changes to
MetastoreEventsProcessorTest.

Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
4 files changed, 110 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/14296/7
-- 
To view, visit http://gerrit.cloudera.org:8080/14296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 7
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

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

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................


Patch Set 7:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 7
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 27 Sep 2019 23:11:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

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

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................


Patch Set 3:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/14296/2/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/14296/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2220
PS2, Line 2220: 
> Any reason this is CatalogException instead of DatabaseNotFound...?
Done


http://gerrit.cloudera.org:8080/#/c/14296/2/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/14296/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1228
PS2, Line 1228: ugLo
> nit: Okay
Done


http://gerrit.cloudera.org:8080/#/c/14296/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1235
PS2, Line 1235: 
> Consider moving the exception handling to updateDbIfExists?
Done


http://gerrit.cloudera.org:8080/#/c/14296/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
File fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java:

http://gerrit.cloudera.org:8080/#/c/14296/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@557
PS2, Line 557: scalar function
> Do you think we need to add coverage for native functions here? There's a i
This change is adding the catalog service identifiers to the DB irrespective of weather it is a persistent java function or not. In case of persistent java functions, hms creates a CREATE_FUNCTION event which we do not support. We ignore such events anyway.


http://gerrit.cloudera.org:8080/#/c/14296/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@580
PS2, Line 580:     eventsProcessor_.processEvents();
> Need to call eventsProcessor_.processEvents() before this?
Good catch. Done.


http://gerrit.cloudera.org:8080/#/c/14296/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@2336
PS2, Line 2336: 
> Typo, impala
Oops!



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 3
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 26 Sep 2019 18:54:27 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

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

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/4667/ : 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/14296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 6
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 27 Sep 2019 18:54:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

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

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................


Patch Set 7:

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5023/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 7
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 28 Sep 2019 23:36:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

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

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................


Patch Set 7:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/4669/ : 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/14296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 7
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 27 Sep 2019 23:11:53 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

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

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14296/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/14296/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1231
PS1, Line 1231:         } else {
> line too long (97 > 90)
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 2
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Sep 2019 23:03:42 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/14296 )

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................

IMPALA-8968: Alter database events on dropped database should not put
events processor in error state.

This change is two-fold:
1. If an alter database event is received on database that does not
   exist, the event can be safely ignored. The events processor should
   only go into an error state if updateDb() fails.

2. This change also adds catalog service identifiers to create/drop
   function operations as Impala generates alter database events
   with these operations and they should be detected as self-events
   and ignored.

Testing:
Add tests to verify both the above changes to
MetastoreEventsProcessorTest.

Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
4 files changed, 110 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/14296/6
-- 
To view, visit http://gerrit.cloudera.org:8080/14296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 6
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

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

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/4655/ : 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/14296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 3
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 26 Sep 2019 19:47:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/14296 )

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................

IMPALA-8968: Alter database events on dropped database should not put
events processor in error state.

This change is two-fold:
1. If an alter database event is received on database that does not
   exist, the event can be safely ignored. The events processor should
   only go into an error state if updateDb() fails.

2. This change also adds catalog service identifiers to create/drop
   function operations as Impala generates alter database events
   with these operations and they should be detected as self-events
   and ignored.

Testing:
Add tests to verify both the above changes to
MetastoreEventsProcessorTest.

Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
4 files changed, 110 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/14296/5
-- 
To view, visit http://gerrit.cloudera.org:8080/14296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 5
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/14296 )

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................

IMPALA-8968: Alter database events on dropped database should not put
events processor in error state.

This change is two-fold:
1. If an alter database event is received on database that does not
   exist, the event can be safely ignored. The events processor should
   only go into an error state if updateDb() fails.

2. This change also adds catalog service identifiers to create/drop
   function operations as Impala generates alter database events
   with these operations and they should be detected as self-events
   and ignored.

Testing:
Add tests to verify both the above changes to
MetastoreEventsProcessorTest.

Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
4 files changed, 111 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/14296/4
-- 
To view, visit http://gerrit.cloudera.org:8080/14296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 4
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

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

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/4635/ : 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/14296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 2
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Sep 2019 23:44:09 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

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

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14296/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/14296/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1231
PS1, Line 1231:           infoLog("Database {} updated after alter database event.", alteredDatabase_.getName());
line too long (97 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 1
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Sep 2019 22:59:00 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

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

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/4666/ : 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/14296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 5
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 27 Sep 2019 18:52:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

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

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/4634/ : 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/14296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 1
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Sep 2019 23:39:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/14296 )

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................

IMPALA-8968: Alter database events on dropped database should not put
events processor in error state.

This change is two-fold:
1. If an alter database event is received on database that does not
   exist, the event can be safely ignored. The events processor should
   only go into an error state if updateDb() fails.

2. This change also adds catalog service identifiers to create/drop
   function operations as Impala generates alter database events
   with these operations and they should be detected as self-events
   and ignored.

Testing:
Add tests to verify both the above changes to
MetastoreEventsProcessorTest.

Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
4 files changed, 117 insertions(+), 2 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 2
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

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

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................


Patch Set 6:

(3 comments)

Still have one more question and a nit

http://gerrit.cloudera.org:8080/#/c/14296/6/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/14296/6/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1226
PS6, Line 1226:   
nit: wrong indent here


http://gerrit.cloudera.org:8080/#/c/14296/6/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/14296/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1313
PS6, Line 1313:           catalog_.addVersionsForInflightEvents(db, newCatalogVersion);
Could you explain why we don't need this for persistent java functions? In previous patch versions, it's in the if-branch below.


http://gerrit.cloudera.org:8080/#/c/14296/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2017
PS6, Line 2017:           catalog_.addVersionsForInflightEvents(db, newCatalogVersion);
same here.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 6
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 27 Sep 2019 19:00:20 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

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

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................


Patch Set 2:

(2 comments)

Just have some comments in the tests

http://gerrit.cloudera.org:8080/#/c/14296/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
File fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java:

http://gerrit.cloudera.org:8080/#/c/14296/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@557
PS2, Line 557: scalar function
Do you think we need to add coverage for native functions here? There's a if-branch inside CatalogOpExecutor.createFunction() checking isPersistentJavaFn. Maybe we need to cover the else-clause of it.


http://gerrit.cloudera.org:8080/#/c/14296/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@580
PS2, Line 580:     assertEquals(EventProcessorStatus.ACTIVE, eventsProcessor_.getStatus());
Need to call eventsProcessor_.processEvents() before this?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 2
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 25 Sep 2019 22:12:19 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

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

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................

IMPALA-8968: Alter database events on dropped database should not put
events processor in error state.

This change is two-fold:
1. If an alter database event is received on database that does not
   exist, the event can be safely ignored. The events processor should
   only go into an error state if updateDb() fails.

2. This change also adds catalog service identifiers to create/drop
   function operations as Impala generates alter database events
   with these operations and they should be detected as self-events
   and ignored.

Testing:
Add tests to verify both the above changes to
MetastoreEventsProcessorTest.

Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Reviewed-on: http://gerrit.cloudera.org:8080/14296
Reviewed-by: Quanlong Huang <hu...@gmail.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
4 files changed, 110 insertions(+), 2 deletions(-)

Approvals:
  Quanlong Huang: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 8
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

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

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................


Patch Set 7: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5019/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 7
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 28 Sep 2019 03:24:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8968: Alter database events on dropped database should not put events processor in error state.

Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has removed Vihang Karajgaonkar from this change.  ( http://gerrit.cloudera.org:8080/14296 )

Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state.
......................................................................


Removed reviewer Vihang Karajgaonkar.
-- 
To view, visit http://gerrit.cloudera.org:8080/14296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced
Gerrit-Change-Number: 14296
Gerrit-PatchSet: 2
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>