You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by Billie Rinaldi <bi...@apache.org> on 2015/04/08 21:41:11 UTC
Review Request 32986: ACCUMULO-3715 Decrease sampling percentage for
tracing
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32986/
-----------------------------------------------------------
Review request for accumulo, Eric Newton and Josh Elser.
Repository: accumulo
Description
-------
Make sampling of traces configurable; clean up and standardize config parameters; update documentation
Diffs
-----
core/src/main/java/org/apache/accumulo/core/conf/Property.java 9dceb1e
core/src/main/java/org/apache/accumulo/core/trace/DistributedTrace.java c5b6eac
core/src/main/java/org/apache/accumulo/core/trace/ProbabilitySampler.java PRE-CREATION
docs/src/main/asciidoc/chapters/administration.txt 0382934
server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java 04f21a1
server/master/src/main/java/org/apache/accumulo/master/replication/ReplicationDriver.java 63c6b20
server/tracer/src/main/java/org/apache/accumulo/tracer/AsyncSpanReceiver.java dbcb335
server/tracer/src/main/java/org/apache/accumulo/tracer/TraceTableStats.java PRE-CREATION
server/tserver/src/main/java/org/apache/accumulo/tserver/replication/AccumuloReplicaSystem.java cb8ae13
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/MinorCompactionTask.java 0f6a98d
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java 43bd9d9
Diff: https://reviews.apache.org/r/32986/diff/
Testing
-------
Thanks,
Billie Rinaldi
Re: Review Request 32986: ACCUMULO-3715 Decrease sampling percentage
for tracing
Posted by Josh Elser <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32986/#review79519
-----------------------------------------------------------
Ship it!
LGTM
core/src/main/java/org/apache/accumulo/core/trace/ProbabilitySampler.java
<https://reviews.apache.org/r/32986/#comment128923>
Some javadoc would be nice.
server/tracer/src/main/java/org/apache/accumulo/tracer/TraceTableStats.java
<https://reviews.apache.org/r/32986/#comment128925>
hdfs and accumulo SpanTypeCount should be static final
- Josh Elser
On April 9, 2015, 3:57 p.m., Billie Rinaldi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32986/
> -----------------------------------------------------------
>
> (Updated April 9, 2015, 3:57 p.m.)
>
>
> Review request for accumulo, Eric Newton and Josh Elser.
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> Make sampling of traces configurable; clean up and standardize config parameters; update documentation
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/accumulo/core/conf/Property.java 9dceb1e
> core/src/main/java/org/apache/accumulo/core/trace/DistributedTrace.java c5b6eac
> core/src/main/java/org/apache/accumulo/core/trace/ProbabilitySampler.java PRE-CREATION
> docs/src/main/asciidoc/chapters/administration.txt 0382934
> server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java 04f21a1
> server/master/src/main/java/org/apache/accumulo/master/replication/ReplicationDriver.java 63c6b20
> server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/trace/ShowTrace.java 4e05b89
> server/tracer/src/main/java/org/apache/accumulo/tracer/AsyncSpanReceiver.java dbcb335
> server/tracer/src/main/java/org/apache/accumulo/tracer/TraceDump.java e4eb70e
> server/tracer/src/main/java/org/apache/accumulo/tracer/TraceTableStats.java PRE-CREATION
> server/tserver/src/main/java/org/apache/accumulo/tserver/replication/AccumuloReplicaSystem.java cb8ae13
> server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/MinorCompactionTask.java 0f6a98d
> server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java 43bd9d9
>
> Diff: https://reviews.apache.org/r/32986/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Billie Rinaldi
>
>
Re: Review Request 32986: ACCUMULO-3715 Decrease sampling percentage
for tracing
Posted by Billie Rinaldi <bi...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32986/
-----------------------------------------------------------
(Updated April 9, 2015, 3:57 p.m.)
Review request for accumulo, Eric Newton and Josh Elser.
Changes
-------
This updated patch contains all the previous changes and also makes some modifications to the ShowTrace servlet and TraceDump to handle unrooted spans better (since they are now considered normal). I've kept all the sampling configuration because I think it could still be useful, even though just dropping 0ms spans is sufficient to get tracing down to a reasonable amount. Also TraceDump's main method appears to have been broken since 2012, so I tried to fix that.
Repository: accumulo
Description
-------
Make sampling of traces configurable; clean up and standardize config parameters; update documentation
Diffs (updated)
-----
core/src/main/java/org/apache/accumulo/core/conf/Property.java 9dceb1e
core/src/main/java/org/apache/accumulo/core/trace/DistributedTrace.java c5b6eac
core/src/main/java/org/apache/accumulo/core/trace/ProbabilitySampler.java PRE-CREATION
docs/src/main/asciidoc/chapters/administration.txt 0382934
server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java 04f21a1
server/master/src/main/java/org/apache/accumulo/master/replication/ReplicationDriver.java 63c6b20
server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/trace/ShowTrace.java 4e05b89
server/tracer/src/main/java/org/apache/accumulo/tracer/AsyncSpanReceiver.java dbcb335
server/tracer/src/main/java/org/apache/accumulo/tracer/TraceDump.java e4eb70e
server/tracer/src/main/java/org/apache/accumulo/tracer/TraceTableStats.java PRE-CREATION
server/tserver/src/main/java/org/apache/accumulo/tserver/replication/AccumuloReplicaSystem.java cb8ae13
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/MinorCompactionTask.java 0f6a98d
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java 43bd9d9
Diff: https://reviews.apache.org/r/32986/diff/
Testing
-------
Thanks,
Billie Rinaldi
Re: Review Request 32986: ACCUMULO-3715 Decrease sampling percentage
for tracing
Posted by Billie Rinaldi <bi...@apache.org>.
> On April 9, 2015, 3 a.m., Mike Drob wrote:
> > core/src/main/java/org/apache/accumulo/core/conf/Property.java, line 385
> > <https://reviews.apache.org/r/32986/diff/1/?file=921041#file921041line385>
> >
> > If this property name changes, should we provide some level of support for the old name?
I introduced this property in 1.7. When I came back to it, I found it confusing because trace.span.receiver. properties are passed down automatically to the span receiver, but this path property is read in DistributedTrace and translated to trace.span.receiver.tracer.zookeeper.path when passed to the span receiver. I figured I should either change its name to trace.span.receiver.tracer.zookeeper.path so it would not need special handling, or remove it from the t.s.r. naming convention so it's clear that it is handled specially.
- Billie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32986/#review79472
-----------------------------------------------------------
On April 9, 2015, 3:57 p.m., Billie Rinaldi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32986/
> -----------------------------------------------------------
>
> (Updated April 9, 2015, 3:57 p.m.)
>
>
> Review request for accumulo, Eric Newton and Josh Elser.
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> Make sampling of traces configurable; clean up and standardize config parameters; update documentation
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/accumulo/core/conf/Property.java 9dceb1e
> core/src/main/java/org/apache/accumulo/core/trace/DistributedTrace.java c5b6eac
> core/src/main/java/org/apache/accumulo/core/trace/ProbabilitySampler.java PRE-CREATION
> docs/src/main/asciidoc/chapters/administration.txt 0382934
> server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java 04f21a1
> server/master/src/main/java/org/apache/accumulo/master/replication/ReplicationDriver.java 63c6b20
> server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/trace/ShowTrace.java 4e05b89
> server/tracer/src/main/java/org/apache/accumulo/tracer/AsyncSpanReceiver.java dbcb335
> server/tracer/src/main/java/org/apache/accumulo/tracer/TraceDump.java e4eb70e
> server/tracer/src/main/java/org/apache/accumulo/tracer/TraceTableStats.java PRE-CREATION
> server/tserver/src/main/java/org/apache/accumulo/tserver/replication/AccumuloReplicaSystem.java cb8ae13
> server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/MinorCompactionTask.java 0f6a98d
> server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java 43bd9d9
>
> Diff: https://reviews.apache.org/r/32986/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Billie Rinaldi
>
>
Re: Review Request 32986: ACCUMULO-3715 Decrease sampling percentage
for tracing
Posted by Billie Rinaldi <bi...@apache.org>.
I am going to experiment with leaving out the sampling and only dropping
0ms spans to see if that is a sufficient solution. Feel free to hold off
on further review until I post the results.
On Wed, Apr 8, 2015 at 8:00 PM, Mike Drob <md...@mdrob.com> wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32986/#review79472
> -----------------------------------------------------------
>
>
>
> core/src/main/java/org/apache/accumulo/core/conf/Property.java
> <https://reviews.apache.org/r/32986/#comment128875>
>
> If this property name changes, should we provide some level of support
> for the old name?
>
>
> - Mike Drob
>
>
> On April 8, 2015, 7:41 p.m., Billie Rinaldi wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/32986/
> > -----------------------------------------------------------
> >
> > (Updated April 8, 2015, 7:41 p.m.)
> >
> >
> > Review request for accumulo, Eric Newton and Josh Elser.
> >
> >
> > Repository: accumulo
> >
> >
> > Description
> > -------
> >
> > Make sampling of traces configurable; clean up and standardize config
> parameters; update documentation
> >
> >
> > Diffs
> > -----
> >
> > core/src/main/java/org/apache/accumulo/core/conf/Property.java 9dceb1e
> >
> core/src/main/java/org/apache/accumulo/core/trace/DistributedTrace.java
> c5b6eac
> >
> core/src/main/java/org/apache/accumulo/core/trace/ProbabilitySampler.java
> PRE-CREATION
> > docs/src/main/asciidoc/chapters/administration.txt 0382934
> >
> server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java
> 04f21a1
> >
> server/master/src/main/java/org/apache/accumulo/master/replication/ReplicationDriver.java
> 63c6b20
> >
> server/tracer/src/main/java/org/apache/accumulo/tracer/AsyncSpanReceiver.java
> dbcb335
> >
> server/tracer/src/main/java/org/apache/accumulo/tracer/TraceTableStats.java
> PRE-CREATION
> >
> server/tserver/src/main/java/org/apache/accumulo/tserver/replication/AccumuloReplicaSystem.java
> cb8ae13
> >
> server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/MinorCompactionTask.java
> 0f6a98d
> >
> server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
> 43bd9d9
> >
> > Diff: https://reviews.apache.org/r/32986/diff/
> >
> >
> > Testing
> > -------
> >
> >
> > Thanks,
> >
> > Billie Rinaldi
> >
> >
>
>
Re: Review Request 32986: ACCUMULO-3715 Decrease sampling percentage
for tracing
Posted by Mike Drob <md...@mdrob.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32986/#review79472
-----------------------------------------------------------
core/src/main/java/org/apache/accumulo/core/conf/Property.java
<https://reviews.apache.org/r/32986/#comment128875>
If this property name changes, should we provide some level of support for the old name?
- Mike Drob
On April 8, 2015, 7:41 p.m., Billie Rinaldi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32986/
> -----------------------------------------------------------
>
> (Updated April 8, 2015, 7:41 p.m.)
>
>
> Review request for accumulo, Eric Newton and Josh Elser.
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> Make sampling of traces configurable; clean up and standardize config parameters; update documentation
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/accumulo/core/conf/Property.java 9dceb1e
> core/src/main/java/org/apache/accumulo/core/trace/DistributedTrace.java c5b6eac
> core/src/main/java/org/apache/accumulo/core/trace/ProbabilitySampler.java PRE-CREATION
> docs/src/main/asciidoc/chapters/administration.txt 0382934
> server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java 04f21a1
> server/master/src/main/java/org/apache/accumulo/master/replication/ReplicationDriver.java 63c6b20
> server/tracer/src/main/java/org/apache/accumulo/tracer/AsyncSpanReceiver.java dbcb335
> server/tracer/src/main/java/org/apache/accumulo/tracer/TraceTableStats.java PRE-CREATION
> server/tserver/src/main/java/org/apache/accumulo/tserver/replication/AccumuloReplicaSystem.java cb8ae13
> server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/MinorCompactionTask.java 0f6a98d
> server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java 43bd9d9
>
> Diff: https://reviews.apache.org/r/32986/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Billie Rinaldi
>
>