zookeeper-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [zookeeper] gmcatsf commented on issue #994: ZOOKEEPER-3432 Improving zookeeper trace for performance and scalability
Date Wed, 26 Jun 2019 00:40:45 GMT
gmcatsf commented on issue #994: ZOOKEEPER-3432 Improving zookeeper trace for performance and
scalability
URL: https://github.com/apache/zookeeper/pull/994#issuecomment-505672661
 
 
   @maoling I will add `zookeeperTracing.md`, please see my answers inline
   
   > 
   > * almost 3000 line changes, too lazy to read. D. I have the following concerns:
   >   
   >   * the trace has something like `traceId`, if so, how to generate it?
   
   There is no traceId for trace message, could you specify which line this refers to?
   
   >   * this trace feature don't have a switch?
   
   TraceLogger can be turned off by ZooTrace.disableTraceLogger via system property zookeeper.disableTraceLogger.
There is also an existing ZooTrace.traceMask which controls which traces to record.
   
   >   * the sendReport Thread is asyn? enabling this feature has any performance issue?
   
   I assume you mean TraceLogger.TraceMessage.send()? this method is asynchronously adding
trace into a queue with limited capacity and return immediately to avoiding slowing down server.
Traces added to the queue may be dropped if queue overflows. Performance issue with existing
ZooTrace is the exact reason for the pull request. Note the implementation uses only single
socket+single thread, and we have been running this on hundreds of facebook production ensembles
over one year without noticing performance impact.
   
   > 
   > > This implementation is sending traces to external process and there is no persistence
in the pull request. It is up to the external process how to persist those traces.
   > 
   > * is it not user-friendly? how to send traces to external process? what thing my external
process need to do?
   
   The pull request includes a testing class named TraceLoggerServer which shows how this
works, it may not be enough and I will enhance it to show how this external process should
read traces. This the same as the comment from @hanm
   
   > * Overall, the trace implements should comply with the opentracing specfication(https://github.com/opentracing/opentracing-java)?
Otw, too much work are exposed to the users.
   
   OpenTracing have a span/span context data model, which is different from existing ZooTrace
implementation of log messages. The focus of this pull request is to replace writing to log4j
part with writing to external process through socket so that server performance wont be affected.
Would it make sense to create a separate jira for OpenTracing?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message