Implement protocol version so that we can have mixed cluster during u… #16
Conversation
cmd/zrepl/zrepl.c
Outdated
| if (count <= 0) { | ||
| ZREPL_ERRLOG("Replica-iSCSI Tgt connection got " | ||
| "disconnected with err:%d\n", errno); | ||
| rc = read_handshake(sfd, &hdr); |
There was a problem hiding this comment.
We need to support/manage many more commands here. Current code assume that it will handle just HANDSHAKE cmd/msg only. We need to have Switch statement here to handle different commands received from iSCSI tgt (Handshake is one of them, now for rebuild I am implementing many more).
There was a problem hiding this comment.
agreed. Although the situation is even a bit more complicated. Reading handshake is not like reading any other command:
- handshake command must come first
- handshake command must not repeat
- only first 2 bytes need to be read from handshake message before anything else is, to decide if the sender's version is supported.
Hence I would process handshake command out of the loop and then have the switch statement inside the loop as you have described, but only for all other commands except handshake. But I would make this change once we add more commands - not now when there is just one.
There was a problem hiding this comment.
hm, it actually may repeat. It should not repeat on data connection but can repeat on mgmt connection in case iscsit requests information (host:port) for multiple volumes. What do you think? In that case we should perhaps decouple handshake message from zvol query. I mean as in the following picture (that is for mgmt conn):
zrepl iscsit
------connect----->
------HS msg----->
<-----HS repl------
<----zvol query---
-----zvol info------>
<----zvol query---
-----zvol info----->
....
That would eliminate one of the current weirdnesses of the protocol when the client (zrepl) which connects to iscsi, expects iscsit to send the first msg on connection. Now it would be the zrepl who sends HS over the wire and iscsit (server from this point of view) replies to the handshake.
There was a problem hiding this comment.
That can be done i.e. first message from Zrepl to iSCSI controller. But here HS is meant for finding volume which iSCSI know off. Zreplica search that volume and reply back to controller with appropriate response. If Replica has to send first message, I am not what info it should send. I think we should handle HS separately (instead of making code generic for all commands, we can have separate handling for HS).
I can see that I need this flexible/generic code very soon as I am implementing many more commands for rebuild that is why I asked if we can make change now only so that subsequent check-ins do not need to make lots of changes around this area.
include/zrepl_prot.h
Outdated
| * TODO: All structures should be defined with "packed" attribute to avoid | ||
| * ambiguous padding added by compiler. | ||
| * We don't expect replica protocol to be used between nodes with different | ||
| * architecture nevetheless we try to be precise in defining size of members |
jkryl
force-pushed
the
zrepl-prot
branch
5 times, most recently
from
b4de692
to
2e8972a
Compare
…er during upgrade Signed-off-by: Jan Kryl <jan.kryl@cloudbyte.com>
… rebuilding (mayadata-io#16) Signed-off-by: Pawan <pawan@mayadata.io>
…pgrade (#55)
This is a proposal for versioning. Suggestions for improvements are welcomed. The most important change is new version field in zrepl messages which is at the first place in "packet". The field is taken into account only in handshake messages. Either client or server first reads initial two bytes which are the version number, and if it determines that the version is incompatible, it does not try to read the rest of the message. zrepl checks the version in handshake message from a target and replies with "version mismatch" error if the version is different from the one supported. In addition to that, it puts the supported version number into reply packet (again in the version field) and closes the connection. So in theory if target supports multiple protocol versions it can wait for a new connection from replica (happens every 5s) and send a handshake with supported version.
Other changes bundled with this change is removal of void pointer from network messages, fixed layout of structures used for network messages (not dependent on cpu architecture and compiler). Other than that I tried to keep the protocol as it was, with minimal changes.
Interesting are C++ unit tests which test three cases: