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 15: (6 comments) http://gerrit.cloudera.org:8080/#/c/15306/15/be/src/runtime/io/data-cache.cc File be/src/runtime/io/data-cache.cc: http://gerrit.cloudera.org:8080/#/c/15306/15/be/src/runtime/io/data-cache.cc@645 PS15, Line 645: if (handle.get() == nullptr) return false; nit: 'if (UNLIKELY(' it looks like the Insert into the "meta cache" can fail, even after data has been written to the cache file? do we need to try and delete the data from the cache file before returning false? http://gerrit.cloudera.org:8080/#/c/15306/15/be/src/util/cache/cache.h File be/src/util/cache/cache.h: http://gerrit.cloudera.org:8080/#/c/15306/15/be/src/util/cache/cache.h@256 PS15, Line 256: the entry is immediately nit: revise 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@46 PS15, Line 46: entriess nit: typo http://gerrit.cloudera.org:8080/#/c/15306/15/tests/custom_cluster/test_data_cache.py File tests/custom_cluster/test_data_cache.py: http://gerrit.cloudera.org:8080/#/c/15306/15/tests/custom_cluster/test_data_cache.py@55 PS15, Line 55: int_test_data_cache_deterministic prefix with '__' instead of 'int' http://gerrit.cloudera.org:8080/#/c/15306/15/tests/custom_cluster/test_data_cache.py@81 PS15, Line 81: int_ same comment as above http://gerrit.cloudera.org:8080/#/c/15306/15/tests/custom_cluster/test_data_cache.py@113 PS15, Line 113: int_ same comment as above -- 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: 15 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: Wed, 18 Mar 2020 21:19:07 +0000 Gerrit-HasComments: Yes
