Adar Dembo has posted comments on this change. Change subject: util: add file cache ......................................................................
Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/5146/2//COMMIT_MSG Commit Message: PS2, Line 41: hareds > shards? Done http://gerrit.cloudera.org:8080/#/c/5146/2/src/kudu/util/file_cache.cc File src/kudu/util/file_cache.cc: Line 62: string::size_type colon = cache_key.find(':'); > Can you rely on the fact that the colon is at position 3 in order to skip t Yeah, I guess between the DCHECK and the test coverage, that would be safe. Line 81: virtual void EvictedEntry(Slice key, Slice value) OVERRIDE { > remove virtual and change OVERRIDE to override. Done Line 343: + once_.memory_footprint_excluding_this() > operator on preceding line Done http://gerrit.cloudera.org:8080/#/c/5146/2/src/kudu/util/file_cache.h File src/kudu/util/file_cache.h: Line 80: class FileCache { > document thread safety guarantees of the class where otherwise unspecified. Done Line 130: std::string Dump() const; > I think we typically call these 'ToStringDebug'. You mean ToDebugString? Sure, I'll rename it. -- To view, visit http://gerrit.cloudera.org:8080/5146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I26d02f71b0a9644de0b669875941adae5f426345 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
