[
https://issues.apache.org/jira/browse/PHOENIX-1691?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14365627#comment-14365627
]
Samarth Jain commented on PHOENIX-1691:
---------------------------------------
Looks good to me. Just a few minor nits:
In TraceStatement.java traceOn and samplingRate should be final.
{code}
+ private TraceScope closeTraceScope(final PhoenixConnection conn,
TraceScope traceScope) {
+ if (traceScope != null) {
+ traceScope.close();
+ traceScope = null;
+ conn.setTraceScope(null);
+ }
+ return null;
+ }
{code}
Taking a look at the code, it seems like you are getting the traceScope by
calling conn.getTraceScope(). So you probably don't need to pass traceScope as
a param here separately. That would also get rid of the
{code}
traceScope = null;
{code}
which wasn't really needed to begin with.
In PhoenixTracingEndToEndIT, the order of params in assertEquals() is inverted.
For example:
{code}
+ assertEquals(pconn.getSampler(), Sampler.ALWAYS);
The method signature is assertEquals(expected, actual);
{code}
A general question, can you help me understand the reason behind closing the
existing trace scope whenever there is a change in the sampler type? Would it
be ok to just carry on with the existing trace scope for cases when you are
switching from ALWAYS to PROBABILITY and vice-versa?
> Allow settting sampling rate while enabling tracing.
> ----------------------------------------------------
>
> Key: PHOENIX-1691
> URL: https://issues.apache.org/jira/browse/PHOENIX-1691
> Project: Phoenix
> Issue Type: Sub-task
> Reporter: Rajeshbabu Chintaguntla
> Assignee: Rajeshbabu Chintaguntla
> Fix For: 5.0.0, 4.4
>
> Attachments: PHOENIX-1691.patch, PHOENIX-1691_v2.patch
>
>
> Now we can dynamically enable/disable tracing from query. We should also be
> able to set sampling rate while enabling tracing.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
