You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org> on 2018/10/07 06:14:01 UTC
[Impala-ASF-CR] IMPALA-7597: wraps retries around CatalogMetaProvider
Vuk Ercegovac has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11608
Change subject: IMPALA-7597: wraps retries around CatalogMetaProvider
......................................................................
IMPALA-7597: wraps retries around CatalogMetaProvider
When configured to use the local catalog, concurrent metadata
reads and writes can cause the CatalogMetaProvider to throw
an InconsistentMetadataFetchException. Queries have been wrapped
with a retry loop, but the other frontend methods, such listing
table or partition information, can also fail from the same error.
These errors were seen under a workload consisting of concurrent
adding and showing partitions.
This change adds a RetryableMetaProvider that wraps the relevant
methods of CatalogMetaProvider with retries/logging. When obtaining
a Catalog from the FeCatalogManager, the caller can switch between
obtaining a Catalog that does not retry and one that does. Several
Frontend call-sites have been updated accordingly. For example,
GetCatalogMetrics does not really care if an exception is thrown
when fetching the number of tables so using a Catalog without retries
is used (as before). However, when showing various metadata such as
partitions or files, a retryable Catalog is now used. Note that when
a local catalog is not used, the retryable catalog is a no-op.
Testing:
- added a test that checks whether inconsistent metadata exceptions
are seen in a concurrent workload.
Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
---
A fe/src/main/java/org/apache/impala/catalog/local/RetryableMetaProvider.java
M fe/src/main/java/org/apache/impala/service/FeCatalogManager.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/MetadataOp.java
M tests/custom_cluster/test_local_catalog.py
5 files changed, 323 insertions(+), 44 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/11608/1
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7597: wraps retries around CatalogMetaProvider
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around CatalogMetaProvider
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py
File tests/custom_cluster/test_local_catalog.py:
http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py@216
PS4, Line 216: class TestLocalCatalogRetries(CustomClusterTestSuite):
flake8: E302 expected 2 blank lines, found 1
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 4
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Oct 2018 22:37:55 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7597: wraps retries around CatalogMetaProvider
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around CatalogMetaProvider
......................................................................
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/11608/1/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/11608/1/fe/src/main/java/org/apache/impala/service/Frontend.java@256
PS1, Line 256: public FeCatalog getRetryableCatalog() { return catalogManager_.getOrCreateRetryableCatalog(); }
line too long (98 > 90)
http://gerrit.cloudera.org:8080/#/c/11608/1/tests/custom_cluster/test_local_catalog.py
File tests/custom_cluster/test_local_catalog.py:
http://gerrit.cloudera.org:8080/#/c/11608/1/tests/custom_cluster/test_local_catalog.py@170
PS1, Line 170: s
flake8: E501 line too long (94 > 90 characters)
http://gerrit.cloudera.org:8080/#/c/11608/1/tests/custom_cluster/test_local_catalog.py@182
PS1, Line 182: t
flake8: E501 line too long (102 > 90 characters)
http://gerrit.cloudera.org:8080/#/c/11608/1/tests/custom_cluster/test_local_catalog.py@225
PS1, Line 225: )
flake8: E501 line too long (92 > 90 characters)
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sun, 07 Oct 2018 06:15:09 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7597: wraps retries around Frontend metadata operations.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around Frontend metadata operations.
......................................................................
Patch Set 9: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 9
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 00:20:11 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7597: wraps retries around Frontend metadata operations.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around Frontend metadata operations.
......................................................................
Patch Set 8:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11608/8/tests/custom_cluster/test_local_catalog.py
File tests/custom_cluster/test_local_catalog.py:
http://gerrit.cloudera.org:8080/#/c/11608/8/tests/custom_cluster/test_local_catalog.py@217
PS8, Line 217: class TestLocalCatalogRetries(CustomClusterTestSuite):
flake8: E302 expected 2 blank lines, found 1
http://gerrit.cloudera.org:8080/#/c/11608/8/tests/custom_cluster/test_local_catalog.py@244
PS8, Line 244: r
flake8: F841 local variable 'ret' is assigned to but never used
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 8
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Oct 2018 23:37:47 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7597: wraps retries around Frontend metadata operations.
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Tianyi Wang, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11608
to look at the new patch set (#7).
Change subject: IMPALA-7597: wraps retries around Frontend metadata operations.
......................................................................
IMPALA-7597: wraps retries around Frontend metadata operations.
When configured to use the local catalog, concurrent metadata
reads and writes can cause the CatalogMetaProvider to throw
an InconsistentMetadataFetchException. Queries have been wrapped
with a retry loop, but the other frontend methods, such listing
table or partition information, can also fail from the same error.
These errors were seen under a workload consisting of concurrent
adding and showing partitions.
This change wraps call-sites (primarily in Frontend.java) that acquire
a Catalog, so have a chance of throwing an InconsistentMetadataFetchExecption.
Testing:
- added a test that checks whether inconsistent metadata exceptions
are seen in a concurrent workload.
Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
---
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/MetadataOp.java
M tests/custom_cluster/test_local_catalog.py
3 files changed, 292 insertions(+), 101 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/11608/7
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 7
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7597: wraps retries around Frontend metadata operations.
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Tianyi Wang, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11608
to look at the new patch set (#6).
Change subject: IMPALA-7597: wraps retries around Frontend metadata operations.
......................................................................
IMPALA-7597: wraps retries around Frontend metadata operations.
When configured to use the local catalog, concurrent metadata
reads and writes can cause the CatalogMetaProvider to throw
an InconsistentMetadataFetchException. Queries have been wrapped
with a retry loop, but the other frontend methods, such listing
table or partition information, can also fail from the same error.
These errors were seen under a workload consisting of concurrent
adding and showing partitions.
This change wraps call-sites (primarily in Frontend.java) that acquire
a Catalog, so have a chance of throwing an InconsistentMetadataFetchExecption.
Testing:
- added a test that checks whether inconsistent metadata exceptions
are seen in a concurrent workload.
Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
---
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/MetadataOp.java
M tests/custom_cluster/test_local_catalog.py
3 files changed, 290 insertions(+), 101 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/11608/6
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 6
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7597: wraps retries around Frontend metadata operations.
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around Frontend metadata operations.
......................................................................
Patch Set 5:
(5 comments)
thx for the suggestions. I brought up lambdas before but need java 8-- added a todo to switch it over, which should make this part nicer.
http://gerrit.cloudera.org:8080/#/c/11608/4/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/11608/4/fe/src/main/java/org/apache/impala/service/Frontend.java@681
PS4, Line 681: RI
> I meant, log the exception stack trace too. LOG.warn("foo", e) instead of L
Done
http://gerrit.cloudera.org:8080/#/c/11608/5/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/11608/5/fe/src/main/java/org/apache/impala/service/Frontend.java@754
PS5, Line 754: List<? extends FeDb> dbs = getCatalog().getDbs(matcher);
> mention no need to retry for this call path?
added a comment in the retry wrapper that explains when retries are not needed.
http://gerrit.cloudera.org:8080/#/c/11608/5/fe/src/main/java/org/apache/impala/service/Frontend.java@1463
PS5, Line 1463: FeTable table = getCatalog().getTable(request.getTable_name().getDb_name(),
> this one?
yup, wrapped this one and another.
http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py
File tests/custom_cluster/test_local_catalog.py:
http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py@280
PS4, Line 280: catalogd_args="--catalog_topic_mode=minimal")
> Ya, i meant to replace that "alter" with "refresh" which bumps the version
good suggestion. simpler now.
http://gerrit.cloudera.org:8080/#/c/11608/5/tests/custom_cluster/test_local_catalog.py
File tests/custom_cluster/test_local_catalog.py:
http://gerrit.cloudera.org:8080/#/c/11608/5/tests/custom_cluster/test_local_catalog.py@248
PS5, Line 248: attempt < 100
> Is this needed if we join for 30s?
I saw that without the patch, the failure comes up pretty quickly. With the patch, there are lots of successes. The more queries run, the more memory was taken up to the point of oom'ing the test runner. So I've capped the number of queries but still guard using a timeout for the case where we're stuck in a longer retry loop.
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 5
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Oct 2018 05:43:26 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7597: wraps retries around CatalogMetaProvider
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around CatalogMetaProvider
......................................................................
Patch Set 1:
(5 comments)
Haven't looked at the tests yet but some high level comments.
http://gerrit.cloudera.org:8080/#/c/11608/1/fe/src/main/java/org/apache/impala/catalog/local/RetryableMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/RetryableMetaProvider.java:
http://gerrit.cloudera.org:8080/#/c/11608/1/fe/src/main/java/org/apache/impala/catalog/local/RetryableMetaProvider.java@52
PS1, Line 52: implements MetaProvider
Doesn't this only apply to CatalogdMetaProvider impl? Wondering why we need retries around something like a DirectMetaProvider.
http://gerrit.cloudera.org:8080/#/c/11608/1/fe/src/main/java/org/apache/impala/catalog/local/RetryableMetaProvider.java@100
PS1, Line 100: exception
Can we say that it exceeded num retries?
http://gerrit.cloudera.org:8080/#/c/11608/1/fe/src/main/java/org/apache/impala/catalog/local/RetryableMetaProvider.java@118
PS1, Line 118: }
Should we include the number of attempts it took for a call to be successful?
http://gerrit.cloudera.org:8080/#/c/11608/1/fe/src/main/java/org/apache/impala/catalog/local/RetryableMetaProvider.java@126
PS1, Line 126: while (true) {
: try {
: return provider_.loadTableNames(dbName);
: } catch (InconsistentMetadataFetchException e) {
: tracker.handleRetryOrThrow(e);
: }
can we factor out while(true) try {} catch {retry} into a helper? I think we can use reflection to invoke a method by name.
retryMethod(tracker, "loadTableNames", params...)
http://gerrit.cloudera.org:8080/#/c/11608/1/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/11608/1/fe/src/main/java/org/apache/impala/service/Frontend.java@834
PS1, Line 834: getCatalog
It looks like the callers should now understand whether an operation should be retryable or not and then "get" a Catalog accordingly. This seems a bit confusing to me. Is there an easy way to fix that? What if someone adds a new operation and gets a wrong type of Catalog in the future? Should we think about that?
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Oct 2018 18:57:29 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7597: wraps retries around CatalogMetaProvider
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around CatalogMetaProvider
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1044/ : 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/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 4
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Oct 2018 23:19:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7597: wraps retries around Frontend metadata operations.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around Frontend metadata operations.
......................................................................
Patch Set 9: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 9
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Oct 2018 20:28:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7597: wraps retries around Frontend metadata operations.
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/11608 )
Change subject: IMPALA-7597: wraps retries around Frontend metadata operations.
......................................................................
IMPALA-7597: wraps retries around Frontend metadata operations.
When configured to use the local catalog, concurrent metadata
reads and writes can cause the CatalogMetaProvider to throw
an InconsistentMetadataFetchException. Queries have been wrapped
with a retry loop, but the other frontend methods, such listing
table or partition information, can also fail from the same error.
These errors were seen under a workload consisting of concurrent
adding and showing partitions.
This change wraps call-sites (primarily in Frontend.java) that acquire
a Catalog, so have a chance of throwing an InconsistentMetadataFetchExecption.
Testing:
- added a test that checks whether inconsistent metadata exceptions
are seen in a concurrent workload.
Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Reviewed-on: http://gerrit.cloudera.org:8080/11608
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/MetadataOp.java
M tests/custom_cluster/test_local_catalog.py
3 files changed, 289 insertions(+), 101 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 10
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7597: wraps retries around CatalogMetaProvider
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around CatalogMetaProvider
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11608/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11608/2//COMMIT_MSG@18
PS2, Line 18: methods of CatalogMetaProvider with retries/logging. When obtaining
> I briefly looked into making it a checked exception back when I initially c
makes sense. also, its an exception that for now, is coming from one flavor of MetaProvider so unclear about insisting on it for all.
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Oct 2018 22:30:31 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7597: wraps retries around CatalogMetaProvider
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around CatalogMetaProvider
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/977/ : 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/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sun, 07 Oct 2018 07:07:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7597: wraps retries around Frontend metadata operations.
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around Frontend metadata operations.
......................................................................
Patch Set 6:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11608/5/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/11608/5/fe/src/main/java/org/apache/impala/service/Frontend.java@1463
PS5, Line 1463: ImpalaInternalAdminUser.getInstance();
> add these cases to the test query list too?
Done
http://gerrit.cloudera.org:8080/#/c/11608/6/tests/custom_cluster/test_local_catalog.py
File tests/custom_cluster/test_local_catalog.py:
http://gerrit.cloudera.org:8080/#/c/11608/6/tests/custom_cluster/test_local_catalog.py@312
PS6, Line 312: def test_replan_on_stale_metadata(self, unique_database):
> Port this test and below to use the helper method check..retries()?
makes sense. they're looking for slightly different things (exception vs. retry). left a todo for now.
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 6
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Oct 2018 06:36:17 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7597: wraps retries around Frontend metadata operations.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around Frontend metadata operations.
......................................................................
Patch Set 9:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3330/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 9
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Oct 2018 20:28:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7597: wraps retries around CatalogMetaProvider
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around CatalogMetaProvider
......................................................................
Patch Set 3:
latest patch wraps retries higher-up at the callsites.
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 3
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Oct 2018 04:31:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7597: wraps retries around Frontend metadata operations.
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Tianyi Wang, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11608
to look at the new patch set (#5).
Change subject: IMPALA-7597: wraps retries around Frontend metadata operations.
......................................................................
IMPALA-7597: wraps retries around Frontend metadata operations.
When configured to use the local catalog, concurrent metadata
reads and writes can cause the CatalogMetaProvider to throw
an InconsistentMetadataFetchException. Queries have been wrapped
with a retry loop, but the other frontend methods, such listing
table or partition information, can also fail from the same error.
These errors were seen under a workload consisting of concurrent
adding and showing partitions.
This change wraps call-sites (primarily in Frontend.java) that acquire
a Catalog, so have a chance of throwing an InconsistentMetadataFetchExecption.
Testing:
- added a test that checks whether inconsistent metadata exceptions
are seen in a concurrent workload.
Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
---
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/MetadataOp.java
M tests/custom_cluster/test_local_catalog.py
3 files changed, 264 insertions(+), 101 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/11608/5
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 5
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7597: wraps retries around Frontend metadata operations.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around Frontend metadata operations.
......................................................................
Patch Set 8:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1094/ : 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/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 8
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Oct 2018 20:59:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7597: wraps retries around CatalogMetaProvider
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around CatalogMetaProvider
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11608/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11608/2//COMMIT_MSG@18
PS2, Line 18: methods of CatalogMetaProvider with retries/logging. When obtaining
I'm confused how this is supposed to work. Don't we need to retry from the "top" of the higher level transaction/operation, not just the individual method call? If we just retry the method call, it's going to just keep on throwing because we are retrying with the original TableMetaRef, no?
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 09 Oct 2018 20:31:46 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7597: wraps retries around Frontend metadata operations.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around Frontend metadata operations.
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1078/ : 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/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 5
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Oct 2018 18:16:08 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7597: wraps retries around Frontend metadata operations.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around Frontend metadata operations.
......................................................................
Patch Set 6:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11608/6/tests/custom_cluster/test_local_catalog.py
File tests/custom_cluster/test_local_catalog.py:
http://gerrit.cloudera.org:8080/#/c/11608/6/tests/custom_cluster/test_local_catalog.py@217
PS6, Line 217: class TestLocalCatalogRetries(CustomClusterTestSuite):
flake8: E302 expected 2 blank lines, found 1
http://gerrit.cloudera.org:8080/#/c/11608/6/tests/custom_cluster/test_local_catalog.py@220
PS6, Line 220: d
flake8: E303 too many blank lines (2)
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 6
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Oct 2018 06:53:43 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7597: wraps retries around Frontend metadata operations.
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around Frontend metadata operations.
......................................................................
Patch Set 5:
Looks fine to me. Another way of doing it would be to take a more functional approach and add a method like RetryTracker#retry(Callable<T> callable), and pass the guts of the frontend methods in as callables there. That might be slightly more composable/reusable than repeating the while loop, and when we are wholesale in Java 8 it would be pretty concise using lambda syntax.
The only downside might be in passing through exceptions, which don't really participate that well with callables/generics.
I'm fine with this as is, though.
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 5
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Oct 2018 19:38:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7597: wraps retries around CatalogMetaProvider
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around CatalogMetaProvider
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11608/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11608/2//COMMIT_MSG@18
PS2, Line 18: methods of CatalogMetaProvider with retries/logging. When obtaining
> good point. most of the top-level "describe" methods use strings as args, n
I'll look into wrapping the frontend call-sites, so similar handling as with the createExecRequest code path. I thought I saw some discussion about making this a checked exception, but otherwise, if there are better, less cumbersome ways to deal with this, let me know.
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Oct 2018 22:21:12 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7597: wraps retries around Frontend metadata operations.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around Frontend metadata operations.
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1086/ : 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/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 7
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Oct 2018 07:08:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7597: wraps retries around CatalogMetaProvider
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around CatalogMetaProvider
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1030/ : 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/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 3
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Oct 2018 04:59:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7597: wraps retries around CatalogMetaProvider
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around CatalogMetaProvider
......................................................................
Patch Set 4:
(12 comments)
Agreed that spreading this retry pattern is not pretty. Some thoughts:
- is it an improvement when compared to the user seeing these exceptions?
- perhaps it indicates that there are too many service-level methods. I'm in favor of funneling these through a common path, but I'd rather do that restructure separately. an example of this is MetadataOp which fwict is used by the hs2 entry point.
Other suggestions welcome.
http://gerrit.cloudera.org:8080/#/c/11608/4/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/11608/4/fe/src/main/java/org/apache/impala/service/Frontend.java@660
PS4, Line 660: * Keeps track of retries when handling InconsistentMetadataFetchExceptions.
> Maybe add more context and documentation. In which cases is this used etc.
Done
http://gerrit.cloudera.org:8080/#/c/11608/4/fe/src/main/java/org/apache/impala/service/Frontend.java@666
PS4, Line 666: private String msg_;
> final
Done
http://gerrit.cloudera.org:8080/#/c/11608/4/fe/src/main/java/org/apache/impala/service/Frontend.java@671
PS4, Line 671:
> ..of..
Done
http://gerrit.cloudera.org:8080/#/c/11608/4/fe/src/main/java/org/apache/impala/service/Frontend.java@680
PS4, Line 680: LOG.warn("Retried {}: {} (retry #{} of {})", msg_, exception.getMessage(),
> Should any of this bubble up to the runtime profiles (if one exists) for th
would make sense to handle these uniformly with queries. added a todo. that plumbling is better done separately.
http://gerrit.cloudera.org:8080/#/c/11608/4/fe/src/main/java/org/apache/impala/service/Frontend.java@681
PS4, Line 681: );
> , exception) to log the trace and you can probably skip exception.getMessag
wasn't entirely sure what's the suggestion here, but I was aiming for consistency with the query-retry message
http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py
File tests/custom_cluster/test_local_catalog.py:
http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py@173
PS4, Line 173: def test_cache_metrics(self, unique_database):
> did something change here or was it just moved to the top? Kind of confusin
separated these two types of tests into two classes so this test moved up here.
http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py@218
PS4, Line 218: _find_inconsistent_metadata
> name seems confusing. Maybe call check_metadata_fetch_retries or some such
renamed to _check_metadata_retries
http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py@221
PS4, Line 221: ,
> mention the randomness part?
Done
http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py@236
PS4, Line 236: def stress_thread(client):
> Add a comment on what this does?
Done
http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py@250
PS4, Line 250: inconsistent_seen[0] += 1
> catch and record other exceptions since this runs in a thread?
Done
http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py@258
PS4, Line 258: 5
> I doubt if this is enough, I noticed some concurrency errors required > 60s
I saw it repro at this level, but I upped in case its too close to the edge. Set it to 30, which is the level used in tests already.
http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py@280
PS4, Line 280: "alter table %s.%s set tblproperties ('numRows'='3')" %
> Run these on 'functional' tables with a concurrent refresh? That way you ca
I didn't want to modify the state (see the alter on L280). That can leak to other tests, which is a mess.
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 4
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Oct 2018 17:36:05 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7597: wraps retries around CatalogMetaProvider
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around CatalogMetaProvider
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11608/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11608/2//COMMIT_MSG@18
PS2, Line 18: methods of CatalogMetaProvider with retries/logging. When obtaining
> I'm confused how this is supposed to work. Don't we need to retry from the
good point. most of the top-level "describe" methods use strings as args, not refs. the refs are mostly used for everything under createExecRequest, and there, we're guarding at the top-level. the method to get column stats and the one that looks up all partition info should change as a result. will confirm with tests.
http://gerrit.cloudera.org:8080/#/c/11608/1/fe/src/main/java/org/apache/impala/catalog/local/RetryableMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/RetryableMetaProvider.java:
http://gerrit.cloudera.org:8080/#/c/11608/1/fe/src/main/java/org/apache/impala/catalog/local/RetryableMetaProvider.java@126
PS1, Line 126: while (true) {
: try {
: return provider_.loadTableNames(dbName);
: } catch (InconsistentMetadataFetchException e) {
: tracker.handleRetryOrThrow(e);
: }
> can we factor out while(true) try {} catch {retry} into a helper? I think
will try it but only after we're settled on this approach.
http://gerrit.cloudera.org:8080/#/c/11608/1/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/11608/1/fe/src/main/java/org/apache/impala/service/Frontend.java@834
PS1, Line 834: ribeResult
> It looks like the callers should now understand whether an operation should
As discussed, I was trading off looping too many times with retry (consider getCatalogMetrics and all query paths under createExecRequest) vs. letting one of these inconsistent metadata exceptions loose. We spend far more cycles in the latter so opted to not go with an approach that changes the default to loop.
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Oct 2018 20:48:56 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7597: wraps retries around CatalogMetaProvider
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Tianyi Wang, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11608
to look at the new patch set (#3).
Change subject: IMPALA-7597: wraps retries around CatalogMetaProvider
......................................................................
IMPALA-7597: wraps retries around CatalogMetaProvider
When configured to use the local catalog, concurrent metadata
reads and writes can cause the CatalogMetaProvider to throw
an InconsistentMetadataFetchException. Queries have been wrapped
with a retry loop, but the other frontend methods, such listing
table or partition information, can also fail from the same error.
These errors were seen under a workload consisting of concurrent
adding and showing partitions.
This change wraps call-sites (primarily in Frontend.java) that acquire
a Catalog, so have a chance of throwing an InconsistentMetadataFetchExecption.
Testing:
- added a test that checks whether inconsistent metadata exceptions
are seen in a concurrent workload.
Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
---
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/MetadataOp.java
M tests/custom_cluster/test_local_catalog.py
3 files changed, 181 insertions(+), 39 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/11608/3
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 3
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7597: wraps retries around CatalogMetaProvider
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11608
to look at the new patch set (#2).
Change subject: IMPALA-7597: wraps retries around CatalogMetaProvider
......................................................................
IMPALA-7597: wraps retries around CatalogMetaProvider
When configured to use the local catalog, concurrent metadata
reads and writes can cause the CatalogMetaProvider to throw
an InconsistentMetadataFetchException. Queries have been wrapped
with a retry loop, but the other frontend methods, such listing
table or partition information, can also fail from the same error.
These errors were seen under a workload consisting of concurrent
adding and showing partitions.
This change adds a RetryableMetaProvider that wraps the relevant
methods of CatalogMetaProvider with retries/logging. When obtaining
a Catalog from the FeCatalogManager, the caller can switch between
obtaining a Catalog that does not retry and one that does. Several
Frontend call-sites have been updated accordingly. For example,
GetCatalogMetrics does not really care if an exception is thrown
when fetching the number of tables so using a Catalog without retries
is used (as before). However, when showing various metadata such as
partitions or files, a retryable Catalog is now used. Note that when
a local catalog is not used, the retryable catalog is a no-op.
Testing:
- added a test that checks whether inconsistent metadata exceptions
are seen in a concurrent workload.
Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
---
A fe/src/main/java/org/apache/impala/catalog/local/RetryableMetaProvider.java
M fe/src/main/java/org/apache/impala/service/FeCatalogManager.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/MetadataOp.java
M tests/custom_cluster/test_local_catalog.py
5 files changed, 328 insertions(+), 44 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/11608/2
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-7597: wraps retries around CatalogMetaProvider
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around CatalogMetaProvider
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/976/ : 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/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sun, 07 Oct 2018 06:49:11 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7597: wraps retries around Frontend metadata operations.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around Frontend metadata operations.
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1084/ : 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/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 6
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Oct 2018 06:22:07 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7597: wraps retries around Frontend metadata operations.
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around Frontend metadata operations.
......................................................................
Patch Set 8: Code-Review+2
test change broke another test. minor fix. core tests pass. carrying .+2
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 8
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Oct 2018 20:27:21 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7597: wraps retries around Frontend metadata operations.
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around Frontend metadata operations.
......................................................................
Patch Set 6:
(2 comments)
A couple of minor comments, otherwise +2 from me
http://gerrit.cloudera.org:8080/#/c/11608/5/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/11608/5/fe/src/main/java/org/apache/impala/service/Frontend.java@1463
PS5, Line 1463: ImpalaInternalAdminUser.getInstance();
> yup, wrapped this one and another.
add these cases to the test query list too?
http://gerrit.cloudera.org:8080/#/c/11608/6/tests/custom_cluster/test_local_catalog.py
File tests/custom_cluster/test_local_catalog.py:
http://gerrit.cloudera.org:8080/#/c/11608/6/tests/custom_cluster/test_local_catalog.py@312
PS6, Line 312: def test_replan_on_stale_metadata(self, unique_database):
Port this test and below to use the helper method check..retries()?
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 6
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Oct 2018 06:06:32 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7597: wraps retries around CatalogMetaProvider
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Tianyi Wang, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11608
to look at the new patch set (#4).
Change subject: IMPALA-7597: wraps retries around CatalogMetaProvider
......................................................................
IMPALA-7597: wraps retries around CatalogMetaProvider
When configured to use the local catalog, concurrent metadata
reads and writes can cause the CatalogMetaProvider to throw
an InconsistentMetadataFetchException. Queries have been wrapped
with a retry loop, but the other frontend methods, such listing
table or partition information, can also fail from the same error.
These errors were seen under a workload consisting of concurrent
adding and showing partitions.
This change wraps call-sites (primarily in Frontend.java) that acquire
a Catalog, so have a chance of throwing an InconsistentMetadataFetchExecption.
Testing:
- added a test that checks whether inconsistent metadata exceptions
are seen in a concurrent workload.
Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
---
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/MetadataOp.java
M tests/custom_cluster/test_local_catalog.py
3 files changed, 247 insertions(+), 100 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/11608/4
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 4
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7597: wraps retries around Frontend metadata operations.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around Frontend metadata operations.
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11608/7/tests/custom_cluster/test_local_catalog.py
File tests/custom_cluster/test_local_catalog.py:
http://gerrit.cloudera.org:8080/#/c/11608/7/tests/custom_cluster/test_local_catalog.py@217
PS7, Line 217: class TestLocalCatalogRetries(CustomClusterTestSuite):
flake8: E302 expected 2 blank lines, found 1
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 7
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Oct 2018 09:03:28 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7597: wraps retries around Frontend metadata operations.
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Tianyi Wang, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11608
to look at the new patch set (#8).
Change subject: IMPALA-7597: wraps retries around Frontend metadata operations.
......................................................................
IMPALA-7597: wraps retries around Frontend metadata operations.
When configured to use the local catalog, concurrent metadata
reads and writes can cause the CatalogMetaProvider to throw
an InconsistentMetadataFetchException. Queries have been wrapped
with a retry loop, but the other frontend methods, such listing
table or partition information, can also fail from the same error.
These errors were seen under a workload consisting of concurrent
adding and showing partitions.
This change wraps call-sites (primarily in Frontend.java) that acquire
a Catalog, so have a chance of throwing an InconsistentMetadataFetchExecption.
Testing:
- added a test that checks whether inconsistent metadata exceptions
are seen in a concurrent workload.
Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
---
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/MetadataOp.java
M tests/custom_cluster/test_local_catalog.py
3 files changed, 289 insertions(+), 101 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/11608/8
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 8
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7597: wraps retries around CatalogMetaProvider
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around CatalogMetaProvider
......................................................................
Patch Set 4:
re-worked the tests a bit to separate out the retries from the other tests and to use less memory (was getting oom killed)
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 4
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Oct 2018 22:38:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7597: wraps retries around CatalogMetaProvider
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around CatalogMetaProvider
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11608/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11608/2//COMMIT_MSG@18
PS2, Line 18: methods of CatalogMetaProvider with retries/logging. When obtaining
> I'll look into wrapping the frontend call-sites, so similar handling as wit
I briefly looked into making it a checked exception back when I initially created the class, but it looked like somewhat of a sprawling change, so figured I'd leave that as a TODO.
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Oct 2018 22:26:29 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7597: wraps retries around Frontend metadata operations.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around Frontend metadata operations.
......................................................................
Patch Set 5:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11608/5/tests/custom_cluster/test_local_catalog.py
File tests/custom_cluster/test_local_catalog.py:
http://gerrit.cloudera.org:8080/#/c/11608/5/tests/custom_cluster/test_local_catalog.py@217
PS5, Line 217: class TestLocalCatalogRetries(CustomClusterTestSuite):
flake8: E302 expected 2 blank lines, found 1
http://gerrit.cloudera.org:8080/#/c/11608/5/tests/custom_cluster/test_local_catalog.py@220
PS5, Line 220: d
flake8: E303 too many blank lines (2)
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 5
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Oct 2018 19:41:36 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7597: wraps retries around CatalogMetaProvider
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around CatalogMetaProvider
......................................................................
Patch Set 4:
(12 comments)
Like we discussed offline, I'm wondering if this can be done is any other way without a major refactoring of code. Basically to enforce that the retries are done where needed (especially for stuff that can potentially be added in the future). I can't think of any other fancy ways. Todd, do you have any thoughts?
http://gerrit.cloudera.org:8080/#/c/11608/4/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/11608/4/fe/src/main/java/org/apache/impala/service/Frontend.java@660
PS4, Line 660: * Keeps track of retries when handling InconsistentMetadataFetchExceptions.
Maybe add more context and documentation. In which cases is this used etc. etc.
http://gerrit.cloudera.org:8080/#/c/11608/4/fe/src/main/java/org/apache/impala/service/Frontend.java@666
PS4, Line 666: private String msg_;
final
http://gerrit.cloudera.org:8080/#/c/11608/4/fe/src/main/java/org/apache/impala/service/Frontend.java@671
PS4, Line 671:
..of..
http://gerrit.cloudera.org:8080/#/c/11608/4/fe/src/main/java/org/apache/impala/service/Frontend.java@680
PS4, Line 680: LOG.warn("Retried {}: {} (retry #{} of {})", msg_, exception.getMessage(),
Should any of this bubble up to the runtime profiles (if one exists) for these requests?
http://gerrit.cloudera.org:8080/#/c/11608/4/fe/src/main/java/org/apache/impala/service/Frontend.java@681
PS4, Line 681: );
, exception) to log the trace and you can probably skip exception.getMessage()
http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py
File tests/custom_cluster/test_local_catalog.py:
http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py@173
PS4, Line 173: def test_cache_metrics(self, unique_database):
did something change here or was it just moved to the top? Kind of confusing in the gerrit view.
http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py@218
PS4, Line 218: _find_inconsistent_metadata
name seems confusing. Maybe call check_metadata_fetch_retries or some such thing?
http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py@221
PS4, Line 221: ,
mention the randomness part?
http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py@236
PS4, Line 236: def stress_thread(client):
Add a comment on what this does?
http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py@250
PS4, Line 250: inconsistent_seen[0] += 1
catch and record other exceptions since this runs in a thread?
http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py@258
PS4, Line 258: 5
I doubt if this is enough, I noticed some concurrency errors required > 60s
http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py@280
PS4, Line 280: "alter table %s.%s set tblproperties ('numRows'='3')" %
Run these on 'functional' tables with a concurrent refresh? That way you can probably avoid these unique_db, table_name creation in find_inconsistent_...()
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 4
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Oct 2018 06:31:48 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7597: wraps retries around Frontend metadata operations.
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around Frontend metadata operations.
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 7
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Oct 2018 06:42:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7597: wraps retries around Frontend metadata operations.
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around Frontend metadata operations.
......................................................................
Patch Set 5:
(6 comments)
LGTM. Just a few clarifications.
http://gerrit.cloudera.org:8080/#/c/11608/4/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/11608/4/fe/src/main/java/org/apache/impala/service/Frontend.java@681
PS4, Line 681: RI
> wasn't entirely sure what's the suggestion here, but I was aiming for consi
I meant, log the exception stack trace too. LOG.warn("foo", e) instead of LOG.warn("foo")
http://gerrit.cloudera.org:8080/#/c/11608/5/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/11608/5/fe/src/main/java/org/apache/impala/service/Frontend.java@754
PS5, Line 754: List<? extends FeDb> dbs = getCatalog().getDbs(matcher);
mention no need to retry for this call path?
http://gerrit.cloudera.org:8080/#/c/11608/5/fe/src/main/java/org/apache/impala/service/Frontend.java@1463
PS5, Line 1463: FeTable table = getCatalog().getTable(request.getTable_name().getDb_name(),
this one?
http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py
File tests/custom_cluster/test_local_catalog.py:
http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py@173
PS4, Line 173: catalogd_args="--catalog_topic_mode=minimal")
> separated these two types of tests into two classes so this test moved up h
I see, thanks.
http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py@280
PS4, Line 280: catalogd_args="--catalog_topic_mode=minimal")
> I didn't want to modify the state (see the alter on L280). That can leak to
Ya, i meant to replace that "alter" with "refresh" which bumps the version too, but your call. That makes the check_metadata_retries() simpler.
http://gerrit.cloudera.org:8080/#/c/11608/5/tests/custom_cluster/test_local_catalog.py
File tests/custom_cluster/test_local_catalog.py:
http://gerrit.cloudera.org:8080/#/c/11608/5/tests/custom_cluster/test_local_catalog.py@248
PS5, Line 248: attempt < 100
Is this needed if we join for 30s?
--
To view, visit http://gerrit.cloudera.org:8080/11608
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b
Gerrit-Change-Number: 11608
Gerrit-PatchSet: 5
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Oct 2018 00:35:42 +0000
Gerrit-HasComments: Yes