💥 engine: Remove --oci-max-parallelism

[gerhard]

Fixes #6894

This is not an option that we should support. Justin puts it best:

I think with (Dagger) Modules, this effectively means that
`max-parallelism` is pretty much just completely broken - it just fits
very badly with the idea of nested containers.

... but to me this kind of highlights that we need a better way of
constraining resources - max-parallelism is just really not fit for
the shape that pipelines tend to take these days. Maybe we should
provide some cgroup config options instead (if the end desire is to
constrain CPU usage/etc).

While this can still be configured if a custom Engine toml config is provided, it means that users went out of their way to have this. It's hard on purpose and we don't recommend using this setting anymore.

This also removes it from the Helm chart.

WDYT @sipsma @jedevc @aluzzardi @vito ?

---

@matipan if everyone is in agreement with this change, we should remove this from all our infra. We can do that before the release goes out.

[gerhard]

Speaking with @aluzzardi, these checks may have failed due to a ClickHouse migration which killed some in-flight traces. Rebasing & force pushing so that we force re-run all failed checks.

[jedevc]

If possible, I'd like to hold off on merging this. If I have the time, I think keeping this option would be best for people using it, but we'd need the improved semantics previously described. But we can keep this open at least as a tracking issue.

@matipan if everyone is in agreement with this change, we should remove this from all our infra. We can do that before the release goes out.

IMO, we should do this as soon as possible - it's definitely a potential candidate for some of the weird ephemeral CI failures we've seen. With this option it's possible to get every running test is a stuck state (if the right combination of tests all start at the right time) - which prevents any progress being made (which sounds like a symptom we've seen before).

[sipsma]

Agree with @jedevc, this option is used already by multiple users and while it's totally buggy once modules are used with too low a value, that feels more like a bug than enough justification to remove this entirely.

• BTW, @gerhard you asked about the num-cpu option; that was added because it was requested by a user a while back. It makes it easier for users to scale this setting across many different hosts with many different CPU counts without having to jump through hoops to update the engine config on each host.

I'd totally be in favor of adding a scary warning message to the help for this though and tracking a long term fix to the problem.

• Actually now that I think about it, all the session related refactoring work should make fixing this a lot more tenable (what would have been a large project to medium at most).

[github-actions]

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[gerhard]

Thank you for the extra context @sipsma & @jedevc.

I will close this & re-open it as a non-breaking change that is restricted to the Helm Chart only.