rkhachatryan commented on a change in pull request #17757:
URL: https://github.com/apache/flink/pull/17757#discussion_r754208421
##########
File path:
flink-dstl/flink-dstl-dfs/src/main/java/org/apache/flink/changelog/fs/StateChangeFsUploader.java
##########
@@ -92,6 +103,16 @@ public void upload(Collection<UploadTask> tasks) throws
IOException {
}
}
+ private LocalResult uploadWithMetrics(Path path, Collection<UploadTask>
tasks)
+ throws IOException {
+ metrics.getUploadsCounter().inc();
+ long start = clock.relativeTimeNanos();
+ LocalResult result = upload(path, tasks);
+ metrics.getUploadLatencies().update(clock.relativeTimeNanos() - start);
Review comment:
Is your question about a) `System.currentTimeMillis()` vs
`System.nanoTime()` or b) whether nanos should be converted to milllis?
As for a), I think `currentTimeMillis` should NOT be used as it's not
monotonic (I consider potential overhead of `nanoTime` is neglibible in context
of IO)
As for b), I don't have a strong opinion. Nanosecond precision is consistent
with State Backends latency tracking and potentially with future changelog
implementations; it can also be useful in local setups (for debugging).
Division can be done closer to UI then if needed.
--
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]