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.

Sign up for GitHub

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

Jump to bottom

dev,bazel: actions/build cache partially discarded when switching between dev and bazel #71835

Closed
irfansharif opened this issue Oct 21, 2021 · 4 comments · Fixed by #71856
Closed

dev,bazel: actions/build cache partially discarded when switching between dev and bazel #71835

irfansharif opened this issue Oct 21, 2021 · 4 comments · Fixed by #71856
Assignees
@rickystewart
Labels
A-build-system C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@irfansharif
Copy link
Contributor

irfansharif commented Oct 21, 2021 •
edited
Loading

Describe the problem

I'm seeing the we're doing additional work when switching between dev and raw bazel. Not a problem in of itself, but curious why. bazelbuild/bazel#10902?

To Reproduce

$ ./dev build cockroach-short
$ bazel build //pkg/cmd/cockroach-short:cockroach-short
INFO: Analyzed target //pkg/cmd/cockroach-short:cockroach-short (1 packages loaded, 2825 targets configured).
INFO: Found 1 target...
Target //pkg/cmd/cockroach-short:cockroach-short up-to-date:
  _bazel/bin/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short
INFO: Elapsed time: 1.891s, Critical Path: 0.75s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
Successfully built binary for target //pkg/cmd/cockroach-short:cockroach-short at cockroach-short

$ ./dev build cockroach-short
$ bazel build //pkg/cmd/cockroach-short:cockroach-short
INFO: Analyzed target //pkg/cmd/cockroach-short:cockroach-short (1 packages loaded, 2825 targets configured).
INFO: Found 1 target...
Target //pkg/cmd/cockroach-short:cockroach-short up-to-date:
  _bazel/bin/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short
INFO: Elapsed time: 1.649s, Critical Path: 0.66s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
Successfully built binary for target //pkg/cmd/cockroach-short:cockroach-short at cockroach-short

$ bazel build //pkg/cmd/cockroach-short:cockroach-short
INFO: Analyzed target //pkg/cmd/cockroach-short:cockroach-short (1 packages loaded, 5 targets configured).
INFO: Found 1 target...
INFO: From Linking external/com_google_protobuf/libprotobuf_lite.a:
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: file: bazel-out/darwin-opt-exec-2B5CBBC6/bin/external/com_google_protobuf/_objs/protobuf_lite/io_win32.o has no symbols
[76 / 279] 16 actions, 15 running
    Compiling src/google/protobuf/util/internal/default_value_objectwriter.cc; 3s darwin-sandbox
    Compiling src/google/protobuf/util/internal/field_mask_utility.cc; 3s darwin-sandbox
    Compiling src/google/protobuf/util/field_comparator.cc; 2s darwin-sandbox
    Compiling src/google/protobuf/util/field_mask_util.cc; 2s darwin-sandbox
    Compiling src/google/protobuf/util/internal/datapiece.cc; 2s darwin-sandbox
    Compiling src/google/protobuf/text_format.cc; 2s darwin-sandbox
    Compiling src/google/protobuf/timestamp.pb.cc; 1s darwin-sandbox
    Compiling src/google/protobuf/type.pb.cc; 1s darwin-sandbox ...

Expected behavior

The final command simply being able to re-use existing cached artifacts/analysis.
@irfansharif irfansharif added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-build-system labels Oct 21, 2021
@rickystewart
Copy link
Collaborator

I'm not seeing that behavior -- for me, the final bazel build //pkg/cmd/cockroach-short:cockroach-short takes just as little time as the previous ./dev build short. Is your ./dev doctor clean?

cockroach$ ./dev build short 
$ bazel build //pkg/cmd/cockroach-short:cockroach-short
INFO: Analyzed target //pkg/cmd/cockroach-short:cockroach-short (711 packages loaded, 10674 targets configured).
INFO: Found 1 target...
Target //pkg/cmd/cockroach-short:cockroach-short up-to-date:
  _bazel/bin/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short
INFO: Elapsed time: 256.142s, Critical Path: 224.03s
INFO: 994 processes: 1 internal, 993 darwin-sandbox.
INFO: Build completed successfully, 994 total actions
Successfully built binary for target //pkg/cmd/cockroach-short:cockroach-short at cockroach-short
cockroach$ ./dev build short 
$ bazel build //pkg/cmd/cockroach-short:cockroach-short
INFO: Analyzed target //pkg/cmd/cockroach-short:cockroach-short (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //pkg/cmd/cockroach-short:cockroach-short up-to-date:
  _bazel/bin/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short
