之前在github上倒是给一些开源的库提交过代码,但用的人一般也不多。这次应该算真正意义上的“为开源项目贡献代码”。所以写篇文章纪念下……
起因是我上周把我们公司的cassandra集群升级到2.1.0了。虽然理论上大版本的第一个正式版坑可能不少,要几个小版本升级后才靠谱,但这次2.1搞了N个beta和rc,感觉还是靠谱的。加上号称性能提升,对整天因为机器太搓扛不住压力之类的俺们公司还是挺有吸引力的。于是就升级了。升级完倒是没觉得性能提升,但发现log里经常有AssertionError。看了下是发现不同节点数据不一致的时候判断每个节点与最新数据的差异(把差异部分写到对应节点从而保持数据一致性)的代码的一个assert挂了。没时间细看就复制粘贴异常信息发JIRA上和maillist里,发了3天没人理……
然后这几天不上班了就看了下具体的代码,是在比较一个节点的对应row的RangeTombstoneList和所有节点合并后的最新状态(因为传输不成功、丢数据等导致某个节点的数据并不是最新的,把对应的row在每个节点上的各种状态按时间戳挨个找最大就是一个row真正的最新状态,当读取数据的时候发现某个节点与最新状态不一致的时候要把需要更新的部分找到然后传给对应节点)的RangeTombstoneList的diff时挂的。写diff函数的人认为单个节点的TombstoneList长度应该小于全局最新状态的TombstoneList长度,因为后者是由每个节点合并而来的。所以这里上来先来个assert确保size是<=的关系。
但实际上每个节点的RangeTombstoneList合并后的List长度是可能小于某个节点的长度的,比如节点A记录某行数据column从1到3在时间戳为T1的时候删除——记做[1,4]@1,左闭右开区间,又记录另一条删除记录是[4,5]@2;而节点B记录的是[1,5]@3,那么三者合并的RangeTombstoneList其实就是[1,5)@3,size是1。而节点A的list size是2。于是diff的时候就挂了……
所以我感觉直接去掉assert就好了,提了个patch删了一行代码加了一个会导致之前的代码挂掉的testcase。但有个committer表示那个代码写的时候就是假设assert一定通过的,如果理论上输入数据不这样那这个函数也得改……于是我仔细看了diff的逻辑,不仅必须是size <=的关系,而且其实即使assert通过也有别的bug……所以干脆重写了这个函数、补了一个testcase,提交了v2的patch。
然后过了快一天,之前写这个代码的人帮我review了下代码,找到个小bug,补了一些testcase,提交了v3的patch,而committer基于v3加了些注释、改了我表述诡异的注释之后提交了代码到2.1分支, 应该会在2.1.1发布的时候修复这个bug。
以后我也能吹牛逼说自己是Cassandra的贡献者了……不过我很想吐槽Cassandra的代码格式规范,第一次见java代码花括号在第二行开头……
发表回复