accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Eric Newton" <eric.new...@gmail.com>
Subject Re: Review Request 20101: ACCUMULO-2212 - ZooReaderWriterFactory
Date Tue, 08 Apr 2014 11:59:27 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20101/#review39782
-----------------------------------------------------------


It's kind of a big structural change for 1.6.1. Can we keep it just to 1.7?

- Eric Newton


On April 7, 2014, 7:39 p.m., Bill Havanki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20101/
> -----------------------------------------------------------
> 
> (Updated April 7, 2014, 7:39 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-2212
>     https://issues.apache.org/jira/browse/ACCUMULO-2212
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> The o.a.a.server.zookeeper.ZooReaderWriter class is converted to a factory class, and
all users of it are switched over. Classes that used server.zookeeper.ZooReaderWriter objects
now directly use the Fate class instead, and the former class is eliminated. The logic that
understands how to construct ZooReaderWriters based on the site configuration resides now
in the new ZooReaderWriterFactory.
> 
> Additional changes:
> * Some method arguments are changed from being of type o.a.a.fate.zookeeper.ZooReader
to o.a.a.fate.zookeeper.IZooReader.
> * The getZooKeeper() method of ZooReader was added to IZooReader, so that ZooCache could
work with the interface type.
> * The invocation handler used to retry ZK calls on connection loss is refactored into
a new RetryingInvocationHandler.
> 
> Notes for reviewers:
> * The Fate classes should not depend on any other Accumulo packages.
> * The goal is to facilitate future changes to the modified classes that add setters for
the factory used, instead of always using 'new ZooReaderWriterFactory()'. The factory can
then be made a mock for testing, or dynamically changed to some alternative implementation
at runtime. This ticket is focused on moving the code to the factory and removing o.a.a.server.zookeeper.ZooReaderWriter.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/trace/DistributedTrace.java 83f5c26 
>   core/src/main/java/org/apache/accumulo/core/trace/ZooTraceClient.java 1315a9d 
>   fate/src/main/java/org/apache/accumulo/fate/zookeeper/IZooReader.java 0610e79 
>   fate/src/main/java/org/apache/accumulo/fate/zookeeper/RetryingInvocationHandler.java
PRE-CREATION 
>   fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java e793a69 
>   fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooReader.java 60660d6 
>   fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooReaderWriter.java 2a327b0

>   fate/src/test/java/org/apache/accumulo/fate/zookeeper/RetryingInvocationHandlerTest.java
PRE-CREATION 
>   server/base/src/main/java/org/apache/accumulo/server/Accumulo.java 4e1eb35 
>   server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java 5cbffc3 
>   server/base/src/main/java/org/apache/accumulo/server/master/LiveTServerSet.java 63bd894

>   server/base/src/main/java/org/apache/accumulo/server/master/state/DeadServerList.java
2f657c4 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/ZooStore.java b0ed03f

>   server/base/src/main/java/org/apache/accumulo/server/monitor/LogService.java 0a5341a

>   server/base/src/main/java/org/apache/accumulo/server/problems/ProblemReport.java b882195

>   server/base/src/main/java/org/apache/accumulo/server/problems/ProblemReports.java 23d4de5

>   server/base/src/main/java/org/apache/accumulo/server/security/handler/ZKAuthenticator.java
1646a28 
>   server/base/src/main/java/org/apache/accumulo/server/security/handler/ZKAuthorizor.java
34d43f2 
>   server/base/src/main/java/org/apache/accumulo/server/security/handler/ZKPermHandler.java
6319653 
>   server/base/src/main/java/org/apache/accumulo/server/tables/TableManager.java 7a61eb6

>   server/base/src/main/java/org/apache/accumulo/server/tablets/UniqueNameAllocator.java
4ae8335 
>   server/base/src/main/java/org/apache/accumulo/server/util/ChangeSecret.java 2926a3f

>   server/base/src/main/java/org/apache/accumulo/server/util/CleanZookeeper.java f2074a1

>   server/base/src/main/java/org/apache/accumulo/server/util/DeleteZooInstance.java 448da86

>   server/base/src/main/java/org/apache/accumulo/server/util/DumpZookeeper.java 30aa2eb

>   server/base/src/main/java/org/apache/accumulo/server/util/MasterMetadataUtil.java e936b97

>   server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java 374017d

>   server/base/src/main/java/org/apache/accumulo/server/util/NamespacePropUtil.java 4e5df9e

>   server/base/src/main/java/org/apache/accumulo/server/util/RestoreZookeeper.java 6e5607e

>   server/base/src/main/java/org/apache/accumulo/server/util/SystemPropUtil.java b6ca527

>   server/base/src/main/java/org/apache/accumulo/server/util/TablePropUtil.java bcaf9b0

>   server/base/src/main/java/org/apache/accumulo/server/util/TabletServerLocks.java 2fc0bd3

>   server/base/src/main/java/org/apache/accumulo/server/util/ZooZap.java 489d4bc 
>   server/base/src/main/java/org/apache/accumulo/server/watcher/MonitorLog4jWatcher.java
ac3426e 
>   server/base/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java
c5a9528 
>   server/base/src/main/java/org/apache/accumulo/server/zookeeper/TransactionWatcher.java
4e0e977 
>   server/base/src/main/java/org/apache/accumulo/server/zookeeper/ZooCache.java bf34ef6

>   server/base/src/main/java/org/apache/accumulo/server/zookeeper/ZooLock.java dce6d38

>   server/base/src/main/java/org/apache/accumulo/server/zookeeper/ZooQueueLock.java f7c1e68

>   server/base/src/main/java/org/apache/accumulo/server/zookeeper/ZooReaderWriter.java
f950077 
>   server/base/src/main/java/org/apache/accumulo/server/zookeeper/ZooReaderWriterFactory.java
PRE-CREATION 
>   server/base/src/test/java/org/apache/accumulo/server/problems/ProblemReportTest.java
dbad326 
>   server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java ab2ab42

>   server/master/src/main/java/org/apache/accumulo/master/Master.java 2440ee4 
>   server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java
8023169 
>   server/master/src/main/java/org/apache/accumulo/master/state/MergeStats.java 4737b6e

>   server/master/src/main/java/org/apache/accumulo/master/state/SetGoalState.java f981bae

>   server/master/src/main/java/org/apache/accumulo/master/tableOps/CancelCompactions.java
49227ef 
>   server/master/src/main/java/org/apache/accumulo/master/tableOps/CompactRange.java e3b0405

>   server/master/src/main/java/org/apache/accumulo/master/tableOps/RenameNamespace.java
41f24cd 
>   server/master/src/main/java/org/apache/accumulo/master/tableOps/RenameTable.java 8c5ed00

>   server/master/src/main/java/org/apache/accumulo/master/tableOps/Utils.java 577f5d5

>   server/master/src/main/java/org/apache/accumulo/master/tserverOps/ShutdownTServer.java
20b7328 
>   server/master/src/main/java/org/apache/accumulo/master/util/FateAdmin.java 4e72832

>   server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java 0e6b37f 
>   server/tracer/src/main/java/org/apache/accumulo/tracer/TraceServer.java 67f2739 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java 3fe60b7 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java 6d73125

>   test/src/main/java/org/apache/accumulo/test/functional/CacheTestClean.java 3fe94e1

>   test/src/main/java/org/apache/accumulo/test/functional/CacheTestWriter.java 20ea55f

>   test/src/main/java/org/apache/accumulo/test/functional/ZombieTServer.java f26c8d7 
>   test/src/test/java/org/apache/accumulo/test/functional/GarbageCollectorIT.java f04f196

>   test/src/test/java/org/apache/accumulo/test/functional/SplitRecoveryIT.java d9de5d1

> 
> Diff: https://reviews.apache.org/r/20101/diff/
> 
> 
> Testing
> -------
> 
> Unit tests pass; functional tests pass.
> 
> 
> Thanks,
> 
> Bill Havanki
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message