Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/21389 )
Change subject: KUDU-613: Add SLRU Cache metrics ...................................................................... Patch Set 11: (3 comments) http://gerrit.cloudera.org:8080/#/c/21389/11//COMMIT_MSG Commit Message: PS11: It's expect this patch come before the integration with block cache, and it seems that was some revisions ago? Why to change the order of the patches? http://gerrit.cloudera.org:8080/#/c/21389/11/src/kudu/util/block_cache_metrics.cc File src/kudu/util/block_cache_metrics.cc: PS11: Shouldn't this file be a part of the follow-up patch https://gerrit.cloudera.org/#/c/21390/ ? http://gerrit.cloudera.org:8080/#/c/21389/11/src/kudu/util/cache_metrics.h File src/kudu/util/cache_metrics.h: http://gerrit.cloudera.org:8080/#/c/21389/11/src/kudu/util/cache_metrics.h@65 PS11, Line 65: scoped_refptr<AtomicGauge<uint64_t>> protected_segment_cache_usage; In addition to these low-level metrics that tracks internals of the SLRU cache, I'd think of introducing functional gauge-based metrics that would allow an end-user to compute all the interesting cache metrics without manually doing the computations. The latter might be error-prone. That's about the following metrics for the SLRU cache as a whole: * inserts * lookups * evictions * hits * misses What do you think? -- To view, visit http://gerrit.cloudera.org:8080/21389 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1c8181ec6bea301605aaef7db0003c3eaef3072d Gerrit-Change-Number: 21389 Gerrit-PatchSet: 11 Gerrit-Owner: Mahesh Reddy <mre...@cloudera.com> Gerrit-Reviewer: Abhishek Chennaka <achenn...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <ale...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy <mre...@cloudera.com> Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Comment-Date: Fri, 26 Jul 2024 23:02:42 +0000 Gerrit-HasComments: Yes