Terry-Mao / goim

goim
https://goim.io/
MIT License
7.21k stars 1.78k forks source link

room broadcast msg在comet里websocket send的时候json marshal错误 #70

Closed liutaihua closed 8 years ago

liutaihua commented 8 years ago

日志里错误打印: dispatch websocket error(json: error calling MarshalJSON for type json.RawMessage: invalid character '\x00' looking for beginning of value)

case就是doc里的 curl -d "{\"test\": 1}" http://127.0.0.1:7172/1/push/room?rid=1 roomId我是在client.js里做auth的时候随意个userId在body里, 让logic里走到 roomId=1的逻辑返回里生成的roomId

我打日志跟了一下, job模块里,在做proto.Body入buf之前都还是正常的,bytes.Writer之后RPC call到comet就不对了, 不知道我看的对不对。 看上去是在做proto.WriteTo的地方构建协议包的地方offset截断了json中的一个字符导致, proto的Operation也由5变成11,12之类的了,很像是offset问题 不知道是否是这样

Terry-Mao commented 8 years ago

多谢,反馈这个是一个bug,目前websocket还不支持room的广播,这块我们尽快修复,如果可能能否用tcp呢?这块我看下尽快跟进;

Terry-Mao commented 8 years ago

@thinkboy

liutaihua commented 8 years ago

tcp是正常的 :) 谢谢

liutaihua commented 8 years ago

补充一个,即时用websocket,不应当限死json结构的,代码里用的websocket.JSON.Send, 蛤蛤, 不过我看到上面的TODO了

Leozki commented 8 years ago

昨天也遇到这个问题,打印出hex看了一下,p.body是tcp包的结构~

liutaihua commented 8 years ago

我自己尝试了fix 原来raw包是完整封在p.Body里, 所以就在ws做send前把 p.Body里的buf读出来构造一个新的Proto, 然后websocket.json.send

如果这么做,这里就还有个问题,job模块会做buffer应该是为了减少小消息的频率,一起做大包发送吧, 所以在ws协议这里, 如果p.Body里是多个proto的对象, 就需要逐个拆包,单独发送了。因为用的json结构。 BTW, ws这里做新Proto构造, 我是直接从Ring里拿一个然后赋值属性, 不知道这样的做法是否妥当。

tcp无此问题

Terry-Mao commented 8 years ago

websocket的做法,可能会使用兼容协议处理,目前还在想怎么办,因为推送方可能是二进制协议内容,我问过前端同事,他们也能处理二进制,所以我在考虑是不是websocket也使用二进制方式放松。

否则就要考虑其他方式兼容了。

Terry-Mao commented 8 years ago

或者还有一种做法,在聚合消息的时候分成两份,其中一份拼凑成json数组,然后在推送的时候,区分websocket还是tcp使用不同的内容,但是这样内网流量会增加

liutaihua commented 8 years ago

我修改了下,然后使用websocket协议. 测试不怎么理想

有几个问题, 不知道是否方便答疑 1, 为了省内存,tcp相关的write buff, read buff我都直接在comet.conf里设到最小了, 这个应该不影响ws协议的吧

2, 我修改的方式, 收到批量的raw里多个消息, 拆包的时候, 是直接产生了一个临时Proto对象, 然后拆包逐个发送, 内存分配应该有一定影响效率, 不知道影响是否会过大

3, 测试的时候, 我一个测试服务器8核心 E5606 @ 2.13GHz, 24G内存,
开3000-5000连接, 在comet服务器上启动发送脚本,每秒50个消息的速率 另外Channel的signal chan buff我也调大到1024, 以防止连接读取速度和发送速度之间的过大差别

会出现测试机读取消息包读不过来的情况, 造成comet server端开始出现 send-Q堆积, 不知道是哪里2了, 待祥查

liutaihua commented 8 years ago

目测ws协议性能会成指数下降。 syscall太多了。需要设计个好的方式 但是目前我们客户端没法配合, 只能以json格式给数据。 否则还能搞个batch方式发了 :( 自己测试用batch方式发送, 负载秒降到低位

job里做的batch方式,到了ws协议来发的时候, 完全木有体现batch的方式, 逐个的发,消息量一多,就是成m*n的增加io write调用,产生一大堆syscall

Terry-Mao commented 8 years ago

这个明白,主要是协议怎么去兼容比较好,这块我还在想,我尽快给个结论

Terry-Mao commented 8 years ago

ping @liutaihua,这块你们现在解决了吗

liutaihua commented 8 years ago

算不上解决,稍微改进了点。 改用gorilla/websocket的ws实现,这个ws有read, write buffer(每次分配,不是内存池) 用batch方式, 在writeWebsocket前对buff做repack,把proto.Body拆出来放一个array里,需要客户端处理一下。 想过改BinaryMessage方式, 不过感觉这样还不如用socket了,所以打算还是等客户端有空了,改socket方式吧,goim性能很赞,现在足够使用 :) 我们要改的还挺多, 现在用json, 比较费流量。

