Add Multiraft transport implementation. #292
Conversation
|
@bdarnell I wasn't sure how does the multiraft server id translates into RaftMessageRequest, I assume it is the sender/to info. |
| @@ -23,6 +23,8 @@ import ( | |||
| "strings" | |||
| "sync" | |||
|
|
|||
| "github.com/cockroachdb/cockroach/gossip" | |||
There was a problem hiding this comment.
multiraft shouldn't import gossip (it is at least in principle independent from the rest of cockroach). This cockroach-specific RPCTransport should live elsewhere, probably either cockroach/rpc or cockroach/server.
|
The change looks good aside from where the new code will live. For testing, I think we need something that starts up two rpc servers (hopefully this can be done with just a net/rpc server and a dummy channel-based implementation of ServerInterface, instead of pulling in the rest of cockroach) and two RPCTransports, gossips their addresses, and sends some messages back and forth. We'll also want to wire this in to the storage/raft.go interfaces (although I wouldn't try to do that in the same PR). If we can get rid of singleNodeRaft completely and just use this that would be ideal, although I don't know if we have enough of the gossip machinery running in all the tests to do that. This will probably inform what package the code will live in - we want to be able to import RPCTransport from storage (I think? or maybe we just pass it in when we construct the Store), which rules out cockroach/server. |
|
FYI I'm going to start working on the storage/raft.go changes I referred to above - replace singleNodeRaft with something that can address multiple nodes, although still using LocalRPCTransport for now. |
tnachen
force-pushed
the
raft_transport
branch
from
a772d48 to
6d40b73
Compare
|
Hi Ben, |
|
OK. This isn't blocking anything yet so we can wait until you're back to finish it up. |
tnachen
force-pushed
the
raft_transport
branch
from
6d40b73 to
20e4579
Compare
tnachen
changed the title
|
@bdarnell Can you take a look? I've added one test and moved to another package. |
| gossip.AddInfo("node-2", rpcServer2.Addr(), time.Hour) | ||
|
|
||
| testServer := &TestTransportServer{} | ||
| transport1.Listen(1, testServer) |
There was a problem hiding this comment.
How does this work? You're only binding one TestTransportServer but both nodes are able to connect. I don't think Connect is a strong enough test; you should actually send messages between the two servers (add a channel to TestTransportServer to get the messages out).
There was a problem hiding this comment.
Good point, I was going to create another test but let me modify this one. I'm still figuring out how to get the channel to work.
tnachen
force-pushed
the
raft_transport
branch
from
20e4579 to
90cdb69
Compare
|
Replaced by #483 |
before: in cluster-ui package we used legacy versions of tootips based on antd and with a bunch of code duplicates and hacks. after: used ui-components tooltip component, removed unneeded code and styles. also on process of work find out that barchats legend tooltips missing styles - added them.
before: in cluster-ui package we used legacy versions of tootips based on antd and with a bunch of code duplicates and hacks. after: used ui-components tooltip component, removed unneeded code and styles. also on process of work find out that barchats legend tooltips missing styles - added them.
before: in cluster-ui package we used legacy versions of tootips based on antd and with a bunch of code duplicates and hacks. after: used ui-components tooltip component, removed unneeded code and styles. also on process of work find out that barchats legend tooltips missing styles - added them. (cherry picked from commit 359271e)
before: in cluster-ui package we used legacy versions of tootips based on antd and with a bunch of code duplicates and hacks. after: used ui-components tooltip component, removed unneeded code and styles. also on process of work find out that barchats legend tooltips missing styles - added them.
I haven't added any test, but this is my first patch for cockroach (and pretty much my foray into golang).
So want to put up a early review to make sure I'm not going down a wrong path.