If a cache entry can't be found on disk, remove the cache entry#894
If a cache entry can't be found on disk, remove the cache entry#894mostynb merged 1 commit intobuchgr:masterfrom
Conversation
Matches upstream PR buchgr#894 by mostynb. When the slow-path retry in availableOrTryProxy() fails to open a blob file, remove the stale LRU entry while still holding the lock. This prevents ghost entries from persisting indefinitely and causing repeated 'Lost inputs no longer available remotely' failures for clients using --remote_download_toplevel. Upstream issue: buchgr#893 Upstream PR: buchgr#894
ulrfa
left a comment
ulrfa
left a comment
There was a problem hiding this comment.
Good! I agree it make sense to delete phantom entries to minimize the impact of issues.
However, see suggestion regarding the logging.
|
The github.com web UI gives weird error messages for my review comment. I'm trying to write it here instead: If os.Open() in the fast path (line 465) is failing with IsNotExist and lru.Get() on line 471 is not finding the key, then I assume it is caused by an undersized cache and the operator needs to address the problem. The existing warning is reasonable for that scenario:
But I expect the os.Open() in the slow path (line 474) to never fail, and if it does it could indicate either:
Therefore I suggest having separate log messages so that we can distinguish between if failing in fast or slow path. Perhaps also including err in the log message in case of failure in slow path? |
mostynb
force-pushed
the
fix_ghost_lru_entries
branch
from
342e7cf to
c283c2c
Compare
Updated the log to show if we took the slow path, and the last reported error. |
mostynb
force-pushed
the
fix_ghost_lru_entries
branch
from
c283c2c to
acdfca9
Compare
Initial mitigation for buchgr#893.
mostynb
force-pushed
the
fix_ghost_lru_entries
branch
from
acdfca9 to
7ee9ad2
Compare
2 participants
Initial mitigation for #893.