Adar Dembo has posted comments on this change. Change subject: Add more trace counters and timings ......................................................................
Patch Set 1: (8 comments) http://gerrit.cloudera.org:8080/#/c/7819/1/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: PS1, Line 1245: int64_t dur = end_time - start_time; : TRACE_COUNTER_INCREMENT("lbm_write_time_us", dur); : const char* counter = BUCKETED_COUNTER_NAME("lbm_writes", dur); : TRACE_COUNTER_INCREMENT(counter, 1); Maybe move this into the container too, for symmetry? PS1, Line 1458: int64_t dur = end_time - start_time; : TRACE_COUNTER_INCREMENT("lbm_read_time_us", dur); : : const char* counter = BUCKETED_COUNTER_NAME("lbm_reads", dur); : TRACE_COUNTER_INCREMENT(counter, 1); And this. http://gerrit.cloudera.org:8080/#/c/7819/1/src/kudu/util/env_posix.cc File src/kudu/util/env_posix.cc: Line 764: virtual Status Read(uint64_t offset, Slice* result) const OVERRIDE { No tracing for reads/writes? Line 786: TRACE_EVENT1("io", "PosixRWFile::PreAllocate", "path", filename_); Want to include the offset/length? Line 827: TRACE_EVENT1("io", "PosixRWFile::PunchHole", "path", filename_); Could include offset/length here too. Line 863: TRACE_EVENT1("io", "PosixRWFile::Flush", "path", filename_); And here too. Line 902: TRACE_EVENT1("io", "PosixRWFile::Close", "path", filename_); Trace close_us and number of close operations? Line 1280: TRACE_EVENT1("io", "PosixEnv::GetFileSizeOnDisk", "path", fname); Trace micros and number of operations? Same for the other stat-based functions below. -- To view, visit http://gerrit.cloudera.org:8080/7819 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie99bc9e06be682e1595745f154b7dded3903bd6e Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
