Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/10171 )
Change subject: IMPALA-6916: Implement COMMENT ON DATABASE ...................................................................... Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/10171/6/common/thrift/JniCatalog.thrift File common/thrift/JniCatalog.thrift: http://gerrit.cloudera.org:8080/#/c/10171/6/common/thrift/JniCatalog.thrift@635 PS6, Line 635: // Name of comment to alter. Comment what happens if the comment is not set http://gerrit.cloudera.org:8080/#/c/10171/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/10171/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3534 PS6, Line 3534: long newVersion = catalog_.incrementAndGetCatalogVersion(); There's a subtle race here with the statestore topic update thread that calls CatalogServiceCatalog.getCatalogDelta(). The main problem here is that incrementing the global version and setting the version on the db are not atomic. So in CatalogServiceCatalog.getCatalogDelta() we could getVersion() and then not include the db even though it falls within the expected version range due to this race. For tables, we solve this problem with a lock in Table. We could follow a similar pattern for the db, but let's check with Dimitris whether this is a path we want to go down now. -- To view, visit http://gerrit.cloudera.org:8080/10171 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifcf909c18f97073346f6f603538bf921e69fbb00 Gerrit-Change-Number: 10171 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Adam Holley <g...@holleyism.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Comment-Date: Mon, 30 Apr 2018 16:49:12 +0000 Gerrit-HasComments: Yes
