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 <huangquanl...@gmail.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@apache.org> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Vihang Karajgaonkar <vih...@cloudera.com> Gerrit-Comment-Date: Tue, 22 Oct 2019 23:18:34 +0000 Gerrit-HasComments: Yes
