Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13112 )
Change subject: [ttl_cache] allow outstanding handles ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/13112/1//COMMIT_MSG Commit Message: PS1: This changelist does not bring much value unless there is a use case where TTLCache is used such a way that it's necessary to allow for outstanding handles. As of now, I'll keep aside. Thank you all for the feedback! http://gerrit.cloudera.org:8080/#/c/13112/1//COMMIT_MSG@9 PS1, Line 9: This patch improves TTLCache so that it's safe to keep a handle to : an entry in the cache even if the cache itself has already gone. > Frankly, I'm finding the ownership semantics difficult to follow due to the SGTM. That makes sense, especially given the tight timeline we have for the integration testing of Kudu+HMS+Sentry. http://gerrit.cloudera.org:8080/#/c/13112/1/src/kudu/util/ttl_cache-test.cc File src/kudu/util/ttl_cache-test.cc: http://gerrit.cloudera.org:8080/#/c/13112/1/src/kudu/util/ttl_cache-test.cc@407 PS1, Line 407: TEST_F(TTLCacheTest, CacheAndOutstandingHandles) { > Is this test a superset of CacheAndOutstandingHandlesNoMetrics? Should we j The initial idea was to check whether there are any problems even with non-metricised instances. The case with metrics could fail while accessing invalid metrics pointer during eviction callback, so I looked elsewhere. Indeed, there were issues, but those were due to triggering DCHECKs in the Cache's code itself :) In other words, the non-metricised scenario was useful for the exploration phase. Right, if going forward with this changelist, probably it makes sense to drop the non-metricised scenario this this one is a super-set of the former, as you noticed. -- To view, visit http://gerrit.cloudera.org:8080/13112 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I280884a630985c5783b7f99f992f530b5f4b9d50 Gerrit-Change-Number: 13112 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 25 Apr 2019 21:57:47 +0000 Gerrit-HasComments: Yes
