[
https://issues.apache.org/jira/browse/ZOOKEEPER-2353?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15098376#comment-15098376
]
Ivan Kelly commented on ZOOKEEPER-2353:
---------------------------------------
[~fpj] 3.4.8 will be in the same situation as all releases < 3.4.7, so there's
no urgency in changing it.
If we are willing to break 3.5-alpha, we could add a version number to the
protocol in 3.4.x, and then reimplement the 3.5 changes with the new version
number.
In 3.4, the sequence on receiving a connection is.
1. Read a long from socket. This is the sid of the connecting server.
2. We check the sid, if we chose to continue with the connection, we start
reading packets and handing them off to FLE.
3. FLE checks if the buffer length is less than 28 bytes, if so discards it.
Otherwise it processes it.
This means we can create a ClientHandshake packet, as long as we keep it below
28bytes.
To handle cases where a client that doesn't send ClientHandshake connects to a
server that does use it, the server can mark and reset the socket InputSteam
(not sure if it's supported). If doesn't even need to do this though. It can
just ignore the first packet and wait for the next round of election to start.
> QuorumCnxManager protocol needs to be upgradable with-in a specific Version
> ---------------------------------------------------------------------------
>
> Key: ZOOKEEPER-2353
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2353
> Project: ZooKeeper
> Issue Type: Improvement
> Affects Versions: 3.4.7, 3.5.1
> Reporter: Powell Molleti
>
> Currently 3.5.X sends its hdr as follows:
> {code:title=QuorumCnxManager.java|borderStyle=solid}
> dout.writeLong(PROTOCOL_VERSION);
> dout.writeLong(self.getId());
> String addr = self.getElectionAddress().getHostString() + ":" +
> self.getElectionAddress().getPort();
> byte[] addr_bytes = addr.getBytes();
> dout.writeInt(addr_bytes.length);
> dout.write(addr_bytes);
> dout.flush();
> {code}
> Since it writes length of host and port byte string there is no simple way to
> append new fields to this hdr anymore. I.e the rx side has to consider all
> bytes after sid for host and port parsing, which is what it does here:
> [QuorumCnxManager.InitialMessage.parse(): http://bit.ly/1Q0znpW]
> {code:title=QuorumCnxManager.java|borderStyle=solid}
> sid = din.readLong();
> int remaining = din.readInt();
> if (remaining <= 0 || remaining > maxBuffer) {
> throw new InitialMessageException(
> "Unreasonable buffer length: %s", remaining);
> }
> byte[] b = new byte[remaining];
> int num_read = din.read(b);
> if (num_read != remaining) {
> throw new InitialMessageException(
> "Read only %s bytes out of %s sent by server %s",
> num_read, remaining, sid);
> }
> // FIXME: IPv6 is not supported. Using something like Guava's
> HostAndPort
> // parser would be good.
> String addr = new String(b);
> String[] host_port = addr.split(":");
> {code}
> This has been captured in the discussion here: ZOOKEEPER-2186.
> Though it is possible to circumvent this problem by various means the request
> here is to design messages with hdr such that there is no need to bump
> version number or hack certain fields (i.e figure out if its length of
> host/port or length of different message etc, in the above case).
> This is the idea here as captured in ZOOKEEPER-2186.
> {code:java}
> dout.writeLong(PROTOCOL_VERSION);
> String addr = self.getElectionAddress().getHostString() + ":" +
> self.getElectionAddress().getPort();
> byte[] addr_bytes = addr.getBytes();
> // After version write the total length of msg sent by sender.
> dout.writeInt(Long.BYTES + addr_bytes.length);
> // Write sid afterwards
> dout.writeLong(self.getId());
> // Write length of host/port string
> dout.writeInt(addr_bytes.length);
> // Write host/port string
> dout.write(addr_bytes);
> {code}
> Since total length of the message and length of each variable field is also
> present it is quite easy to provide backward compatibility, w.r.t to parsing
> of the message.
> Older code will read the length of message it knows and ignore the rest.
> Newer revision(s), that wants to keep things compatible, will only append to
> hdr and not change the meaning of current fields.
> I am guessing this was the original intent w.r.t the introduction of protocol
> version here: ZOOKEEPER-1633
> Since 3.4.x code does not parse this and 3.5.x is still in alpha mode perhaps
> it is possible to consider this change now?.
> Also I would like to propose to carefully consider the option of using
> protobufs for the next protocol version bump. This will prevent issues like
> this in the future.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
