Would you mind to fire one issue to track whether to ignore serialization ID for heartbeat message? I think it's worth it.
On Tue, Jun 19, 2018 at 3:55 PM 风暴角的企鹅 <[email protected]> wrote: > To Lan: > > > Thank you. I agree with what you said. > > > Whether ignoring the serialization ID or not, it is up to your committer's > choice. Wait for your further decision. > > > > > > > > ------------------ 原始邮件 ------------------ > 发件人: "Ian Luo"<[email protected]>; > 发送时间: 2018年6月19日(星期二) 下午3:28 > 收件人: "dev"<[email protected]>; > > 主题: Re: 服务端收到心跳包的时候不要检查 header 里面的 serialID > > > > Let's use dubbo protocol [1] to discuss. > > When you refer 'serial ID' here, do you mean serialization ID in protocol > from 16 bit to 20 bit? If this is the case, I am afraid I cannot agree with > you, since serialization ID between 16-bit and 20-bit is used to indicate > how the data should be deserialized, which has the possible values like > hessian2 (2), java (3), kryo (8) and so on. In fact, Dubbo reserves 5 bits > for the different serialization options. > > But for heartbeat, it's kinda event (which is decided by bit 21) instead of > serialization type (bit 16 - bit 20). > > I agree that it is reasonable to ignore serialization ID when dubbo finds > the message is a heartbeat, but it's based on the fact that heartbeat is > the only one event type implemented so far. Heartbeat is an event with no > body attached, but in the future it is possible to have other event type > which may have body, then it starts to make sense to have the serialization > ID defined in an event message. > > Regards, > -Ian. > > 1. http://dubbo.apache.org/books/dubbo-dev-book/implementation.html > > On Tue, Jun 19, 2018 at 12:09 PM 风暴角的企鹅 <[email protected]> wrote: > > > Thank you for your reply. At least, the heartbeat packet serial ID does > > not conflict with the logic packet serial ID. What my suggestion is the > > logic packet should filter the serial ID including [1, 3, 4, 5, 6, 7] > while > > the heartbeat packet should not filter the serial ID array [1, 3, 4, 5, > 6, > > 7]. Because the serial ID array [1, 3, 4, 5, 6, 7] is logic instance > > serialization ID while it can not be the the heartbeat instance > > serialization ID at the same time. > > > > > > For example, if the heartbeat packet serialization ID is 8, the dubbo > > heartbeat handle function just need to filter 8. It need not care about > > [1, 3, 4, 5, 6, 7]. > > > > > > > > > > ------------------ 原始邮件 ------------------ > > 发件人: "Ian Luo"<[email protected]>; > > 发送时间: 2018年6月15日(星期五) 下午2:05 > > 收件人: "dev"<[email protected]>; > > > > 主题: Re: 服务端收到心跳包的时候不要检查 header 里面的 serialID > > > > > > > > The original design allows heartbeat packet to have data included, though > > the current implementation is in fact an empty packet. You can take a > look > > at com.alibaba.dubbo.remoting.exchange.Response#HEARTBEAT_EVENT, which is > > set to null right now. > > > > If in the future a valid data is required in heartbeat packet, then the > > current logic for serialID will be necessary. I'd like to listen to > others' > > comments before we decide either to ignore the check or keep the current > > design. > > > > -Ian. > > > > On Thu, Jun 14, 2018 at 9:44 PM 风暴角的企鹅 <[email protected]> wrote: > > > > > Dr All: > > > 我写了一个 dubbo 原生协议（hessian2）的go语言测试客户端，代码如下： > > > > > https://github.com/alexstocks/go-practice/blob/master/hessian/main.go > > > > > > > > > 我构造了一个二进制心跳包对服务端进行测试。 > > > > > > > > > 示例代码中我循环增大serialID从0开始直到其最大值31。其中 serialID 取值 [1, 3, 4, 5, 6, 7] > > > 时，服务端有以下异常日志： > > > > > > > > > 2018-06-14 14:50:04,598 [New I/O server worker #1-1] WARN > > > alibaba.dubbo.rpc.protocol.dubbo.DubboCodec (DubboCodec.java:140) - > > > [DUBBO] Decode request failed: Flag error, expect > > > OBJECT_NULL|OBJECT_DUMMY|OBJECT_DESC|OBJECT_DESC_ID, get 78, dubbo > > version: > > > 2.5.4, current host: 127.0.0.1 > > > java.io.IOException: Flag error, expect > > > OBJECT_NULL|OBJECT_DUMMY|OBJECT_DESC|OBJECT_DESC_ID, get 78 > > > at > > > > > > com.alibaba.dubbo.common.serialize.support.dubbo.GenericObjectInput.readObject(GenericObjectInput.java:79) > > > at > > > > > > com.alibaba.dubbo.remoting.exchange.codec.ExchangeCodec.decodeHeartbeatData(ExchangeCodec.java:381) > > > at > > > > > > com.alibaba.dubbo.rpc.protocol.dubbo.DubboCodec.decodeBody(DubboCodec.java:121) > > > at > > > > > > com.alibaba.dubbo.remoting.exchange.codec.ExchangeCodec.decode(ExchangeCodec.java:118) > > > at > > > > > > com.alibaba.dubbo.remoting.exchange.codec.ExchangeCodec.decode(ExchangeCodec.java:79) > > > at > > > > > > com.alibaba.dubbo.rpc.protocol.dubbo.DubboCountCodec.decode(DubboCountCodec.java:46) > > > at > > > > > > com.alibaba.dubbo.remoting.transport.netty.NettyCodecAdapter$InternalDecoder.messageReceived(NettyCodecAdapter.java:134) > > > at > > > > > > org.jboss.netty.channel.SimpleChannelUpstreamHandler.handleUpstream(SimpleChannelUpstreamHandler.java:80) > > > at > > > > > > org.jboss.netty.channel.DefaultChannelPipeline.sendUpstream(DefaultChannelPipeline.java:564) > > > at > > > > > > org.jboss.netty.channel.DefaultChannelPipeline.sendUpstream(DefaultChannelPipeline.java:559) > > > at > > > org.jboss.netty.channel.Channels.fireMessageReceived(Channels.java:274) > > > at > > > org.jboss.netty.channel.Channels.fireMessageReceived(Channels.java:261) > > > at > > org.jboss.netty.channel.socket.nio.NioWorker.read(NioWorker.java:349) > > > at > > > > > > org.jboss.netty.channel.socket.nio.NioWorker.processSelectedKeys(NioWorker.java:280) > > > at > org.jboss.netty.channel.socket.nio.NioWorker.run(NioWorker.java:200) > > > at > > > > > > org.jboss.netty.util.ThreadRenamingRunnable.run(ThreadRenamingRunnable.java:108) > > > at > > > > > > org.jboss.netty.util.internal.DeadLockProofWorker$1.run(DeadLockProofWorker.java:44) > > > at > > > > > > java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) > > > at > > > > > > java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) > > > at java.lang.Thread.run(Thread.java:748) > > > 2018-06-14 14:50:05,599 [New I/O server worker #1-1] WARN > > > com.alibaba.dubbo.remoting.transport.AbstractServer > > > (AbstractServer.java:199) - [DUBBO] All clients has discontected from > / > > > 127.0.0.1:20000. You can graceful shutdown now., dubbo version: 2.5.4, > > > current host: 127.0.0.1 > > > > > > > > > > > > 经过与 dubbo commiter 们讨论，说是这几个 ID 是内部保留的实例ID，如 > > > > > > https://github.com/apache/incubator-dubbo/blob/2.5.x/dubbo-common/src/main/java/com/alibaba/dubbo/common/serialize/support/json/FastJsonSerialization.java > > > 。 > > > > > > > > > 针对这个例子，这个解释是 okay 的，可以解释这个错误。不过个人建议：服务端收到心跳包的时候不要检查 header 里面的 > > > serialID，直接放过，可否？ > > > > > > > > > > > > > > > 因为心跳包不设计逻辑处理，不需要获取serialID对应的实例，这里检查这个字段有什么意义呢？因为这个原因就导致服务端抛出异常，个人觉得这个处理欠妥。
