[ https://issues.apache.org/jira/browse/ZOOKEEPER-2353?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15093353#comment-15093353 ]
Raul Gutierrez Segales commented on ZOOKEEPER-2353: --------------------------------------------------- Tackling changing the serialization mechanism probably needs to be decoupled from this. We'll probably have to support Jute forever, so we can start with that and then explore using protobuf for server to server messages. > 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)