> On May 15, 2014, 1:53 p.m., kturner wrote: > > Ship It!
I was concerned about wider sharing of zoocache objects than before AND calls to zoocahce.clear(). Looking at the code it seems that most of the code that calls zoocache.clear() creates its own zoocache via "new zoocache". So the code that was calling clear was not using a shared zoocache and that patch does not change this. - kturner ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21072/#review43106 ----------------------------------------------------------- On May 5, 2014, 2:23 p.m., Bill Havanki wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21072/ > ----------------------------------------------------------- > > (Updated May 5, 2014, 2:23 p.m.) > > > Review request for accumulo. > > > Bugs: ACCUMULO-2635 > https://issues.apache.org/jira/browse/ACCUMULO-2635 > > > Repository: accumulo > > > Description > ------- > > This diff introduces a ZooCacheFactory class to centralize and maximize reuse > of ZooCache instances and to facilitate testing. The caching mechanism > formerly in ZooCache was moved to the factory; also, the separate caching > mechanism in ServerClient was removed in favor of the factory. > > To make reviewing easier: Start with the fate classes, and then move on to > the rest. Three of the remaining classes - RootTabletLocator, > ZooKeeperInstance, and ZookeeperLockChecker - were changed more than the > others to explore writing unit tests that take advantage of ZooCacheFactory. > The new unit tests are also included here. > > > Diffs > ----- > > core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java > 43b315df496b3cbb92495a0b4a4019315b0ff9b0 > > core/src/main/java/org/apache/accumulo/core/client/impl/ConditionalWriterImpl.java > 01e4b95386c48ce7ba36559928e82e7aa0abd4f2 > > core/src/main/java/org/apache/accumulo/core/client/impl/InstanceOperationsImpl.java > 12e491243fdc4840382e1ba367938cfcd49420c5 > core/src/main/java/org/apache/accumulo/core/client/impl/Namespaces.java > c134c0492b911e63ba731f7d4f5b37ae0b57e9cf > > core/src/main/java/org/apache/accumulo/core/client/impl/RootTabletLocator.java > 1224207f8838fe9705cb3f60fdaefb51e357504f > core/src/main/java/org/apache/accumulo/core/client/impl/ServerClient.java > bf3e1bc063838482da6c95b6f5693a23446d6f60 > core/src/main/java/org/apache/accumulo/core/client/impl/Tables.java > 5988bda3b2853ab6113b1c8b71d632aa323ac995 > > core/src/main/java/org/apache/accumulo/core/client/impl/ZookeeperLockChecker.java > 85bd171f7a87a60ed340b6e51c3c63f967d7ed46 > > core/src/test/java/org/apache/accumulo/core/client/ZooKeeperInstanceTest.java > PRE-CREATION > > core/src/test/java/org/apache/accumulo/core/client/impl/RootTabletLocatorTest.java > PRE-CREATION > > core/src/test/java/org/apache/accumulo/core/client/impl/ZookeeperLockCheckerTest.java > PRE-CREATION > fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java > e793a696b66f1cf6fa160bd2829f6ab5b3ae0b67 > fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCacheFactory.java > PRE-CREATION > fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooLock.java > 03e159f38e0d0abd32d5361a5a4b07648d7926e1 > > fate/src/test/java/org/apache/accumulo/fate/zookeeper/ZooCacheFactoryTest.java > PRE-CREATION > > server/base/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java > 620188ccbec40ff5c669cd3ceb7872ca91fe7fd4 > > server/base/src/main/java/org/apache/accumulo/server/conf/NamespaceConfiguration.java > 99532ca0ef29b45fe406ab631510a04c26398505 > > server/base/src/main/java/org/apache/accumulo/server/conf/TableConfiguration.java > c134e315ad73f5e4034b63d2de3697b64fce7f00 > > server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfiguration.java > 0c03aaca897cfbc9b58113b1b5d480469b810509 > > Diff: https://reviews.apache.org/r/21072/diff/ > > > Testing > ------- > > Unit tests and functional tests pass. > > > Thanks, > > Bill Havanki > >
