Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12118 )
Change subject: IMPALA-7970 : Add support for metastore event based automatic invalidate ...................................................................... Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/12118/8/fe/src/main/java/org/apache/impala/catalog/Db.java File fe/src/main/java/org/apache/impala/catalog/Db.java: http://gerrit.cloudera.org:8080/#/c/12118/8/fe/src/main/java/org/apache/impala/catalog/Db.java@112 PS8, Line 112: synchronized (thriftDb_) { thriftDb_.setMetastore_db(msDb); } > I see your point. Initially, I wanted to model the changes to metastore db The locking scheme does require the cooperation of multiple classes so method access modifiers aren't likely to be sufficient to prevent other classes from doing bad things (it's good if we make fewer things public for sure though). I think the precedent to look at here are the various alterDatabase operations in CatalogOpExecutor. Those follow the protocol described in the CatalogOpExecutor class comment. In those cases metastoreDdlLock_ is the lock that's preventing concurrent updates to thriftDb_. I was trying to understand if anything prevents concurrent reads + writes to thriftDb_ though and I don't think there currently is anything, so it seems like some anomalies could result from that. There's also some logic in CatalogOpExecutor that mutates thriftDb_ in-place. -- To view, visit http://gerrit.cloudera.org:8080/12118 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic70b27780560b7ac9b33418d132b36cd0ca4abf7 Gerrit-Change-Number: 12118 Gerrit-PatchSet: 8 Gerrit-Owner: Anonymous Coward <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Paul Rogers <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Thu, 27 Dec 2018 20:11:03 +0000 Gerrit-HasComments: Yes
