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/02/26 21:26:23 UTC
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Anurag Mantripragada has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12601
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
IMPALA-8240: Event processor should keep trying when metastore
is unavailable.
When metastore is unavailable, EventProcessor state is changed
to ERROR when there is a MetastoreFetchNotificationException.
After the change, the exception handler will not change the state and
EventProcessor continues trying when metastore is unavailable.
Testing:
Added test in MetastoreEventProcessorTest to check event processor
state is active even after multiple NotificationFetchExceptions
Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
---
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
2 files changed, 16 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/12601/1
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 1
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12601/5/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/12601/5/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@556
PS5, Line 556: // Roughly half of the time an exception is thrown. Make sure the event processor
: // is still active.
: for(int i = 0 ; i < 11 ; i++) {
: fetchProcessor.processEvents();
: assertEquals(EventProcessorStatus.ACTIVE, fetchProcessor.getStatus());
: }
> I'm sorry, not sure what you mean here. The test class makes sure that exce
right. But since the exception is thrown at random, it is possible that the above loop can finish without a single exception being thrown. Essentially the test is not doing what is meant to do.
What I meant above is to loop until an exception is thrown, break out of loop and assert that the status is still ACTIVE. Did I get something wrong?
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 5
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 00:33:52 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/12601/3/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java
File fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java:
http://gerrit.cloudera.org:8080/#/c/12601/3/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@25
PS3, Line 25: public class HMSFetchNotificationsEventProcessorForTests
java doc explaining what it does would be great
http://gerrit.cloudera.org:8080/#/c/12601/3/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@40
PS3, Line 40: else {
format to inline with }
http://gerrit.cloudera.org:8080/#/c/12601/3/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/12601/3/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@531
PS3, Line 531: fetchProcessor_ = new HMSFetchNotificationsEventProcessorForTests(
Since this is used only this method, you don't need to declare the variable at class level. So something like below if cleaner. Note that you can get the currentNotificationId from eventsProcessor_ too. So you don't need to make it class level variable as well. Also, setting it to retries to 10 may take more time without any added benefit. Setting it to 2-3 is good enough.
MetastoreEventsProcessor fetchProcessor = new HMSFetchNotificationsEventProcessorForTests(catalog_, eventsProcessor_.getCurrentEventId(), 2L);
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 3
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Feb 2019 23:29:55 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
IMPALA-8240: Event processor should keep trying when metastore
is unavailable.
When metastore is unavailable, EventProcessor state is changed
to ERROR when there is a MetastoreFetchNotificationException.
After this change, the exception handler will not change the state and
EventProcessor continues trying when metastore is unavailable.
Testing:
Added test in MetastoreEventProcessorTest to check event processor
state is active even after multiple NotificationFetchExceptions
Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
---
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
2 files changed, 52 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/12601/6
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 6
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 4:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/12601/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java:
http://gerrit.cloudera.org:8080/#/c/12601/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@328
PS1, Line 328: // No need to change the EventProcessor state to error since we want the
> Can you make the error log state that the metastore might be unavailable, a
Done
http://gerrit.cloudera.org:8080/#/c/12601/3/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java
File fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java:
http://gerrit.cloudera.org:8080/#/c/12601/3/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@25
PS3, Line 25:
> java doc explaining what it does would be great
Done
http://gerrit.cloudera.org:8080/#/c/12601/3/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@40
PS3, Line 40: throws MetastoreNotificationFetchException {
> format to inline with }
Done
http://gerrit.cloudera.org:8080/#/c/12601/3/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/12601/3/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@531
PS3, Line 531: catalog_, eventsProcessor_.getCurrentEventId(), 2L);
> Since this is used only this method, you don't need to declare the variable
Done
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 4
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Feb 2019 23:54:13 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 8: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 8
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 01:21:34 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 9:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3851/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 9
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 22:28:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
IMPALA-8240: Event processor should keep trying when metastore
is unavailable.
When metastore is unavailable, EventProcessor state is changed
to ERROR when there is a MetastoreFetchNotificationException.
After this change, the exception handler will not change the state and
EventProcessor continues trying when metastore is unavailable.
Testing:
Added test in MetastoreEventProcessorTest to check event processor
state is active even after multiple NotificationFetchExceptions
Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
---
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
2 files changed, 45 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/12601/5
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 5
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 6: Code-Review+2
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12601/6/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/12601/6/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@38
PS6, Line 38: import org.apache.calcite.avatica.Meta;
remove
http://gerrit.cloudera.org:8080/#/c/12601/6/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@542
PS6, Line 542: (r
nit: space if (
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 6
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 01:12:09 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 5:
(18 comments)
http://gerrit.cloudera.org:8080/#/c/12601/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12601/4//COMMIT_MSG@12
PS4, Line 12: this
> typo
Done
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java:
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@327
PS4, Line 327: ex) {
> nit: can be shortened to ex
Done
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@328
PS4, Line 328: // No need to change the EventProcessor state to error since we want the
> How many times does it retry until it becomes unsuccessful? Will HMS throw
There is no bound on number of retries. It will keep retrying
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java
File fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java:
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@25
PS4, Line 25:
> nit: extra \n
Done
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@27
PS4, Line 27:
:
:
> I'd rephrase this to something like, events processor that simulates HMS fa
Done
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@30
PS4, Line 30:
> I don't think this warrants a separate class. A private static inner class
Done
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@31
PS4, Line 31:
> indentation off
Done
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@36
PS4, Line 36:
> nit: rename to counter_ and move this before constructor.
Done
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@36
PS4, Line 36:
> class members use _ suffix. Move to the top. Add a comment
Done
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@42
PS4, Line 42:
> This looks weird. How about throw an error randomly 50% of the times?
Done
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@42
PS4, Line 42:
:
:
> This code is also not generic enough. If the goal of this class to be reusa
Done
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@45
PS4, Line 45:
> nit: else isn't necessary after a throw or return.
Done
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@45
PS4, Line 45:
> not needed.
Done
http://gerrit.cloudera.org:8080/#/c/12601/4/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/12601/4/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@528
PS4, Line 528:
> nit: space before {
Done
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@529
PS4, Line 529:
> nit: we usually don't use _ for non-member variables.
Done
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@529
PS4, Line 529: Processor {
> local variables don't need a suffix _
Done
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@536
PS4, Line 536: storeNotificationFetchExcep
> Should we also have a test where EventProessorState becomes ERROR?
For FetchExceptions, it will never become ERROR since the EventProcessor needs to trying.
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@534
PS4, Line 534: @Override
: public List<NotificationEvent> getNextMetastoreEvents()
: throws MetastoreNotificationFetchException {
:
> it may not be obvious that when we process the events 3 times, we'll get an
Moved to Private Static class. Also changed the code to produce exception roughly half the time.
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 5
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Feb 2019 19:54:34 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 8: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3848/
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 8
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 05:25:34 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12601/5/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/12601/5/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@556
PS5, Line 556: // Roughly half of the time an exception is thrown. Make sure the event processor
: // is still active.
: for(int i = 0 ; i < 11 ; i++) {
: fetchProcessor.processEvents();
: assertEquals(EventProcessorStatus.ACTIVE, fetchProcessor.getStatus());
: }
> Looks like this can even pass when no exceptions are thrown. I think we sho
I'm sorry, not sure what you mean here. The test class makes sure that exceptions are thrown roughly half of the times this method is called..This is actually an overkill. I want to test the status has not changed to ERROR after exceptions. This was the behavior before the change - Even a single exception would change the status to ERROR which would prevent the EventProcessor to run the next time.
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 5
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 00:20:38 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 4:
(9 comments)
Bunch of nits.
http://gerrit.cloudera.org:8080/#/c/12601/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12601/4//COMMIT_MSG@12
PS4, Line 12: thise
typo
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java
File fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java:
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@25
PS4, Line 25:
nit: extra \n
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@27
PS4, Line 27: * A MetastoreEventsProcessor test class useful for testing impacts of
: * HMS crashes/restarts on MetastoreEventsProcessor status.
: */
I'd rephrase this to something like, events processor that simulates HMS failures...
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@30
PS4, Line 30: ForTests
remove.
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@31
PS4, Line 31: extends MetastoreEventsProcessor {
indentation off
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@36
PS4, Line 36: private int counter = 0;
class members use _ suffix. Move to the top. Add a comment
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@42
PS4, Line 42: counter < 3
This looks weird. How about throw an error randomly 50% of the times?
if (rand.nextInt() % 2) throw
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@45
PS4, Line 45: else {
not needed.
http://gerrit.cloudera.org:8080/#/c/12601/4/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/12601/4/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@529
PS4, Line 529: fetchProcessor_
local variables don't need a suffix _
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 4
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Feb 2019 03:20:18 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 1:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/2260/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 1
Gerrit-Owner: Anurag Mantripragada <an...@gmail.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, 26 Feb 2019 21:59:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Zoram Thanga (Code Review)" <ge...@cloudera.org>.
Zoram Thanga has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12601/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java:
http://gerrit.cloudera.org:8080/#/c/12601/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@328
PS1, Line 328: LOG.error("Unable to fetch the next batch of metastore events", fetchEx);
Can you make the error log state that the metastore might be unavailable, and that we're going to keep retrying?
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 1
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Feb 2019 22:45:15 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 1:
(2 comments)
Thanks for the quick turnaround on this. Overall looks good to me. Couple of minor comments.
http://gerrit.cloudera.org:8080/#/c/12601/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java:
http://gerrit.cloudera.org:8080/#/c/12601/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@a328
PS1, Line 328:
Can you add a comment here saying why we don't need to change the event processor status to ERROR?
http://gerrit.cloudera.org:8080/#/c/12601/1/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/12601/1/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@125
PS1, Line 125: fetchProcessor_ = new HMSFetchNotificationsEventProcessorForTests(
this doesn't need to be at class level since its only used by one test case. May be move this to the testEventProcessorFetchAfterHMSRestart method itself as a local variable?
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 1
Gerrit-Owner: Anurag Mantripragada <an...@gmail.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, 26 Feb 2019 21:30:44 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12601/6/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/12601/6/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@38
PS6, Line 38: import org.apache.calcite.avatica.Meta;
nit, unused import. RRest looks good to me.
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 6
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 01:12:31 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 8:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2291/ : 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/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 8
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 02:01:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2264/ : 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/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 4
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Feb 2019 00:25:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 5:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12601/5/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/12601/5/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@534
PS5, Line 534: @Override
nit: newline
http://gerrit.cloudera.org:8080/#/c/12601/5/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@556
PS5, Line 556: // Roughly half of the time an exception is thrown. Make sure the event processor
: // is still active.
: for(int i = 0 ; i < 11 ; i++) {
: fetchProcessor.processEvents();
: assertEquals(EventProcessorStatus.ACTIVE, fetchProcessor.getStatus());
: }
Looks like this can even pass when no exceptions are thrown. I think we should do something like,
while (exceptionNotThrown) {
fetchProcessor.processEvents();
}
assertEquals(ACTIVE, fetchProcessor.status());
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 5
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Feb 2019 21:57:22 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2261/ : 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/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 2
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Feb 2019 23:44:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2276/ : 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/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 5
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Feb 2019 20:38:52 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
IMPALA-8240: Event processor should keep trying when metastore
is unavailable.
When metastore is unavailable, EventProcessor state is changed
to ERROR when there is a MetastoreFetchNotificationException.
After this change, the exception handler will not change the state and
EventProcessor continues trying when metastore is unavailable.
Testing:
Added test in MetastoreEventProcessorTest to check event processor
state is active even after multiple NotificationFetchExceptions
Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Reviewed-on: http://gerrit.cloudera.org:8080/12601
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
2 files changed, 50 insertions(+), 3 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 10
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2262/ : 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/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 3
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Feb 2019 23:51:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
IMPALA-8240: Event processor should keep trying when metastore
is unavailable.
When metastore is unavailable, EventProcessor state is changed
to ERROR when there is a MetastoreFetchNotificationException.
After thise change, the exception handler will not change the state and
EventProcessor continues trying when metastore is unavailable.
Testing:
Added test in MetastoreEventProcessorTest to check event processor
state is active even after multiple NotificationFetchExceptions
Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
---
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
A fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
3 files changed, 65 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/12601/2
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 2
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
IMPALA-8240: Event processor should keep trying when metastore
is unavailable.
When metastore is unavailable, EventProcessor state is changed
to ERROR when there is a MetastoreFetchNotificationException.
After thise change, the exception handler will not change the state and
EventProcessor continues trying when metastore is unavailable.
Testing:
Added test in MetastoreEventProcessorTest to check event processor
state is active even after multiple NotificationFetchExceptions
Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
---
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
A fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
3 files changed, 66 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/12601/3
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 3
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/12601/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java:
http://gerrit.cloudera.org:8080/#/c/12601/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@328
PS2, Line 328: // No need to change the EventProcessor state to error since we want the EventProcessor
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/12601/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@328
PS2, Line 328: // No need to change the EventProcessor state to error since we want the EventProcessor
line too long (94 > 90)
http://gerrit.cloudera.org:8080/#/c/12601/2/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java
File fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java:
http://gerrit.cloudera.org:8080/#/c/12601/2/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@25
PS2, Line 25: public class HMSFetchNotificationsEventProcessorForTests extends MetastoreEventsProcessor {
line too long (91 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 2
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Feb 2019 23:13:24 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12601/5/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/12601/5/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@556
PS5, Line 556: // Roughly half of the time an exception is thrown. Make sure the event processor
: // is still active.
: for(int i = 0 ; i < 11 ; i++) {
: fetchProcessor.processEvents();
: assertEquals(EventProcessorStatus.ACTIVE, fetchProcessor.getStatus());
: }
> I'm sorry, not sure what you mean here. The test class makes sure that exce
I think I see your point here. I will make the change. Thanks for the comments.
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 5
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 00:31:42 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 5:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12601/5/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/12601/5/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@534
PS5, Line 534: @Override
> nit: newline
Done
http://gerrit.cloudera.org:8080/#/c/12601/5/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@556
PS5, Line 556: // Roughly half of the time an exception is thrown. Make sure the event processor
: // is still active.
: for(int i = 0 ; i < 11 ; i++) {
: fetchProcessor.processEvents();
: assertEquals(EventProcessorStatus.ACTIVE, fetchProcessor.getStatus());
: }
> right. But since the exception is thrown at random, it is possible that the
Understood. Thanks for clarifying.
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 5
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 00:37:01 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 8:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/12601/6/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/12601/6/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@38
PS6, Line 38: import org.apache.hadoop.hive.metastore.api.CurrentNotificationEventId;
> nit, unused import. RRest looks good to me.
Done
http://gerrit.cloudera.org:8080/#/c/12601/6/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@38
PS6, Line 38: import org.apache.hadoop.hive.metastore
> remove
Done
http://gerrit.cloudera.org:8080/#/c/12601/6/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@542
PS6, Line 542: T
> nit: space if (
Done
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 8
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 01:18:56 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
IMPALA-8240: Event processor should keep trying when metastore
is unavailable.
When metastore is unavailable, EventProcessor state is changed
to ERROR when there is a MetastoreFetchNotificationException.
After thise change, the exception handler will not change the state and
EventProcessor continues trying when metastore is unavailable.
Testing:
Added test in MetastoreEventProcessorTest to check event processor
state is active even after multiple NotificationFetchExceptions
Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
---
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
A fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
3 files changed, 67 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/12601/4
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 4
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 8:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3848/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 8
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 01:21:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
IMPALA-8240: Event processor should keep trying when metastore
is unavailable.
When metastore is unavailable, EventProcessor state is changed
to ERROR when there is a MetastoreFetchNotificationException.
After this change, the exception handler will not change the state and
EventProcessor continues trying when metastore is unavailable.
Testing:
Added test in MetastoreEventProcessorTest to check event processor
state is active even after multiple NotificationFetchExceptions
Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
---
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
2 files changed, 50 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/12601/8
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 8
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 4:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java:
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@327
PS4, Line 327: fetchEx
nit: can be shortened to ex
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@328
PS4, Line 328: // No need to change the EventProcessor state to error since we want the
How many times does it retry until it becomes unsuccessful? Will HMS throw a different exception if it has used all the retries?
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java
File fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java:
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@30
PS4, Line 30: public class HMSFetchNotificationsEventProcessorForTests
I don't think this warrants a separate class. A private static inner class inside MetastoreEventsProcessorTest should be sufficient.
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@36
PS4, Line 36: private int counter = 0;
nit: rename to counter_ and move this before constructor.
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@42
PS4, Line 42: if(counter < 3){
: throw new MetastoreNotificationFetchException("Fetch Exception");
: }
This code is also not generic enough. If the goal of this class to be reusable, maybe we should make it more generic specify how many times before it throws an exception?
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/HMSFetchNotificationsEventProcessorForTests.java@45
PS4, Line 45: else
nit: else isn't necessary after a throw or return.
http://gerrit.cloudera.org:8080/#/c/12601/4/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/12601/4/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@528
PS4, Line 528: {
nit: space before {
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@529
PS4, Line 529: _
nit: we usually don't use _ for non-member variables.
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@536
PS4, Line 536: EventProcessorStatus.ACTIVE
Should we also have a test where EventProessorState becomes ERROR?
http://gerrit.cloudera.org:8080/#/c/12601/4/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@534
PS4, Line 534: for(int i = 0 ; i < 4 ; i++) {
: fetchProcessor_.processEvents();
: assertEquals(EventProcessorStatus.ACTIVE, fetchProcessor_.getStatus());
: }
it may not be obvious that when we process the events 3 times, we'll get an exception. Maybe we should move the dummy MetastoreEventsProcesor into a private static class instead of a separate file.
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 4
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Feb 2019 03:25:50 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 9: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 9
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 22:28:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 9: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 9
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Mar 2019 02:52:30 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2288/ : 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/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 6
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 01:27:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8240: Event processor should keep trying when metastore is unavailable.
Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/12601 )
Change subject: IMPALA-8240: Event processor should keep trying when metastore is unavailable.
......................................................................
Patch Set 4: Code-Review+1
Thanks for the changes. Looks good to me. You will need a committer +2 to get this patch merged.
--
To view, visit http://gerrit.cloudera.org:8080/12601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I570a52462e6d3b634b2c227dfcb98e20ad2a0023
Gerrit-Change-Number: 12601
Gerrit-PatchSet: 4
Gerrit-Owner: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Feb 2019 00:48:36 +0000
Gerrit-HasComments: No