You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/09/09 15:37:21 UTC
[GitHub] [pinot] richardstartin opened a new issue #7414: Tracing should not rely on the system clock
richardstartin opened a new issue #7414:
URL: https://github.com/apache/pinot/issues/7414
There's code throughout the codebase which performs tracing of sensitive operations, such as in `BaseOperator`:
```java
@Override
public final T nextBlock() {
if (Thread.interrupted()) {
throw new EarlyTerminationException();
}
if (TraceContext.traceEnabled()) {
long start = System.currentTimeMillis();
T nextBlock = getNextBlock();
long end = System.currentTimeMillis();
String operatorName = getOperatorName();
LOGGER.trace("Time spent in {}: {}", operatorName, (end - start));
TraceContext.logTime(operatorName, (end - start));
return nextBlock;
} else {
return getNextBlock();
}
}
```
Calls to `System.currentTimeMillis()` call in to the system clock, so are subject to NTP/PTP adjustments, meaning that `(end - start)` can be negative or misleadingly long. `System.nanoTime()` should be used instead, and converted to milliseconds if so desired.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] kishoreg commented on issue #7414: Tracing should not rely on the system clock
Posted by GitBox <gi...@apache.org>.
kishoreg commented on issue #7414:
URL: https://github.com/apache/pinot/issues/7414#issuecomment-916216423
Good point. We have this in many places.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on issue #7414: Tracing should not rely on the system clock
Posted by GitBox <gi...@apache.org>.
richardstartin commented on issue #7414:
URL: https://github.com/apache/pinot/issues/7414#issuecomment-916273412
I just recalled that [JFR uses RDTSC](https://github.com/openjdk/jdk/blob/739769c8fc4b496f08a92225a12d07414537b6c0/src/hotspot/share/jfr/utilities/jfrTime.cpp#L58) so can measure time more efficiently, so replacing manual tracing with JFR events might be a good idea if performance is a concern.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mcvsubbu commented on issue #7414: Tracing should not rely on the system clock
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on issue #7414:
URL: https://github.com/apache/pinot/issues/7414#issuecomment-916248377
I had the same thought as well, way early on in Pinot development. However, when Adwait (a former pinot team member) and I researched a bit, we came across some articles that effectively said `System.currentTimeMillis()` was more efficient than nanos. I remember I was surprised because I thought getting nanos would be far cheaper, usually a relative clock that does not reach across cpus. Perhaps the performance penalty is platform dependent, I am not sure.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on issue #7414: Tracing should not rely on the system clock
Posted by GitBox <gi...@apache.org>.
richardstartin commented on issue #7414:
URL: https://github.com/apache/pinot/issues/7414#issuecomment-916270036
> `System.currentTimeMillis()` was more efficient than nanos
This is the first time I have heard that and I must admit I am very skeptical about those articles' claims. On linux, `System.currentTimeMillis` delegates to `CLOCK_REALTIME` (see [here](https://github.com/openjdk/jdk/blob/20a373a0d00b72f0c6dceb19efca3b7cc34335e5/src/hotspot/os/posix/os_posix.cpp#L1387)) whereas `System.nanoTime()` delegates to `CLOCK_MONOTONIC` ([here](https://github.com/openjdk/jdk/blob/20a373a0d00b72f0c6dceb19efca3b7cc34335e5/src/hotspot/os/posix/os_posix.cpp#L1409)). There are [faster clocks](https://people.cs.rutgers.edu/~pxk/416/notes/c-tutorials/gettime.html) than `CLOCK_MONOTONIC` but Java users don't have access to them. Performance wise, the dominating factor in time measurement tends to be whether the `clock_gettime ` syscall is mapped into userspace by vDSO or not, and e.g. Xen doesn't do that, which can confound measurement.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org