Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14134 )
Change subject: IMPALA-8797: Support database and table blacklist ...................................................................... Patch Set 5: (7 comments) http://gerrit.cloudera.org:8080/#/c/14134/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14134/4//COMMIT_MSG@9 PS4, Line 9: Add catalogd and coordinator startup options for database and table > Can we also add them to the coordinator startup flags and intercept any ref Yes. Added this. But now we need to take care if the blacklist configs are inconsistent between catalogd and coordinators, which happens a few times in my tests. Added some complexity. http://gerrit.cloudera.org:8080/#/c/14134/4/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/14134/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@274 PS4, Line 274: > nit: you can also do a Set<TableName> Done http://gerrit.cloudera.org:8080/#/c/14134/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@290 PS4, Line 290: tedTables( > why pass class members? I'm guessing it is for unit tests, mention that? Yes. It's for unit tests and let the constructor codes clean. Decide to extract it to be a util class since other places need it too. http://gerrit.cloudera.org:8080/#/c/14134/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@332 PS4, Line 332: */ > nit: call parseBlackListedDbsAndTbls() or something similar? Done http://gerrit.cloudera.org:8080/#/c/14134/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@335 PS4, Line 335: } > nit: Splitter is a neater way of doing the same thing. It has helpers to tr Done http://gerrit.cloudera.org:8080/#/c/14134/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@338 PS4, Line 338: * Check whether the table is in blacklist > nit: I think this can be info, since this is once per process lifetime and Done http://gerrit.cloudera.org:8080/#/c/14134/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@356 PS4, Line 356: * Returns a Metastore event processor object if > worthy of logging? Done -- To view, visit http://gerrit.cloudera.org:8080/14134 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I02dbb07f8e08793b57b2a88d09b30fd32cff26dc Gerrit-Change-Number: 14134 Gerrit-PatchSet: 5 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Tue, 27 Aug 2019 14:47:56 +0000 Gerrit-HasComments: Yes
