wal: add "etcd_wal_writes_bytes_total" #11738
Conversation
Signed-off-by: Gyuho Lee <leegyuho@amazon.com>
Signed-off-by: Gyuho Lee <leegyuho@amazon.com>
Signed-off-by: Gyuho Lee <leegyuho@amazon.com>
Codecov Report
@@ Coverage Diff @@
## master #11738 +/- ##
==========================================
- Coverage 66.29% 65.95% -0.34%
==========================================
Files 403 403
Lines 36771 36787 +16
==========================================
- Hits 24376 24264 -112
- Misses 10895 11017 +122
- Partials 1500 1506 +6
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Thanks @gyuho
LGTM and a small comment, we should document the usage of this new metric. I guess probably in here https://github.com/etcd-io/etcd/blob/master/Documentation/metrics/latest ? Thanks!
|
Will do. Thanks! |
| // Flush flushes buffered data. | ||
| func (pw *PageWriter) Flush() error { | ||
| _, err := pw.flush() | ||
| return err | ||
| } |
There was a problem hiding this comment.
Should Flush() be deprecated or even removed? Otherwise, the metrics where FlushN captured isn't complete?
There was a problem hiding this comment.
It was an external package, so there could be other projects importing this method. So we rather keep it.
| start := time.Now() | ||
| if err = fileutil.Fsync(f.File); err != nil { |
There was a problem hiding this comment.
Would it make sense to calculate metrics inside fileutil.Fsync so that a) consumer won't forget to add metrics in new places where Fsync is called and 2) capture the latency even for the failed calls?
There was a problem hiding this comment.
fileutil.Fsync is used in other packages, so we don't want to break the interface and introduce a new prometheus metrics for a simple package. We may consider wrapper in the next release, so we need not do this.
While we track latencies around WAL fsync, actual number of written data aren't being tracked. This will be useful when we look into the write throughput of etcd.