michaeljmarshall commented on a change in pull request #13057:
URL: https://github.com/apache/pulsar/pull/13057#discussion_r761617545
##########
File path:
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java
##########
@@ -1223,13 +1223,13 @@ void sendComplete(final Exception e) {
if (finalEx != null && finalEx instanceof TimeoutException) {
TimeoutException te = (TimeoutException) e;
long sequenceId = te.getSequenceId();
- long ns = System.nanoTime();
+ long ms = System.currentTimeMillis();
String errMsg = String.format(
- "%s : createdAt %s ns ago, firstSentAt %s ns ago,
lastSentAt %s ns ago, retryCount %s",
+ "%s : createdAt %s ms ago, firstSentAt %s ms ago,
lastSentAt %s ms ago, retryCount %s",
Review comment:
+1 for seconds.
##########
File path:
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java
##########
@@ -1173,7 +1173,7 @@ static OpSendMsg create(MessageImpl<?> msg, ByteBufPair
cmd, long sequenceId, Se
op.cmd = cmd;
op.callback = callback;
op.sequenceId = sequenceId;
- op.createdAt = System.nanoTime();
+ op.createdAt = System.currentTimeMillis();
Review comment:
I think `System.nanoTime()` was likely an intentional choice here. There
is a general, although not universal, paradigm in the project to use `nanoTime`
instead of `currentTimeMillis` when computing durations. I believe this choice
stems from the fact that nanoTime is supposed to be always increasing while
currentTimeMillis does not technically have that guarantee. That being said, I
discovered this [blog
post](http://steveloughran.blogspot.com/2015/09/time-on-multi-core-multi-socket-servers.html)
from this [stack overflow
comment](https://stackoverflow.com/questions/351565/system-currenttimemillis-vs-system-nanotime#comment103640369_351571)
recently when researching this exact subject. If we want to change the pattern
to use `currentTimeMillis` for durations, I think we should make the change for
the whole project.
@lhotari @eolivelli @merlimat - do you have any opinions here?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
