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

x/build/cmd/relui: "Upload modules to CDN" can fail and is not idempotent #60921

Open
dmitshur opened this issue Jun 21, 2023 · 3 comments
Open
Labels
Builders x/build issues (builders, bots, dashboards) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@dmitshur
Copy link
Contributor

dmitshur commented Jun 21, 2023

Seeing BuildReleaseTasks.uploadModules sometimes make partial progress, then fail, and begin to retry. Retrying requires additional permissions to be able to overwrite files it has already started to upload on a previous attemt.

It doesn't seem to log or otherwise make it apparent why a retry was needed in the first place. Tracking issue to resolve this.

Also see related issue #45893 (it was closed after releasebot went away).

CC @golang/release.

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 21, 2023
@dmitshur dmitshur added this to the Unreleased milestone Jun 21, 2023
@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label Jun 21, 2023
@rsc
Copy link
Contributor

rsc commented Jun 21, 2023

Noting here that a straightforward fix would be to handle the err != nil case by checking whether the file we want to upload already exists and has the right SHA256 checksum. If so, err = nil. This would handle restarted relui jobs as well as any internal retries that may or may not be in the GCS uploading library.

@dmitshur dmitshur changed the title x/build/cmd/relui: "Upload modules to CDN" sometimes fails x/build/cmd/relui: "Upload modules to CDN" can fail and is not idempotent Jun 21, 2023
@gopherbot
Copy link

Change https://go.dev/cl/504820 mentions this issue: internal/releasetargets: remove duplicate linux-arm target

@heschi
Copy link
Contributor

heschi commented Jun 21, 2023

Turns out to have been a real bug, not just a glitch, and retrying would've masked it. I still agree that we should make uploads idempotent, though.

gopherbot pushed a commit to golang/build that referenced this issue Jun 21, 2023
The code that added missing secondary ports didn't know about the v6l
suffix. Teach it.

Also change the testing gcsfs implementation to reject overwrites so the
problem would have been caught in tests.

For golang/go#60921

Change-Id: I61f7bd9c250425f0418d92885710382dccd60591
Reviewed-on: https://go-review.googlesource.com/c/build/+/504820
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Builders x/build issues (builders, bots, dashboards) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Status: Planned
Development

No branches or pull requests

4 participants