Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/15170 )
Change subject: IMPALA-8690 (prep 2): Switch DataCache to Impala's cache implementation ...................................................................... Patch Set 6: (3 comments) http://gerrit.cloudera.org:8080/#/c/15170/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15170/6//COMMIT_MSG@16 PS6, Line 16: 3. Remove CacheMetrics code (not used by Impala) > I assume we have some other way of tracking these metrics? Or are they not We currently do not use them for the data cache, and we maintain equivalent metrics inside the data cache. We may want metrics directly from the cache code in future, but it would be easy to reintroduce this code using Impala metrics infrastructure (and avoid bringing in the Kudu dependencies). http://gerrit.cloudera.org:8080/#/c/15170/6/.clang-tidy File .clang-tidy: http://gerrit.cloudera.org:8080/#/c/15170/6/.clang-tidy@67 PS6, Line 67: -clang-diagnostic-unreachable-code-break,\ > Might be nice to briefly mention the rationale for this in the commit messa I ended up changing the code to remove the extra break calls. This seemed consistent with what we did elsewhere, so I've removed this. http://gerrit.cloudera.org:8080/#/c/15170/6/be/src/runtime/io/data-cache.cc File be/src/runtime/io/data-cache.cc: http://gerrit.cloudera.org:8080/#/c/15170/6/be/src/runtime/io/data-cache.cc@556 PS6, Line 556: Cache::UniqueHandle handle( > nit: this will fit on a single line now, here and I think a couple of other Done -- To view, visit http://gerrit.cloudera.org:8080/15170 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I917f8352c9276373dd2761af986bf3487855271c Gerrit-Change-Number: 15170 Gerrit-PatchSet: 6 Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Sahil Takiar <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Comment-Date: Wed, 19 Feb 2020 01:47:20 +0000 Gerrit-HasComments: Yes
