Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bazel: disable sandboxing for local development #79785

Open
Tracked by #75453
irfansharif opened this issue Apr 11, 2022 · 6 comments
Open
Tracked by #75453

bazel: disable sandboxing for local development #79785

irfansharif opened this issue Apr 11, 2022 · 6 comments
Labels
A-build-system C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-dev-inf

Comments

@irfansharif
Copy link
Contributor

irfansharif commented Apr 11, 2022

Is your feature request related to a problem? Please describe.

Sandboxing on MacOS have measurable overhead, we bazelbuild/bazel#8230; it comes from
symlinks being much slower to create.

Describe the solution you'd like

Disabling sandboxing for local development (still enabling it for CI). It should help speed up bazel builds noticeably.

Describe alternatives you've considered

Rolling out sandboxfs from https://bazel.build/docs/sandboxing, but that seems a lot more experimental and harder to manage.

Additional context

An earlier attempt to disable sandboxing (#79360) was reverted (#79577) after running into opaque build errors. The current theory is that it's due to misconfigured toolchains (see internal chat). I don't think it applies on the M1s; I've been running without sandboxing for weeks now and saw no issues (build --spawn_strategy=local --strategy=Genrule=sandboxed --strategy_regexp='Action pkg/ui'=sandboxed in my .bazelrc.user).

Epic CRDB-8036

Jira issue: CRDB-15915

Epic CRDB-17171

@irfansharif irfansharif added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-build-system T-dev-inf labels Apr 11, 2022
@erikgrinaker
Copy link
Contributor

erikgrinaker commented Apr 11, 2022

Will this be worth it though? One of the nice benefits of Bazel is that it uses a canned build environment, which is managed and deterministic, such that engineers/dev-inf don't have to wrangle with build env problems. I think this would save a non-trivial amount of engineering time. Then again, maybe we could make it configurable.

PS: not a big deal for me personally, since I build on a Linux gceworker. Of course, the one time when I did have to build on macOS, it failed precisely because I never build on macOS so my build env isn't maintained. But I think it's worth considering.

@irfansharif
Copy link
Contributor Author

irfansharif commented Apr 11, 2022

It's worth understanding what build env problems actually arise when disabling sandboxing. The measured overhead I've seen is easily >10% of build time, which adds up. It'll be easy to re-enable sandboxing of course, just a matter of build --spawn_strategy=sandboxed in your .bazelrc).

@erikgrinaker
Copy link
Contributor

erikgrinaker commented Apr 11, 2022

Well, anecdotally it takes e.g. new hires a day to get their first CRDB build. Pretty sweet if they could just do a dev build instead, no? (I have no idea if that's the reality, but it's the dream :))

@irfansharif
Copy link
Contributor Author

My understanding of disabled-sandboxing-mode is that we can still specify exactly what compilers/toolchains to use, and dev build would fetch those exact versions on the fly without relying on the system-installed programs. When disabling sandboxing, I'm talking specifically about how bazel restricts filesystem access: for each build rule it crafts a directory with symlinks out to other files you've explicitly declared and nothing else. See https://bazel.build/docs/sandboxing for more details. We don't do much by-hand-maintenance of BUILD.bazel files anyway, so I think it's unlikely to unintentionally introduce rules to "work locally" and is a debugging nightmare (I could be wrong); especially given we'd still use sandboxing in CI.

@erikgrinaker
Copy link
Contributor

Ah, I misunderstood then -- my bad. Thanks for clarifying!

@jlinder jlinder added sync-me and removed sync-me labels May 20, 2022
Copy link

github-actions bot commented May 1, 2024

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
10 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-system C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-dev-inf
Projects
None yet
Development

No branches or pull requests

4 participants