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 2019/09/26 04:05:13 UTC
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Quanlong Huang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14307
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
In local catalog mode, the coordinator does not cache all the metadata.
Instead, it caches them on-demand (based on query requests), and
removes them based on the Guava cache configurations (e.g. size or TTL).
We use the catalog version as part of the cache key for fine-grained
metadata, e.g partition meta. When invalidating a table, we simply
invalidate the top-level table entry, and allow other information to
remain in the cache. The old metadata will be lazily removed by Guava
cache since they won't be touched anymore. Thus, there're bunch of stale
metadata in the cache so we can't track the minimal catalog version of
valid catalog objects efficiently.
The minimal catalog version of valid catalog objects is used to
implement global invalidate metadata. In legacy catalog mode, all cached
catalog objects are valid in fact. Coordinator gets the expected min
catalog version in the RPC response from Catalogd. It's the version when
Catalogd starts to reset the entire catalog, which means when the reset
is done, all valid catalog objects should be associated with a catalog
version larger than it. Coordinator will wait until its min catalog
version exceeds this value, which means it has processed all the updates
of the reset propagated from the catalogd via statestored. If SYNC_DDL
is set, the coordinator will also wait until other coordinators reach
the same catalog version with it, so they can also see the latest update
of reset.
This patch adds a new field (lastResetCatalogVersion) in TCatalog to
keep the catalog version when catalogd starts to reset the entire
metadata. Each time when catalogd generates a new topic update for
catalog topic, it will generate a TCatalogObject in CATALOG type
containing the state of the catalog which includes this new field.
When coordinator receives a new value of lastResetCatalogVersion in a
topic update, it means catalogd has reset the entire catalog and all the
relative updates are also included in the same topic update. This is
guaranteed by the fact that the write lock of versionLock is held when
catalogd resetting the entire catalog. So the update thread which
requires holding the read lock of versionLock don't have chance to
propagate partial results. Thus, all metadata with catalog version <=
lastResetCatalogVersion can be considered stale after coordinator finish
processing the topic update. lastResetCatalogVersion + 1 is the lower
bound (included) of min catalog version of a coordinator.
Tests:
- Recover all existing tests that have been disabled due to this
missing feature
Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M tests/authorization/test_ranger.py
M tests/common/skip.py
M tests/custom_cluster/test_local_catalog.py
M tests/metadata/test_hms_integration.py
M tests/metadata/test_metadata_query_statements.py
9 files changed, 52 insertions(+), 75 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/14307/1
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 11:
(3 comments)
Thanks for your comments, Vihang!
Changes in the new patch:
1) Found and fixed IMPALA-9136
2) Rename all min_catalog_object_version stuffs to catalog_object_version_lower_bound.
3) Add more logging to better diagnosing catalog version issues.
http://gerrit.cloudera.org:8080/#/c/14307/8/be/src/util/impalad-metrics.cc
File be/src/util/impalad-metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14307/8/be/src/util/impalad-metrics.cc@220
PS8, Line 220: _
> Should this be initialized to -1?
I think it's ok to be 0 since no catalog objects will have catalog version = -1. It keeps the invariant that all cached catalog object versions >= 0.
http://gerrit.cloudera.org:8080/#/c/14307/11/tests/custom_cluster/test_concurrent_ddls.py
File tests/custom_cluster/test_concurrent_ddls.py:
http://gerrit.cloudera.org:8080/#/c/14307/11/tests/custom_cluster/test_concurrent_ddls.py@152
PS11, Line 152: if "CatalogException: Table" in err and \
: "was modified while operation was in progress, aborting execution" in err:
: return True
: if sync_ddl and "Couldn't retrieve the catalog topic version for the SYNC_DDL " \
: "operation after 5 attempts.The operation has been successfully " \
: "executed but its effects may have not been broadcast to all the " \
: "coordinators." in err:
: return True
> Can you add a comment explaining why these are acceptable errors? its not v
Sure, added the comments. I actually find two bugs (IMPALA-9135 and IMPALA-9136) when digged deeper.
IMPALA-9135 is simple so I fixed it by the way in this patch.
Thanks for raising this!
http://gerrit.cloudera.org:8080/#/c/14307/11/tests/custom_cluster/test_local_catalog.py
File tests/custom_cluster/test_local_catalog.py:
http://gerrit.cloudera.org:8080/#/c/14307/11/tests/custom_cluster/test_local_catalog.py@50
PS11, Line 50: get_min_catalog_object_version
> Was this function not useful? https://github.com/apache/impala/blob/master/
Nice find! I missed it...
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 11
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Nov 2019 04:03:35 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 6:
Refactored some codes and add the fix for IMPALA-9062 that don't need to acquire table lock in gathering topic updates in minimal topic mode.
Thanks Bharath's comments! Todd, Vihang, hope can hear your opinions for this approach.
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Fri, 18 Oct 2019 14:05:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 11:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4956/ : 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/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 11
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Wed, 06 Nov 2019 01:08:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 10:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4946/ : 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/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 10
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Nov 2019 16:47:40 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4647/ : 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/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 26 Sep 2019 14:23:51 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
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/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
The minimal catalog object version of valid catalog objects is used to
implement global invalidate metadata in legacy catalog mode. Coordinator
sends DDL RPC to catalogd for global invalidate metadata and gets the
expected min catalog version in the response. It's the version when
catalogd starts to reset the entire catalog, which means when the reset
is done, all valid catalog objects should be associated with a catalog
version larger than it. Coordinator will wait until its min catalog
version exceeds this value, which means it has processed all the updates
of the reset propagated from the catalogd via statestored. If SYNC_DDL
is set, the coordinator will also wait until other coordinators reach
the same statestore topic version with it, so they have also processed
the same updates and had the latest catalog after reset.
In local catalog mode, the coordinator does not cache all the metadata.
Instead, it caches them on-demand (based on query requests), and removes
them based on the Guava cache configurations (size or TTL) or explicit
invalidation from the catalog topic updates. So it's hard to track the
minimal catalog object version correctly.
This patch adds a new field (lastResetCatalogVersion) in TCatalog to
propagate the catalog version when catalogd starts to reset the entire
metadata. Each time when catalogd generates a new topic update, it will
generate a TCatalogObject of CATALOG type containing the state of the
catalog which includes this new field.
When coordinator receives a new value of lastResetCatalogVersion in a
topic update, it means catalogd has reset the entire catalog.
Coordinator will then clear its cache to remove all stale catalog
objects. It's possible that some fresh items being removed too. They
will be refetched on demand.
After the invalidation, there are no catalog object cached with catalog
version <= lastResetCatalogVersion. Because stale cache has been cleared
and all metadata from catalogd is newer than lastResetCatalogVersion. So
lastResetCatalogVersion + 1 is the lower bound (included) of min catalog
object version of a coordinator.
This patch also exposes the lower bound of catalog object version of via
a new metric "catalog.catalog-object-version-lower-bound" to ease
debugging.
IMPALA-9136 is also fixed in this patch.
Tests:
- Recover all existing tests that have been disabled due to this
missing feature
- Add custom cluster test for concurrent DDLs with INVALIDATE METADATA
- Run CORE tests
Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Reviewed-on: http://gerrit.cloudera.org:8080/14307
Reviewed-by: Vihang Karajgaonkar <vi...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/util/impalad-metrics.cc
M be/src/util/impalad-metrics.h
M common/thrift/CatalogObjects.thrift
M common/thrift/Frontend.thrift
M common/thrift/metrics.json
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/catalog/CatalogObjectImpl.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M tests/authorization/test_grant_revoke.py
M tests/authorization/test_ranger.py
M tests/common/skip.py
A tests/custom_cluster/test_concurrent_ddls.py
M tests/custom_cluster/test_local_catalog.py
M tests/metadata/test_hms_integration.py
M tests/metadata/test_metadata_query_statements.py
19 files changed, 383 insertions(+), 115 deletions(-)
Approvals:
Vihang Karajgaonkar: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 14
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 5:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/14307/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/14307/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1104
PS5, Line 1104: if (ctx.collectFullUpdates || tbl.getCatalogVersion() <= ctx.toVersion) {
> I've tried tracking the min catalog versions in local-catalog mode coordina
I see what you are saying, nice find. Agree that we shouldn't take any locks while operating inside Guava's cache context, might lead to performance problems. I can't think of a race-free approach without synchronizing both the operations.
http://gerrit.cloudera.org:8080/#/c/14307/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1104
PS5, Line 1104: if (ctx.collectFullUpdates || tbl.getCatalogVersion() <= ctx.toVersion) {
> For your concerns, I think it's ok since we already have the logic to force
I guess its ok since invalidate metadata is special operation on the entire catalog state. We should probably document this if we end up taking this approach. I don't know what the other reviewers think of this approach. Todd, Vihang?
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 17 Oct 2019 16:28:54 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 8:
(3 comments)
I generally like this solution -- seems simpler than trying to track the full contents of the cache.
Is it possible to use this solution for catalog v1 as well, and remove the CatalogObjectVersionSet thing? I think that thing is buggy/complex, maybe would be nice to just have one implementation to worry about. (ok to do that in a follow-up if it's not trivial, or if you think it's a risky change, maybe we shouldn't do it at all)
http://gerrit.cloudera.org:8080/#/c/14307/8/be/src/util/impalad-metrics.h
File be/src/util/impalad-metrics.h:
http://gerrit.cloudera.org:8080/#/c/14307/8/be/src/util/impalad-metrics.h@119
PS8, Line 119: /// Minimal catalog object version in local catalog cache of Coordinator.
this isn't actually true -- it's no longer the minimum, but rather a lower bound, right? ie it's not a tight bound like it used to be in v1. We should probably rename the metric, variables, thrift fields, etc, to reflect that.
http://gerrit.cloudera.org:8080/#/c/14307/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/14307/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1587
PS8, Line 1587: lastResetStartVersion_ = startVersion;
What if there are two concurrent calls to INVALIDATE METADATA? is it possible that the lastResetStartVersion_ will end up going downward, because the setting of startVersion is under a different lock acquisition from the version writelock above?
http://gerrit.cloudera.org:8080/#/c/14307/8/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:
http://gerrit.cloudera.org:8080/#/c/14307/8/tests/authorization/test_grant_revoke.py@364
PS8, Line 364: self.client.execute("grant role {0} to group foobar".format(role_name))
why were these changes necessary?
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 8
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Oct 2019 23:18:34 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4642/ : 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/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Sep 2019 04:55:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Vihang Karajgaonkar, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14307
to look at the new patch set (#6).
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
The minimal catalog object version of valid catalog objects is used to
implement global invalidate metadata in legacy catalog mode. Coordinator
sends DDL RPC to catalogd for global invalidate metadata and gets the
expected min catalog version in the response. It's the version when
catalogd starts to reset the entire catalog, which means when the reset
is done, all valid catalog objects should be associated with a catalog
version larger than it. Coordinator will wait until its min catalog
version exceeds this value, which means it has processed all the updates
of the reset propagated from the catalogd via statestored. If SYNC_DDL
is set, the coordinator will also wait until other coordinators reach
the same statestore topic version with it, so they have also processed
the same updates and had the latest catalog after reset.
In local catalog mode, the coordinator does not cache all the metadata.
Instead, it caches them on-demand (based on query requests), and removes
them based on the Guava cache configurations (size or TTL) or explicit
invalidation from the catalog topic updates. So it's hard to track the
minimal catalog object version correctly.
This patch adds a new field (lastResetCatalogVersion) in TCatalog to
propagate the catalog version when catalogd starts to reset the entire
metadata. Each time when catalogd generates a new topic update, it will
generate a TCatalogObject of CATALOG type containing the state of the
catalog which includes this new field.
To make all changes of the reset being added in the same topic update
with this TCatalog object. Rapidly changed tables that have catalog
version exceeding the version range of this update will also be included.
When coordinator receives a new value of lastResetCatalogVersion in a
topic update, it means catalogd has reset the entire catalog and all the
relative updates are whether included in the same or previous topic
updates. This is guaranteed by three facts:
1) No topic updates are sent from catalogd when the write lock of
versionLock is held in CatalogServiceCatalog.reset(). Note that the
update thread requires holding the read lock of versionLock.
2) Authz changes before holding the write lock can only be sent in a
previous topic update or in the next topic update after reset().
3) No catalog objects are skipped in the topic update right after
reset(). See changes in GetCatalogDeltaContext.
Thus, all metadata with catalog version <= lastResetCatalogVersion can be
considered stale after coordinator finish processing the topic update.
lastResetCatalogVersion + 1 is the lower bound (included) of min catalog
object version of a coordinator.
To avoid catalogd's update collector thread being blocked by concurrent
DDLs that holding the table locks, this patch also fixes IMPALA-9062. In
local catalog mode, we just need to propagate the table name of a
changed table, so don't need to acquire table lock to get a full TTable
object.
This patch also exposes the min catalog object version of coordinator
via a new metric "catalog.min-catalog-object-version" to ease debugging.
Tests:
- Recover all existing tests that have been disabled due to this
missing feature
Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
---
M be/src/service/impala-server.cc
M be/src/util/impalad-metrics.cc
M be/src/util/impalad-metrics.h
M common/thrift/CatalogObjects.thrift
M common/thrift/metrics.json
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M tests/authorization/test_grant_revoke.py
M tests/authorization/test_ranger.py
M tests/common/skip.py
M tests/custom_cluster/test_local_catalog.py
M tests/metadata/test_hms_integration.py
M tests/metadata/test_metadata_query_statements.py
14 files changed, 180 insertions(+), 106 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/14307/6
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14307
to look at the new patch set (#4).
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
The minimal catalog object version of valid catalog objects is used to
implement global invalidate metadata in legacy catalog mode. Coordinator
sends DDL RPC to catalogd for global invalidate metadata and gets the
expected min catalog version in the response. It's the version when
catalogd starts to reset the entire catalog, which means when the reset
is done, all valid catalog objects should be associated with a catalog
version larger than it. Coordinator will wait until its min catalog
version exceeds this value, which means it has processed all the updates
of the reset propagated from the catalogd via statestored. If SYNC_DDL
is set, the coordinator will also wait until other coordinators reach
the same statestore topic version with it, so they have also processed
the same updates and had the latest catalog after reset.
In local catalog mode, the coordinator does not cache all the metadata.
Instead, it caches them on-demand (based on query requests), and removes
them based on the Guava cache configurations (size or TTL) or explicit
invalidation from the catalog topic updates. So it's hard to track the
minimal catalog object version correctly.
This patch adds a new field (lastResetCatalogVersion) in TCatalog to
propagate the catalog version when catalogd starts to reset the entire
metadata. Each time when catalogd generates a new topic update, it will
generate a TCatalogObject of CATALOG type containing the state of the
catalog which includes this new field.
To make all changes of the reset being added in the same topic update
with this TCatalog object. Rapidly changed tables that have catalog
version exceeding the version range of this update will also be included.
When coordinator receives a new value of lastResetCatalogVersion in a
topic update, it means catalogd has reset the entire catalog and all the
relative updates are whether included in the same or previous topic
updates. This is guaranteed by three facts:
1) No topic updates are sent from catalogd when the write lock of
versionLock is held in CatalogServiceCatalog.reset(). Note that the
update thread requires holding the read lock of versionLock.
2) Authz changes before holding the write lock can only be sent in a
previous topic update or in the next topic update after reset().
3) No catalog objects are skipped in the topic update right after
reset(). See changes in GetCatalogDeltaContext.
Thus, all metadata with catalog version <= lastResetCatalogVersion can be
considered stale after coordinator finish processing the topic update.
lastResetCatalogVersion + 1 is the lower bound (included) of min catalog
object version of a coordinator.
Tests:
- Recover all existing tests that have been disabled due to this
missing feature
Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
---
M be/src/service/impala-server.cc
M be/src/util/impalad-metrics.cc
M be/src/util/impalad-metrics.h
M common/thrift/CatalogObjects.thrift
M common/thrift/metrics.json
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M tests/authorization/test_grant_revoke.py
M tests/authorization/test_ranger.py
M tests/common/skip.py
M tests/custom_cluster/test_local_catalog.py
M tests/metadata/test_hms_integration.py
M tests/metadata/test_metadata_query_statements.py
14 files changed, 179 insertions(+), 104 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/14307/4
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Vihang Karajgaonkar, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14307
to look at the new patch set (#12).
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
The minimal catalog object version of valid catalog objects is used to
implement global invalidate metadata in legacy catalog mode. Coordinator
sends DDL RPC to catalogd for global invalidate metadata and gets the
expected min catalog version in the response. It's the version when
catalogd starts to reset the entire catalog, which means when the reset
is done, all valid catalog objects should be associated with a catalog
version larger than it. Coordinator will wait until its min catalog
version exceeds this value, which means it has processed all the updates
of the reset propagated from the catalogd via statestored. If SYNC_DDL
is set, the coordinator will also wait until other coordinators reach
the same statestore topic version with it, so they have also processed
the same updates and had the latest catalog after reset.
In local catalog mode, the coordinator does not cache all the metadata.
Instead, it caches them on-demand (based on query requests), and removes
them based on the Guava cache configurations (size or TTL) or explicit
invalidation from the catalog topic updates. So it's hard to track the
minimal catalog object version correctly.
This patch adds a new field (lastResetCatalogVersion) in TCatalog to
propagate the catalog version when catalogd starts to reset the entire
metadata. Each time when catalogd generates a new topic update, it will
generate a TCatalogObject of CATALOG type containing the state of the
catalog which includes this new field.
When coordinator receives a new value of lastResetCatalogVersion in a
topic update, it means catalogd has reset the entire catalog.
Coordinator will then clear its cache to remove all stale catalog
objects. It's possible that some fresh items being removed too. They
will be refetched on demand.
After the invalidation, there are no catalog object cached with catalog
version <= lastResetCatalogVersion. Because stale cache has been cleared
and all metadata from catalogd is newer than lastResetCatalogVersion. So
lastResetCatalogVersion + 1 is the lower bound (included) of min catalog
object version of a coordinator.
This patch also exposes the lower bound of catalog object version of via
a new metric "catalog.catalog-object-version-lower-bound" to ease
debugging.
IMPALA-9136 is also fixed in this patch.
Tests:
- Recover all existing tests that have been disabled due to this
missing feature
- Add custom cluster test for concurrent DDLs with INVALIDATE METADATA
- Run CORE tests
Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
---
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/util/impalad-metrics.cc
M be/src/util/impalad-metrics.h
M common/thrift/CatalogObjects.thrift
M common/thrift/Frontend.thrift
M common/thrift/metrics.json
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/catalog/CatalogObjectImpl.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M tests/authorization/test_grant_revoke.py
M tests/authorization/test_ranger.py
M tests/common/skip.py
A tests/custom_cluster/test_concurrent_ddls.py
M tests/custom_cluster/test_local_catalog.py
M tests/metadata/test_hms_integration.py
M tests/metadata/test_metadata_query_statements.py
19 files changed, 385 insertions(+), 117 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/14307/12
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 12
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 13: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 13
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Nov 2019 22:35:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 8:
(8 comments)
Thanks for your comments! Sorry for uploading the buggy patch set 8. I finally revert the changes for IMPALA-9062.
> Is it possible to use this solution for catalog v1 as well, and remove the CatalogObjectVersionSet thing? I think that thing is buggy/complex, maybe would be nice to just have one implementation to worry about. (ok to do that in a follow-up if it's not trivial, or if you think it's a risky change, maybe we shouldn't do it at all)
I think it needs a careful design to remove CatalogObjectVersionSet in catalog-v1. The reason we introduced it is step-by-step:
1) We don't want topic update blocks concurrent DDL/DMLs (IMPALA-5058). So when gathering topic update, we read current catalog version (as toVersion) and free the version lock to let concurrent DDL/DMLs acquire it.
2) Some rapidly changed tables are skipped in topic update since their catalog version exceeds toVersion. Note that if a table is skipped due to version too large in 2 times (controlled by MAX_NUM_SKIPPED_TOPIC_UPDATES), we'll force it to be collected once.
3) Coordinators still have stale cache for these tables until receiving them in topic update. CatalogObjectVersionSet is used to know whether there are stale cached tables. If so, "invalidate metadata" still needs to wait.
It might be feasible that we simply reset the cache in v1 when receiving lastResetStartVersion changed, i.e. all tables come back to IncompleteTable and associate them with the version as lastResetStartVersion. But we are lack of test coverage on concurrent DDL/DMLs to support such a big change.
I plan to add enough test coverage before uploading a new patch. Thanks again for your comments!
http://gerrit.cloudera.org:8080/#/c/14307/8/be/src/util/impalad-metrics.h
File be/src/util/impalad-metrics.h:
http://gerrit.cloudera.org:8080/#/c/14307/8/be/src/util/impalad-metrics.h@119
PS8, Line 119: /// Minimal catalog object version in local catalog cache of Coordinator.
> this isn't actually true -- it's no longer the minimum, but rather a lower
Yeah, done.
http://gerrit.cloudera.org:8080/#/c/14307/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/14307/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@604
PS8, Line 604: lastResetStartVersion
> nit, I think its better to make this variable as final (same for fromVersio
Good point!
http://gerrit.cloudera.org:8080/#/c/14307/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@763
PS8, Line 763: catalog.setCatalog(new TCatalog(catalogServiceId_, ctx.lastResetStartVersion));
> Just a thought to further optimize this. If we know that the lastResetStart
Yeah, good point! I'd like to have a try.
http://gerrit.cloudera.org:8080/#/c/14307/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1135
PS8, Line 1135: catalogTbl.setTable(new TTable(tbl.db_.getName(), tbl.name_));
: catalogTbl.setCatalog_version(tbl.getCatalogVersion());
> I think its really good that we are not serializing the whole table object
Yes, these changes do introduce bugs. The GVO failure is due to I moved the checking of "tbl.getCatalogVersion() <= ctx.fromVersion" before acquiring the table lock...
I finally closed IMPALA-9062 since it's wrong. Though we just need the catalog version in catalog-v2, we still need to acquire the table lock. It's possible that a concurrent thread is holding the table lock and bumping its version into the (fromVersion, toVersion] range. E.g. in AlterTable, we increase and get a new catalog version before modifying the table. Table version is only bumped when operation succeeds. The table lock is held during the operation. So without waiting on the table lock, the table version we see may lower than fromVersion but is going to bump into the range. We will end up missing this table forever.
http://gerrit.cloudera.org:8080/#/c/14307/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1138
PS8, Line 1138: return
> Do we need to add this to topicUpdateEntry before returning? Otherwise, any
Updating topicUpdateLog_ is done inside ctx.addCatalogObject(). However, I'm going to remove this if-clause since we do need the table lock...
http://gerrit.cloudera.org:8080/#/c/14307/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1587
PS8, Line 1587: lastResetStartVersion_ = startVersion;
> What if there are two concurrent calls to INVALIDATE METADATA? is it possib
Nice find! We should avoid lastResetStartVersion_ going downward. I'm thinking adding more test coverage on concurrent DDL/DMLs to avoid bugs like this.
http://gerrit.cloudera.org:8080/#/c/14307/8/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:
http://gerrit.cloudera.org:8080/#/c/14307/8/tests/authorization/test_grant_revoke.py@364
PS8, Line 364: self.client.execute("grant role {0} to group foobar".format(role_name))
> why were these changes necessary?
I found the bug in line 372 and 375 so want to fix them by the way since this test also uses "invalidate metadata"...
For these two lines, they are needed otherwise user "foobar" and "FOOBAR" don't have privilege to create database below.
http://gerrit.cloudera.org:8080/#/c/14307/8/tests/authorization/test_ranger.py
File tests/authorization/test_ranger.py:
http://gerrit.cloudera.org:8080/#/c/14307/8/tests/authorization/test_ranger.py@a60
PS8, Line 60:
> so we don't need --use_local_catalog passed to catalogd anymore?
Yes, catalogd only cares the catalog_topic_mode.
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 8
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 29 Oct 2019 13:36:32 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 8:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4842/ : 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/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 8
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Mon, 21 Oct 2019 13:20:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 7: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 7
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Mon, 21 Oct 2019 05:50:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 10:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/14307/10/tests/custom_cluster/test_concurrent_ddls.py
File tests/custom_cluster/test_concurrent_ddls.py:
http://gerrit.cloudera.org:8080/#/c/14307/10/tests/custom_cluster/test_concurrent_ddls.py@170
PS10, Line 170: c
flake8: E306 expected 1 blank line before a nested definition, found 0
http://gerrit.cloudera.org:8080/#/c/14307/10/tests/custom_cluster/test_concurrent_ddls.py@175
PS10, Line 175: d
flake8: E306 expected 1 blank line before a nested definition, found 0
http://gerrit.cloudera.org:8080/#/c/14307/10/tests/custom_cluster/test_concurrent_ddls.py@189
PS10, Line 189: e
flake8: F841 local variable 'e' is assigned to but never used
http://gerrit.cloudera.org:8080/#/c/14307/10/tests/stress/test_ddl_stress.py
File tests/stress/test_ddl_stress.py:
http://gerrit.cloudera.org:8080/#/c/14307/10/tests/stress/test_ddl_stress.py@18
PS10, Line 18: import time
flake8: F401 'time' imported but unused
http://gerrit.cloudera.org:8080/#/c/14307/10/tests/stress/test_ddl_stress.py@20
PS10, Line 20: from random import random
flake8: F401 'random.random' imported but unused
http://gerrit.cloudera.org:8080/#/c/14307/10/tests/stress/test_ddl_stress.py@22
PS10, Line 22: from tests.beeswax.impala_beeswax import ImpalaBeeswaxException
flake8: F401 'tests.beeswax.impala_beeswax.ImpalaBeeswaxException' imported but unused
http://gerrit.cloudera.org:8080/#/c/14307/10/tests/stress/test_ddl_stress.py@23
PS10, Line 23: from tests.common.impala_test_suite import ImpalaTestSuite, IMPALAD_HOST_PORT_LIST
flake8: F401 'tests.common.impala_test_suite.IMPALAD_HOST_PORT_LIST' imported but unused
http://gerrit.cloudera.org:8080/#/c/14307/10/tests/stress/test_ddl_stress.py@96
PS10, Line 96:
flake8: W391 blank line at end of file
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 10
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Nov 2019 16:02:15 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14307/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/14307/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1104
PS5, Line 1104: if (ctx.collectFullUpdates || tbl.getCatalogVersion() <= ctx.toVersion) {
> I've tried tracking the min catalog versions in local-catalog mode coordina
For your concerns, I think it's ok since we already have the logic to force the table being propogated if it has been skipped more than MAX_NUM_SKIPPED_TOPIC_UPDATES(2) times. Why can't we do the same thing just after global invalidate metadata?
If there are too many concurrent DDLs, global invalidate metadata may also hang for a while in the existing implementation. Because the coordinator is waiting for catalogd to propogate the skipped table, and catalogd is trying to propogate it after skips it 2 times but waiting for acquiring the table lock that is hold by the concurrent DDLs. (same as IMPALA-6671)
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Wed, 16 Oct 2019 02:41:21 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4827/ : 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/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Fri, 18 Oct 2019 14:45:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has restored this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Restored
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: restore
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 13: Code-Review+2
Thanks for making the changes. This would help fill an important gap in catalog-v2 for the users.
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 13
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Nov 2019 18:02:02 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 8:
> Patch Set 6:
>
> (4 comments)
>
> Thanks Vihang's comments!
>
> > I looks like you are sending all the objects in the catalogd when you detect the reset. Is it possible to not send the objects and just invalidate the whole local catalog cache when you detect that the last reset version is changed?
>
> Reseting coordinator's local catalog is ok. But coordinator needs to know when the reset() in catalogd finishs. For SYNC_DDL, it also needs to know the catalog topic version (not catalog version, it's the version of the statestore topic) so it can wait for other coordinators to be ready. I have an explanation with code links in "Why can’t we simply reset the cache of local catalog mode Coordinator for global INVALIDATE METADATA?" in the doc: https://docs.google.com/document/d/1-AoigzsQPgSGosW4vtVP8E7TiwoJJfLkAq3Rd6KvKBg
Ah, I realize what you say is different! It's a good idea to invalidate the whole cache when detecting last reset version is changed! Then we no longer need to change the skipping logics in catalogd, making this patch much simpler.
Update the patch according to this. BTW, still contains the fix for IMPALA-9062.
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 8
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Mon, 21 Oct 2019 12:41:50 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 6:
(4 comments)
Thanks Vihang's comments!
> I looks like you are sending all the objects in the catalogd when you detect the reset. Is it possible to not send the objects and just invalidate the whole local catalog cache when you detect that the last reset version is changed?
Reseting coordinator's local catalog is ok. But coordinator needs to know when the reset() in catalogd finishs. For SYNC_DDL, it also needs to know the catalog topic version (not catalog version, it's the version of the statestore topic) so it can wait for other coordinators to be ready. I have an explanation with code links in "Why can’t we simply reset the cache of local catalog mode Coordinator for global INVALIDATE METADATA?" in the doc: https://docs.google.com/document/d/1-AoigzsQPgSGosW4vtVP8E7TiwoJJfLkAq3Rd6KvKBg
http://gerrit.cloudera.org:8080/#/c/14307/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14307/5//COMMIT_MSG@41
PS5, Line 41: 1) No topic updates are sent from catalogd when the write lock of
: versionLock is held in CatalogServiceCatalog.reset(). Note that the
: update thread requires holding the read lock of versionLock.
: 2) Authz changes before holding the write lock can only be sent in a
: previous topic update or in the next topic update after reset().
: 3) No catalog objects are skipped in the topic update right after
: reset(). See changes in GetCatalogDeltaContext
> Did you consider the case when a reset is executed during execution of getC
Oh, it's possible when reset() runs again after reset(), so getCatalogDelta thread is running with collectFullUpdates being set and will collect everything.
In normal cases if getCatalogDelta() and reset() run concurrently, it's ok since getCatalogDelta runs with collectFullUpdates unset so will only collect updates in range of (fromVersion, toVersion]. After reset() holding the write lock, the updates are with version larger than toVersion.
I'll update the proof here. It's ok for a previous update to contain some results of the reset(). The most important update, CATALOG type TCatalog object, can only get the correct lastResetCatalogVersion_ after reset() finishes. Before that, it just gets an old value of lastResetCatalogVersion_. (lastResetCatalogVersion_ is updated at the end of reset())
http://gerrit.cloudera.org:8080/#/c/14307/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/14307/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@721
PS5, Line 721: public long getCatalogDelta(long nativeCatalogServerPtr, long fromVersion) throws
: TException {
: long toVersion;
: boolean collectFullUpdates;
: versionLock_.readLock().lock();
: try {
: toVersion = catalogVersion_;
:
> what happens if the reset thread takes a write lock after this code block?
Then toVersion is small enough that won't cover updates after reset() acquiring the write lock. However, if reset() runs again after reset(), hasResetCatalog is true so all updates will be collected. As in the above comment, I think it's ok for a previous update to contain some results of a running reset() as long as it don't propagate the lastResetCatalogVersion_.
BTW, with the changes in addTableToCatalogDeltaHelper(), in local catalog mode we no longer need to acquire the table locks. So collecting all table updates won't be blocked by concurrent DDLs.
http://gerrit.cloudera.org:8080/#/c/14307/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1104
PS5, Line 1104: * which would also violate the semantics of SYNC_DDL.
> Does the collectFullUpdates flag come in play in v1 as well? If yes, that s
Yes, it should be used only in MINIMAL topic mode. I'll constrain this.
http://gerrit.cloudera.org:8080/#/c/14307/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1601
PS5, Line 1601: sing for id just befo
> It looks like the currentCatalogVersion may not be the right reset version
Sorry, I should rename it to "lastResetCatalogVersion" or "lastResetBeginVersion" to avoid confusion. It's the same version that we return to the coordinator, meaning that all catalog objects with versions <= this will be invalidated. Coordinator first gets this version in the RPC response, then when receiving this again in the CATALOG type TCatalog object update (via statestore topic update), it knows that reset() has finished and all results are received.
Propagating (catalogVersion_-1) is not we want since it's the current version - 1 after reset().
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Sat, 19 Oct 2019 02:02:58 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 13:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5197/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 13
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Nov 2019 18:02:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 8: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5120/
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 8
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Oct 2019 00:02:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 12:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14307/12/tests/custom_cluster/test_local_catalog.py
File tests/custom_cluster/test_local_catalog.py:
http://gerrit.cloudera.org:8080/#/c/14307/12/tests/custom_cluster/test_local_catalog.py@440
PS12, Line 440: g
flake8: F821 undefined name 'get_catalog_cache_metrics'
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 12
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Nov 2019 04:05:24 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 6:
(4 comments)
I looks like you are sending all the objects in the catalogd when you detect the reset. Is it possible to not send the objects and just invalidate the whole local catalog cache when you detect that the last reset version is changed?
http://gerrit.cloudera.org:8080/#/c/14307/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14307/5//COMMIT_MSG@41
PS5, Line 41: 1) No topic updates are sent from catalogd when the write lock of
: versionLock is held in CatalogServiceCatalog.reset(). Note that the
: update thread requires holding the read lock of versionLock.
: 2) Authz changes before holding the write lock can only be sent in a
: previous topic update or in the next topic update after reset().
: 3) No catalog objects are skipped in the topic update right after
: reset(). See changes in GetCatalogDeltaContext
Did you consider the case when a reset is executed during execution of getCatalogDelta? Looks like it is possible for this to happen since the getCatalogDelta takes a read lock and releases in the for loop. So the reset thread can take a write lock in between the getCatalogDelta which effectively means that the topic may have some state before reset and some state after the reset.
http://gerrit.cloudera.org:8080/#/c/14307/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/14307/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@721
PS5, Line 721: public long getCatalogDelta(long nativeCatalogServerPtr, long fromVersion) throws
: TException {
: long toVersion;
: boolean collectFullUpdates;
: versionLock_.readLock().lock();
: try {
: toVersion = catalogVersion_;
:
what happens if the reset thread takes a write lock after this code block?
http://gerrit.cloudera.org:8080/#/c/14307/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1104
PS5, Line 1104: * which would also violate the semantics of SYNC_DDL.
> I see what you are saying, nice find. Agree that we shouldn't take any lock
Does the collectFullUpdates flag come in play in v1 as well? If yes, that seems unnecessary.
http://gerrit.cloudera.org:8080/#/c/14307/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1601
PS5, Line 1601: sing for id just befo
It looks like the currentCatalogVersion may not be the right reset version since once we assign currentCatalogVersion and the place where we take the write lock is not atomic. May be you should directly use (catalogVersion_-1) here since you already hold the write lock
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Fri, 18 Oct 2019 17:00:05 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4765/ : 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/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 09 Oct 2019 16:07:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4772/ : 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/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 09 Oct 2019 23:22:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Vihang Karajgaonkar, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14307
to look at the new patch set (#13).
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
The minimal catalog object version of valid catalog objects is used to
implement global invalidate metadata in legacy catalog mode. Coordinator
sends DDL RPC to catalogd for global invalidate metadata and gets the
expected min catalog version in the response. It's the version when
catalogd starts to reset the entire catalog, which means when the reset
is done, all valid catalog objects should be associated with a catalog
version larger than it. Coordinator will wait until its min catalog
version exceeds this value, which means it has processed all the updates
of the reset propagated from the catalogd via statestored. If SYNC_DDL
is set, the coordinator will also wait until other coordinators reach
the same statestore topic version with it, so they have also processed
the same updates and had the latest catalog after reset.
In local catalog mode, the coordinator does not cache all the metadata.
Instead, it caches them on-demand (based on query requests), and removes
them based on the Guava cache configurations (size or TTL) or explicit
invalidation from the catalog topic updates. So it's hard to track the
minimal catalog object version correctly.
This patch adds a new field (lastResetCatalogVersion) in TCatalog to
propagate the catalog version when catalogd starts to reset the entire
metadata. Each time when catalogd generates a new topic update, it will
generate a TCatalogObject of CATALOG type containing the state of the
catalog which includes this new field.
When coordinator receives a new value of lastResetCatalogVersion in a
topic update, it means catalogd has reset the entire catalog.
Coordinator will then clear its cache to remove all stale catalog
objects. It's possible that some fresh items being removed too. They
will be refetched on demand.
After the invalidation, there are no catalog object cached with catalog
version <= lastResetCatalogVersion. Because stale cache has been cleared
and all metadata from catalogd is newer than lastResetCatalogVersion. So
lastResetCatalogVersion + 1 is the lower bound (included) of min catalog
object version of a coordinator.
This patch also exposes the lower bound of catalog object version of via
a new metric "catalog.catalog-object-version-lower-bound" to ease
debugging.
IMPALA-9136 is also fixed in this patch.
Tests:
- Recover all existing tests that have been disabled due to this
missing feature
- Add custom cluster test for concurrent DDLs with INVALIDATE METADATA
- Run CORE tests
Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
---
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/util/impalad-metrics.cc
M be/src/util/impalad-metrics.h
M common/thrift/CatalogObjects.thrift
M common/thrift/Frontend.thrift
M common/thrift/metrics.json
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/catalog/CatalogObjectImpl.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M tests/authorization/test_grant_revoke.py
M tests/authorization/test_ranger.py
M tests/common/skip.py
A tests/custom_cluster/test_concurrent_ddls.py
M tests/custom_cluster/test_local_catalog.py
M tests/metadata/test_hms_integration.py
M tests/metadata/test_metadata_query_statements.py
19 files changed, 383 insertions(+), 115 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/14307/13
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 13
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 7:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5118/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 7
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Mon, 21 Oct 2019 01:32:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 7:
Thanks Vihang's comment! There is a bug when getCatalogDelta() runs concurrently with reset(). If reset() finishes first, it'll update lastResetCatalogVersion_ (now renamed to lastResetStartVersion_) and this got propagated at the end of getCatalogDelta(). We should keep the value of lastResetCatalogVersion_ when creating GetCatalogDeltaContext and use it instead. Fixed this bug.
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 7
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Sat, 19 Oct 2019 05:14:07 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14307/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/14307/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1104
PS5, Line 1104: if (ctx.collectFullUpdates || tbl.getCatalogVersion() <= ctx.toVersion) {
> For your concerns, I think it's ok since we already have the logic to force
I think the concerns of concurrent DDLs can be fixed by IMPALA-9062, that we don't need to acquire table locks in minimal topic mode, since we just need the db name, table name and catalog version.
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 17 Oct 2019 03:10:18 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 12:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4982/ : 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/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 12
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Nov 2019 04:48:52 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Vihang Karajgaonkar, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14307
to look at the new patch set (#10).
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
The minimal catalog object version of valid catalog objects is used to
implement global invalidate metadata in legacy catalog mode. Coordinator
sends DDL RPC to catalogd for global invalidate metadata and gets the
expected min catalog version in the response. It's the version when
catalogd starts to reset the entire catalog, which means when the reset
is done, all valid catalog objects should be associated with a catalog
version larger than it. Coordinator will wait until its min catalog
version exceeds this value, which means it has processed all the updates
of the reset propagated from the catalogd via statestored. If SYNC_DDL
is set, the coordinator will also wait until other coordinators reach
the same statestore topic version with it, so they have also processed
the same updates and had the latest catalog after reset.
In local catalog mode, the coordinator does not cache all the metadata.
Instead, it caches them on-demand (based on query requests), and removes
them based on the Guava cache configurations (size or TTL) or explicit
invalidation from the catalog topic updates. So it's hard to track the
minimal catalog object version correctly.
This patch adds a new field (lastResetCatalogVersion) in TCatalog to
propagate the catalog version when catalogd starts to reset the entire
metadata. Each time when catalogd generates a new topic update, it will
generate a TCatalogObject of CATALOG type containing the state of the
catalog which includes this new field.
When coordinator receives a new value of lastResetCatalogVersion in a
topic update, it means catalogd has reset the entire catalog.
Coordinator will then clear its cache to remove all stale catalog
objects. It's possible that some fresh items being removed too. They
will be refetched on demand.
After the invalidation, there are no catalog object cached with catalog
version <= lastResetCatalogVersion. Because stale cache has been cleared
and all metadata from catalogd is newer than lastResetCatalogVersion. So
lastResetCatalogVersion + 1 is the lower bound (included) of min catalog
object version of a coordinator.
This patch also exposes the lower bound of catalog object version of via
a new metric "catalog.catalog-object-version-lower-bound" to ease
debugging.
Tests:
- Recover all existing tests that have been disabled due to this
missing feature
- Add custom cluster test for concurrent DDLs with INVALIDATE METADATA
- Run CORE tests
Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
---
M be/src/service/impala-server.cc
M be/src/util/impalad-metrics.cc
M be/src/util/impalad-metrics.h
M common/thrift/CatalogObjects.thrift
M common/thrift/metrics.json
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M tests/authorization/test_grant_revoke.py
M tests/authorization/test_ranger.py
M tests/common/skip.py
A tests/custom_cluster/test_concurrent_ddls.py
M tests/custom_cluster/test_local_catalog.py
M tests/metadata/test_hms_integration.py
M tests/metadata/test_metadata_query_statements.py
M tests/stress/test_ddl_stress.py
16 files changed, 363 insertions(+), 108 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/14307/10
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 10
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has removed Todd Lipcon from this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Removed reviewer Todd Lipcon.
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14307/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/14307/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1104
PS5, Line 1104: if (ctx.collectFullUpdates || tbl.getCatalogVersion() <= ctx.toVersion) {
> I'm a little curious why this approach instead of tracking the min versions
I've tried tracking the min catalog versions in local-catalog mode coordinator (Solution 2 in the doc: https://docs.google.com/document/d/1-AoigzsQPgSGosW4vtVP8E7TiwoJJfLkAq3Rd6KvKBg). But finally I found we need to replace Guava cache with something else that supports holding an external lock while evicting items and calling the RemovalListener.onRemoval(). The reason is:
- To delete catalog versions due to cache eviction resulting from timed expiration or exceeding a maximum size, we have to use RemovalListener. Then we only call CatalogObjectVersionSet.removeVersion() in our customized RemovalListener.onRemoval().
- We have no way to introduce a lock protecting the deletion of the cache item and the deletion of its catalog version.
- Without the lock, there's a chance for a version not being removed forever:
- - Thread 1 in CatalogdMetaProvider.loadWithCaching() put a new catalog object (with version v1) into the Guava cache
- - Thread 2 in CatalogdMetaProvider.invalidateCacheForObject() remove this catalog object
- - Thread 2 in RemovalListener.onRemoval() calls CatalogObjectVersionSet.removeVersion(v1). It's ignored since v1 hasn't been added yet.
- - Thread 1 in CatalogdMetaProvider.loadWithCaching() calls CatalogObjectVersionSet.addVersion(v1). v1 will permanantly exist in the version set, causing INVALIDATE METADATA to hang.
One approach to fix this case is adding a wait in CatalogObjectVersionSet.removeVersion() until the version is added. But the code paths of legacy catalog mode assumes that a version can be removed several times, and CatalogObjectVersionSet.removeVersion can be called on an unexisting version. Fixsing the assumption is also complex. Guava doc also mentions that we should avoid performing blocking calls or synchronizing on shared resources in the RemovalListener.
I've also written down other problems in the doc. The main obstacle is the lack of protection on modifying Guava cache and the version set in the same time. Any thoughts are welcome.
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Mon, 14 Oct 2019 10:20:54 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 10:
(1 comment)
Added custom cluster tests to cover bugs found in the review, i.e.
1) get table version without acquiring table lock in CatalogServiceCatalog.addTableToCatalogDeltaHelper()
2) concurrent INVALIDATE METADATA commands could downward the lastResetStartVersion_
http://gerrit.cloudera.org:8080/#/c/14307/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/14307/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@763
PS8, Line 763: catalog.setCatalog(new TCatalog(catalogServiceId_, ctx.lastResetStartVersion));
> Yeah, good point! I'd like to have a try.
I finally found that we should at least add the updated dbs and tables into topicUpdateLog_, otherwise DDLs with sync_ddl will hang in waitForSyncDdlVersion().
To add them into topicUpdateLog_, we should at least get their catalog versions. For tables we still need to acquire the table locks. So the existing code paths in this function can't be ignored.
Finally, we still collect all TCatalogObjects for updated tables. In catalog-v2, these objects just contain table names. It's already light weght enough. So I don't want to add more if-else clauses to ignore them...
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 10
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Nov 2019 16:05:12 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 13:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4983/ : 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/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 13
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Nov 2019 05:52:31 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/14307/4/tests/custom_cluster/test_local_catalog.py
File tests/custom_cluster/test_local_catalog.py:
http://gerrit.cloudera.org:8080/#/c/14307/4/tests/custom_cluster/test_local_catalog.py@33
PS4, Line 33: def get_catalog_cache_metrics(impalad, prefix):
> flake8: E302 expected 2 blank lines, found 1
Done
http://gerrit.cloudera.org:8080/#/c/14307/4/tests/custom_cluster/test_local_catalog.py@48
PS4, Line 48: def get_min_catalog_object_version(impalad):
> flake8: E302 expected 2 blank lines, found 1
Done
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 09 Oct 2019 22:37:52 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 8:
(5 comments)
The approach looks good to me. Took another pass at this and left some suggestions below. Thanks!
http://gerrit.cloudera.org:8080/#/c/14307/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/14307/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@604
PS8, Line 604: lastResetStartVersion
nit, I think its better to make this variable as final (same for fromVersion, toVersion and nativeCatalogServerPtr)
http://gerrit.cloudera.org:8080/#/c/14307/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@763
PS8, Line 763: catalog.setCatalog(new TCatalog(catalogServiceId_, ctx.lastResetStartVersion));
Just a thought to further optimize this. If we know that the lastResetStartVersion has changed here and if we are using local catalog mode, do we need to send all the dbs, tables, etc? Can we just send a light weight topic update which only has a TCatalogObject? It seems like on the coordinator side, all we really care of is the min catalog version, current catalog version which is both available in the TCatalogObject type. Do you see any problems with this?
http://gerrit.cloudera.org:8080/#/c/14307/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1135
PS8, Line 1135: catalogTbl.setTable(new TTable(tbl.db_.getName(), tbl.name_));
: catalogTbl.setCatalog_version(tbl.getCatalogVersion());
I think its really good that we are not serializing the whole table object here. However, I am not sure if can do this without taking the tbl lock. For instance, we are getting the tbl version couple of times here. It is possible that the version changes between line 1128 and 1136. Can the table name change here too from another thread?
http://gerrit.cloudera.org:8080/#/c/14307/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1138
PS8, Line 1138: return
Do we need to add this to topicUpdateEntry before returning? Otherwise, any other DDL with sync_ddl turned on will not be able to find a covering topic for this object. right?
http://gerrit.cloudera.org:8080/#/c/14307/8/tests/authorization/test_ranger.py
File tests/authorization/test_ranger.py:
http://gerrit.cloudera.org:8080/#/c/14307/8/tests/authorization/test_ranger.py@a60
PS8, Line 60:
so we don't need --use_local_catalog passed to catalogd anymore?
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 8
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Mon, 28 Oct 2019 23:58:21 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 11:
Passed exhaustive tests: https://jenkins.impala.io/job/pre-review-test/447/
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 11
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Wed, 06 Nov 2019 23:16:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 2:
Just to capture the summary of our discussion, after IMPALA-5058, Catalog server can propagate out of band updates especially if a table is being modified repeatedly. So we cannot rely on the [start-end] versions of each update to track the min version and we might need something more sophisticated.
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 26 Sep 2019 20:09:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Vihang Karajgaonkar, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14307
to look at the new patch set (#11).
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
The minimal catalog object version of valid catalog objects is used to
implement global invalidate metadata in legacy catalog mode. Coordinator
sends DDL RPC to catalogd for global invalidate metadata and gets the
expected min catalog version in the response. It's the version when
catalogd starts to reset the entire catalog, which means when the reset
is done, all valid catalog objects should be associated with a catalog
version larger than it. Coordinator will wait until its min catalog
version exceeds this value, which means it has processed all the updates
of the reset propagated from the catalogd via statestored. If SYNC_DDL
is set, the coordinator will also wait until other coordinators reach
the same statestore topic version with it, so they have also processed
the same updates and had the latest catalog after reset.
In local catalog mode, the coordinator does not cache all the metadata.
Instead, it caches them on-demand (based on query requests), and removes
them based on the Guava cache configurations (size or TTL) or explicit
invalidation from the catalog topic updates. So it's hard to track the
minimal catalog object version correctly.
This patch adds a new field (lastResetCatalogVersion) in TCatalog to
propagate the catalog version when catalogd starts to reset the entire
metadata. Each time when catalogd generates a new topic update, it will
generate a TCatalogObject of CATALOG type containing the state of the
catalog which includes this new field.
When coordinator receives a new value of lastResetCatalogVersion in a
topic update, it means catalogd has reset the entire catalog.
Coordinator will then clear its cache to remove all stale catalog
objects. It's possible that some fresh items being removed too. They
will be refetched on demand.
After the invalidation, there are no catalog object cached with catalog
version <= lastResetCatalogVersion. Because stale cache has been cleared
and all metadata from catalogd is newer than lastResetCatalogVersion. So
lastResetCatalogVersion + 1 is the lower bound (included) of min catalog
object version of a coordinator.
This patch also exposes the lower bound of catalog object version of via
a new metric "catalog.catalog-object-version-lower-bound" to ease
debugging.
Tests:
- Recover all existing tests that have been disabled due to this
missing feature
- Add custom cluster test for concurrent DDLs with INVALIDATE METADATA
- Run CORE tests
Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
---
M be/src/service/impala-server.cc
M be/src/util/impalad-metrics.cc
M be/src/util/impalad-metrics.h
M common/thrift/CatalogObjects.thrift
M common/thrift/metrics.json
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M tests/authorization/test_grant_revoke.py
M tests/authorization/test_ranger.py
M tests/common/skip.py
A tests/custom_cluster/test_concurrent_ddls.py
M tests/custom_cluster/test_local_catalog.py
M tests/metadata/test_hms_integration.py
M tests/metadata/test_metadata_query_statements.py
15 files changed, 360 insertions(+), 107 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/14307/11
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 11
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/14307/4/tests/custom_cluster/test_local_catalog.py
File tests/custom_cluster/test_local_catalog.py:
http://gerrit.cloudera.org:8080/#/c/14307/4/tests/custom_cluster/test_local_catalog.py@33
PS4, Line 33: def get_catalog_cache_metrics(impalad, prefix):
flake8: E302 expected 2 blank lines, found 1
http://gerrit.cloudera.org:8080/#/c/14307/4/tests/custom_cluster/test_local_catalog.py@48
PS4, Line 48: def get_min_catalog_object_version(impalad):
flake8: E302 expected 2 blank lines, found 1
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 09 Oct 2019 15:26:17 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14307
to look at the new patch set (#2).
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
In local catalog mode, the coordinator does not cache all the metadata.
Instead, it caches them on-demand (based on query requests), and
removes them based on the Guava cache configurations (e.g. size or TTL).
We use the catalog version as part of the cache key for fine-grained
metadata, e.g partition meta. When invalidating a table, we simply
invalidate the top-level table entry, and allow other information to
remain in the cache. The old metadata will be lazily removed by Guava
cache since they won't be touched anymore. Thus, there're bunch of stale
metadata in the cache so we can't track the minimal catalog version of
valid catalog objects efficiently.
The minimal catalog version of valid catalog objects is used to
implement global invalidate metadata. In legacy catalog mode, all cached
catalog objects are valid in fact. Coordinator gets the expected min
catalog version in the RPC response from Catalogd. It's the version when
Catalogd starts to reset the entire catalog, which means when the reset
is done, all valid catalog objects should be associated with a catalog
version larger than it. Coordinator will wait until its min catalog
version exceeds this value, which means it has processed all the updates
of the reset propagated from the catalogd via statestored. If SYNC_DDL
is set, the coordinator will also wait until other coordinators reach
the same catalog version with it, so they can also see the latest update
of reset.
This patch adds a new field (lastResetCatalogVersion) in TCatalog to
keep the catalog version when catalogd starts to reset the entire
metadata. Each time when catalogd generates a new topic update for
catalog topic, it will generate a TCatalogObject in CATALOG type
containing the state of the catalog which includes this new field.
When coordinator receives a new value of lastResetCatalogVersion in a
topic update, it means catalogd has reset the entire catalog and all the
relative updates are whether included in the same or previous topic
updates. This is guaranteed by the fact that the write lock of
versionLock is held when catalogd resetting the entire catalog, and we
update lastResetCatalogVersion at last before releasing the write lock.
So the update thread which requires holding the read lock of versionLock
don't have chance to propagate partial results with the new
lastResetCatalogVersion value. Thus, all metadata with catalog version <=
lastResetCatalogVersion can be considered stale after coordinator finish
processing the topic update. lastResetCatalogVersion + 1 is the lower
bound (included) of min catalog version of a coordinator.
Tests:
- Recover all existing tests that have been disabled due to this
missing feature
Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M tests/authorization/test_ranger.py
M tests/common/skip.py
M tests/custom_cluster/test_local_catalog.py
M tests/metadata/test_hms_integration.py
M tests/metadata/test_metadata_query_statements.py
9 files changed, 51 insertions(+), 75 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/14307/2
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 8:
(4 comments)
The patch is good to go from my side. I had some questions and some nits below.
http://gerrit.cloudera.org:8080/#/c/14307/8/be/src/util/impalad-metrics.cc
File be/src/util/impalad-metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14307/8/be/src/util/impalad-metrics.cc@220
PS8, Line 220: 0
Should this be initialized to -1?
http://gerrit.cloudera.org:8080/#/c/14307/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/14307/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@763
PS8, Line 763: catalog.setCatalog(new TCatalog(catalogServiceId_, ctx.lastResetStartVersion));
> I finally found that we should at least add the updated dbs and tables into
I see. Thanks for exploring this suggestion.
http://gerrit.cloudera.org:8080/#/c/14307/11/tests/custom_cluster/test_concurrent_ddls.py
File tests/custom_cluster/test_concurrent_ddls.py:
http://gerrit.cloudera.org:8080/#/c/14307/11/tests/custom_cluster/test_concurrent_ddls.py@152
PS11, Line 152:
:
:
:
:
:
:
:
Can you add a comment explaining why these are acceptable errors? its not very clear to me.
http://gerrit.cloudera.org:8080/#/c/14307/11/tests/custom_cluster/test_local_catalog.py
File tests/custom_cluster/test_local_catalog.py:
http://gerrit.cloudera.org:8080/#/c/14307/11/tests/custom_cluster/test_local_catalog.py@50
PS11, Line 50: get_min_catalog_object_version
Was this function not useful? https://github.com/apache/impala/blob/master/tests/common/impala_service.py#L82
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 8
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Nov 2019 01:45:53 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14307
to look at the new patch set (#5).
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
The minimal catalog object version of valid catalog objects is used to
implement global invalidate metadata in legacy catalog mode. Coordinator
sends DDL RPC to catalogd for global invalidate metadata and gets the
expected min catalog version in the response. It's the version when
catalogd starts to reset the entire catalog, which means when the reset
is done, all valid catalog objects should be associated with a catalog
version larger than it. Coordinator will wait until its min catalog
version exceeds this value, which means it has processed all the updates
of the reset propagated from the catalogd via statestored. If SYNC_DDL
is set, the coordinator will also wait until other coordinators reach
the same statestore topic version with it, so they have also processed
the same updates and had the latest catalog after reset.
In local catalog mode, the coordinator does not cache all the metadata.
Instead, it caches them on-demand (based on query requests), and removes
them based on the Guava cache configurations (size or TTL) or explicit
invalidation from the catalog topic updates. So it's hard to track the
minimal catalog object version correctly.
This patch adds a new field (lastResetCatalogVersion) in TCatalog to
propagate the catalog version when catalogd starts to reset the entire
metadata. Each time when catalogd generates a new topic update, it will
generate a TCatalogObject of CATALOG type containing the state of the
catalog which includes this new field.
To make all changes of the reset being added in the same topic update
with this TCatalog object. Rapidly changed tables that have catalog
version exceeding the version range of this update will also be included.
When coordinator receives a new value of lastResetCatalogVersion in a
topic update, it means catalogd has reset the entire catalog and all the
relative updates are whether included in the same or previous topic
updates. This is guaranteed by three facts:
1) No topic updates are sent from catalogd when the write lock of
versionLock is held in CatalogServiceCatalog.reset(). Note that the
update thread requires holding the read lock of versionLock.
2) Authz changes before holding the write lock can only be sent in a
previous topic update or in the next topic update after reset().
3) No catalog objects are skipped in the topic update right after
reset(). See changes in GetCatalogDeltaContext.
Thus, all metadata with catalog version <= lastResetCatalogVersion can be
considered stale after coordinator finish processing the topic update.
lastResetCatalogVersion + 1 is the lower bound (included) of min catalog
object version of a coordinator.
Tests:
- Recover all existing tests that have been disabled due to this
missing feature
Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
---
M be/src/service/impala-server.cc
M be/src/util/impalad-metrics.cc
M be/src/util/impalad-metrics.h
M common/thrift/CatalogObjects.thrift
M common/thrift/metrics.json
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M tests/authorization/test_grant_revoke.py
M tests/authorization/test_ranger.py
M tests/common/skip.py
M tests/custom_cluster/test_local_catalog.py
M tests/metadata/test_hms_integration.py
M tests/metadata/test_metadata_query_statements.py
14 files changed, 181 insertions(+), 104 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/14307/5
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has abandoned this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Abandoned
There is one unsolvable bug for this solution:
Some tables invalidated in the reset() may be updated frequently while the background thread gets current catalog version and generating topic update delta. These tables will be ignored in the topic update. So it's unsafe for coordinators to advance their min catalog version unless they receive updates of these skipped tables.
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14307/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/14307/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1104
PS5, Line 1104: if (ctx.collectFullUpdates || tbl.getCatalogVersion() <= ctx.toVersion) {
I'm a little curious why this approach instead of tracking the min versions using a data-structure. I think mucking with the logic here makes the code even more complex and harder to reason about. Also since this undos the "skipping logic", it looks like invalidate metadata can potentially increase the likelihood of "hangs" when run concurrently with other DDLs (probably something to keep in mind).
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Mon, 14 Oct 2019 06:55:59 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 11:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/14307/10/tests/custom_cluster/test_concurrent_ddls.py
File tests/custom_cluster/test_concurrent_ddls.py:
http://gerrit.cloudera.org:8080/#/c/14307/10/tests/custom_cluster/test_concurrent_ddls.py@170
PS10, Line 170:
> flake8: E306 expected 1 blank line before a nested definition, found 0
Done
http://gerrit.cloudera.org:8080/#/c/14307/10/tests/custom_cluster/test_concurrent_ddls.py@175
PS10, Line 175: t
> flake8: E306 expected 1 blank line before a nested definition, found 0
Done
http://gerrit.cloudera.org:8080/#/c/14307/10/tests/custom_cluster/test_concurrent_ddls.py@189
PS10, Line 189:
> flake8: F841 local variable 'e' is assigned to but never used
Done
http://gerrit.cloudera.org:8080/#/c/14307/10/tests/stress/test_ddl_stress.py
File tests/stress/test_ddl_stress.py:
http://gerrit.cloudera.org:8080/#/c/14307/10/tests/stress/test_ddl_stress.py@18
PS10, Line 18: import pytest
> flake8: F401 'time' imported but unused
Done
http://gerrit.cloudera.org:8080/#/c/14307/10/tests/stress/test_ddl_stress.py@20
PS10, Line 20: from tests.common.impala_test_suite import ImpalaTestSuite
> flake8: F401 'random.random' imported but unused
Done
http://gerrit.cloudera.org:8080/#/c/14307/10/tests/stress/test_ddl_stress.py@22
PS10, Line 22:
> flake8: F401 'tests.beeswax.impala_beeswax.ImpalaBeeswaxException' imported
Done
http://gerrit.cloudera.org:8080/#/c/14307/10/tests/stress/test_ddl_stress.py@23
PS10, Line 23: # Number of tables to create per thread
> flake8: F401 'tests.common.impala_test_suite.IMPALAD_HOST_PORT_LIST' import
Done
http://gerrit.cloudera.org:8080/#/c/14307/10/tests/stress/test_ddl_stress.py@96
PS10, Line 96:
> flake8: W391 blank line at end of file
Done
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 11
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Wed, 06 Nov 2019 00:23:02 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 8:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5120/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 8
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Mon, 21 Oct 2019 14:02:20 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 12:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14307/12/tests/custom_cluster/test_local_catalog.py
File tests/custom_cluster/test_local_catalog.py:
http://gerrit.cloudera.org:8080/#/c/14307/12/tests/custom_cluster/test_local_catalog.py@440
PS12, Line 440: g
> flake8: F821 undefined name 'get_catalog_cache_metrics'
Oops! Forgot to rollback this line.
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 12
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Nov 2019 05:00:36 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 11: Code-Review+1
Would be great if Todd or someone else also take a look at this. If not, I will be happy to promote my +1 to +2
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 11
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Nov 2019 01:46:46 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Vihang Karajgaonkar, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14307
to look at the new patch set (#8).
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
The minimal catalog object version of valid catalog objects is used to
implement global invalidate metadata in legacy catalog mode. Coordinator
sends DDL RPC to catalogd for global invalidate metadata and gets the
expected min catalog version in the response. It's the version when
catalogd starts to reset the entire catalog, which means when the reset
is done, all valid catalog objects should be associated with a catalog
version larger than it. Coordinator will wait until its min catalog
version exceeds this value, which means it has processed all the updates
of the reset propagated from the catalogd via statestored. If SYNC_DDL
is set, the coordinator will also wait until other coordinators reach
the same statestore topic version with it, so they have also processed
the same updates and had the latest catalog after reset.
In local catalog mode, the coordinator does not cache all the metadata.
Instead, it caches them on-demand (based on query requests), and removes
them based on the Guava cache configurations (size or TTL) or explicit
invalidation from the catalog topic updates. So it's hard to track the
minimal catalog object version correctly.
This patch adds a new field (lastResetCatalogVersion) in TCatalog to
propagate the catalog version when catalogd starts to reset the entire
metadata. Each time when catalogd generates a new topic update, it will
generate a TCatalogObject of CATALOG type containing the state of the
catalog which includes this new field.
When coordinator receives a new value of lastResetCatalogVersion in a
topic update, it means catalogd has reset the entire catalog.
Coordinator will then clear its cache to remove all stale catalog
objects. It's possible that some fresh items being removed too. They
will be refetched on demand.
After the invalidation, there are no catalog object cached with catalog
version <= lastResetCatalogVersion. Because stale cache has been cleared
and all metadata from catalogd is newer than lastResetCatalogVersion. So
lastResetCatalogVersion + 1 is the lower bound (included) of min catalog
object version of a coordinator.
This patch also fixes IMPALA-9062 to avoid catalogd's update collector
thread being blocked by concurrent DDLs that holding the table locks.
In local catalog mode, we just need to propagate the table name of a
changed table, so don't need to acquire table lock to get the entire
TTable object.
This patch also exposes the min catalog object version of coordinator
via a new metric "catalog.min-catalog-object-version" to ease debugging.
Tests:
- Recover all existing tests that have been disabled due to this
missing feature
- Run CORE tests
Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
---
M be/src/service/impala-server.cc
M be/src/util/impalad-metrics.cc
M be/src/util/impalad-metrics.h
M common/thrift/CatalogObjects.thrift
M common/thrift/metrics.json
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M tests/authorization/test_grant_revoke.py
M tests/authorization/test_ranger.py
M tests/common/skip.py
M tests/custom_cluster/test_local_catalog.py
M tests/metadata/test_hms_integration.py
M tests/metadata/test_metadata_query_statements.py
14 files changed, 176 insertions(+), 109 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/14307/8
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 8
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4834/ : 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/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 7
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Sat, 19 Oct 2019 05:55:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Vihang Karajgaonkar, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14307
to look at the new patch set (#7).
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
......................................................................
IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
The minimal catalog object version of valid catalog objects is used to
implement global invalidate metadata in legacy catalog mode. Coordinator
sends DDL RPC to catalogd for global invalidate metadata and gets the
expected min catalog version in the response. It's the version when
catalogd starts to reset the entire catalog, which means when the reset
is done, all valid catalog objects should be associated with a catalog
version larger than it. Coordinator will wait until its min catalog
version exceeds this value, which means it has processed all the updates
of the reset propagated from the catalogd via statestored. If SYNC_DDL
is set, the coordinator will also wait until other coordinators reach
the same statestore topic version with it, so they have also processed
the same updates and had the latest catalog after reset.
In local catalog mode, the coordinator does not cache all the metadata.
Instead, it caches them on-demand (based on query requests), and removes
them based on the Guava cache configurations (size or TTL) or explicit
invalidation from the catalog topic updates. So it's hard to track the
minimal catalog object version correctly.
This patch adds a new field (lastResetCatalogVersion) in TCatalog to
propagate the catalog version when catalogd starts to reset the entire
metadata. Each time when catalogd generates a new topic update, it will
generate a TCatalogObject of CATALOG type containing the state of the
catalog which includes this new field.
To make all changes of the reset being added in the same topic update
with this TCatalog object. Rapidly changed tables that have catalog
version exceeding the version range of this update will also be included.
When coordinator receives a new value of lastResetCatalogVersion in a
topic update, it means catalogd has reset the entire catalog and all the
relative updates are whether included in the same or previous topic
updates. This is guaranteed by three facts:
1) In catalogd, lastResetCatalogVersion_ is only updated at the end of
reset() and is protected by versionLock_.
2) In catalogd's update gathering thread, reading
lastResetCatalogVersion_ requires the read lock of versionLock_. The
value is then saved into GetCatalogDeltaContext and finally got
propagated.
3) If reset() is done, the next update won't skip any tables. The
update gathering thread will collect all catalog objects.
Thus, all metadata with catalog version <= lastResetCatalogVersion can be
considered stale after coordinator finish processing the topic update.
lastResetCatalogVersion + 1 is the lower bound (included) of min catalog
object version of a coordinator.
To avoid catalogd's update collector thread being blocked by concurrent
DDLs that holding the table locks, this patch also fixes IMPALA-9062. In
local catalog mode, we just need to propagate the table name of a
changed table, so don't need to acquire table lock to get a full TTable
object.
This patch also exposes the min catalog object version of coordinator
via a new metric "catalog.min-catalog-object-version" to ease debugging.
Tests:
- Recover all existing tests that have been disabled due to this
missing feature
Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
---
M be/src/service/impala-server.cc
M be/src/util/impalad-metrics.cc
M be/src/util/impalad-metrics.h
M common/thrift/CatalogObjects.thrift
M common/thrift/metrics.json
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M tests/authorization/test_grant_revoke.py
M tests/authorization/test_ranger.py
M tests/common/skip.py
M tests/custom_cluster/test_local_catalog.py
M tests/metadata/test_hms_integration.py
M tests/metadata/test_metadata_query_statements.py
14 files changed, 187 insertions(+), 109 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/14307/7
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 7
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>