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

x/build: use streaming RPC to list gomote files #69732

Closed
neild opened this issue Oct 1, 2024 · 9 comments
Closed

x/build: use streaming RPC to list gomote files #69732

neild opened this issue Oct 1, 2024 · 9 comments
Assignees
@neild
Labels
Builders x/build issues (builders, bots, dashboards) NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unreleased

Comments

@neild
Copy link
Contributor

neild commented Oct 1, 2024

The number of files on a gomote has passed an RPC size limit event horizon. I can no longer run gomote push more than once:

$ gomote push
# Using group "debug" from GOMOTE_GROUP
# Pushing GOROOT "/Users/dneil/src/project/openat/go" to "dneil-gotip-windows-amd64-0"...
# Error running push: error listing buildlet's existing files: rpc error: code = ResourceExhausted desc = grpc: received message larger than max (4523362 vs. 4194304)

Perhaps we can increase that limit, but a more future-proof solution would be to use a streaming RPC to list files.
@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label Oct 1, 2024
@gopherbot gopherbot added this to the Unreleased milestone Oct 1, 2024
@gabyhelp
Copy link

gabyhelp commented Oct 1, 2024

Related Issues and Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@neild neild self-assigned this Oct 1, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/617155 mentions this issue: all: update gRPC and protobuf modules

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/617056 mentions this issue: maintner: compare proto messages with proto.Equal

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/617057 mentions this issue: all: regenerate .pb.go files, unify generation process

@dmitshur dmitshur moved this to In Progress in Go Release Oct 1, 2024
@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 1, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/617161 mentions this issue: cmd/gomote: use ListDirectoryStreaming

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/617160 mentions this issue: internal/gomote: add ListDirectoryStreaming RPC

gopherbot pushed a commit to golang/build that referenced this issue Oct 1, 2024
@neild
all: update gRPC and protobuf modules
4d9c15e 
A following CL regenerates the gomote protobuf files,
so update to the most recent gRPC/protobuf modules
while we're at it.

For golang/go#69732

Change-Id: I5de0a5c536ee027556e7bd66d485e6c7658d8a73
Reviewed-on: https://go-review.googlesource.com/c/build/+/617155
Reviewed-by: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
gopherbot pushed a commit to golang/build that referenced this issue Oct 1, 2024
@neild
maintner: compare proto messages with proto.Equal
010e739 
Direct comparison of proto messages (*a == *b) is not supported.
Use proto.Equal instead.

For golang/go#69732

Change-Id: I14db4806e37519a1b988db99ed6c5f5c0284c52b
Reviewed-on: https://go-review.googlesource.com/c/build/+/617056
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
@ArjitGopher
Copy link

I am interested in working for this issue.

gopherbot pushed a commit to golang/build that referenced this issue Oct 2, 2024
@neild
all: regenerate .pb.go files, unify generation process
785db46 
Add a internal/cmd/update-protos command which regenerates all
protobuf files within the repo, using the versions of the
code generators listed in the go.mod file.

Replace the per-directory //go:generate commands with a single
top-level one.

Regenerate all .pb.go files.

For golang/go#69732

Change-Id: I337badfb49fc29f638d71eee609d5995262ccd75
Reviewed-on: https://go-review.googlesource.com/c/build/+/617057
Reviewed-by: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
gopherbot pushed a commit to golang/build that referenced this issue Oct 2, 2024
@neild
internal/gomote: add ListDirectoryStreaming RPC
d74ce56 
ListDirectory responses can overflow the maximum RPC message size.
Add a streaming version which returns its results in chunks.

For golang/go#69732

Change-Id: I3f7d50a3899f0cbc9954da89a26906f1f5850a93
Reviewed-on: https://go-review.googlesource.com/c/build/+/617160
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/617515 mentions this issue: internal/gomote: fix panic in ListDirectoryStreaming

@gabyhelp
Copy link

gabyhelp commented Oct 2, 2024

Related Issues and Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

gopherbot pushed a commit to golang/build that referenced this issue Oct 2, 2024
@neild @mknyszek
internal/gomote: fix panic in ListDirectoryStreaming
25d5d8f 
For golang/go#69732

Change-Id: I72ef0672cfffa7d33afd44f886c67933242801fb
Reviewed-on: https://go-review.googlesource.com/c/build/+/617515
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
gopherbot pushed a commit to golang/build that referenced this issue Oct 2, 2024
@neild
cmd/gomote: use ListDirectoryStreaming
0b78e17 
Use the streaming directory listing RPC to avoid
overflowing the maximum RPC response size.

For golang/go#69732

Change-Id: I2529d3788843844e3ccdde79c12c5c453b09bb25
Reviewed-on: https://go-review.googlesource.com/c/build/+/617161
Reviewed-by: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@neild neild closed this as completed Oct 2, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in Go Release Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@neild neild

Labels
Builders x/build issues (builders, bots, dashboards) NeedsFix The path to resolution is known, but the work has not been done.
Projects
Go Release
Archived in project
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@neild @dmitshur @gopherbot @gabyhelp @ArjitGopher