Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15306 )
Change subject: IMPALA-8690: Add LIRS cache eviction algorithm ...................................................................... Patch Set 18: (5 comments) http://gerrit.cloudera.org:8080/#/c/15306/15/be/src/util/cache/lirs-cache.cc File be/src/util/cache/lirs-cache.cc: http://gerrit.cloudera.org:8080/#/c/15306/15/be/src/util/cache/lirs-cache.cc@83 PS15, Line 83: erreference recency), and it has three different types of e > Yeah, this needed more clarity. I reworked this paragraph. makes a lot more sense, thanks! http://gerrit.cloudera.org:8080/#/c/15306/15/be/src/util/cache/lirs-cache.cc@122 PS15, Line 122: > Added a description here along with the lifecycle. makes sense, thanks! http://gerrit.cloudera.org:8080/#/c/15306/18/be/src/util/cache/lirs-cache.cc File be/src/util/cache/lirs-cache.cc: http://gerrit.cloudera.org:8080/#/c/15306/18/be/src/util/cache/lirs-cache.cc@77 PS18, Line 77: // If the key has only been accessed once, its reuse distance is considered infinite. if i'm reading the hdfs-file-reader.cc and data-cache.cc code correctly, it looks like it calls Lookup twice whenever there is a cache miss. one call occurs when trying to read the data: HdfsFileReader::ReadDataCache --> DataCache::Partition::Lookup --> Cache::Lookup if that results in a cache miss, it calls tries to insert the data into the cache resulting in another call to Lookup: HdfsFileReader::WriteDataCache --> DataCache::Partition::Store --> Cache::Lookup so pretty much every entry in the cache will have a non-infinite reuse distance will this cause problems for LIRS? http://gerrit.cloudera.org:8080/#/c/15306/18/be/src/util/cache/lirs-cache.cc@150 PS18, Line 150: ref_count nit: could you add some docs for 'ref_count' its not clear to me when it needs to be incremented http://gerrit.cloudera.org:8080/#/c/15306/18/be/src/util/cache/lirs-cache.cc@411 PS18, Line 411: HandleTable table_; nit: add docs -- To view, visit http://gerrit.cloudera.org:8080/15306 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I670fa4b2b7c93998130dc4e8b2546bb93e9a84f8 Gerrit-Change-Number: 15306 Gerrit-PatchSet: 18 Gerrit-Owner: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Sahil Takiar <stak...@cloudera.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Comment-Date: Mon, 23 Mar 2020 18:42:50 +0000 Gerrit-HasComments: Yes
