> On Nov. 21, 2014, 11:06 p.m., Mike Drob wrote: > > core/src/main/java/org/apache/accumulo/core/client/impl/ReplicationOperationsImpl.java, > > lines 91-99 > > <https://reviews.apache.org/r/28312/diff/1/?file=771864#file771864line91> > > > > Are these doing the same thing? This looks like a big change. > > Christopher Tubbs wrote: > Yes, they were doing the same thing. Look at InstanceOperationsImpl. This > was simply removing redundant code. > > Mike Drob wrote: > Can we split this out into a separate issue? It muddles the changeset > here. > > Josh Elser wrote: > As the dude who wrote this goofy bit of code (asking a the master to > change a property instead of just changing the property through the API), I > don't think this needs to be split into its own issue. My $0.02. > > Christopher Tubbs wrote: > Is there a particular problem with this change that migth warrant > additional review/consideration? If so, I'll break it out. As is, I see it as > just one more bit of code that is simplified by the introduction of a > context. Essentially, what you're asking me to do is modify this code to look > like the following as an interim step, and I see no reason for this if it can > be replaced with something that already does this: > MasterClient.execute(context.getInstance(), new ClientExec<Client>() { > @Override > public void execute(Client client) throws Exception { > client.setSystemProperty(Tracer.traceInfo(), context.rpcCreds(), > Property.REPLICATION_PEERS.getKey() + name, replicaType); > } > });
I was actually suggesting the opposite - if the change does not actually depend on the introduction of a context, then you can apply it separately (and immediately). - Mike ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28312/#review62669 ----------------------------------------------------------- On Nov. 22, 2014, 2:21 a.m., Christopher Tubbs wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28312/ > ----------------------------------------------------------- > > (Updated Nov. 22, 2014, 2:21 a.m.) > > > Review request for accumulo and Mike Drob. > > > Bugs: ACCUMULO-3199 and ACCUMULO-3252 > https://issues.apache.org/jira/browse/ACCUMULO-3199 > https://issues.apache.org/jira/browse/ACCUMULO-3252 > > > Repository: accumulo > > > Description > ------- > > This patch introduces a new ClientContext object that contains Credentials, > Instance, and Configuration provided from the client API. This new object is > passed around internally in place of the previous three objects. An > AccumuloServerContext is also introduced, which extends the ClientContext. > Together, these objects ensure the proper configuration, credentials, and > everything else needed to communicate with other system components are > available to any RPC-related code. > > These new object types also reduce the need to create multiple references to > commonly used internal objects (such as HdfsZooInstance and > SystemCredentials), and avoids storing information in static fields. As a > side-effect, this should allow for better testing with mocked components. > > This fixes ACCUMULO-3252, and may lay some groundwork for ACCUMULO-2589. > > > Diffs > ----- > > core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java a6a5e81 > core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java > 4e11a14 > > core/src/main/java/org/apache/accumulo/core/client/impl/BatchWriterImpl.java > bd76a50 > > core/src/main/java/org/apache/accumulo/core/client/impl/ClientConfigurationHelper.java > 603172f > core/src/main/java/org/apache/accumulo/core/client/impl/ClientContext.java > PRE-CREATION > > core/src/main/java/org/apache/accumulo/core/client/impl/ConditionalWriterImpl.java > cb0b90e > core/src/main/java/org/apache/accumulo/core/client/impl/ConnectorImpl.java > 2d077d9 > > core/src/main/java/org/apache/accumulo/core/client/impl/InstanceOperationsImpl.java > 9848612 > core/src/main/java/org/apache/accumulo/core/client/impl/MasterClient.java > 53f178a > > core/src/main/java/org/apache/accumulo/core/client/impl/MultiTableBatchWriterImpl.java > b09582a > > core/src/main/java/org/apache/accumulo/core/client/impl/NamespaceOperationsImpl.java > f756ef1 > > core/src/main/java/org/apache/accumulo/core/client/impl/ReplicationClient.java > 901128b > > core/src/main/java/org/apache/accumulo/core/client/impl/ReplicationOperationsImpl.java > e45bcc9 > > core/src/main/java/org/apache/accumulo/core/client/impl/RootTabletLocator.java > 97d476b > core/src/main/java/org/apache/accumulo/core/client/impl/ScannerImpl.java > 670782e > > core/src/main/java/org/apache/accumulo/core/client/impl/ScannerIterator.java > b0bfc20 > > core/src/main/java/org/apache/accumulo/core/client/impl/SecurityOperationsImpl.java > 8a17a77 > core/src/main/java/org/apache/accumulo/core/client/impl/ServerClient.java > 2d76695 > > core/src/main/java/org/apache/accumulo/core/client/impl/TableOperationsImpl.java > 3c450c3 > core/src/main/java/org/apache/accumulo/core/client/impl/TabletLocator.java > 6c57c29 > > core/src/main/java/org/apache/accumulo/core/client/impl/TabletLocatorImpl.java > c550f15 > > core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchDeleter.java > 4858c7b > > core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchReader.java > c6f3a70 > > core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchReaderIterator.java > 0cb9f7f > > core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchWriter.java > a73fdec > core/src/main/java/org/apache/accumulo/core/client/impl/ThriftScanner.java > 8e601d2 > > core/src/main/java/org/apache/accumulo/core/client/impl/ThriftTransportKey.java > 3791d63 > > core/src/main/java/org/apache/accumulo/core/client/impl/ThriftTransportPool.java > 4f3c661 > > core/src/main/java/org/apache/accumulo/core/client/impl/TimeoutTabletLocator.java > bcbe561 > core/src/main/java/org/apache/accumulo/core/client/impl/Writer.java 7b307d8 > > core/src/main/java/org/apache/accumulo/core/client/mock/impl/MockTabletLocator.java > 35f160f > > core/src/main/java/org/apache/accumulo/core/metadata/MetadataLocationObtainer.java > c7fe137 > core/src/main/java/org/apache/accumulo/core/metadata/MetadataServicer.java > 7a503b7 > > core/src/main/java/org/apache/accumulo/core/metadata/ServicerForMetadataTable.java > 0802994 > > core/src/main/java/org/apache/accumulo/core/metadata/ServicerForRootTable.java > 4da517c > > core/src/main/java/org/apache/accumulo/core/metadata/ServicerForUserTables.java > fd64e05 > > core/src/main/java/org/apache/accumulo/core/metadata/TableMetadataServicer.java > a3800ed > > core/src/main/java/org/apache/accumulo/core/replication/AccumuloReplicationReplayer.java > 620381d > core/src/main/java/org/apache/accumulo/core/util/ThriftUtil.java 88f5afc > > core/src/test/java/org/apache/accumulo/core/client/impl/ClientConfigurationHelperTest.java > 663ce22 > > core/src/test/java/org/apache/accumulo/core/client/impl/ClientContextTest.java > PRE-CREATION > > core/src/test/java/org/apache/accumulo/core/client/impl/RootTabletLocatorTest.java > b7be982 > > core/src/test/java/org/apache/accumulo/core/client/impl/ScannerImplTest.java > 311bbf8 > > core/src/test/java/org/apache/accumulo/core/client/impl/TableOperationsImplTest.java > 20e068b > > core/src/test/java/org/apache/accumulo/core/client/impl/TabletLocatorImplTest.java > 21bf2b9 > > core/src/test/java/org/apache/accumulo/core/metadata/MetadataServicerTest.java > 687e543 > > core/src/test/java/org/apache/accumulo/core/replication/ReplicationOperationsImplTest.java > b404d3d > > mapreduce/src/main/java/org/apache/accumulo/core/client/mapred/AbstractInputFormat.java > 5af78d2 > > mapreduce/src/main/java/org/apache/accumulo/core/client/mapreduce/AbstractInputFormat.java > 836cff9 > > mapreduce/src/main/java/org/apache/accumulo/core/client/mapreduce/lib/impl/InputConfigurator.java > 90a8622 > > minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java > 5149d9d > > minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterStartStopTest.java > 2031b11 > > server/base/src/main/java/org/apache/accumulo/server/AccumuloServerContext.java > PRE-CREATION > > server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java > a2ff172 > > server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java > 1b54ebd > > server/base/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java > efbd882 > > server/base/src/main/java/org/apache/accumulo/server/conf/ServerConfigurationFactory.java > 7e36b40 > > server/base/src/main/java/org/apache/accumulo/server/conf/TableConfiguration.java > 8cf46c7 > server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java > baad742 > server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java > a15e05e > > server/base/src/main/java/org/apache/accumulo/server/master/LiveTServerSet.java > 3388b5b > > server/base/src/main/java/org/apache/accumulo/server/master/balancer/TableLoadBalancer.java > d96f9b0 > > server/base/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java > 30a2460 > > server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataStateStore.java > 63617d6 > > server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java > 155a5d2 > > server/base/src/main/java/org/apache/accumulo/server/master/state/RootTabletStateStore.java > 219fd49 > > server/base/src/main/java/org/apache/accumulo/server/master/state/TabletStateStore.java > 7c75454 > > server/base/src/main/java/org/apache/accumulo/server/problems/ProblemReport.java > 99349b1 > > server/base/src/main/java/org/apache/accumulo/server/problems/ProblemReportingIterator.java > 69d73e1 > > server/base/src/main/java/org/apache/accumulo/server/problems/ProblemReports.java > 23d4de5 > > server/base/src/main/java/org/apache/accumulo/server/replication/ReplicationUtil.java > 50b15f5 > > server/base/src/main/java/org/apache/accumulo/server/security/AuditedSecurityOperation.java > 8049003 > > server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java > 71dcdcf > > server/base/src/main/java/org/apache/accumulo/server/security/SystemCredentials.java > 33a5158 > server/base/src/main/java/org/apache/accumulo/server/util/Admin.java > 254b62c > > server/base/src/main/java/org/apache/accumulo/server/util/FindOfflineTablets.java > 721d4e2 > > server/base/src/main/java/org/apache/accumulo/server/util/ListVolumesUsed.java > b876392 > > server/base/src/main/java/org/apache/accumulo/server/util/MasterMetadataUtil.java > 01eb477 > > server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java > 97b8cff > > server/base/src/main/java/org/apache/accumulo/server/util/RandomizeVolumes.java > 002659d > > server/base/src/main/java/org/apache/accumulo/server/util/RemoveEntriesForMissingFiles.java > 0b4f896 > > server/base/src/main/java/org/apache/accumulo/server/util/ReplicationTableUtil.java > 8f0656c > server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java > 470394d > > server/base/src/main/java/org/apache/accumulo/server/util/VerifyTabletAssignments.java > 50d7014 > > server/base/src/test/java/org/apache/accumulo/server/client/BulkImporterTest.java > 3680341 > > server/base/src/test/java/org/apache/accumulo/server/conf/TableConfigurationTest.java > e66ec98 > > server/base/src/test/java/org/apache/accumulo/server/master/balancer/TableLoadBalancerTest.java > 5c134a5 > > server/base/src/test/java/org/apache/accumulo/server/problems/ProblemReportingIteratorTest.java > 6c61469 > > server/base/src/test/java/org/apache/accumulo/server/security/SystemCredentialsTest.java > c8610d5 > > server/base/src/test/java/org/apache/accumulo/server/util/ReplicationTableUtilTest.java > 88f13c9 > > server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java > 4a9dc3e > server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java > f6ea8f7 > > server/gc/src/main/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferences.java > b0fd0f4 > > server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogsTest.java > 71e5f7d > > server/gc/src/test/java/org/apache/accumulo/gc/SimpleGarbageCollectorTest.java > 6195f42 > > server/gc/src/test/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferencesTest.java > 03a2678 > server/master/src/main/java/org/apache/accumulo/master/Master.java 93691fb > > server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java > 9a3e532 > > server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java > f5ed941 > > server/master/src/main/java/org/apache/accumulo/master/metrics/ReplicationMetrics.java > 762b18d > > server/master/src/main/java/org/apache/accumulo/master/replication/MasterReplicationCoordinator.java > 8218c8a > > server/master/src/main/java/org/apache/accumulo/master/replication/ReplicationDriver.java > f822a90 > > server/master/src/main/java/org/apache/accumulo/master/replication/WorkDriver.java > 975d06c > > server/master/src/main/java/org/apache/accumulo/master/replication/WorkMaker.java > e2ac08b > > server/master/src/main/java/org/apache/accumulo/master/tableOps/BulkImport.java > 66c3fcc > > server/master/src/main/java/org/apache/accumulo/master/tableOps/CancelCompactions.java > 374fc24 > > server/master/src/main/java/org/apache/accumulo/master/tableOps/ChangeTableState.java > f1878b0 > > server/master/src/main/java/org/apache/accumulo/master/tableOps/CloneTable.java > da0afd8 > > server/master/src/main/java/org/apache/accumulo/master/tableOps/CompactRange.java > 13ef68e > > server/master/src/main/java/org/apache/accumulo/master/tableOps/CreateNamespace.java > 8d0aa26 > > server/master/src/main/java/org/apache/accumulo/master/tableOps/CreateTable.java > 1e4d40e > > server/master/src/main/java/org/apache/accumulo/master/tableOps/DeleteNamespace.java > b6a9578 > > server/master/src/main/java/org/apache/accumulo/master/tableOps/DeleteTable.java > 6a49a05 > > server/master/src/main/java/org/apache/accumulo/master/tableOps/ExportTable.java > 9c397db > > server/master/src/main/java/org/apache/accumulo/master/tableOps/ImportTable.java > 35067ce > > server/master/src/main/java/org/apache/accumulo/master/tableOps/RenameTable.java > 5d511ac > > server/master/src/main/java/org/apache/accumulo/master/tableOps/TableRangeOp.java > 12849b6 > server/master/src/test/java/org/apache/accumulo/master/TestMergeState.java > 29dfefb > > server/master/src/test/java/org/apache/accumulo/master/replication/MasterReplicationCoordinatorTest.java > 1ec3f24 > > server/master/src/test/java/org/apache/accumulo/master/replication/WorkMakerTest.java > 0455c44 > > server/monitor/src/main/java/org/apache/accumulo/monitor/EmbeddedWebServer.java > 46fe54b > server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java > 8bc255d > > server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/BasicServlet.java > 2feb804 > > server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/LogServlet.java > f877664 > > server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/MasterServlet.java > 1b613e5 > > server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/OperationServlet.java > c6c75cd > > server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/ProblemServlet.java > 60cece6 > > server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/ReplicationServlet.java > 94765f8 > > server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/TServersServlet.java > 4409dff > > server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/TablesServlet.java > 428880e > > server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/VisServlet.java > fb56573 > > server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/XMLServlet.java > 405d6df > > server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/trace/Basic.java > 53cb172 > server/tserver/src/main/java/org/apache/accumulo/tserver/FileManager.java > fb14ca5 > server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java > 93161ee > > server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java > ba86522 > > server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java > 91ec141 > > server/tserver/src/main/java/org/apache/accumulo/tserver/replication/AccumuloReplicaSystem.java > 71643e8 > > server/tserver/src/main/java/org/apache/accumulo/tserver/replication/BatchWriterReplicationReplayer.java > 358857d > > server/tserver/src/main/java/org/apache/accumulo/tserver/replication/ReplicationServicerHandler.java > 30361b1 > > server/tserver/src/main/java/org/apache/accumulo/tserver/replication/ReplicationWorker.java > 20da0d6 > > server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Compactor.java > 4f19ff9 > > server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java > 3eb7229 > > server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/MinorCompactor.java > 115aed7 > server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java > 1f3306e > > server/tserver/src/test/java/org/apache/accumulo/server/tabletserver/LargestFirstMemoryManagerTest.java > 26ec264 > > server/tserver/src/test/java/org/apache/accumulo/tserver/replication/BatchWriterReplicationReplayerTest.java > fee5dcd > shell/src/main/java/org/apache/accumulo/shell/Shell.java 996b2af > test/src/main/java/org/apache/accumulo/test/GetMasterStats.java 00f8f74 > test/src/main/java/org/apache/accumulo/test/WrongTabletTest.java 9089b60 > > test/src/main/java/org/apache/accumulo/test/continuous/ContinuousStatsCollector.java > 6ead9a6 > test/src/main/java/org/apache/accumulo/test/functional/ZombieTServer.java > 592e177 > > test/src/main/java/org/apache/accumulo/test/performance/metadata/MetadataBatchScanTest.java > 96eb214 > > test/src/main/java/org/apache/accumulo/test/performance/scan/CollectTabletStats.java > bf21818 > > test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java > 438b83e > > test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/Shutdown.java > 69d54a9 > > test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/StartAll.java > 6b8c43c > > test/src/main/java/org/apache/accumulo/test/randomwalk/security/WalkingSecurity.java > 8a66951 > > test/src/test/java/org/apache/accumulo/server/security/SystemCredentialsIT.java > 7524943 > > test/src/test/java/org/apache/accumulo/test/MasterRepairsDualAssignmentIT.java > da2d20f > test/src/test/java/org/apache/accumulo/test/MetaConstraintRetryIT.java > 5dc75d0 > test/src/test/java/org/apache/accumulo/test/MultiTableBatchWriterIT.java > 484c048 > test/src/test/java/org/apache/accumulo/test/TableConfigurationUpdateIT.java > 0d9a211 > test/src/test/java/org/apache/accumulo/test/TotalQueuedIT.java a794088 > test/src/test/java/org/apache/accumulo/test/functional/AbstractMacIT.java > 644b05e > > test/src/test/java/org/apache/accumulo/test/functional/BalanceAfterCommsFailureIT.java > c32375b > > test/src/test/java/org/apache/accumulo/test/functional/ConfigurableMacIT.java > d828675 > > test/src/test/java/org/apache/accumulo/test/functional/DynamicThreadPoolsIT.java > 3ec3ccf > > test/src/test/java/org/apache/accumulo/test/functional/MasterAssignmentIT.java > 46f6b23 > > test/src/test/java/org/apache/accumulo/test/functional/MetadataMaxFiles.java > 98adbf6 > > test/src/test/java/org/apache/accumulo/test/functional/SimpleBalancerFairnessIT.java > 33dfa15 > test/src/test/java/org/apache/accumulo/test/functional/SimpleMacIT.java > 3f04b94 > test/src/test/java/org/apache/accumulo/test/functional/SplitRecoveryIT.java > 5223439 > > test/src/test/java/org/apache/accumulo/test/replication/GarbageCollectorCommunicatesWithTServersIT.java > 1044677 > > test/src/test/java/org/apache/accumulo/test/replication/UnorderedWorkAssignerReplicationIT.java > ad9c9fc > > Diff: https://reviews.apache.org/r/28312/diff/ > > > Testing > ------- > > Full ITs passed (after re-running failures manually). Also, ran > ReplicationITs with SSL enabled. > > > Thanks, > > Christopher Tubbs > >
