Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/22478 )
Change subject: KUDU-613: Fix BlockCache Constructor ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/22478/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/22478/1//COMMIT_MSG@11 PS1, Line 11: policy. This patch fixes this miscalculation. : > Could you add a trivial check for the expected sizes of each of the SLRU ca I added a DCHECK_EQ statement for now. I had some trouble accessing the private members of the slru cache within the block_cache-test even after declaring it a friend test in the slru cache. Needed to be able to access the private member shards_ to know how many shards there are. Let me know if the DCHECK isn't sufficient. http://gerrit.cloudera.org:8080/#/c/22478/1/src/kudu/cfile/block_cache.cc File src/kudu/cfile/block_cache.cc: http://gerrit.cloudera.org:8080/#/c/22478/1/src/kudu/cfile/block_cache.cc@253 PS1, Line 253: 4_t probationary > I don't think precision loss is something we should be concerned about: FLA Ack http://gerrit.cloudera.org:8080/#/c/22478/2/src/kudu/cfile/block_cache.cc File src/kudu/cfile/block_cache.cc: http://gerrit.cloudera.org:8080/#/c/22478/2/src/kudu/cfile/block_cache.cc@248 PS2, Line 248: cy::LRU) { > style nit: does it make sense to introduce a named constant for this and re Done -- To view, visit http://gerrit.cloudera.org:8080/22478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icfde56fd766ba7160052e88ca09a63845f3297c6 Gerrit-Change-Number: 22478 Gerrit-PatchSet: 3 Gerrit-Owner: Mahesh Reddy <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy <[email protected]> Gerrit-Comment-Date: Thu, 13 Feb 2025 07:29:56 +0000 Gerrit-HasComments: Yes
