https://pantsbuild.org/ logo
Join Slack
Powered by
We have GOT to figure out how to get CI to be cons...
# development
h
We have GOT to figure out how to get CI to be consistently green, this constant re-running is untenable. I think this is approaching code red status.
➕ 3
b
I'm still on my kick of "make Pants development as breezey as Pants makes development", looking at tests was something I was musing after the docs/blog kick
w
The failures tend to happen in the integration/e2e, correct?
f
Definitely we want to fix this sooner rather than later. My $current_job waiting too long to get this right is why I have no time to devote to Pants 😅
l
I will take https://github.com/pantsbuild/pants/actions/runs/6549550831/job/17787048388?pr=20043 and catalog what all failed and share it back.
All timeouts but not necessarily labeled as integration tests:
Copy code
## 2/10
✕ src/python/pants/backend/adhoc/run_system_binary_integration_test.py:tests failed in 60.07s.
## 3/10
✕ src/python/pants/backend/javascript/goals/lockfile_test.py:tests failed in 60.07s.
✕ src/python/pants/backend/javascript/subsystems/nodejs_test.py:tests failed in 240.10s.
## 4/10
✕ src/python/pants/backend/openapi/lint/spectral/rules_integration_test.py:tests failed in 120.07s.
✕ src/python/pants/backend/python/goals/coverage_py_integration_test.py:tests failed in 540.07s.
✕ src/python/pants/init/load_backends_integration_test.py:tests failed in 800.07s.
## 5/10
✕ src/python/pants/engine/streaming_workunit_handler_integration_test.py:streaming_workunit_handler_integration_test failed in 167.38s.
E       AssertionError: After 15 attempts in 30 seconds: The waiter process should have written its pid.
## 7/10
✕ src/python/pants/backend/javascript/dependency_inference/rules_test.py:tests failed in 60.10s.
## 8/10
✕ src/python/pants/option/global_options_test.py:tests failed in 60.10s.
w
👍 I was just reading through them. weird timeouts too. 60, 120, 800 seconds
f
could be resource contention, especially if some of these tests pass well in isolation?
w
Do each of the shards run in separate containers?
f
those GHA agents are not super powerful
l
Same story for another build, where a much smaller number of tests failed.
Copy code
## 0/10
✕ src/python/pants/backend/javascript/package/rules_integration_test.py:test failed in 120.07s.
✕ src/python/pants/backend/python/typecheck/pyright/rules_integration_test.py:tests failed in 240.07s.
I am going to look for successful CI builds how long these particular tests take and whether the timeout is close or very far from the typical runtime.
In the first build these same tests passed with:
Copy code
✓ src/python/pants/backend/javascript/package/rules_integration_test.py:test succeeded in 60.13s.
✓ src/python/pants/backend/python/typecheck/pyright/rules_integration_test.py:tests succeeded in 88.09s.
b
I do suspect on the non-timeout tests, I've seen subtle 
set
-based bugs.
l
and just to get an idea of the variance while passing: in build 20042:
Copy code
✓ src/python/pants/backend/javascript/package/rules_integration_test.py:test succeeded in 42.07s.
✓ src/python/pants/backend/python/typecheck/pyright/rules_integration_test.py:tests succeeded in 126.14s.
w
Just to put it down again, some suggestions I've proposed in a meeting at some point (arguably better in a Discussion, but I have to dash shortly). • Run 
--changed-since
and 
--transitive
tests ◦ Though, since so many of our tests are integration tests, this may not help much, but if the system works well, this shouldn't hurt either • Run/shard tests by backend/plugin - maybe we'd notice some trends if certain backends are kept together in tests/resources ◦ Alternatively, run regular unit tests together, and then do "something" special for the long-running integration tests (again, this might help isolate backends, or timeouts, or resource contention, etc) • Longer term: Figure out some way to pull the integration tests that use "Type 4" of Pants testing (e2e basically), to not use that. ◦ Super slow, used as too much of a crutch
👍 1
b
Run
--changed-since` and 
--transitive
tests
Since we have a remote cache, this would be inappropriate I think. We only ever run what we MUST be running because everything else gets pulled from the cache. (Also not running the world in CI means things slip through the cracks, since they aren't modeled in Pants targets)
w
Wasn't thinking too much of remote cache
(Also not running the world in CI means things slip through the cracks, since they aren't modeled in Pants targets)
Well. That's terrifying.
b
Very technically, the crack-slipping is any file which is loaded in rule code (via something like 
PathGlobs
) which isn't wired up as a Target with the correct dependencies. The most common of these are config files. Touch a config file, ask Pants what changed it'll say "nothing?" But the (capital P) Processes that transitively depend on those files will need to run.
w
Oh, I think my original suggestion, btw, was full blown testing nightly or at some reduced rate - to catch a lot of the stuff we miss, but I stand by my "well, that's terrifying" comment
The most common of these are config files. Touch a config file, ask Pants what changed it'll say "nothing?"
I don't want to diverge this thread too much, but... Is that a good thing? Anyways, discussion for another day
b
I'd be very -1 on not running the full test suite on PR CI. 1. Who has been looking at the CI failures on pushes to 
master
? I'm gonna wager "no one". So why should we expect any different from our nightly test suite? We're volunteers... 2. It papers over the problem. We should fix this at the core.
w
Yeah, I'd like to solve the underlying problem in lieu of mitigating it, but also a decent part of that original proposal was: • If we suggest people to use it, we should use it ourselves • Being a good internet citizen, and not having servers crunching for hours on untouched/unchanged code None of that solves this underlying problem though, of why this bizarreness is happening. I've been a googlin' to see if anyone else's actions becomes flaky the longer actions run
l
Josh, I agree. 
--changed-since
is not reliable and at best could be used as a quick-fail. Like running test with 
--changed-since
first to save running the whole test suite when there are already known failures. I think it is a bit odd that we recommend to our users to use 
--changed-since
in CI. It is more suitable to the development workflow.
b
I never liked it listed as "option 1": https://www.pantsbuild.org/docs/using-pants-in-ci#approach-1-only-run-over-changed-files But there is a little warning thingy
➕ 1
f
I'm for using 
--changed-since
in CI for a lot of reasons. The "let the cache work it out" model isn't always viable for repos that use Pants: • Not everyone has a distributed cache • It can take quite a bit of time to actually calculate the cache keys, depending on where you are in a graph • For truly large monorepos, it's almost nonsensical. But that's a bigger problem with multiple elements of Pants' design By not using 
--changed
it we make it a second class citizen. There's some really annoying bits with 
--changed
like how if you touch BUILD files all deps get marked changed.
That said, using 
changed
wouldn't solve or even mitigate our problem here I don't think. If you have write access to the cache, I think caching gets you an even more surgical subset of tests that actually run than the changed subsystem does.
👍 1
h
Yes, each shard is a separate GHA runner, but they are weak-ass dual core runners.
Anecdotally it appears to me that timeouts are usually on those shards, not the MacStadium or ARM dedicated machines
One option that John has suggested in the past is only running our full test suite on the ARM machine, which has 80 cores I think
And just run the platform-specific tests on x86_64
b
There'd be a lot more contention for that resource then...
h
Yep, but maybe we have enough capacity?
Also, re the 
main
builds, right now they almost always fail, and so we don't pay attention to them.
A CI failure on 
main
should be a stop-the-world event
b
Considering we occasionally wait for the Arm machine today I'd suspect blasting it with every PR push would lead toa lot of waiting
w
Hmm something I might test out on one of my machines tonight - create a Linux container with 1 core or thread assigned and see if we can even complete the tests
Even if it takes an hour or something, it “shouldn’t” fail, right? Like, there is no minimal number of cores right? Then, maybe just monitor the memory usage if it fails
f
You need at least two cores probably, because we would run processes=1 if there's only one core. In my experience, these kinds of issues arise when schedulers decided we can set parallelism=$(nproc) but the tests themselves actually need more than one proc, which can result in catastrophic load cascades as the spawned processes/threads compete for time
Or worse memory pressure puts you in swap hell
w
16 cores, 32GB on my Linux box - so I can play around a bit. I would like to narrow down the scope of the problem to bite sized bits • Thread/process contention? Remove that with 1 threaded running • Memory exhaustion? Give it 31 GB of ram • File contention? … do something • Memory leak? Well damn I would just like to come up with a reproducible failure, and a reproducible success… Generally, on my overpowered computers, everything passes
f
I wonder if the memory issues are not to blame. Pants can be super greedy with memory, and if these 2 core nodes only have 4 GB of ram, it's pretty easy to hit that during routine Pants use
🎯 1
w
The sporadic errors at sporadic times feel like they could be memory related. Anytime I have the same failure at stuff that used to pass, then it usually comes down to some sort of resource contention. If we can force it, happy days
f
This is very similar to patterns I've seen at work, and it's almost always come down to load driven by waiting for the OS to swap pages back into memory
👀 1
those nodes might be 8 GB though, which makes this less likely
you could test it with cgroups though
w
@happy-kitchen-89482 Do we have any logging hooks to spit out periodic cpu, memory, and file usage stats? Like, to the pants.d folder or anything?
f
7 GB apparently
h
For context, a few weeks ago I merged a change to lower the concurrency on those x86 machines to 1, and it hasn't helped much
w
Doh
l
What about retry of failed tests?
Like ... couldn't we figure out a way to have 
pants test
be able to automatically retry failed tests? Maybe the feature is already there
👆 1
Imagine the shard runs, it remembers what all failed and why, and if the failures were by timeout it runs a second job with just those tests.
we could combine that with some kind of policy that puts a cap on the timeouts so that the chunk size of tests is not too big. Some of the failed test files had timeouts of 
800s
and 
540s
and one of them I recall has previously been a spurious failure for me, even with my tenure of like 2-3 prs.
that is a > 10min test run. which would fail or succeed as just one. We can break these up.
f
if there's local cache then just re-running pants test should do retry correctly
👆 1
h
There has been discussion of a Pants feature to retry failed tests.
Well, timed-out tests, specifically, not all failures
b
There was a PR too. So it might already be on main? I've lost track of so many things
h
I don't think there was a PR
I don't recall one
but it would be a great project to work on
b
I definitely saw PRs to support it.
w
Anyone know why running pants test in a Podman container, I'd be getting this?
Copy code
pants.core.util_rules.system_binaries.BinaryNotFoundError: Cannot find `unzip` on `['/bin', '/opt/homebrew/bin', '/usr/bin', '/usr/local/bin']`. Please ensure that it is installed so that Pants can download the tools Pants needs to run.
Copy code
which zip
/usr/bin/zip
which unzip
/usr/bin/unzip
Upon further inspection, some of these numbers are strange. Long running tasks that are batched together seems to have what I would call 'non-random' timing. I don't know if those long-running tasks are sorted by time, but this all just feels really weird to me. I have 2 cores, and 8gb of memory, so why is this reporting monotonically increasing failure times across like 5-10 tests? Is the timeout on a per-test, or per batch basis? I've only really started experimenting, but from my limited time - the "feeling" I'm getting is that we're starting a timer on a batch of work, then we expect the limited number of cores we have available to finish that work in some expected time, and when it doesn't, all the times are reported as summations of the previous, rather than independent
Copy code
01:41:34.51 [INFO] Completed: Run Pytest - src/python/pants/backend/python/framework/stevedore/python_target_dependencies_test.py:tests - succeeded.
01:41:34.70 [INFO] Long running tasks:
  60.90s	Run Pytest for src/python/pants/backend/python/util_rules/vcs_versioning_test.py:tests
  64.20s	Run Pytest for src/python/pants/backend/python/lint/ruff/rules_integration_test.py:tests
  67.30s	Run Pytest for src/python/pants/backend/python/lint/docformatter/rules_integration_test.py:tests
  71.20s	Run Pytest for src/python/pants/backend/cc/lint/clangformat/rules_integration_test.py:tests
  74.70s	Run Pytest for src/python/pants/backend/google_cloud_function/python/rules_test.py:rules_test
  91.00s	Run Pytest for src/python/pants/backend/kotlin/compile/kotlinc_test.py:tests
  97.70s	Run Pytest for src/python/pants/jvm/package/war_test.py:tests
  101.00s	Run Pytest for src/python/pants/backend/docker/util_rules/docker_build_context_test.py:tests
  111.60s	Run Pytest for src/python/pants/backend/java/lint/google_java_format/rules_integration_test.py:tests
01:41:44.70 [WARN] Failed to generate JUnit XML data for src/python/pants/backend/java/lint/google_java_format/rules_integration_test.py:tests.
01:41:44.70 [ERROR] Completed: Run Pytest - src/python/pants/backend/java/lint/google_java_format/rules_integration_test.py:tests - failed (exit code -15).
============================= test session starts ==============================
collecting ... collected 3 items

src/python/pants/backend/java/lint/google_java_format/rules_integration_test.py::test_passing 


Exceeded timeout of 120.0 seconds when executing local process: Run Pytest for src/python/pants/backend/java/lint/google_java_format/rules_integration_test.py:tests
01:42:35.60 [INFO] Completed: Run Pytest - src/python/pants/backend/helm/resolve/fetch_test.py:tests - succeeded.
01:42:55.30 [INFO] Completed: Run Pytest - src/python/pants/jvm/resolve/coursier_fetch_integration_test.py:tests - succeeded.
01:43:04.90 [INFO] Long running tasks:
  80.20s	Run Pytest for src/python/pants/jvm/package/deploy_jar_test.py:tests
  80.20s	Run Pytest for src/python/pants/backend/tools/semgrep/rules_integration_test.py:tests
  145.10s	Run Pytest for src/python/pants/backend/python/lint/bandit/rules_integration_test.py:tests
  149.70s	Run Pytest for src/python/pants/backend/codegen/protobuf/scala/rules_integration_test.py:tests
  154.40s	Run Pytest for src/python/pants/backend/python/lint/ruff/rules_integration_test.py:tests
  157.50s	Run Pytest for src/python/pants/backend/python/lint/docformatter/rules_integration_test.py:tests
  161.40s	Run Pytest for src/python/pants/backend/cc/lint/clangformat/rules_integration_test.py:tests
  181.20s	Run Pytest for src/python/pants/backend/kotlin/compile/kotlinc_test.py:tests
  191.20s	Run Pytest for src/python/pants/backend/docker/util_rules/docker_build_context_test.py:tests
01:43:06.30 [INFO] Completed: Run Pytest - src/python/pants/backend/helm/util_rules/chart_test.py:tests - succeeded.
01:43:12.50 [INFO] Completed: Run Pytest - src/python/pants/engine/internals/engine_test.py:tests - succeeded.
01:43:13.70 [INFO] Completed: Run Pytest - src/python/pants/backend/openapi/dependency_inference_test.py:tests - succeeded.
01:43:16.20 [WARN] Failed to generate JUnit XML data for src/python/pants/backend/docker/util_rules/docker_build_context_test.py:tests.
01:43:16.20 [ERROR] Completed: Run Pytest - src/python/pants/backend/docker/util_rules/docker_build_context_test.py:tests - failed (exit code -15).
Copy code
01:45:35.70 [INFO] Long running tasks:
  62.79s	Run Pytest for src/python/pants/backend/helm/dependency_inference/deployment_test.py:tests
  86.20s	Run Pytest for src/python/pants/backend/helm/goals/deploy_test.py:tests
  91.39s	Run Pytest for src/python/pants/backend/codegen/avro/java/rules_integration_test.py:tests
  107.60s	Run Pytest for src/python/pants/backend/awslambda/python/rules_test.py:rules_test
  143.29s	Run Pytest for src/python/pants/backend/python/goals/pytest_runner_integration_test.py:tests
  160.39s	Run Pytest for src/python/pants/backend/scala/compile/scalac_test.py:tests
  200.69s	Run Pytest for src/python/pants/jvm/test/junit_test.py:tests
  231.00s	Run Pytest for src/python/pants/backend/tools/semgrep/rules_integration_test.py:tests
Some stats to be taken with a reasonable grain of salt: 2core, 4gb => Memory was pretty well behaved, around 50-60% usage, CPUs were 100% 2core, 8gb => Memory 45-50% usage, CPUs were 100% 4core, 8gb => Memory 50% usage, CPUs were 100% 8core, 8gb => Memory 80-90%, cores averaging 80-90% (feels like memory contention reducing CPU usage) In all of these cases I was getting timeouts galore, but at different locations, and tests that had previously failed, may have succeeded as cores increased. I'm going to try playing with the test batch size, as I think that might be a culprit here
-- some time later -- Alright, that was a fail. Still getting all kinds of these long running tasks infos and timeouts - which, with two cores and a 
--test-batch-size=1
I dunno, just surprised that I would have all these concurrently running tasks.
Copy code
Exceeded timeout of 120.0 seconds when executing local process: Run Pytest for src/python/pants/backend/docker/subsystems/dockerfile_parser_test.py:tests


02:22:27.51 [INFO] Long running tasks:
  84.19s	Run Pytest for src/python/pants/engine/internals/scheduler_integration_test.py:scheduler_integration_test
  85.19s	Run Pytest for src/python/pants/help/help_integration_test.py:integration
  86.39s	Run Pytest for src/python/pants/core/goals/run_integration_test.py:integration
  87.00s	Run Pytest for src/python/pants/base/exception_sink_integration_test.py:exception_sink_integration_test
  122.30s	Run Pytest for src/python/pants/goal/stats_aggregator_integration_test.py:integration
  130.50s	Run Pytest for tests/python/pants_test/integration/goal_rule_integration_test.py:goal_rule_integration
  133.40s	Run Pytest for src/python/pants/backend/terraform/dependency_inference_test.py:dependency_inference_test
  144.30s	Run Pytest for src/python/pants/backend/javascript/lint/prettier/rules_integration_test.py:tests
  146.90s	Run Pytest for src/python/pants/backend/javascript/goals/test_integration_test.py:tests
  146.90s	Run Pytest for src/python/pants/backend/python/typecheck/pyright/rules_integration_test.py:tests
  152.41s	Run Pytest for src/python/pants/backend/javascript/subsystems/nodejs_test.py:tests
02:22:28.01 [WARN] Failed to generate JUnit XML data for src/python/pants/goal/stats_aggregator_integration_test.py:integration.
02:22:28.01 [ERROR] Completed: Run Pytest - src/python/pants/goal/stats_aggregator_integration_test.py:integration - failed (exit code -15).
============================= test session starts ==============================
collecting ... collected 3 items

src/python/pants/goal/stats_aggregator_integration_test.py::test_counters_and_histograms 


Exceeded timeout of 120.0 seconds when executing local process: Run Pytest for src/python/pants/goal/stats_aggregator_integration_test.py:integration
l
@flat-zoo-31952 is it true that rerunning 
pants test
will only rerun failed tests if there is a cache? Oh ... is it because we do not batch tests so each one is in its own pytest invocation. How should we look for this retry-on-timeout functionality. I can try to hunt around for the existing PRs that may have been created in support of it. Or should we pilot plainly adding a 
pants test
retry if the first one failed in github actions?
f
Yeah a previous successful test with the same inputs is a cache hit. Failed tests are cache misses. I'm not sure how batching affects this logic now. Making it so only timed-out tests are re-run sounds complicated to me
l
oh wow, @careful-address-89803 has already been looking into it: https://github.com/pantsbuild/pants/pull/19760
➕ 1
f
Honestly I'd start with just 
for i in $(seq 1 $num_retries); do pants test :: && break; done
and see if that helps at all. Just to validate if this even helps. Sure it's wasteful to re-run tests that fail because of broken code but I think we should at least see if this re-running technique can buy us some stability
➕ 1
l
I agree Josh
Should I take a crack at it? I'll be slower since I am somewhat unfamiliar with gha. Or are you able to make the change?
b
I'd honestly rather us take the retries over the finish line
l
We can do both in parallel. both are useful.
w
Retries are nice and all, but am I the only one confused as to why cores=1, batch-size=1 seems to allow > 10 concurrent tests? > Pants will limit the total number of parallel tests running across all scheduled processes so that it does not exceed the configured value of 
[GLOBAL].process_execution_local_parallelism
(by default, the number of CPUs available on the machine running Pants). For example, if your machine has 8 CPUs and Pants schedules 8 concurrent 
pytest
processes with 
pytest-xdist
enabled, it will pass 
-n 1
to each process so that the total concurrency is 8. > By default, Pants will schedule concurrent 
pytest
runs for each Python test file passed to the 
test
goal. This approach provides parallelism with fine-grained caching, but can have drawbacks in some situations: I would have thought that those two setups could at least reduce the concurrency problem, so that only 1 pytest runs on 1 file at a time
🤔 1
Running with 
--debug
to see if t hat helps
Alright, so before it bombs out complaining about debug, I get about 16-20 "Set up Pytest to run interactively" logs, for a variety of different tests, and a slew of pytest_runner.pex files
Copy code
14:14:04.60 [INFO] Completed: Building pytest_runner.pex
14:14:04.60 [INFO] Starting: Building pytest_runner.pex
14:14:08.40 [INFO] Long running tasks:
  403.88s	Set up Pytest to run interactively - src/python/pants/backend/tools/semgrep/rules_integration_test.py:tests
  403.94s	Set up Pytest to run interactively - src/python/pants/jvm/jdk_rules_test.py:tests
  404.44s	Set up Pytest to run interactively - src/python/pants/backend/codegen/thrift/apache/python/python_thrift_module_mappper_test.py:tests
14:14:08.90 [INFO] Completed: Building pytest_runner.pex
14:14:08.90 [INFO] Completed: Building pytest_runner.pex
14:14:08.90 [INFO] Starting: Building pytest_runner.pex
14:14:08.90 [INFO] Starting: Building pytest_runner.pex
14:14:10.80 [INFO] Completed: Building pytest_runner.pex
14:14:10.80 [INFO] Starting: Building pytest_runner.pex
14:14:12.70 [INFO] Completed: Building pytest_runner.pex
14:14:12.70 [INFO] Starting: Building pytest_runner.pex
14:14:12.80 [INFO] Completed: Building pytest_runner.pex
14:14:12.80 [INFO] Starting: Building pytest_runner.pex
14:14:16.20 [INFO] Completed: Building pytest_runner.pex
14:14:16.20 [INFO] Starting: Building pytest_runner.pex
14:14:18.00 [INFO] Completed: Building pytest_runner.pex
14:14:18.00 [INFO] Starting: Building pytest_runner.pex
14:14:18.30 [INFO] Completed: Building pytest_runner.pex
14:14:18.30 [INFO] Starting: Building pytest_runner.pex
14:14:29.60 [INFO] Completed: Building pytest_runner.pex
14:14:29.60 [INFO] Starting: Building pytest_runner.pex
14:14:32.00 [INFO] Completed: Building pytest_runner.pex
14:14:38.50 [INFO] Long running tasks:
  90.00s	Run setuptools.build_meta:__legacy__ for src/python/pants:pants-packaged
14:14:40.10 [INFO] Completed: Building pytest_runner.pex
14:14:40.60 [INFO] Completed: Building pytest_runner.pex
14:14:42.90 [INFO] Completed: Building pytest_runner.pex
14:14:43.20 [INFO] Completed: Building pytest_runner.pex
14:14:45.80 [INFO] Completed: Building pytest_runner.pex
14:14:46.00 [INFO] Completed: Building pytest_runner.pex
14:14:47.20 [INFO] Completed: Building pytest_runner.pex
14:14:47.80 [INFO] Completed: Building pytest_runner.pex
14:14:49.41 [INFO] Completed: Building pytest_runner.pex
14:14:49.80 [INFO] Completed: Building pytest_runner.pex
14:14:50.11 [INFO] Completed: Building pytest_runner.pex
14:14:50.11 [INFO] Completed: Building pytest_runner.pex
14:14:50.30 [INFO] Completed: Building pytest_runner.pex
14:14:50.74 [INFO] Completed: Building pytest_runner.pex
@happy-kitchen-89482 @witty-crayon-22786 Is there a mechanism I'm missing to run a single pytest executable against a single file, one at a time?
Copy code
./pants --test-batch-size=1 test --debug ::

podman run --cpus=1 --memory=24g --rm my-pants-test-image
l
SJ, we probably need to control the concurrency in pants. There is an extra arg for that.
w
I think the results are somewhere in this thread. But, Pants's process concurrency is based on number of available cores (by default), which is 1 (process_execution_local_parallelism) https://pantsbuild.slack.com/archives/C046T6T9U/p1643829377652609
l
oh interesting `default: 
#cores
`
w
I set it explicitly to one, in case somewhere that value was overwritten. This all might be a red herring, but yeah, something isn't jiving with me about this set of tests I did last night. In my tests up to about 8 cores, it's a guarantee that we run into these timeouts? That's insane
l
If you run 
pants help-advanced global
in the container and look at what it reports for "current value" for --process-execution-local-parellelism I wonder what it will say
In the old days there were occasionally issues that looking for the # of cores from within a container would return the number of cores on the host machine. I assume however we determine the n cores would be able to return the cpu rationed out to the container.
w
I explicitly set the number of cores, and the behaviour feels more normal
Lol, what. Pants has outsmarted podman... Well played Pants..... Well played.
I have to look through it, I'm guessing that Podman just passes through the system's cpuinfo when requested, and limits the cores at a higher level - but at least this explains what I've been seeing. How more processes are created than system resources I've allocated
l
So if you set 
--process-execution-local-parallelism=1
in the pants invocation does it look right?
w
Yeah, tests are running and completing sequentially now, so the unit tests are no longer timing out due to starvation. We'll see when we get to integration tests though
Copy code
14:40:05.45 [INFO] Completed: Run Pytest - src/python/pants/engine/download_file_integration_test.py:tests - succeeded.
14:40:06.92 [INFO] Completed: Run Pytest - src/python/pants/backend/docker/util_rules/docker_build_args_test.py:tests - succeeded.
14:40:07.91 [INFO] Completed: Run Pytest - src/python/pants/core/util_rules/config_files_test.py:tests - succeeded.
14:40:09.18 [INFO] Completed: Run Pytest - src/python/pants/backend/docker/util_rules/dockerfile_test.py:tests - succeeded.
14:40:10.18 [INFO] Completed: Run Pytest - src/python/pants/core/goals/generate_snapshots_test.py:tests - succeeded.
14:40:13.01 [INFO] Completed: Run Pytest - src/python/pants/backend/visibility/rule_types_test.py:tests - succeeded.
14:40:14.16 [INFO] Completed: Run Pytest - src/python/pants/backend/visibility/lint_integration_test.py:tests - succeeded.
14:40:15.21 [INFO] Completed: Run Pytest - src/python/pants/backend/openapi/goals/tailor_test.py:tests - succeeded.
14:40:17.30 [INFO] Completed: Run Pytest - src/python/pants/engine/internals/build_files_test.py:tests - succeeded.
14:40:18.61 [INFO] Completed: Run Pytest - src/python/pants/backend/go/lint/golangci_lint/rules_integration_test.py:tests - succeeded.
14:40:20.16 [INFO] Completed: Run Pytest - src/python/pants/backend/codegen/protobuf/go/rules_integration_test.py:tests - succeeded.
l
Nice 🙌 . But I guess also 🐌 🐢
w
🤷 Curious about the timeouts now - this will take forever, but it'll be interesting to find out
l
hacking on the tiny change to do the blunt retry in CI as Josh R suggested in https://github.com/pantsbuild/pants/pull/20053
I think we should also chunk down some of the tests. Retry is not as helpful when the task has an 800s timeout, but we can see how this first bit goes.
w
If we could run/cache on a per-test, rather than per test file - man, game changer for these integration tests. I'm getting timeouts on the last Helm test, for example, but the first two passed after 100 seconds or whatever
l
Files are the unit of caching and everything in pants, at least for the source files. We can break up the files.
w
Yep, I agree - would be nicer to be seamless somehow, rather than having tests written/organized for the system, rather than the system taking care - but that's not (yet) the world we live in 🤷 I'm flagged some items that are failing in my non-CI environment, so, not our biggest concern yet - but they could become a problem at some point
l
Waiting to see what the test shards in https://github.com/pantsbuild/pants/pull/20053 show. Whether there end up being failures and if the retry is enough to rescue them.
🙏 1
b
I'd hesitate to break them up. Reminder we're only papering over the underlying issue here. If shared fixtures, like say RuleRunners cache state between tests then breaking them up leads to longer overall times
f
Yep, I agree - would be nicer to be seamless somehow, rather than having tests written/organized for the system, rather than the system taking care - but that's not (yet) the world we live in
Do you mean sub-file target granularity?
> Reminder we're only papering over the underlying issue here Sorta, but it's also a phenomenally common way underlying issues surface. Having good ways do deal with it and mitigate its impact while you continue investigating for real root causes is important.
w
> Reminder we're only papering over the underlying issue here Well, if we've set a timeout for a test at 180s, and if fails on my machine at 180s, and passes on CI at 170s - and then maybe spuriously passes on re-test (either because it's within timeout, or some ambient cache, or or or) - I don't call that papering over anything. We have tests that just take forever to run sometimes. I don't think that's the only problem we have, but it is A problem we have. > Do you mean sub-file target granularity? Yeah, more or less. Like, run, cache, and apply timeouts, etc, per test, not per file or per batch. So, looking at some of the timeouts I'm having on my machine - 3 long running tests in one file causing a timeout, but in 3 files, they would pass maybe (heavy on the maybe, as I haven't tested this out)
b
But we haven't investigated it yet, when tests were failing. I'm worried we'll have less incentive to investigate it when tests pass
w
> I'm worried we'll have less incentive to investigate it when tests pass True - highly likely, but reliable passing is the first standard in my opinion. Optimizing/fixing underlying would be a close second
f
My experience dealing with flaky tests says that basically no-one has the incentives to fix flaky tests if re-runs work well enough, even if they're manual. That line of thinking presumes that people will context shift to fixing a hairy unrelated issue when they are blocked because that blockage acts as an "incentive" to fix things. But it doesn't. It just irritates people and they try to find ways around it. Flakes have to hunted down and killed with intentionality. In the meantime, retries and timeout increases and just plain having more resources can help stabilize builds and avoid casual reliance on manual re-runs. If we can collect data about what is being retried then we have a shitlist of tests that need to be looked at. We could probably script GHA to create github issues for retries, and then we deal with them in bug-squashing sprints or similar.
w
To Josh's point, we've had flaky CI for a while (my first PRs required retries over a year or more ago).
f
tl;dr you can't irritate people into fixing flaky tests. I've been trying at my job for 3 years now 😂
😆 1
b
Well it actually was my next target 😏
w
So, just as a brief overview of what I'm seeing on this multi-hour, single test-at-a-time - my machine is getting spanked anytime we need to call out to the network to download/resolve something. Helm, NodeJS, pyenv all cause timeouts where we don't have cached downloads.
b
Yeah that honestly was going to be my first suggestion
Check if we're heavy on network
w
Maybe throttled, but these are some big installations, all contending with 1 core or whatever, so, its unsurprising. I wonder if we can pre-cache some of those downloads into our actions/runner/env
b
I know a build tool designed with caching 😁
😆 1
w
I mean, in the spirit of the test, we'd want to emulate downloading from "somewhere" - but yeah, ideally from the filesystem
Ooof, yeah, gigs of downloads
b
Well, things are looking up 🙂
Alright, I'm gonna turn off notifications for this thread. I'm going homebuying. I'll check in later or feel free to @mention me 🙂
w
🏠
💰 1
Retry is not as helpful when the task has an 800s timeout
pantsd_integration_test => 2100s timeout
😮 1
l
Unfortunately the tests in the retrying PR passed on the first go so I couldn't see if retrying would help anything. https://github.com/pantsbuild/pants/pull/20053 . If it is harmless should we try to incorporate it into main and see later if it helped?
Something I don't quite understand is the role of remote caching in the gha runner. We go through the trouble of setting up this bazel cache apparently, with read access to our S3 cache. So it is unclear why the test results need to be recomputed seemingly every time.
w
As a sanity, did you want to force a test error to see if it retries, and then, eventually stops?
l
I am not sure how to do that
I checked that it will retry on failure with a toy example in gha
w
👍
l
(and that the retry uses a memoized result for an already passing test)
I'll mark the PR as ready for review and request comments.
Worked up https://github.com/pantsbuild/pants/pull/20053 and requested review from a few folks.
w
@careful-address-89803’s https://github.com/pantsbuild/pants/pull/19760 is effectively ready to go, so that would be my preference.
➕ 1
it’s all well and good to say “the tests ought to be better and more isolated”, but that’s always the case (as Josh Reed mentioned), and it’s not scalable.
l
Happy to close the naive retry in favor of the more intelligent timeout-only rerunning. So the steps left are perhaps some adjustments to 19760 and then setting pants own CI to use it? Who can pick up the baton on that?
w
Dan pinged me about it a few minutes ago (after seeing this thread presumably), and he indicated he'll apply the feedback soon.
🙌 1
But if that's taking too long, I expect that anyone else could pick up that PR to land it.
c
Stu helped me get this good and merged, hopefully that keep main passing. It could also help identifying flaky tests since they'll have the "+" sigil
🎉 2
3 Views
Open in Slack
PreviousNext
Powered by