libpomelo2无法解析protobuf格式的返回数据

(已在github提交issue, 在这里再发一下以防万一没看到

如果返回的是pb格式的数据(就是在serverProtos.json里声明的),必然解析不出来

https://github.com/NetEase/libpomelo2/blob/master/src/tr/uv/pr_msg.c#L202 这里使得route=NULL,后面的逻辑就必然只通过json方式解析了

关键是libpomelo倒是对的……老版本会自动去读取对应request。。。新版本写错这种事情也是比较少见~~

不过稳定性比libpomelo强不少哈哈

标签:无
qklxtlx 在 2014-10-13 00:13发布
qklxtlx 在 2014-10-13 00:14重新编辑 分享到 weibo
3 回复
#1 {3} wangxy 2014-10-13 10:29 回复

@qklxtlx

  • 因为在libpomelo2的实现中,考虑到client与transport解耦,transport与decode/encode解耦, 而req_id 与client中的request对应关系是在client层的。当时为了简单化,直接进行了不支持, 这里可以考虑增加一个req_id 与 route 的对应关系,作为参数传给decoder使用。

  • 在libpomelo中,各个层之间没有分离,所以decoder可以方便地获取到对应关系。

  • 由于libuv的所有与loop相关的调用中(除了uv库提供的util函数之外的所有调用),除了uv_async_send是线程安全的,其他的都是非线程安全的,而在libpomelo中,有在其他线程中操作loop情况,比如pc\\async_write的实现中,就给loop挂了个uv_async_t, 而pc\\async_write一定是在其他线程被调用,因此,这里就有潜在崩溃的危险。另外,libpomelo中还有一些需要锁保护的地方,没有做保护,这些也是决定做大的重构,实现libpomelo2的原因。

  • 后面会考虑在libpomelo2中修复这个问题,感谢你的关注和支持,如果有时间的话,关于libpomelo2的各个平台编译方式以及更多的语言绑定方面,欢迎多多贡献。

qklxtlx 2014-10-13 11:09 回复

en代码我读了下,确实比libpomelo舒服不少,线程安全是个很大的问题……
但是pb解析这玩意儿不能用是属于功能上的bug了。。。我确认下就是,这个东西官方会快速出fix么?不然我就想办法自己改了
因为确实如你所说,分离之后不太好改(不然我直接就改好提交PR了嘿嘿

wangxy 2014-10-13 11:13 回复

@qklxtlx

会尽快出fix,最近肯定会出来的

qklxtlx 2014-10-13 11:22 回复

@wangxy 嗯嗯麻烦了!
gyp用起来好麻烦,我现在是根据Libpomelo手动改的Android.mk编译,等有空了我试试CMake

#2 {12} wangxy 2014-10-13 14:02 回复

@qklxtlx

pomelo 协议的response里面不带route是不太合理的,比如,超时处理,当request超时的时候,肯定要清掉,但是如果在request清掉后,response又回来了,req_id 已经被清掉了,一样拿不到route。 如果超时后不清掉,那么清除时机放到哪, 放到重连之前吗,如果连接一直没问题,没有发生重连呢? 如果不设超时,那么如果出现大量的request没有response回来呢,将造成大量的挂起, 因此这个地方要考虑的不仅仅是是否fix,主要要考虑怎样做才最合理。

libpomelo中同样有这样的问题,只不过没有加超时处理,如果有request没有response回来,那就一直挂着,直到重连或销毁client的时候再做清理.

libpomelo2中默认给每一个request都加了超时处理,因此这里处理起来有点怪怪的

总之,让底层的协议解析依赖高层数据是不合理的

qklxtlx 2014-10-13 14:37 回复

我也觉得最好带上route...但是目前pomelo的通信格式是这么定义的
https://github.com/NetEase/pomelo/wiki/%E5%8D%8F%E8%AE%AE%E6%A0%BC%E5%BC%8F
如果从协议开始变,就会伤筋动骨的改了……
超时这东西在libpomelo里hack出来了,确实是有必要的。。。目前看来就是没有特别方便的fix?除非就禁用返回protobuf格式这个功能了

wangxy 2014-10-13 14:55 回复

@qklxtlx

是的,我也倾向于response不使用proto压缩,如果,有很大的response的话,服务端可以先response一个很小的值,表示收到request,然后具体的response通过push下推

qklxtlx 2014-10-13 21:00 回复

@wangxy 单独还要推一次这个就麻烦了。。。如果能一次返回完的就必然一次搞定

fantasyni 2014-10-14 08:53 回复

@qklxtlx 对的,单独推一次就需要维护状态了,也不符合开发常用逻辑

qklxtlx 2014-10-14 10:33 回复

@fantasyni 现在暂时关了serverProtos.json 凑合用用先~不然再写一个缓存数组记录就太麻烦了。。。

hitstanley 2014-10-16 11:09 回复

我的一点想法,我觉得request超时的处理应该开放给业务层,业务层针对不同的request超时可以选择简单清掉完事或者继续等待或者断线重连等策略,如果清掉后response回来了就直接忽略掉,服务器业务层应该保证所有request都在一个合理时间内得到响应,同时对于超时的情况应该记录下来以供未来分析超时原因。

wangxy 2014-10-16 13:12 回复

@hitstanley 目前的request支持disable掉timeout的,timeout参数传入PC_WITHOUT_TIMEOUT就好

但是如果开启timeout后,把timeout时的操作交给用户的话,会引入复杂性,这个bug最近会fix,尽量使用优雅点的方式进行fix

wangxy 2014-10-16 13:30 回复

@hitstanley

现在的主要问题是不管怎么fix,都会导致协议的解码层与client层的耦合,除非让协议解析层自己来维护req_id 与route的对应关系,而这样又造成了协议解析层与client层维护的信息的重复,不管怎么修都感觉到不够优雅

wangxy 2014-10-16 13:40 回复

@hitstanley @qklxtlx

打算按如下的方式修复:

增加一个调用

const char* pc_trans_get_route_by_req_id(pc_client_t* client, unsigned int req_id);

然后在解码的时候,将client传入, 然后从client和req_id信息中获取route,如果拿不到的话,就直接丢掉这个包。

大家来评估一下这样修的可行性,虽然不够优雅.

hitstanley 2014-10-16 19:09 回复

@wangxy 我觉得优雅的解决方案只有改协议格式了,message type为response时,route为空,不知道当初为什么这么设计呢?现在如果改为不为空工作量会很大吗?应该也不会存在兼容性问题。

wangxy 2014-10-16 19:54 回复

@hitstanley 工作量不大,但是因为是二进制协议,并且协议没有带版本号,因此很难兼容

qklxtlx 2014-10-16 21:57 回复

@wangxy 之前我也是这么做的,但是后来衡量了下就放弃serverProto了。从修bug的角度来说,这么写是破坏性最小的

#3 {5} wangxy 2014-10-15 16:48 回复

@qklxtlx
libpomelo2 又修了几个fatal bug, 建议更新到v0.1.3

hitstanley 2014-10-15 19:07 回复

请问这个问题fix了吗?

wangxy 2014-10-15 22:05 回复

@hitstanley

你可以看前面的讨论,这个问题,目前没有好的fix方案,建议不在serverProtos.json中启用对response的支持,server 主动push是没问题的.

qklxtlx 2014-10-15 22:39 回复

收到,已整合……看来我还是要多测测才敢弄到线上去了

qklxtlx 2014-10-15 22:40 回复

@hitstanley 目前我们就是禁掉了serverProtos.json 而且记下不能打开这么搞。。。头大,但是勉强能忍吧

hitstanley 2014-10-16 11:12 回复

@wangxy 是可以,但总是觉得用起来不方便而已,呵呵。

回到顶部