Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/20607 )
Change subject: KUDU-613: Introduce SLRU cache ...................................................................... Patch Set 9: (3 comments) http://gerrit.cloudera.org:8080/#/c/20607/8/src/kudu/util/block_cache_metrics.h File src/kudu/util/block_cache_metrics.h: PS8: > +1 Moved the block cache metrics out of this patch, will be part of a follow up patch. http://gerrit.cloudera.org:8080/#/c/20607/8/src/kudu/util/cache.h File src/kudu/util/cache.h: http://gerrit.cloudera.org:8080/#/c/20607/8/src/kudu/util/cache.h@67 PS8, Line 67: // Recency list handle. An entry is a variable length heap-allocated structure. > +1 Done http://gerrit.cloudera.org:8080/#/c/20607/8/src/kudu/util/slru_cache.h File src/kudu/util/slru_cache.h: http://gerrit.cloudera.org:8080/#/c/20607/8/src/kudu/util/slru_cache.h@50 PS8, Line 50: // UniquePendingHandle -- a wrapper around opaque PendingHandle structure : // to facilitate automatic reference counting newly allocated cache's handles. : typedef std::unique_ptr<Cache::PendingHandle, : Cache::PendingHandleDeleter<SLRUCache>> UniquePendingHandle; : : // Sets probationary and protected segments metrics to update corresponding counters accordingly. : // : // Entries are reconstructed on any movement between the segments (upgrade/downgrade). : // : // An upgrade of one entry will increment the number of evictions from the probationary segment by : // one and the number of insertions into the protected segment by one. : // : // A downgrade of one entry will increment the number of evictions from the protected segment by : // one and the number of insertions into the probationary segment by one. : // : // To calculate the number of inserts for the SLRU cache, subtract the number of evictions : // of the protected segment from the number of inserts of the probationary segment. : // : // > nit: maybe, it's time to templatize this class and re-use it throughout var I did so, but for any uses of this templatized class both Free() and Release() will have to be made public as it was done so for SLRUCache. If this is not an acceptable tradeoff, let me know and I can revert these changes. -- To view, visit http://gerrit.cloudera.org:8080/20607 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I45531534a2049dd38c002f4dc7982df9fd46e0bb Gerrit-Change-Number: 20607 Gerrit-PatchSet: 9 Gerrit-Owner: Mahesh Reddy <mre...@cloudera.com> Gerrit-Reviewer: Abhishek Chennaka <achenn...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <ale...@apache.org> Gerrit-Reviewer: Attila Bukor <abu...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy <mre...@cloudera.com> Gerrit-Reviewer: Marton Greber <greber...@gmail.com> Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 07 Feb 2024 20:35:21 +0000 Gerrit-HasComments: Yes
