You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Quanlong Huang (Code Review)" <ge...@cloudera.org> on 2023/11/15 10:50:07 UTC
[Impala-ASF-CR] IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Quanlong Huang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20707
Change subject: IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
......................................................................
IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Any failures in fetching HMS events should be retriable. Event-processor
should not go into the ERROR state which can only be recovered by a
global INVALIDATE METADATA command.
This patch deals with the failure when creating a new MetaStoreClient
and throws a MetastoreClientInstantiationException instead of an
IllegalStateException. Callers can catch the exception and convert it
into MetastoreNotificationFetchException which won't fail the
event-processor.
Also makes sure MetastoreEventsProcessor.getCurrentEventId() only throws
MetastoreNotificationFetchException. Previously it throws
CatalogException which will fail the event-processor. Note that
CatalogException is used for errors in accessing objects in the Catalog,
e.g. table not found. We shouldn't throw it when fetching HMS events
fails.
Tests:
- Add test in custom_cluster/test_catalog_hms_failures.py. The test
class previously only runs in exhaustive jobs due to long running
time. Optimize the test to only restart HMS. Adds a new option,
-if_not_running, for run-hive-server.sh to avoid unneccessary
restarts.
Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
A fe/src/main/java/org/apache/impala/catalog/MetastoreClientInstantiationException.java
M fe/src/main/java/org/apache/impala/catalog/events/ExternalEventsProcessor.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
M testdata/bin/run-hive-server.sh
M tests/custom_cluster/test_catalog_hms_failures.py
8 files changed, 118 insertions(+), 44 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/20707/1
--
To view, visit http://gerrit.cloudera.org:8080/20707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Gerrit-Change-Number: 20707
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20707 )
Change subject: IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14823/ : 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/20707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Gerrit-Change-Number: 20707
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Dec 2023 14:21:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20707 )
Change subject: IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10096/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/20707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Gerrit-Change-Number: 20707
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Sat, 23 Dec 2023 12:32:10 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/20707 )
Change subject: IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
......................................................................
Patch Set 3: Code-Review+1
(1 comment)
> Patch Set 3:
>
> (1 comment)
http://gerrit.cloudera.org:8080/#/c/20707/3/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/20707/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1122
PS3, Line 1122: event.processIfEnabled();
> I think it depends on where the failure happens. If the table metadata is h
I agree with Quanlong's suggestion. +1
--
To view, visit http://gerrit.cloudera.org:8080/20707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Gerrit-Change-Number: 20707
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Mon, 27 Nov 2023 23:29:56 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20707 )
Change subject: IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20707/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/20707/4/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@3391
PS4, Line 3391: testHMSClientFailureInFetchingEvents
> nit: Should there be another FE test for getNextMetastoreEventsInBatches()
Done
--
To view, visit http://gerrit.cloudera.org:8080/20707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Gerrit-Change-Number: 20707
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Sat, 23 Dec 2023 01:52:16 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20707 )
Change subject: IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14460/ : 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/20707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Gerrit-Change-Number: 20707
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Nov 2023 03:22:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20707 )
Change subject: IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20707/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20707/3//COMMIT_MSG@15
PS3, Line 15: Callers can catch the exception and convert it
: into MetastoreNotificationFetchException which won't fail the
: event-processor.
> nit: There are several places where getMetastoreClient() is called. This pa
Good point. Previously we just looked into the codes of different event types. In MetastoreEventsProcessor we do need to catch the new exception in some other places.
Added codes to catch the exception in getNextMetastoreEventsInBatches() and getEventTimeFromHMS().
getNextMetastoreEventsInBatches() already throws MetastoreNotificationFetchException (but not all its callers handle that). We can convert MetastoreClientInstantiationException to MetastoreNotificationFetchException.
getEventTimeFromHMS() is introduced by a recent patch. It should catch MetastoreClientInstantiationException since it's used in the main loop of event-processor (i.e. in processEvents()) when fetching events.
--
To view, visit http://gerrit.cloudera.org:8080/20707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Gerrit-Change-Number: 20707
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Dec 2023 13:53:57 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20707 )
Change subject: IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
......................................................................
Patch Set 3:
(1 comment)
This patch looks good to me. I just have 1 question.
http://gerrit.cloudera.org:8080/#/c/20707/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20707/3//COMMIT_MSG@15
PS3, Line 15: Callers can catch the exception and convert it
: into MetastoreNotificationFetchException which won't fail the
: event-processor.
nit: There are several places where getMetastoreClient() is called. This patch, however, only catch MetastoreClientInstantiationException in 2 out of 6 call site at MetastoreEventProcessor.java. Why not catch it in getNextMetastoreEventsInBatches() as well?
From the other comment thread, I assume it just a matter of priority now and other call site should be investigated if they are retriable as well? If that is true, do you mind mentioning in commit message why only catch MetastoreClientInstantiationException in this two place for now?
--
To view, visit http://gerrit.cloudera.org:8080/20707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Gerrit-Change-Number: 20707
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Dec 2023 05:46:26 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20707 )
Change subject: IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20707/3/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/20707/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1122
PS3, Line 1122: event.processIfEnabled();
> How about catching MetastoreClientInstantiationException in MetastoreEvents
I think it depends on where the failure happens. If the table metadata is half updated and become corrupt, the retry might fail. That's also the reason we throw MetastoreNotificationNeedsInvalidateException in many places. I think we are conservative. There should be some failure points that are retriable. It needs more time to go through all event code paths and requires a larger change.
As a hot fix, maybe the current patch + bumping HMS client retry configs is simpler.
--
To view, visit http://gerrit.cloudera.org:8080/20707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Gerrit-Change-Number: 20707
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Tue, 21 Nov 2023 01:15:34 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20707 )
Change subject: IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14449/ : 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/20707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Gerrit-Change-Number: 20707
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Wed, 15 Nov 2023 11:18:21 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
k.venureddy2103@gmail.com has posted comments on this change. ( http://gerrit.cloudera.org:8080/20707 )
Change subject: IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20707/3/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/20707/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1122
PS3, Line 1122: event.processIfEnabled();
catalog_.getMetaStoreClient() throws MetastoreClientInstantiationException now and it is an unchecked exception. Certain events like addpartition, alterpartition, altertable, committxn etc make calls to HMS during the respective event processing(i.e., they do getMetaStoreClient() in the event.process() call). If getMetaStoreClient() the exception during that time, MetastoreClientInstantiationException is not caught and it propagted all the way till MetastoreEventsProcessor#processEvents() and we update eventProcessorStatus_ with EventProcessorStatus.ERROR.
--
To view, visit http://gerrit.cloudera.org:8080/20707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Gerrit-Change-Number: 20707
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Nov 2023 11:18:18 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20707 )
Change subject: IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14832/ : 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/20707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Gerrit-Change-Number: 20707
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Sat, 23 Dec 2023 02:19:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, k.venureddy2103@gmail.com, Sai Hemanth Gantasala, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20707
to look at the new patch set (#4).
Change subject: IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
......................................................................
IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Any failures in fetching HMS events should be retriable. Event-processor
should not go into the ERROR state which can only be recovered by a
global INVALIDATE METADATA command.
This patch deals with the failure in creating a new MetaStoreClient
by throwing a MetastoreClientInstantiationException instead of an
IllegalStateException. Previously the IllegalStateException could fail
the process of fetching HMS events. Now callers can catch the
MetastoreClientInstantiationException and convert it into
MetastoreNotificationFetchException if the process is retriable. So the
event-processor can retry in the next round. There are still other
callers of Catalog#getMetaStoreClient() that don't catch the new
exception since their work can't be easily retried.
Also makes sure MetastoreEventsProcessor.getCurrentEventId() only throws
MetastoreNotificationFetchException. Previously it throws
CatalogException which will fail the event-processor. Note that
CatalogException is used for errors in accessing objects in the Catalog,
e.g. table not found. We shouldn't throw it when fetching HMS events
fails.
Tests:
- Add FE unit test to verify MetastoreNotificationFetchException is
thrown as expected. To mimic HMS connection failures, use a
customized MetastoreClientPool that uses wrong HMS port.
- Add e2e test in custom_cluster/test_catalog_hms_failures.py. The test
class previously only runs in exhaustive jobs due to long running
time. Optimize the test to only restart HMS. Adds a new option,
-if_not_running, for run-hive-server.sh to avoid unneccessary
restarts.
Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
---
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
A fe/src/main/java/org/apache/impala/catalog/MetastoreClientInstantiationException.java
M fe/src/main/java/org/apache/impala/catalog/events/ExternalEventsProcessor.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
A fe/src/test/java/org/apache/impala/testutil/IncompetentMetastoreClientPool.java
M testdata/bin/run-hive-server.sh
M tests/custom_cluster/test_catalog_hms_failures.py
10 files changed, 213 insertions(+), 46 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/20707/4
--
To view, visit http://gerrit.cloudera.org:8080/20707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Gerrit-Change-Number: 20707
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
[Impala-ASF-CR] IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20707 )
Change subject: IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20707/3/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/20707/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1122
PS3, Line 1122: event.processIfEnabled();
> catalog_.getMetaStoreClient() throws MetastoreClientInstantiationException
Yeah, that's a concern. Our goal is to survive in intermittent HMS errors. It could also happen in processing events. Note that previously catalog_.getMetaStoreClient() throws IllegalStateException which is also an unchecked exception. So the situation is not worse.
The current patch set mainly deals with failures in fetching events. Agree that there are some retriable steps in processing events. But it requires a more complex change to catch MetastoreClientInstantiationException in those places and add appropriate retry codes.
Do you think bumping the Hive configs of hive.metastore.connect.retries and hive.metastore.client.connect.retry.delay is enough? We might need to tolerate a failure duration of 5 minutes.
--
To view, visit http://gerrit.cloudera.org:8080/20707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Gerrit-Change-Number: 20707
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Nov 2023 08:53:08 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello k.venureddy2103@gmail.com, Sai Hemanth Gantasala, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20707
to look at the new patch set (#2).
Change subject: IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
......................................................................
IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Any failures in fetching HMS events should be retriable. Event-processor
should not go into the ERROR state which can only be recovered by a
global INVALIDATE METADATA command.
This patch deals with the failure when creating a new MetaStoreClient
and throws a MetastoreClientInstantiationException instead of an
IllegalStateException. Callers can catch the exception and convert it
into MetastoreNotificationFetchException which won't fail the
event-processor.
Also makes sure MetastoreEventsProcessor.getCurrentEventId() only throws
MetastoreNotificationFetchException. Previously it throws
CatalogException which will fail the event-processor. Note that
CatalogException is used for errors in accessing objects in the Catalog,
e.g. table not found. We shouldn't throw it when fetching HMS events
fails.
Tests:
- Add FE unit test to verify MetastoreNotificationFetchException is
thrown as expected. To mimic HMS connection failures, use a
customized MetastoreClientPool that uses wrong HMS port.
- Add e2e test in custom_cluster/test_catalog_hms_failures.py. The test
class previously only runs in exhaustive jobs due to long running
time. Optimize the test to only restart HMS. Adds a new option,
-if_not_running, for run-hive-server.sh to avoid unneccessary
restarts.
Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
---
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
A fe/src/main/java/org/apache/impala/catalog/MetastoreClientInstantiationException.java
M fe/src/main/java/org/apache/impala/catalog/events/ExternalEventsProcessor.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
A fe/src/test/java/org/apache/impala/testutil/IncompetentMetastoreClientPool.java
M testdata/bin/run-hive-server.sh
M tests/custom_cluster/test_catalog_hms_failures.py
10 files changed, 210 insertions(+), 45 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/20707/2
--
To view, visit http://gerrit.cloudera.org:8080/20707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Gerrit-Change-Number: 20707
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
[Impala-ASF-CR] IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
k.venureddy2103@gmail.com has posted comments on this change. ( http://gerrit.cloudera.org:8080/20707 )
Change subject: IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20707/3/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/20707/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1122
PS3, Line 1122: event.processIfEnabled();
> Yeah, that's a concern. Our goal is to survive in intermittent HMS errors.
How about catching MetastoreClientInstantiationException in MetastoreEventsProcessor#processEvents() and logging it? We will be able to avoid updating eventProcessorStatus_ to EventProcessorStatus.ERROR. Anyway, in the subsequent scheduled runs of processEvents() when hms becomes available again, it starts to process from the last failed event.
--
To view, visit http://gerrit.cloudera.org:8080/20707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Gerrit-Change-Number: 20707
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Nov 2023 16:34:16 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20707 )
Change subject: IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20707/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/20707/4/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@3391
PS4, Line 3391: testHMSClientFailureInFetchingEvents
nit: Should there be another FE test for getNextMetastoreEventsInBatches() and getEventTimeFromHMS()?
--
To view, visit http://gerrit.cloudera.org:8080/20707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Gerrit-Change-Number: 20707
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Dec 2023 15:41:56 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20707 )
Change subject: IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/20707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Gerrit-Change-Number: 20707
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Sat, 23 Dec 2023 17:02:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20707 )
Change subject: IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20707/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/20707/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@3351
PS2, Line 3351: * Test getNextMetastoreEvents() throws MetastoreNotificationFetchException when there are
line too long (92 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/20707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Gerrit-Change-Number: 20707
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Nov 2023 02:55:13 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20707 )
Change subject: IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14461/ : 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/20707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Gerrit-Change-Number: 20707
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Nov 2023 04:37:07 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello k.venureddy2103@gmail.com, Sai Hemanth Gantasala, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20707
to look at the new patch set (#3).
Change subject: IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
......................................................................
IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Any failures in fetching HMS events should be retriable. Event-processor
should not go into the ERROR state which can only be recovered by a
global INVALIDATE METADATA command.
This patch deals with the failure when creating a new MetaStoreClient
and throws a MetastoreClientInstantiationException instead of an
IllegalStateException. Callers can catch the exception and convert it
into MetastoreNotificationFetchException which won't fail the
event-processor.
Also makes sure MetastoreEventsProcessor.getCurrentEventId() only throws
MetastoreNotificationFetchException. Previously it throws
CatalogException which will fail the event-processor. Note that
CatalogException is used for errors in accessing objects in the Catalog,
e.g. table not found. We shouldn't throw it when fetching HMS events
fails.
Tests:
- Add FE unit test to verify MetastoreNotificationFetchException is
thrown as expected. To mimic HMS connection failures, use a
customized MetastoreClientPool that uses wrong HMS port.
- Add e2e test in custom_cluster/test_catalog_hms_failures.py. The test
class previously only runs in exhaustive jobs due to long running
time. Optimize the test to only restart HMS. Adds a new option,
-if_not_running, for run-hive-server.sh to avoid unneccessary
restarts.
Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
---
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
A fe/src/main/java/org/apache/impala/catalog/MetastoreClientInstantiationException.java
M fe/src/main/java/org/apache/impala/catalog/events/ExternalEventsProcessor.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
A fe/src/test/java/org/apache/impala/testutil/IncompetentMetastoreClientPool.java
M testdata/bin/run-hive-server.sh
M tests/custom_cluster/test_catalog_hms_failures.py
10 files changed, 210 insertions(+), 45 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/20707/3
--
To view, visit http://gerrit.cloudera.org:8080/20707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Gerrit-Change-Number: 20707
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
[Impala-ASF-CR] IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
k.venureddy2103@gmail.com has posted comments on this change. ( http://gerrit.cloudera.org:8080/20707 )
Change subject: IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
......................................................................
Patch Set 3: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20707/3/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/20707/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1122
PS3, Line 1122: event.processIfEnabled();
> I think it depends on where the failure happens. If the table metadata is h
makes sense
--
To view, visit http://gerrit.cloudera.org:8080/20707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Gerrit-Change-Number: 20707
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Dec 2023 04:40:12 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, k.venureddy2103@gmail.com, Sai Hemanth Gantasala, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20707
to look at the new patch set (#5).
Change subject: IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
......................................................................
IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Any failures in fetching HMS events should be retriable. Event-processor
should not go into the ERROR state which can only be recovered by a
global INVALIDATE METADATA command.
This patch deals with the failure in creating a new MetaStoreClient
by throwing a MetastoreClientInstantiationException instead of an
IllegalStateException. Previously the IllegalStateException could fail
the process of fetching HMS events. Now callers can catch the
MetastoreClientInstantiationException and convert it into
MetastoreNotificationFetchException if the process is retriable. So the
event-processor can retry in the next round. There are still other
callers of Catalog#getMetaStoreClient() that don't catch the new
exception since their work can't be easily retried.
Also makes sure MetastoreEventsProcessor.getCurrentEventId() only throws
MetastoreNotificationFetchException. Previously it throws
CatalogException which will fail the event-processor. Note that
CatalogException is used for errors in accessing objects in the Catalog,
e.g. table not found. We shouldn't throw it when fetching HMS events
fails.
Tests:
- Add FE unit test to verify MetastoreNotificationFetchException is
thrown as expected. To mimic HMS connection failures, use a
customized MetastoreClientPool that uses wrong HMS port.
- Add e2e test in custom_cluster/test_catalog_hms_failures.py. The test
class previously only runs in exhaustive jobs due to long running
time. Optimize the test to only restart HMS. Adds a new option,
-if_not_running, for run-hive-server.sh to avoid unneccessary
restarts.
Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
---
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
A fe/src/main/java/org/apache/impala/catalog/MetastoreClientInstantiationException.java
M fe/src/main/java/org/apache/impala/catalog/events/ExternalEventsProcessor.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
A fe/src/test/java/org/apache/impala/testutil/IncompetentMetastoreClientPool.java
M testdata/bin/run-hive-server.sh
M tests/custom_cluster/test_catalog_hms_failures.py
10 files changed, 246 insertions(+), 47 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/20707/5
--
To view, visit http://gerrit.cloudera.org:8080/20707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Gerrit-Change-Number: 20707
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
[Impala-ASF-CR] IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20707 )
Change subject: IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
......................................................................
Patch Set 5: Code-Review+2
LGTM! Carry +1 from prior reviews.
--
To view, visit http://gerrit.cloudera.org:8080/20707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Gerrit-Change-Number: 20707
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Sat, 23 Dec 2023 01:54:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
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/20707 )
Change subject: IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
......................................................................
IMPALA-12561: Event-processor shouldn't go into ERROR state for failures in fetching events
Any failures in fetching HMS events should be retriable. Event-processor
should not go into the ERROR state which can only be recovered by a
global INVALIDATE METADATA command.
This patch deals with the failure in creating a new MetaStoreClient
by throwing a MetastoreClientInstantiationException instead of an
IllegalStateException. Previously the IllegalStateException could fail
the process of fetching HMS events. Now callers can catch the
MetastoreClientInstantiationException and convert it into
MetastoreNotificationFetchException if the process is retriable. So the
event-processor can retry in the next round. There are still other
callers of Catalog#getMetaStoreClient() that don't catch the new
exception since their work can't be easily retried.
Also makes sure MetastoreEventsProcessor.getCurrentEventId() only throws
MetastoreNotificationFetchException. Previously it throws
CatalogException which will fail the event-processor. Note that
CatalogException is used for errors in accessing objects in the Catalog,
e.g. table not found. We shouldn't throw it when fetching HMS events
fails.
Tests:
- Add FE unit test to verify MetastoreNotificationFetchException is
thrown as expected. To mimic HMS connection failures, use a
customized MetastoreClientPool that uses wrong HMS port.
- Add e2e test in custom_cluster/test_catalog_hms_failures.py. The test
class previously only runs in exhaustive jobs due to long running
time. Optimize the test to only restart HMS. Adds a new option,
-if_not_running, for run-hive-server.sh to avoid unneccessary
restarts.
Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Reviewed-on: http://gerrit.cloudera.org:8080/20707
Reviewed-by: Riza Suminto <ri...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
A fe/src/main/java/org/apache/impala/catalog/MetastoreClientInstantiationException.java
M fe/src/main/java/org/apache/impala/catalog/events/ExternalEventsProcessor.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
A fe/src/test/java/org/apache/impala/testutil/IncompetentMetastoreClientPool.java
M testdata/bin/run-hive-server.sh
M tests/custom_cluster/test_catalog_hms_failures.py
10 files changed, 246 insertions(+), 47 deletions(-)
Approvals:
Riza Suminto: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/20707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I775684d473fdbfb9f0531234f59a6239bd0873e3
Gerrit-Change-Number: 20707
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>