Alexey Serbin 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.
> I'm still not convinced that a separate base implementation for the TTL-bas
I didn't try to implement that cache using cache.{h,cc} -- I abandoned that 
though right after looking at interface in cache.h and reading of optimizations 
to achieve 5% optimizations for gcc 4.4.x.

My points after looking at that LRU cache implementation are:
  1) The interface doesn't seem simple enough for the simple task I want to 
accomplish, e.g. using Allocate(), using Slice for parameters, wrappers for 
Handles, sounds too much for this simple thing.
  2) The LRU cache tends to cache usage in memory, while for the entries I was 
thinking to cache I'm not sure there is a good estimation for memory usage 
(apache::thrift::TBase-derived  sentry::TListSentryPrivilegesResponse).  
Instead, I'm thinking to report the usage of that cache as number of items.

If you still think adopting LRU cache for that is a good idea, I can go try LRU 
cache for that.

BTW, I think this changelist will be needed even if going along the 'use the 
LRU-cache for all' route because it's necessary to separate instantiating 
metrics for various caches  at least to have proper descriptions for the metric 
entities (former cache_metrics.{cc,h} were all about kBytes and block cache).



--
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: Mon, 11 Mar 2019 21:42:20 +0000
Gerrit-HasComments: Yes

Reply via email to