https://pantsbuild.org/ logo
Join Slack
Powered by
This remote cache error is brutal - I think I've h...
# development
w
This remote cache error is brutal - I think I've hit it on 1/3rd of my PRs 
tests/python/pants_test/integration/remote_cache_integration_test.py:remote_cache_integration failed after 3 attempts in 65.46s.
f
It's the same one where Pants runs the second command instead of getting it from the cache?
which "should" have been fixed by https://github.com/pantsbuild/pants/pull/22263
w
f
since the test literally verifies that the CAS/AC have the top-level digests
(but not every digest)
w
I haven't looked into it - as the resolution is a re-run failed jobs, and I'm a bit locked in on call-by-name
f
I'm fine with marking the test as flaky
it's currently too dependent on write timings and completeness
w
👍
f
so myabe we ignore the test for now?
w
We can mark it as ignored, or just keep re-running failed. At the moment, I don't mind one way or the other. @happy-kitchen-89482 ?
h
My worry about skipping tests is that we never unskip them
💯 1
so maybe after call-by-name week we need an unskip-tests-week
f
> so maybe after call-by-name week we need an unskip-tests-week or a "test health" this week -- evaluate whether it is even being tested correctly
like this test needs to be to understand why Pants thought the remote cache was "empty"
as for this test, https://github.com/pantsbuild/pants/pull/22286 will skip it
and in an unskip tests week, I could implement the full remote cache digest check which checks for presence of every digest and not just the top-level ones
f
sorry for the flake 😓 Do you have a theory on why its being marked as ran @fast-nail-55400? The only thing that comes to mind for me is a pure data race (between local and remote cache check). So the test needs to account for that better
f
a few theories: 1. It could be a data race based on write timings, indeed my initial fix added better checking for the top-level digests. 2. The Pants remote cache code has "speculation" on cache reads and tries to run the process concurrently with trying to check the cache. The speculative local run could be "winning" the race with the cache check.
for (2) , we would need to have a way to disable speculation for the remote cache check, (It might even be a useful option to be able to disable it.)
I'm leaning toward (2) as the reason for the flaky behavior.
f
Yeah I agree. I think there is code for this already 
remote_cache_speculation_delay_millis
let me see if I fix the test
f
in any event, I just landed the PR to skip the test for now.
👍 1
The speculation delay may not fix the issue. It does not disable the local execution, just adds a delay before Pants checks for results: https://github.com/pantsbuild/pants/blob/18c670f190001ea665064cc7627c84e37f717cfa/src/rust/engine/process_execution/remote/src/remote_cache.rs#L326
Although increasing the delay might give the cache the chance to win the race.
I wonder if we could have the command being executed check outside the sandbox and delay itself if it is told it is the second execution. Thus, it would never complete before the cache wins.
f
Yeah I think that would be doable, though might not be worth the effort? I was trying to reproduce the flake locally but having a hard time with it (was reliably passing) Are the CI boxes under ~100% cpu load?
f
> I was trying to reproduce the flake locally but having a hard time with it (was reliably passing) Are the CI boxes under ~100% cpu load? They are GitHub-hosted runners with whatever the standard CPU is.
10 Views
Open in Slack
PreviousNext
Powered by