Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13196 )
Change subject: [util] interface to invalidate cache entries ...................................................................... Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/13196/2/src/kudu/util/cache.h File src/kudu/util/cache.h: http://gerrit.cloudera.org:8080/#/c/13196/2/src/kudu/util/cache.h@243 PS2, Line 243: from the oldest (less relevant) entries to the newest (more relevant) ones. > Does the order depend on the kind of recency list it is? Caches with both LRU and FIFO eviction policies keep their lists organized the way where the best candidates for eviction is the beginning of the list. Of course, that's an implementation detail, but this comment reflects the contract the implementation should follow to make invalidation more effective. http://gerrit.cloudera.org:8080/#/c/13196/2/src/kudu/util/cache.h@265 PS2, Line 265: Ivalidate > Invalidate Done http://gerrit.cloudera.org:8080/#/c/13196/2/src/kudu/util/cache.h@266 PS2, Line 266: For example, in case of : // multi-shard cache, when the 'iteration_func' returns 'false', : // the invalidation at current shard stops and switches to the next : // non-yet-processed shard, if any is present. > I agree, I wasn't sure what the motivation behind these functions were, giv My concern was concurrent access to the cache from 'outside' while running the invalidation from 'inside', so I wanted to have a means to control the number of invalidated entries per one run of lazy scrubbing threads (like in the TTLCache) to reduce contention. As of now, I addressed that in the TTLCache and how it's used in the SentryPrivilegesFetcher. Basically, I don't want the scrubbing thread to behave like another JVM's GC-like global lock. So, those use-cases have materialized in the follow-up changelists. If you feel strongly against it anyway, let's continue the discussion. http://gerrit.cloudera.org:8080/#/c/13196/2/src/kudu/util/cache.h@304 PS2, Line 304: to for > nit: for Done http://gerrit.cloudera.org:8080/#/c/13196/2/src/kudu/util/cache.cc File src/kudu/util/cache.cc: http://gerrit.cloudera.org:8080/#/c/13196/2/src/kudu/util/cache.cc@63 PS2, Line 63: const Cache::ValidityFunc Cache::kInvalidateAllEntriesFunc = []( : Slice /* key */, Slice /* value */) { : return false; : }; : : const Cache::IterationFunc Cache::kIterateOverAllEntriesFunc = []( : size_t /* valid_entries_num */, size_t /* valid_entries_num */) { : return true; : }; > These seem more expressive than they need to be for the purpose of invalida The constructor of InvalidationControl uses these. http://gerrit.cloudera.org:8080/#/c/13196/2/src/kudu/util/cache.cc@63 PS2, Line 63: const Cache::ValidityFunc Cache::kInvalidateAllEntriesFunc = []( : Slice /* key */, Slice /* value */) { : return false; : }; : : const Cache::IterationFunc Cache::kIterateOverAllEntriesFunc = []( : size_t /* valid_entries_num */, size_t /* valid_entries_num */) { : return true; : }; > I was still somewhat fuzzy about the use of "invalid_entries_num", and of " Some of that is now implemented in TTLCache, its tests and how the TTLCache is used in SentryPrivilegesFetcher. Done. http://gerrit.cloudera.org:8080/#/c/13196/2/src/kudu/util/cache.cc@63 PS2, Line 63: const Cache::ValidityFunc Cache::kInvalidateAllEntriesFunc = []( : Slice /* key */, Slice /* value */) { : return false; : }; : : const Cache::IterationFunc Cache::kIterateOverAllEntriesFunc = []( : size_t /* valid_entries_num */, size_t /* valid_entries_num */) { : return true; : }; > Ah, looking at https://gerrit.cloudera.org/c/13201/5/src/kudu/util/ttl_cach Right. -- To view, visit http://gerrit.cloudera.org:8080/13196 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1e340c3270b5580114151cdd40afe82576c18099 Gerrit-Change-Number: 13196 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 02 May 2019 01:37:32 +0000 Gerrit-HasComments: Yes
