Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/20607 )
Change subject: KUDU-613: Introduce SLRU cache ...................................................................... Patch Set 11: (5 comments) Running into an ASAN error after the latest changes, it might also be related to the templatized code for HandleDeleter/PendingHandleDeleter since I don't recall seeing it before those changes. http://gerrit.cloudera.org:8080/#/c/20607/10/src/kudu/util/slru_cache-test.cc File src/kudu/util/slru_cache-test.cc: http://gerrit.cloudera.org:8080/#/c/20607/10/src/kudu/util/slru_cache-test.cc@76 PS10, Line 76: > nit: the indent should be 2 + 4 = 6 spaces here Done http://gerrit.cloudera.org:8080/#/c/20607/10/src/kudu/util/slru_cache-test.cc@80 PS10, Line 80: > nit: wrong indent Done http://gerrit.cloudera.org:8080/#/c/20607/10/src/kudu/util/slru_cache-test.cc@209 PS10, Line 209: RETURN_IF_NO_NVM_CACHE(GetParam().first); > Here and below: does it mean the test is run only on NVM cache? If so, how It means that the test will be skipped for caches with memory type NVM if the machine does not support NVM caches. Caches with memory type DRAM are unaffected by this macro. http://gerrit.cloudera.org:8080/#/c/20607/10/src/kudu/util/slru_cache-test.cc@244 PS10, Line 244: Erase > Does it make sense to test the erase operation for both the cases when an e Done http://gerrit.cloudera.org:8080/#/c/20607/10/src/kudu/util/slru_cache.h File src/kudu/util/slru_cache.h: http://gerrit.cloudera.org:8080/#/c/20607/10/src/kudu/util/slru_cache.h@136 PS10, Line 136: // Creates a new SLRU cache with 'probationary_capacity' being the capacity of > It would be great to document the parameters of this NewSLRUCache() functio Done -- 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: 11 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: Thu, 08 Feb 2024 22:05:59 +0000 Gerrit-HasComments: Yes
