You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by Bill Havanki <bh...@clouderagovt.com> on 2014/05/05 16:23:21 UTC
Review Request 21072: ACCUMULO-2635 - ZooCacheFactory (1.6.1)
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21072/
-----------------------------------------------------------
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
Re: Review Request 21072: ACCUMULO-2635 - ZooCacheFactory (1.6.1)
Posted by ke...@deenlo.com.
> 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
>
>
Re: Review Request 21072: ACCUMULO-2635 - ZooCacheFactory (1.6.1)
Posted by ke...@deenlo.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21072/#review43106
-----------------------------------------------------------
Ship it!
Ship It!
- kturner
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
>
>
Re: Review Request 21072: ACCUMULO-2635 - ZooCacheFactory (1.6.1)
Posted by Bill Havanki <bh...@clouderagovt.com>.
Bumping this review one more time, as my first bump got was engulfed by the
ASF email outage.
Bill
On Wed, May 7, 2014 at 12:56 PM, Bill Havanki <bh...@clouderagovt.com>wrote:
> Just bumping this available review - comments welcome.
>
>
> On Mon, May 5, 2014 at 10:23 AM, Bill Havanki <bh...@clouderagovt.com>wrote:
>
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/21072/
>> Review request for accumulo.
>> By Bill Havanki.
>> *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.
>>
>> Testing
>>
>> Unit tests and functional tests pass.
>>
>> 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)
>>
>> View Diff <https://reviews.apache.org/r/21072/diff/>
>>
>
>
>
> --
> // Bill Havanki
> // Solutions Architect, Cloudera Govt Solutions
> // 443.686.9283
>
--
// Bill Havanki
// Solutions Architect, Cloudera Govt Solutions
// 443.686.9283
Re: Review Request 21072: ACCUMULO-2635 - ZooCacheFactory (1.6.1)
Posted by Bill Havanki <bh...@clouderagovt.com>.
Just bumping this available review - comments welcome.
On Mon, May 5, 2014 at 10:23 AM, Bill Havanki <bh...@clouderagovt.com>wrote:
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21072/
> Review request for accumulo.
> By Bill Havanki.
> *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.
>
> Testing
>
> Unit tests and functional tests pass.
>
> 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)
>
> View Diff <https://reviews.apache.org/r/21072/diff/>
>
--
// Bill Havanki
// Solutions Architect, Cloudera Govt Solutions
// 443.686.9283