Skip to content

Conversation

rpoyner-tri
Copy link
Contributor

@rpoyner-tri rpoyner-tri commented Aug 5, 2025

…" (#23239)

This reverts commit 218b815.


This change is Reviewable

Copy link
Contributor Author

@rpoyner-tri rpoyner-tri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The plan here is to get exposure to CI again, and consider what might be done to mitigate out-of-disk-space errors.

Reviewable status: needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", missing label for release notes (waiting on @rpoyner-tri)

Copy link
Contributor Author

@rpoyner-tri rpoyner-tri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", missing label for release notes (waiting on @rpoyner-tri)


a discussion (no related file):
The current set of CI results are probably lies. Need a rebase or a "retest please" once CI is fixed.

@rpoyner-tri
Copy link
Contributor Author

Previously, rpoyner-tri (Rick Poyner (rico)) wrote…

The current set of CI results are probably lies. Need a rebase or a "retest please" once CI is fixed.

@drake-jenkins-bot retest this please

@tyler-yankee
Copy link
Contributor

@drake-jenkins-bot retest this please

Attention contributor: in order for Jenkins to pass, you will need to rebase (or merge) your PR up to the latest master branch, and re-push. If you need help with that, let us know. You can push that change separately, or you can combine it with a push of review fixes.

Copy link
Contributor Author

@rpoyner-tri rpoyner-tri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. There are sill failures, but not from the Jenkins hiccup.

Reviewable status: needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", missing label for release notes (waiting on @rpoyner-tri)

Copy link
Contributor Author

@rpoyner-tri rpoyner-tri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", missing label for release notes (waiting on @rpoyner-tri)


a discussion (no related file):
The only issue remaining is disk space in the CI execution nodes. I see two possible paths forward:

  • give the execution nodes more disk space
  • use bazel configuration tricks (somehow?) to lop off some of the more disk-hogging and debug-dubious targets. The //:py/install_test target is probably the worst offender.

Copy link
Collaborator

@jwnimmer-tri jwnimmer-tri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", missing label for release notes (waiting on @rpoyner-tri)


a discussion (no related file):

Previously, rpoyner-tri (Rick Poyner (rico)) wrote…

The only issue remaining is disk space in the CI execution nodes. I see two possible paths forward:

  • give the execution nodes more disk space
  • use bazel configuration tricks (somehow?) to lop off some of the more disk-hogging and debug-dubious targets. The //:py/install_test target is probably the worst offender.

My 2c:

The main question is what developer experience looks like. For a from-scratch full debug build (without any caching), what is the total footprint for a local developer on master vs with this PR?

If the space increase is in a sensible range (<25%), then we just buy more disk in the Jenkins nodes.

If the space increase is crazy (> 100%) then we'll need to think about impact on developers and whether we might need to pursue additional mitigations to keep local development in the realm of plausibility.

Copy link
Contributor Author

@rpoyner-tri rpoyner-tri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", missing label for release notes (waiting on @rpoyner-tri)


a discussion (no related file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

My 2c:

The main question is what developer experience looks like. For a from-scratch full debug build (without any caching), what is the total footprint for a local developer on master vs with this PR?

If the space increase is in a sensible range (<25%), then we just buy more disk in the Jenkins nodes.

If the space increase is crazy (> 100%) then we'll need to think about impact on developers and whether we might need to pursue additional mitigations to keep local development in the realm of plausibility.

I don't think local dev is at all a problem; especially in the current draft with compress-debug-sections turned on.

I'll see if I can get some disk measurements. AFAIK, just comparing a bazel-bin from master -c dbg vs this branch should get me an absolute difference. Seems like coming up with a percentage increase that's at all meaningful would be harder.

Copy link
Collaborator

@jwnimmer-tri jwnimmer-tri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", missing label for release notes (waiting on @rpoyner-tri)


a discussion (no related file):

Previously, rpoyner-tri (Rick Poyner (rico)) wrote…

I don't think local dev is at all a problem; especially in the current draft with compress-debug-sections turned on.

I'll see if I can get some disk measurements. AFAIK, just comparing a bazel-bin from master -c dbg vs this branch should get me an absolute difference. Seems like coming up with a percentage increase that's at all meaningful would be harder.

We want more than just bazel-bin. We want the total size of bazel's outputBase (see https://bazel.build/remote/output-directories) for a from-scratch build, with local disk caching turned off. That will be an total number of MiB, and we can A/B compare master vs this PR. I can make the numbers, if that's easier?

Copy link
Contributor Author

@rpoyner-tri rpoyner-tri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", missing label for release notes (waiting on @rpoyner-tri)


a discussion (no related file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

We want more than just bazel-bin. We want the total size of bazel's outputBase (see https://bazel.build/remote/output-directories) for a from-scratch build, with local disk caching turned off. That will be an total number of MiB, and we can A/B compare master vs this PR. I can make the numbers, if that's easier?

go for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants