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: systematically avoid "cannot open autom4te.cache/requests: Read-only file system" issue #70986

Closed
rickystewart opened this issue Oct 1, 2021 · 0 comments · Fixed by #71227
Assignees
Labels
A-build-system C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-dev-inf

Comments

@rickystewart
Copy link
Collaborator

In the Bazel build, compiling the C dependencies can often fail like this:

autom4te: cannot open autom4te.cache/requests: Read-only file system
autom4te: cannot open autom4te.cache/requests: Read-only file system
autoreconf: /usr/bin/autoconf failed with exit status: 1

i.e. autmo4te is trying to use its own cache but Bazel sandboxing is breaking it. I haven't seen this happen on macOS, but it routinely happens on Linux. Unfortunately there isn't an environment variable we can set to unilaterally turn off the cache, and my experimentation with trying to pass the --no-cache argument down to autom4te from Bazel was not productive. In CI we have a wrapper script that passes the --no-cache argument down to the real autom4te, but it's not realistic to ask everyone to put this shim script into their environment.

If we can't figure out how to address it inside of the actual Bazel build, consider adding something to dev.

ref:

Epic CRDB-8036

@rickystewart rickystewart added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-build-system T-dev-inf labels Oct 1, 2021
craig bot pushed a commit that referenced this issue Oct 7, 2021
71083: opt, parser: add support for nulls first, nulls last r=nehageorge a=nehageorge

Previously, we only supported the default Postgres ordering for
ORDER BY. That is, NULLS come first for ascending order and NULLS
come last for descending order. Postgres also supports the keywords
"NULLS FIRST" and "NULLS LAST", which explicitly places all the NULLS
first or last, respectively. This PR adds support for this non-default
ordering.

Informs #6224.

Release note (sql change): NULLS FIRST and NULLS LAST specifiers now
supported for ORDER BY. 

71219: sql: UserHasAdminRole should return true for the admin role itself r=rafiss a=tukeJonny

This PR makes `planner.UserHasAdminRole` returns true on the `admin` role for improving introspection.

This PR had checked by running `make testbaselogic` .

Fixes #70779 

Release note: None

71220: dev: update how we test for presence of `--dev` config in `doctor` r=rail a=rickystewart

Some people are running Linux and can therefore use the `crosslinux`
config on a day-to-day basis. This is OK and `dev doctor` should allow
this. Instead of failing the `doctor` check if `--config dev` isn't set,
we instead test whether the user has stamping enabled on a `bazel
build`. The built `cockroach` binary won't start if stamping is disabled
and both the `dev` and `crosslinux` configs enable stamping.

Release note: None

71227: bazel: avoid `read-only file system` error when calling into `autom4te` r=rail a=rickystewart

See 3f50217 for more context. Many
people are encountering the error described in that commit and it's not
feasible to ask everyone to install an `autom4te` shim. Instead we
include the shim in the build definition. We also remove it from the
builder image to demonstrate that the fix works.

Closes #70986.

Release note: None

71265: changefeedccl: don't warn when Timestamp() is called on flush event r=miretskiy a=stevendanna

When investigating a recent test failure, I noticed the following log
messages:

    ccl/changefeedccl/kvevent/event.go:153 ⋮ [-] 1830  setting empty timestamp for unknown event type
    ccl/changefeedccl/kvevent/event.go:153 ⋮ [-] 1831  setting empty timestamp for unknown event type
    ccl/changefeedccl/kvevent/event.go:153 ⋮ [-] 1832  setting empty timestamp for unknown event type
    ccl/changefeedccl/kvevent/event.go:153 ⋮ [-] 1833  setting empty timestamp for unknown event type
    ccl/changefeedccl/kvevent/event.go:153 ⋮ [-] 1834  setting empty timestamp for unknown event type

One possibility is that these were generated by our new Flush event,
which is an in-band signal between the kvFeed and the changeAggregator
and has no associated timestamp.

This PR adds handling for TypeFlush to cut down on the log spam.

Not however, this raises the question of why we were hitting the
condition that forces a flush so often.

One possibility is that we were unable to allocate resources to add
items to the queue because of some other user of our memory monitor
and thus while we kept trying to flush, these flushes had no
effect. We may consider tracking whether there has been a successful
resource acquisition since the last flush and only resending a flush
if there has been.

Release note: None

71280: sql: clear (rather than emptying) bound account r=[Azhng,miretskiy] a=stevendanna

Empty() leaves some number of bytes in the bound account as
reserved. When we abandon a Container, those "reserved" bytes were
never released back to the parent. I believe we currently abandon one
Container at the end of each explicit transaction.

Fixes #71262
Fixes #71263
Fixes #71264
Fixes #65279

We may want to provide two different methods. One that calls Empty() and one that
calls Clear() in case there are users of this who really need to keep their reserved bytes.

Release note: None

Co-authored-by: Neha George <neha.george@cockroachlabs.com>
Co-authored-by: tukeJonny <ne250143@yahoo.co.jp>
Co-authored-by: Ricky Stewart <ricky@cockroachlabs.com>
Co-authored-by: Steven Danna <danna@cockroachlabs.com>
@craig craig bot closed this as completed in 3764d2a Oct 7, 2021
ericharmeling pushed a commit to ericharmeling/cockroach that referenced this issue Oct 20, 2021
See 3f50217 for more context. Many
people are encountering the error described in that commit and it's not
feasible to ask everyone to install an `autom4te` shim. Instead we
include the shim in the build definition. We also remove it from the
builder image to demonstrate that the fix works.

Closes cockroachdb#70986.

Release note: None
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

Successfully merging a pull request may close this issue.

1 participant