INFO: Elapsed time: 1.822s, Critical Path: 1.39s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
Successfully built binary for target //pkg/cmd/cockroach-short:cockroach-short at cockroach-short
cockroach$ bazel build //pkg/cmd/cockroach-short:cockroach-short
INFO: Analyzed target //pkg/cmd/cockroach-short:cockroach-short (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //pkg/cmd/cockroach-short:cockroach-short up-to-date:
  _bazel/bin/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short
INFO: Elapsed time: 1.765s, Critical Path: 1.36s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action

@rickystewart
Copy link
Collaborator

Oh, and are you at the latest master?

@irfansharif
Copy link
Contributor Author

Yup, seeing the same thing as of 6ef0b11.

@irfansharif
Copy link
Contributor Author

Tried debugging a bit further, it looks like our protobuf dependency keeps getting recompiled when switching between the two subsystems. That led me to bazelbuild/intellij#1169 and bazelbuild/bazel#7095. Adding https://docs.bazel.build/versions/main/command-line-reference.html#flag--incompatible_strict_action_env to my .bazelrc.user got rid of the dependency on PATH and it's back to being what I expect.

irfansharif added a commit to irfansharif/cockroach that referenced this issue Oct 22, 2021
@irfansharif
bazel: skip recompilation when using dev+bazel
1b47263 
Fixes cockroachdb#71835. When switching between using `dev` and `bazel` raw, I kept
seeing our C++ protobuf dependency getting recompiled (slowly).
It appears that the bazel build for protobuf has a dependency on $PATH
(see bazelbuild/intellij#1169 and bazelbuild/bazel#7095). Specifying
`--incompatible_strict_action_env` pins PATH and avoids the build cache
thrashing we were seeing before.

Release note: None
@irfansharif irfansharif mentioned this issue Oct 22, 2021
Merged
craig bot pushed a commit that referenced this issue Oct 22, 2021
@rafiss @celiala @irfansharif
Merge #71632 #71836 #71856
df06437 
71632: sql: make sure pgwire bind always happens in a transaction r=otan a=rafiss

fixes #70378
and maybe #64140

The approach of using a transaction matches what we do for pgwire Parse
messages already. This is important to make sure that user-defined types
are leased correctly.

This also updated the SQL EXECUTE command to resolve user-defined types
so that it gets the latest changes.

Release note (bug fix): Adding new values to a user-defined enum type
will previously would cause a prepared statement using that type to not
work. This now works as expected.

71836: clusterversion: introduce 22.1 development versions r=celiala a=celiala

Fix: #69828

Looking at past Version values for `Start{XX_X}`, I think the `Version` value for `Key: Start22_1` should instead be `Major: 21, Minor: 2, ...`.


Start21_1 (from #70268):
```
// v21.1 versions. Internal versions defined here-on-forth must be even.
{
  Key:     Start21_1,
  Version: roachpb.Version{Major: 20, Minor: 2, Internal: 2},
},
```

Start 21_2:
```
{
  Key:     Start21_2,
  Version: roachpb.Version{Major: 21, Minor: 1, Internal: 1102},
},
```

Release justification: Non-production code change.
Release note: None

71856: bazel: skip recompilation when using dev+bazel r=irfansharif a=irfansharif

Fixes #71835. When switching between using `dev` and `bazel` raw, I kept
seeing our C++ protobuf dependency getting recompiled (slowly).
It appears that the bazel build for protobuf has a dependency on $PATH
(see bazelbuild/intellij#1169 and bazelbuild/bazel#7095). Specifying
[`--incompatible_strict_action_env`](https://docs.bazel.build/versions/main/command-line-reference.html#flag--incompatible_strict_action_env) pins PATH and avoids the build cache
thrashing we were seeing before.

Release note: None

Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Celia La <[email protected]>
Co-authored-by: irfan sharif <[email protected]>
@craig craig bot closed this as completed in ff59b0d Oct 22, 2021
@craig craig bot closed this as completed in #71856 Oct 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rickystewart rickystewart

Labels
A-build-system C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

bazel: skip recompilation when using dev+bazel irfansharif/cockroach
2 participants
@rickystewart @irfansharif