Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/21389 )
Change subject: KUDU-613: Add SLRU Cache metrics ...................................................................... Patch Set 12: (4 comments) 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<Counter> protected_segment_cache_misses_caching; > Whoops, it seems I went to many cycles of re-hydrating the context of this I'll add a TODO for this. http://gerrit.cloudera.org:8080/#/c/21389/11/src/kudu/util/slru_cache.h File src/kudu/util/slru_cache.h: http://gerrit.cloudera.org:8080/#/c/21389/11/src/kudu/util/slru_cache.h@193 PS11, Line 193: > Whoops, there was a typo: I mean SLRUCacheShard, not SLRUHandle in the comm Added a template parameter for SLRUCacheShard, thanks for the suggestion. http://gerrit.cloudera.org:8080/#/c/21389/11/src/kudu/util/slru_cache.cc File src/kudu/util/slru_cache.cc: http://gerrit.cloudera.org:8080/#/c/21389/11/src/kudu/util/slru_cache.cc@380 PS11, Line 380: > just FYI nit (no need to address this, especially if you aren't excited rev Good catch, removed it for the metrics related code in this patch. I'll put out a separate patch that removes all instances of this in the rest of this file and other cache related code. http://gerrit.cloudera.org:8080/#/c/21389/11/src/kudu/util/slru_cache.cc@567 PS11, Line 567: shards_[Shard(hash)]->Lookup(key, hash, caching == EXPECT_IN_CACHE), > nit: does it make sense to add a DCHECK() for dynamic_cast<SLRUCacheMetrics Done -- 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: 12 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: Tue, 30 Jul 2024 06:57:04 +0000 Gerrit-HasComments: Yes