Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/22215 )
Change subject: IMPALA-13478: Sync tuple cache files to disk asynchronously ...................................................................... Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/22215/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/22215/8//COMMIT_MSG@24 PS8, Line 24: writing. UpdateWriteSize() can fail if it hits the limit on > What are the odds that this results in not writing any cache hits because r Yes, I think there are scenarios where lots of writers are failing by hitting the limit. Ignoring eviction for a second, I think out of a batch of writers starting with no outstanding writes, at least one should succeed if its write fits in the cache. As each writer in the batch hits the limit, it stops and releases its reservation. Assuming they don't all hit the limit simultaneously, one of them should be able to benefit from the others stopping. There is a bias in favor of smaller entries, because they ask for a reservation increase fewer times. Given the batching, entries smaller than the batch size (currently 128KB) only ask once and then should not hit the limit. Pro-active eviction means we can evict up to the outstanding write limit for in progress writes. If the outstanding write limit is a large percentage of the cache, that makes it unusable. We should probably have a warning. Let's assume it is a smaller percentage (e.g. 5%). I think the rate at which we can churn the cache is limited by the speed of the disk. Let's say the disk can't flush at all, just blocks forever. Every new write will eventually hit the limit and fail. In LRU, we're adding entries at the top of list, but then when it hits the limit, we delete those entries. I think the rest of the cache would stay constant. One thing I didn't handle in this change is that an in-progress write could be evicted from the cache due to other activity. I was thinking about a follow-up change where UpdateCharge() can fail if the entry is no longer part of the cache. This would be more common for LIRS than LRU. -- To view, visit http://gerrit.cloudera.org:8080/22215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I646bb56300656d8b8ac613cb8fe2f85180b386d3 Gerrit-Change-Number: 22215 Gerrit-PatchSet: 8 Gerrit-Owner: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Kurt Deschler <kdesc...@cloudera.com> Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com> Gerrit-Reviewer: Yida Wu <wydbaggio...@gmail.com> Gerrit-Comment-Date: Tue, 06 May 2025 18:15:47 +0000 Gerrit-HasComments: Yes
