[kv] include hidden potential internal flush time in KV flush latency#2206
[kv] include hidden potential internal flush time in KV flush latency#2206wuchong merged 1 commit intoapache:mainfrom
Conversation
wuchong
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I think you are right. The previous implementation failed to account for certain flush operations.
However, simply moving the start time before all put calls might bundle multiple flush operations into a single latency measurement, which could distort the metrics.
A better approach would be to move the flushLatencyHistogram into RocksDBWriteBatchWrapper and update it directly within RocksDBWriteBatchWrapper#flush. This ensures accurate, per-flush latency tracking.
Make sense. Let me follow your idea. thanks @wuchong |
|
Hi @zuston , Just checking in to see how things are going with this issue. No pressure at all — I'm just wondering if you're still able to work on it. If you're busy or need help, we'd be happy to assist or reassign it. Thanks again for taking it on! |
I will continue to followup this and maybe later in this week |
|
Rebased to trigger CI again. |
Purpose
This change includes the hidden internal flush time during KV flushing.
RocksDBWriteBatchWrappermay trigger a flush implicitly when performing put or delete operations, which caused the previous latency measurement to be incomplete.Brief change log
Move startTime to the first line of the method.
Tests
API and Format
Documentation