Skip to content

Conversation

@heyitsanthony
Copy link

No description provided.

@gyuho
Copy link
Contributor

gyuho commented Jun 8, 2016

Maybe rebase the grpc dep with your fix grpc/grpc-go#717? Just got merged.

"google.golang.org/grpc"
)

type balancer struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

randomizedBalancer?

Copy link
Contributor

Choose a reason for hiding this comment

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

any reason for not using the default roundrobin balancer provided by grpc?

Copy link
Author

Choose a reason for hiding this comment

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

using the grpc roundrobin balancer would just as complicated (or more so) since it needs resolver and watcher interface implementations

Copy link
Contributor

Choose a reason for hiding this comment

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

ok.

@xiang90
Copy link
Contributor

xiang90 commented Jun 8, 2016

LGTM after fixing tests.

@gyuho
Copy link
Contributor

gyuho commented Jun 8, 2016

So rpctypes.ErrConnClosed == grpc.ErrClientConnClosing, now?

LGTM.

/cc @purpleidea @siddontang regarding the issue #5495.

@heyitsanthony
Copy link
Author

@gyuho yes. It probably didn't belong in rpctypes in the first place since the error isn't transmitted from the server.

@heyitsanthony heyitsanthony force-pushed the grpc-nuke-waitstate branch 4 times, most recently from deba127 to 70f642d Compare June 8, 2016 07:32
Tests need to disconnect the network connection for the client to check
reconnection paths but closing a grpc connection closes the logical connection.
To disconnect the client, instead have a bridge between the server and
the client which can monitor and reset connections.
@heyitsanthony heyitsanthony force-pushed the grpc-nuke-waitstate branch 2 times, most recently from 0be40af to 6b13c7f Compare June 8, 2016 08:02
@heyitsanthony heyitsanthony force-pushed the grpc-nuke-waitstate branch 3 times, most recently from 782283b to 810f11c Compare June 8, 2016 16:05
@heyitsanthony heyitsanthony force-pushed the grpc-nuke-waitstate branch from 810f11c to 4a13c9f Compare June 8, 2016 16:27
@heyitsanthony heyitsanthony merged commit ff2b24a into etcd-io:master Jun 8, 2016
@heyitsanthony heyitsanthony deleted the grpc-nuke-waitstate branch June 8, 2016 17:58
@purpleidea
Copy link
Contributor

There are virtually no commit messages or docs associated with this that I can find. Is there a short summary somewhere that we can read to learn more? Thanks!

@xiang90
Copy link
Contributor

xiang90 commented Jun 9, 2016

@purpleidea
Copy link
Contributor

@heyitsanthony
Copy link
Author

@purpleidea the motivation was so that go get github.com/coreos/etcd/clientv3 would work/compile again after a grpc API change. The functional difference is network reconnection logic is being handled by grpc now so the client is much simpler. grpc will be able to dynamically configure the client's endpoints as it's running so it won't be necessary to tear down the client to change endpoints

@purpleidea
Copy link
Contributor

@heyitsanthony Gahhh! Sounds like #5491 ... I wish I knew this was coming, great feature, but I've just spent three or so days fixing my code so that it can handle this scenario! Oh well glad to know this is coming.

I have a bunch of code that depends on it if you'd like to test your new API for this before it goes mainstream. I hope it includes dealing with the change when an endpoint crashes or is fenced somehow.

As an aside: where is the best place to learn about all these new and planned changes. Is there a mailing list where it's discussed or is it internal only chats? It would probably save me and other developers time to know what's going on.

Cheers!

@heyitsanthony
Copy link
Author

@purpleidea https://github.com/coreos/etcd/milestones has the planning. It's difficult to capture everything to the last detail though.

@purpleidea
Copy link
Contributor

On Wed, Jun 8, 2016 at 11:46 PM, Anthony Romano [email protected]
wrote:

@purpleidea https://github.com/purpleidea
https://github.com/coreos/etcd/milestones has the planning. It's
difficult to capture everything to the last detail though.

This is somewhat useful, thanks! I am interested in the big picture ideas,
so if there are ghangouts or similar where discussions are happening and
you feel like including me, please do! Cheers!

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants