DEPS: upgrade grpc to v1.57.2 #138283
DEPS: upgrade grpc to v1.57.2 #138283
Conversation
|
This change is |
|
This builds just fine but bazel-remote seems to throw a spanner in the works: https://github.com/buchgr/bazel-remote/blob/v1.3.3/go.mod#L24 When we get that version of because of this https://github.com/googleapis/go-genproto/blob/6b3ec007d9bb/googleapis/bytestream/go.mod#L6 I don't think that requirement is particularly hard. Looking at the So one thing we could do is to fork @rickystewart what do you think? |
|
Hmm. I'm pretty uncertain about upgrading I don't fully understand the train of thought in your comment. The goal is to upgrade |
|
FYI, it looks like the |
I don't like this either. The problem is that I find myself unable to upgrade to v1.57.2 without hacks.
It's less a train of thought than a reaction to circumstances beyond my grasp. I did the following:
I can never quite follow what The only import for this package is through bazel-remote. As of the first commit (pre go mod tidy): and this matches what happens on Sadly, so It's not fair to blame https://github.com/buchgr/bazel-remote/blob/400101af9d14bae5fb400b80b8dbd279b4b30075/go.mod#L23-L24 but just the fact that it depends on I'm unsure how to proceed, but I hope that I've given you an idea of what's going on here. |
|
Btw: upgrading |
|
Another attempt at refining the narrative. Upgrading gRPC forces However, nobody seems to really depend on Something is nudging us into the After the grpc bump (before the https://github.com/googleapis/go-genproto/tree/0005af68ea5 Before (i.e. master), it was this one: https://github.com/googleapis/go-genproto/tree/daa745c078e1 In the old one, Who depends on the package? I think it's just and in particular nobody imports the module (yet): however, this isn't stable. Because Let's feed it a sane one, the oldest possible one in fact: googleapis/go-genproto@2805bf8 This is good! It didn't mess with grpc yet. We keep v1.57.2 and Omg I think this did the trick. |
A few former subpackages are now proper modules, which requires some gazelle/bzl wrangling. This caused a few spurious diffs where unrelated lines got reordered. I didn't do this manually.
|
Added benchmark results - I wouldn't call them "favorable" exactly (more small allocs), but at least it's not a major regression and the single-thread latency is same or slightly better in the areas where we most care about it. Given the pain this saves with the deps, I'll have to say it's worth it. bors r+ |
|
@tbg I think this commit is causing a problem. On my GCE worker: |
This broke silently in cockroachdb#138283.
139041: DEPS: fix build when cache clear r=tbg a=tbg This broke silently in #138283. As of that PR, you'd get ``` $ ./dev cache --down; ./dev cache --clear; bazel clean --expunge; ./dev build short ``` fail with ``` ERROR: no such package '`@@org_golang_google_genproto_googleapis//rpc/code':` The repository '`@@org_golang_google_genproto_googleapis'` could not be resolved: Repository '`@@org_golang_google_genproto_googleapis'` is not defined ERROR: /private/var/tmp/_bazel_tbg/b1346cddcc70d57afdaa90f7f09f9b2c/external/com_github_buchgr_bazel_remote/server/BUILD.bazel:3:11: no such package '`@@org_golang_google_genproto_googleapis//rpc/code':` The repository '`@@org_golang_google_genproto_googleapis'` could not be resolved: Repository '`@@org_golang_google_genproto_googleapis'` is not defined and referenced by '`@@com_github_buchgr_bazel_remote//server:go_default_library'` ERROR: Analysis of target '`@@com_github_buchgr_bazel_remote//:bazel-remote'` failed; build aborted: Analysis failed ``` Now it works. Closes #139038. Epic: none Release note: none Co-authored-by: Tobias Grieger <tobias.b.grieger@gmail.com>
This broke silently in cockroachdb#138283.
See #136278 (comment).
grpchas gotten a little worse at allocations, but it's overall similarly fast, perhaps even a little faster in the smaller RPCs we care most about.Benchmark results
Epic: None
Release note: None