Return-Path: X-Original-To: apmail-accumulo-dev-archive@www.apache.org Delivered-To: apmail-accumulo-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id E10D3172BF for ; Fri, 10 Oct 2014 20:33:30 +0000 (UTC) Received: (qmail 49440 invoked by uid 500); 10 Oct 2014 20:33:30 -0000 Delivered-To: apmail-accumulo-dev-archive@accumulo.apache.org Received: (qmail 49398 invoked by uid 500); 10 Oct 2014 20:33:30 -0000 Mailing-List: contact dev-help@accumulo.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@accumulo.apache.org Delivered-To: mailing list dev@accumulo.apache.org Received: (qmail 49376 invoked by uid 99); 10 Oct 2014 20:33:30 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 10 Oct 2014 20:33:30 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id E94021DDE70; Fri, 10 Oct 2014 20:33:25 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============3686893922103552464==" MIME-Version: 1.0 Subject: Re: Review Request 26572: ACCUMULO-898 From: "Eric Newton" To: "Eric Newton" , "accumulo" Date: Fri, 10 Oct 2014 20:33:25 -0000 Message-ID: <20141010203325.24817.74506@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org Auto-Submitted: auto-generated Sender: "Eric Newton" X-ReviewGroup: accumulo X-ReviewRequest-URL: https://reviews.apache.org/r/26572/ X-Sender: "Eric Newton" References: <20141010191244.24818.42311@reviews.apache.org> In-Reply-To: <20141010191244.24818.42311@reviews.apache.org> Reply-To: "Eric Newton" X-ReviewRequest-Repository: accumulo --===============3686893922103552464== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26572/#review56204 ----------------------------------------------------------- DistributedTrace.enable, Trace.on, Trace.off should be re-created and deprecated to help users transition away from documented APIs. Is there some reason TraceUtil isn't named Tracer? Seems like it would save a lot of the machanical changes. core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java Why not call TraceUtil Trace? Seems like it would eliminate most of the mechanical changes. core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchWriter.java Span could be a trivial sub-class of TraceScope, and maintain compatibility. core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchWriter.java I think there's still a way to maintain some compatibility, though I'm not concerned about maintaining it inside accumulo. core/src/main/java/org/apache/accumulo/core/util/ThriftUtil.java nit: whitespace? minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java Why add the IPv4Stack change? Is this related to tracing? minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java What's the reason for this change? server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java Why move this? It is probably the right change, but it could probably go in a separate ticket. server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/trace/ShowTrace.java This expression is now long enough to push into a local method. server/monitor/src/test/java/org/apache/accumulo/monitor/ShowTraceLinkTypeTest.java Hoist Collections.emptyMap() into a constant for readability? server/tserver/src/main/java/org/apache/accumulo/tserver/replication/AccumuloReplicaSystem.java Why not make a version of TraceUtil.data that takes a TraceScope? test/src/test/java/org/apache/accumulo/test/ConditionalWriterIT.java Is this related to tracing changes? - Eric Newton On Oct. 10, 2014, 7:12 p.m., Eric Newton wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26572/ > ----------------------------------------------------------- > > (Updated Oct. 10, 2014, 7:12 p.m.) > > > Review request for accumulo. > > > Repository: accumulo > > > Description > ------- > > Switch to HTrace > > > Diffs > ----- > > core/pom.xml 086ebc0 > core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java e582160 > core/src/main/java/org/apache/accumulo/core/client/impl/ConditionalWriterImpl.java e8af187 > core/src/main/java/org/apache/accumulo/core/client/impl/ConnectorImpl.java 62ab6a4 > core/src/main/java/org/apache/accumulo/core/client/impl/InstanceOperationsImpl.java c5f7634 > core/src/main/java/org/apache/accumulo/core/client/impl/NamespaceOperationsImpl.java 7087ac5 > core/src/main/java/org/apache/accumulo/core/client/impl/ReplicationOperationsImpl.java f820aa4 > core/src/main/java/org/apache/accumulo/core/client/impl/SecurityOperationsImpl.java e9c057e > core/src/main/java/org/apache/accumulo/core/client/impl/TableOperationsImpl.java e46b9c9 > core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchReaderIterator.java d2ca60e > core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchWriter.java 5eec397 > core/src/main/java/org/apache/accumulo/core/client/impl/ThriftScanner.java c49330e > core/src/main/java/org/apache/accumulo/core/client/impl/Writer.java 0358a88 > core/src/main/java/org/apache/accumulo/core/trace/DistributedTrace.java 83f5c26 > core/src/main/java/org/apache/accumulo/core/trace/SpanTree.java 772a133 > core/src/main/java/org/apache/accumulo/core/trace/TraceDump.java b44cc3e > core/src/main/java/org/apache/accumulo/core/trace/TraceFormatter.java 9d860d9 > core/src/main/java/org/apache/accumulo/core/trace/ZooTraceClient.java 4c8b837 > core/src/main/java/org/apache/accumulo/core/util/ThriftUtil.java da4e567 > examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/TracingExample.java a542263 > minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java 1fb5901 > pom.xml ebc2f2f > server/base/pom.xml 60762be > server/base/src/main/java/org/apache/accumulo/server/Accumulo.java 7bb3d71 > server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java 7097079 > server/base/src/main/java/org/apache/accumulo/server/master/LiveTServerSet.java 3dbc6ba > server/base/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java 59d3e0b > server/base/src/main/java/org/apache/accumulo/server/trace/TraceFSDataInputStream.java 5162e01 > server/base/src/main/java/org/apache/accumulo/server/trace/TraceFileSystem.java d3fbad7 > server/base/src/main/java/org/apache/accumulo/server/util/Admin.java d85d61e > server/base/src/main/java/org/apache/accumulo/server/util/VerifyTabletAssignments.java 0fbeb6b > server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java 9646be9 > server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectionAlgorithm.java 0f5cada > server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java 10fd7f5 > server/gc/src/main/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferences.java 74da72d > server/master/src/main/java/org/apache/accumulo/master/Master.java b435b0f > server/master/src/main/java/org/apache/accumulo/master/metrics/ReplicationMetrics.java 06a653d > server/master/src/main/java/org/apache/accumulo/master/replication/ReplicationDriver.java a52f743 > server/master/src/main/java/org/apache/accumulo/master/replication/StatusMaker.java 68dd005 > server/master/src/main/java/org/apache/accumulo/master/replication/WorkMaker.java da17bba > server/master/src/main/java/org/apache/accumulo/master/tableOps/BulkImport.java b55b315 > server/master/src/main/java/org/apache/accumulo/master/tableOps/TraceRepo.java 9b1a735 > server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java 7a724f8 > server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/TServersServlet.java 452f790 > server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/trace/ShowTrace.java a476201 > server/monitor/src/test/java/org/apache/accumulo/monitor/ShowTraceLinkTypeTest.java a630434 > server/tracer/src/main/java/org/apache/accumulo/tracer/TraceServer.java 189bb39 > server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java 2e1eb2c > server/tserver/src/main/java/org/apache/accumulo/tserver/mastermessage/SplitReportMessage.java 2eb1077 > server/tserver/src/main/java/org/apache/accumulo/tserver/mastermessage/TabletStatusMessage.java b168625 > server/tserver/src/main/java/org/apache/accumulo/tserver/replication/AccumuloReplicaSystem.java 4079a65 > server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Compactor.java 2eee5ea > server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java 78a2ed6 > server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/MinorCompactionTask.java 5e5f31f > server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java 0194778 > shell/src/main/java/org/apache/accumulo/shell/Shell.java 16742b8 > shell/src/main/java/org/apache/accumulo/shell/commands/TraceCommand.java 7f63570 > start/pom.xml 61e5b32 > test/pom.xml 78909e1 > test/src/main/java/org/apache/accumulo/test/GetMasterStats.java 0ba4ab2 > test/src/main/java/org/apache/accumulo/test/TestIngest.java 0548f4c > test/src/main/java/org/apache/accumulo/test/VerifyIngest.java 74b03e4 > test/src/main/java/org/apache/accumulo/test/WrongTabletTest.java b563ed9 > test/src/main/java/org/apache/accumulo/test/continuous/ContinuousIngest.java d6a16df > test/src/main/java/org/apache/accumulo/test/continuous/ContinuousStatsCollector.java b36dbeb > test/src/main/java/org/apache/accumulo/test/continuous/ContinuousWalk.java 262f7b0 > test/src/main/java/org/apache/accumulo/test/functional/ZombieTServer.java 3b1aeaf > test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/Shutdown.java ad93c78 > test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/StartAll.java 4fc94d8 > test/src/test/java/org/apache/accumulo/test/ConditionalWriterIT.java 570a53c > test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 5a068af > test/src/test/java/org/apache/accumulo/test/VolumeIT.java 5e54957 > test/src/test/java/org/apache/accumulo/test/functional/AbstractMacIT.java 644b05e > test/src/test/java/org/apache/accumulo/test/functional/BalanceAfterCommsFailureIT.java 7b4a774 > test/src/test/java/org/apache/accumulo/test/functional/ConfigurableMacIT.java eab14f5 > test/src/test/java/org/apache/accumulo/test/functional/DynamicThreadPoolsIT.java 87497b9 > test/src/test/java/org/apache/accumulo/test/functional/ExamplesIT.java 210e057 > test/src/test/java/org/apache/accumulo/test/functional/MetadataMaxFiles.java 6b8d9b3 > test/src/test/java/org/apache/accumulo/test/functional/SimpleBalancerFairnessIT.java 966c150 > test/src/test/java/org/apache/accumulo/test/replication/CyclicReplicationIT.java 93c8650 > trace/pom.xml aacfb56 > trace/src/main/java/org/apache/accumulo/trace/instrument/CountSampler.java 9a5bdbb > trace/src/main/java/org/apache/accumulo/trace/instrument/Sampler.java 4abb40a > trace/src/main/java/org/apache/accumulo/trace/instrument/Span.java 5267174 > trace/src/main/java/org/apache/accumulo/trace/instrument/SpanReceiverHost.java PRE-CREATION > trace/src/main/java/org/apache/accumulo/trace/instrument/Trace.java 19171c4 > trace/src/main/java/org/apache/accumulo/trace/instrument/TraceCallable.java c3072b1 > trace/src/main/java/org/apache/accumulo/trace/instrument/TraceExecutorService.java 04dcc39 > trace/src/main/java/org/apache/accumulo/trace/instrument/TraceProxy.java cb93210 > trace/src/main/java/org/apache/accumulo/trace/instrument/TraceRunnable.java 41c765d > trace/src/main/java/org/apache/accumulo/trace/instrument/TraceUtil.java PRE-CREATION > trace/src/main/java/org/apache/accumulo/trace/instrument/Tracer.java d70aeea > trace/src/main/java/org/apache/accumulo/trace/instrument/impl/MilliSpan.java b641a2c > trace/src/main/java/org/apache/accumulo/trace/instrument/impl/NullSpan.java 916b6cf > trace/src/main/java/org/apache/accumulo/trace/instrument/impl/RootMilliSpan.java c25e644 > trace/src/main/java/org/apache/accumulo/trace/instrument/receivers/AsyncSpanReceiver.java 4eebd69 > trace/src/main/java/org/apache/accumulo/trace/instrument/receivers/LogSpans.java dfed660 > trace/src/main/java/org/apache/accumulo/trace/instrument/receivers/SendSpansViaThrift.java 4967d97 > trace/src/main/java/org/apache/accumulo/trace/instrument/receivers/SpanReceiver.java b44e51e > trace/src/main/java/org/apache/accumulo/trace/instrument/receivers/ZooSpanClient.java 0ed92fc > trace/src/main/java/org/apache/accumulo/trace/instrument/receivers/ZooTraceClient.java PRE-CREATION > trace/src/main/java/org/apache/accumulo/trace/instrument/thrift/RpcClientInvocationHandler.java ea57fe7 > trace/src/main/java/org/apache/accumulo/trace/instrument/thrift/RpcServerInvocationHandler.java 4188bf4 > trace/src/main/java/org/apache/accumulo/trace/thrift/Annotation.java PRE-CREATION > trace/src/main/java/org/apache/accumulo/trace/thrift/RemoteSpan.java 416ae17 > trace/src/main/thrift/trace.thrift 76bcafe > trace/src/test/java/org/apache/accumulo/trace/instrument/CountSamplerTest.java 7b5e970 > trace/src/test/java/org/apache/accumulo/trace/instrument/PerformanceTest.java 2e5aaa5 > trace/src/test/java/org/apache/accumulo/trace/instrument/TracerTest.java f338bd8 > > Diff: https://reviews.apache.org/r/26572/diff/ > > > Testing > ------- > > > Thanks, > > Eric Newton > > --===============3686893922103552464==--