jt2594838 commented on a change in pull request #1827:
URL: https://github.com/apache/iotdb/pull/1827#discussion_r505531952
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/LogDispatcher.java
##########
@@ -244,25 +244,20 @@ private void appendEntriesAsync(List<ByteBuffer> logList,
AppendEntriesRequest r
private void appendEntriesSync(List<ByteBuffer> logList,
AppendEntriesRequest request,
List<SendLogRequest> currBatch) {
- long start;
- if (Timer.ENABLE_INSTRUMENTING) {
- start = System.nanoTime();
- }
+ Timer.Statistic.RAFT_SENDER_WAIT_FOR_PREV_LOG.setStartTime();
if (!member.waitForPrevLog(peer, currBatch.get(0).getLog())) {
logger.warn("{}: node {} timed out when appending {}",
member.getName(), receiver,
currBatch.get(0).getLog());
return;
}
- Timer.Statistic.RAFT_SENDER_WAIT_FOR_PREV_LOG.addNanoFromStart(start);
+ Timer.Statistic.RAFT_SENDER_WAIT_FOR_PREV_LOG.calCostTime();
Review comment:
I don't think the change is thread-safe, please confirm it.
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/Timer.java
##########
@@ -183,14 +184,33 @@ public void add(long val) {
}
/**
- * This method equals `add(System.nanoTime() - start)`. We wrap
`System.nanoTime()` in this
- * method to avoid unnecessary calls when instrumenting is disabled.
- *
- * @param start
+ * set now time as the operation's start time
*/
- public void addNanoFromStart(long start) {
+ public void setStartTime() {
+ setStartTime(System.nanoTime());
+ }
+
+ public void setStartTime(long startTime) {
+ if (ENABLE_INSTRUMENTING) {
+ this.startTime = startTime;
+ }
+ }
Review comment:
The condition should be put into the first method, as
`System.nanoTime()` is a heavy call and should be avoided when not using
instrumenting.
----------------------------------------------------------------
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:
[email protected]
