Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12716 )
Change subject: [util] separate generalized CacheMetrics ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/12716/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12716/1//COMMIT_MSG@15 PS1, Line 15: These changes are introduced to accommodate a few follow-up : modifications introducing a TTL-based cache. > '... The LRU cache tends to cache usage in memory ...' -- I meant 'The LRU Regarding #1, I agree that the implementation has some rough edges, though users of a TTL-based cache need not see that (see FileCache for an example as to how that could be hidden). Regarding #2, I'm concerned that if TTL is the only thing expiring cached objects, our memory footprint may grow without bound based on how an external service is configured. Meaning, I'd be more comfortable if we used TTL _and_ memory as bounds for the cache. And to do that, you'll need something like util/cache.{h,cc}. Unless you can show me that this isn't a legitimate concern, that Sentry won't be able to inadvertently DoS the master in this way. It's frustrating that Thrift-based objects don't provide methods that measure memory footprint, but if all we're caching is TListSentryPrivilegesResponse (or TSentryPrivilege, or whatever), we can measure it ourselves. Looking at the generated code, a malloc_usable_size(ptr) call plus some string capacity calls should do the trick. Look at PosixSequentialFile::memory_footprint() for an example. -- To view, visit http://gerrit.cloudera.org:8080/12716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee18214b57e4e0c86e23c3b2e9cc2ee481812844 Gerrit-Change-Number: 12716 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Tue, 12 Mar 2019 01:51:01 +0000 Gerrit-HasComments: Yes