Terry-Mao commented 8 years ago

gorilla 这个能否推一个patch过来

liutaihua commented 8 years ago

好,我整理下 :)

hanwujibaby commented 8 years ago

gorrila的websocket性能比原生的好很多。我们之前用原生的ws放在直播的聊天服务上;人数上来的时候,下发消息会堵塞;看了下源码,原生的加了锁,gorrila没有。猜测性能好的原因在这。

On Fri, Jun 17, 2016 at 11:00 PM, 刘太华 notifications@github.com wrote:

好,我整理下 :)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Terry-Mao/goim/issues/70#issuecomment-226793013, or mute the thread https://github.com/notifications/unsubscribe/AE9GOESlEXNP8MEcXYsW6WvaJ1lzikitks5qMraMgaJpZM4IIQ3r .

from wei4liverpool's gmail

liudanking commented 8 years ago

@liutaihua 用上你的patch了。解决了一直困扰我的room推送问题。 发现一个小问题,单用户推送的时候消息格式是: curl -d "{\"test\":1}" http://127.0.0.1:7172/1/push\?uid=2

{"ver":0,"op":5,"seq":0,"body":{"test":1}}

room推送的时候消息格式是这样: curl -d "{\"test\":1}" http://127.0.0.1:7172/1/push/room\?rid=2

[{"test":1}]

后者似乎跟goim协议格式设计不符。room推送消息格式化这种格式的原因是什么呢?

liutaihua commented 8 years ago

@liudanking 不好意思, 我这边正好没用到点对点的msg,没测试到。 OP_SEND_SMS_REPLY也需要加进 WriteWebsocket内的判断才行,另外当时主要是不知道使用者是否会需要在客户端做proto的协议结构,如果你客户端不需要 协议结构,仅仅需要body内的msg, 那直接在WriteWebsocket方法内增加对 OP_SEND_SMS_REPLY的判断一下. 我放个patch吧。

liudanking commented 8 years ago

@liutaihua thx~

weisd commented 8 years ago

@liutaihua 现在websocket还是没有协议字段,应该把拼好的buffer 斌值到 p.Body 而不是直接返回吧

func (p *Proto) WriteWebsocket(wr *websocket.Conn) (err error) {
    if p.Body == nil {
        p.Body = emptyJSONBody
    }
    if p.Operation == define.OP_RAW || p.Operation == define.OP_SEND_SMS_REPLY {
        // batch mod
        var b = bytes.NewWriterSize(len(p.Body))
        if err = p.WriteBodyTo(b); err != nil {
            return
        }
        err = wr.WriteMessage(websocket.TextMessage,  b.Buffer()) // 这没带协议体就直接发送了!
        //err = wr.WriteJSON(b.Buffer())
        return
    }
    err = wr.WriteJSON(p)
    return
}
weisd commented 8 years ago

改了下代码 websocket 统一返回 json数组,方便客户端统一处理


func (p *Proto) WriteWebsocket(wr *websocket.Conn) (err error) {
    if p.Operation == define.OP_RAW {
        // batch mod
        var b = bytes.NewWriterSize(len(p.Body))
        if err = p.WriteBodyTo(b); err != nil {
            return
        }
        p.Operation = define.OP_SEND_SMS_REPLY
        p.Body = b.Buffer()
        err = wr.WriteJSON(p)
        return
        // err = wr.WriteMessage(websocket.TextMessage,  b.Buffer())
        //err = wr.WriteJSON(b.Buffer())
        // return
    }

    if p.Body == nil {
        p.Body = []byte("[]")
        err = wr.WriteJSON(p)
        return
    }

    var b = bytes.NewWriterSize(len(p.Body) + 2)
    b.Write([]byte("["))
    b.Write(p.Body)
    b.Write([]byte("]"))

    p.Body = b.Buffer()

    err = wr.WriteJSON(p)
    return
}
tonybase commented 8 years ago

@Terry-Mao 关于websocket聚合消息这里,我看到有朋友是 {"ver":1,"op":8,"seq":1,"body":[{msg}, {{msg}]},而我觉得可以直接返回一个packet数组:[{"ver":1,"op":8,"seq":1,"body":{}}, {"ver":1,"op":3,"seq":2,"body":{}}],不知道那种方式比较合适呢?

tonybase commented 8 years ago

这样子做的好处是,以后客户端上线后,可以返回一堆离线消息,各种op都不影响,客户端可以进行简单的解析消息列表,也达到ws聚合消息。

Terry-Mao commented 8 years ago

这个考虑过,内网流量 double 一次,然后做一个双协议适配。可以提供下看看

liudanking commented 8 years ago

https://github.com/Terry-Mao/goim/pull/121 @Terry-Mao websocket 批量写下行消息修了一个 bug. 空了 review 下。 @im-qq 之前消息重复的问题 fix 了。详见 pr.

tonybase commented 8 years ago

如果消息广播,建议在job编码好,一次Marshal好直接广播多份。 如果是私信消息,应该都是在logic层控制按客户端编码和解码版本进行处理。