Don't eagerly invalidate remote metadata when its TTL expires #26318
Conversation
|
@tjgq This implements #26140 (comment), which breaks the existing TTL test cases in the BwoB integration test. I tried to modify them so that they verify the new behavior of the lease extension (deleting known stale action cache entries), but the lack of synchronization makes it difficult to assert on its effect. Can I just delete the tests? |
|
cc @coeuvre |
How about enabling build rewinding for these tests and assert build succeeded? |
src/main/java/com/google/devtools/build/lib/remote/RemoteLeaseExtension.java
Outdated
Show resolved
Hide resolved
That unfortunately doesn't work because the tests aren't full integration tests and don't go through |
|
@coeuvre I adapted the tests by simulating the retry. I think that the metadata stored in |
Build and action rewinding can recover from metadata whose corresponding contents are no longer available remotely, but are usually more efficient since they only invalidate action cache entries if the contents really are gone, not simply because their TTL expired.
This reverts commit 828628e.
fmeum
force-pushed
the
26140-no-ttl-invalidation
branch
from
cda58be to
77e4be6
Compare
github-actions
bot
added
team-Performance
| buildTarget("//a:bar"); | ||
| waitDownloads(); | ||
|
|
||
| // Assert: target was successfully built | ||
| assertValidOutputFile("a/bar.out", "foo\nupdated bar\n"); |
There was a problem hiding this comment.
nit: Keep this assert. (I will do the change during import)
Let's keep it around for now to make this change simple. |
github-actions
bot
removed
the
awaiting-review
I can send a follow-up to remove it if you think that's the right move. We can also wait for this commit to settle for a while. |
Build and action rewinding can recover from metadata whose corresponding contents are no longer available remotely, but are usually more efficient since they only invalidate action cache entries if the contents really are gone, not simply because their TTL expired.
Fixes #26140