vendor: update grpc-go and update non-pinned deps #14297
Conversation
|
Please include the "future work" in this PR. I am strongly opposed to
bumping deps separately from the work that depends on that bump.
…On Mar 21, 2017 16:48, "Andrei Matei" ***@***.***> wrote:
Switch from Peter's fork of grpc to my own fork, since Peter's out of
office. My fork is https://github.com/grpc/grpc-go head + Peter's one
commit increasing the per-stream flow-control window.
The motivation of syncing to newer gRPC is to include grpc/grpc-go#993
<grpc/grpc-go#993>
for helping with #13989
<#13989> - we will move to
gRPC internal heartbeats
instead of using our own connection heartbeats in a future cockroach
commit.
Other dep updates:
- the Lightstep update is a single commit that seems innocuous
cc @bdarnell <https://github.com/bdarnell>
------------------------------
You can view, comment on, or merge this pull request online at:
#14297
Commit Summary
- vendor: update grpc-go and update non-pinned deps
File Changes
- *M* glide.lock
<https://github.com/cockroachdb/cockroach/pull/14297/files#diff-0>
(19)
- *M* glide.yaml
<https://github.com/cockroachdb/cockroach/pull/14297/files#diff-1> (4)
- *M* vendor
<https://github.com/cockroachdb/cockroach/pull/14297/files#diff-2> (2)
Patch Links:
- https://github.com/cockroachdb/cockroach/pull/14297.patch
- https://github.com/cockroachdb/cockroach/pull/14297.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#14297>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABdsPICZRiRz49dtcQZUFL1m12yZHfCvks5roDeBgaJpZM4MkYfo>
.
|
Switch from Peter's fork of grpc to my own fork, since Peter's out of office. My fork is https://github.com/grpc/grpc-go head + Peter's one commit increasing the per-stream flow-control window. The motivation of syncing to newer gRPC is to include grpc/grpc-go#993 for helping with cockroachdb#13989 - we will move to gRPC internal heartbeats instead of using our own connection heartbeats in a future cockroach commit. Other dep updates: - the Lightstep update is a single commit that seems innocuous
andreimatei
force-pushed
the
grpc-update
branch
from
52cd449 to
371f3c7
Compare
| version: d3f1ab4a4dca70781cf6dc850a79ed38632660a6 | ||
| repo: https://github.com/petermattis/grpc-go | ||
| version: 9d55a95b90eb10d08a07e0213d23797dbd7b7552 | ||
| repo: https://github.com/andreimatei/grpc-go |
There was a problem hiding this comment.
Let's fork this to cockroachdb org so we all push new refs there, rather than ping-pong-ing between personal forks every time we have a patch.
|
In this case the dep change is literally dead code absent other changes.
…On Mar 21, 2017 17:17, "David Taylor" ***@***.***> wrote:
***@***.**** approved this pull request.
for a core/stability-affecting dep, I actually like the dep-only change
followed by the code-change, just since that helps minimize the
simultaneously moving parts in case we later need to troubleshoot anything.
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#14297 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABdsPKBVHhAiAm8AOWfs8E4-sn6DC36qks5roD5hgaJpZM4MkYfo>
.
|
|
I'm also ok with not forking to CockroachDB because this is not intended to
be a long lived fork.
…On Mar 21, 2017 17:18, "Tamir Duberstein" ***@***.***> wrote:
In this case the dep change is literally dead code absent other changes.
On Mar 21, 2017 17:17, "David Taylor" ***@***.***> wrote:
> ***@***.**** approved this pull request.
>
> for a core/stability-affecting dep, I actually like the dep-only change
> followed by the code-change, just since that helps minimize the
> simultaneously moving parts in case we later need to troubleshoot anything.
>
> —
> You are receiving this because you were assigned.
> Reply to this email directly, view it on GitHub
> <#14297 (review)>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABdsPKBVHhAiAm8AOWfs8E4-sn6DC36qks5roD5hgaJpZM4MkYfo>
> .
>
|
|
Right, I didn't fork it under the Cockroach org cause hopefully our fork can go away when grpc gets better at bandwidth management, for example with grpc/grpc-go#1126 Future work is to use the new grpc options on our conns to enable their heartbeats, and change our heartbeats to only perform clock sync and not destroy conns when they timeout. There's currently a problem that Peter's change to make the conn window equal to the stream window causes a timeout in a grpc test. If I don't make them equal, but I still increase them significantly from the default spec value (which Peter also did), the timeout is gone but we get a failure of a Review status: 0 of 4 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful. Comments from Reviewable |
Switch from Peter's fork of grpc to my own fork, since Peter's out of
office. My fork is https://github.com/grpc/grpc-go head + Peter's one
commit increasing the per-stream flow-control window.
The motivation of syncing to newer gRPC is to include grpc/grpc-go#993
for helping with #13989 - we will move to gRPC internal heartbeats
instead of using our own connection heartbeats in a future cockroach
commit.
Other dep updates:
cc @bdarnell
This change is