码迷,mamicode.com
首页 > 编程语言 > 详细

一个排序引发的BUG

时间:2021-06-29 16:08:48      阅读:0      评论:0      收藏:0      [点我收藏+]

标签:shadow   cal   stact   mat   测试用例   要求   monitor   16px   compareto   

你好呀,我是why。

前两天在 Git 上闲逛的时候又不知不觉逛到 Dubbo 那里去了。

看了一下最近一个月的数据,社区活跃度还是很高的:

技术图片

然后看了一下最新的 issue,大家提问都很积极。

其中看到了这样的一个 issue,发现有点意思:

https://github.com/apache/dubbo/issues/8055

技术图片

于是写下这篇文章给你分享一下这个 BUG 和 BUG 背后的故事。

放心,就算你完全不懂 Dubbo,也不影响你了解这个 BUG。

先说一下,下文中提到的 Dubbo 代码,没有特别说明的地方,都是我从 git 上拉下来的 Master 分支上的代码

啥 BUG 啊?

先给你描述一个这个 BUG 是啥样的。

其实就是这个 issue 的作者写出来的:Dubbo 框架里面的 Filter 排序过程有问题,即使按照框架要求写好规则后,最终生成的 Filter 链并不是我们想要的。

那么完全不懂 Dubbo 的朋友可能就遇到了第一个问题:啥是 Filter 呢?

其实就是一个过滤器而已,和 web 服务里面过滤器在概念上没啥两样。而 Dubbo 有非常多的 Filter,这些 Filter 共同组成了一个 Filter 调用链。

引用官网上的一个调用链路图,在 Filter 的地方我框起来了:

技术图片

可以看到 Filter 是 Dubbo 框架的一个非常核心的组成部分,很多很多的功能都是从 Filter 扩展出来的。

你要是还不明白也没关系,你只要知道有这样的一个 Filter 调用链就行了,链上的 Filter 各司其职,各干各的事儿。

好的,那么现在需求来了:

我现在要求链上的 Filter 的执行顺序是我能控制的,即我定义 Filter 的时候你得给我留个地方设置它的优先级。

听起来是很简单的一个需求,对吧?

我直接给你留个口子,让你输入 order 参数,不输入给个默认值,然后组装 Filter 链的时候根据 Order 排个序。

不是我吹牛,十分钟就能写完,中间还带着三分钟的摸鱼。

技术图片

但是,就这么个需求出 BUG 了。

具体啥现象呢?

我这里把项目拉下来,基于官方的测试用例,改巴改巴,给你演示一下这个 BUG 的体现是啥。

在 Dubbo 里面有这样的一个注解:

org.apache.dubbo.common.extension.Activate

技术图片

这里的 Order 就是做排序用的。简单演示一下,你看我现在有 5 个 Filter:

技术图片

排序规则是 Order 越小的越先执行,那么这个 Filter 链的执行顺序应该是这样的:

Filter4 -> Filter3 -> Filter2 -> Filter1 -> Filter5

搞个测试案例,我们验证一下:

技术图片

符合预期,没有任何毛病。

另外说明一下,官方的关于 Filter 的测试用例在这里,你有兴趣,源码拉下来就可以看:

org.apache.dubbo.common.extension.support.ActivateComparatorTest#testActivateComparator

不管是官方的案例,还是我自己写的案例,其中最关键的排序功能是这一行代码实现的:

Collections.sort(filters, ActivateComparator.COMPARATOR)

而这一行代码里面最关键的就是 ActivateComparator.COMPARATOR 这个东西。

这个东西就是 BUG 之源,不慌,等下再说。

那么为什么说它有 BUG 呢?

前面演示了正常的情况下,是符合预期的。

但是你看 Activate 注解,里面还有这样的两个东西:

技术图片

before、after,含义是指定 Filter A 在 Filter B 之前或者之后执行。

但是被打上了 @Deprecated 注解,字段说明上也备注了:

Deprecated since 2.7.0

2.7.0 之后被废除。

那么就有点意思了,为啥被废除?

来,看个例子,还是刚刚的那个测试用例。

我就稍微的这么一改:

@Activate(before = "_2")
public class Filter5 implements Filter0{
}

改动点就是在 Filter5 上配置了:

@Activate(before = "_2")

含义就是 Filter5 在 “_2” 之前执行。

“_2” 是啥?

就是 Filter2 的一个映射而已:

技术图片

那么问题就来了,作为一个正常的程序猿,自信的对 Filter5 进行了这个改动之后,他内心的想法一定是想要把这样的 Filter 链:

Filter4 -> Filter3 -> Filter2 -> Filter1 -> Filter5

修改为这样:(Filter5 在 Filter2 之前执行):

Filter4 -> Filter3 -> Filter5 -> Filter2 -> Filter1

那么实际情况是怎样的呢?

来跑一把:

技术图片

咋回事?这不是我预期的执行链啊?

是的,这就是 BUG 的表现。

技术图片

咋回事啊?

到底是咋回事呢?

且听我给你分析一波。

上一小节我说了,问题出在排序算法上。

org.apache.dubbo.common.extension.support.ActivateComparator

来,一起看一下:

技术图片

首先标号为 ① 的地方就是把 before、after、order 封装了一下,然后提供了几个比较的方法。你知道 ActivateInfo 这个实体里面有这些东西就行了,后面的代码会用到。

然后说说标号为 ② 的地方。

这个地方你别看挺长的,但是其实逻辑特别简单,当前对比的两个 filter 中的任何一个配置了 before、after 就会进入到标号为 ② 的部分的逻辑。

然后这里面的一坨逻辑是的这样的:

技术图片

具体逻辑不细说了,等会给你来个直观的演示。

最后标号为 ③ 这个地方,有点意思,稍微多说几句。

能走到标号为 ③ 的地方,说明当前对比的两个 filter 都没有配置 after、before 这两个属性。

直接对比 Order 就行了。

这个地方对 Order 相等的情况还做了一个特殊处理:

o1.getSimpleName().compareTo(o2.getSimpleName()) > 0 ? 1 : -1

如果 Order 相等,再比较类名。

这样做的原因也是保证排序的稳定性。

举个例子,比如这两 Filter,都没有指定 Order:

技术图片

那如果我们去掉这个判断:

技术图片

代码就变成了这样:

if (a1.order > a2.order) {
    return 1 
else {
    return -1 
    
}

简化一下就是这样:

return a1.order > a2.order ? 1:-1

那么这一块的代码,整体就会变成这样:

技术图片

这样仔细一看:咦,好像还能再优化一下。78 行和 80 行是一样的,所以可以去掉 78 行。

技术图片

好的,经过这样的一番改造。

恭喜你,获得了一个老版本的代码:

左边是之前版本的代码,右边是现在 Master 分支的代码:

技术图片

为什么会发生变化,必然是有原因的。

看一眼提交记录:

技术图片

这次提交指向了编号为 7778 的提交:

https://github.com/apache/dubbo/pull/7778

而这次提交指向了编号为 7757 的 issue:

技术图片

https://github.com/apache/dubbo/issues/7757

而这个 issue 在前面提到的编号为 8055 的 issue 里也提到了:

技术图片

这个 issue 主要就是两张图。

第一张图是这样的:

在没有任何自定义 Filter,仅有官方原有的 Filter 的情况下,构建出来的 Filter 链,ExecuteLimitFilter 在 MonitorFilter 之前。

技术图片

第二张图是这样的:

技术图片

在加入了一系列自定义的 Filter(没有指定 Order)之后,ExecuteLimitFilter 就排在了 MonitorFilter 之后了。

至于这两个 Filter 排前排后的影响是什么,和文本关系不大,就不扩展了,你有兴趣的可以去看看对应的链接。

总之,只有这样的判断逻辑是不稳当的:

return a1.order > a2.order ? 1:-1

来个例子演示一下:

技术图片

左边是测试用例,右边是排序规则,下面是输出结果。

从输出结果可以看到,最终的 Filter 链取决于 list 的添加顺序。

这也就是 7757 这个 issues 说的:

list 的遍历顺序会影响到排序的顺序。

技术图片

因此,才会有了这样的一次提交:

技术图片

好,现在我们把排序顺序改回来,同样的测试用例再跑一次,就稳定了:

技术图片

眼睛尖的朋友可能还发现了一个问题。

这个地方还有一次提交:

技术图片
  • 第一种判断:return o1.getSimpleName().compareTo(o2.getSimpleName())
  • 第二种判断:return o1.getSimpleName().compareTo(o2.getSimpleName()) > 0 ? 1 : -1;

你说这是在干啥?

第一种判断还疏忽了这样的一种情况,包名不同但是类名相同的情况:

  • com.why.a.whyFilter
  • com.why.b.whyFilter

这个时候 o1.getSimpleName().compareTo(o2.getSimpleName()) 返回的是 0。

返回 0 会发生啥?

直接吞掉一个 Filter 你信不信?

比如你的集合是 HashSet,或者是 TreeMap。

这就巧了,Dubbo 用的就是 TreeMap。

来个测试用例演示一下。

如果采用第一种判断,最后 TreeMap 里面只有一个 Filter 了:

技术图片

如果采用第二种判断,最后 TreeMap 里面会有两个 Filter :

技术图片

细节,魔鬼都在细节里面。

哎呀,真的是防不胜防啊。

好了,比较器我就说完了,但是你发现没有,我到现在都还没给你说排序过程不稳定这个 BUG 到底是啥,只是给你引申了一个其他的 BUG 出来。

莫慌,这不是我还没想好怎么给你描述嘛。

这个过程其实比较复杂,涉及到 Timsort 排序方法,就这方法就得另起一篇文章才能说清楚。

所以,我换了一个思路,主要给你看比较的过程,至于这个过程背后的原因。

就是 Timsort 在搞鬼,欢迎你自己去探索一番。

那过程是啥呢?

我在比较方法的入口处加上这样的输出语句:

技术图片

五个 Filter 是这样的:

技术图片

测试用例是这样的:

@Test
public void whyTest(){
    List<Class> filters = new ArrayList<>();
    filters.add(Filter4.class);
    filters.add(Filter3.class);
    filters.add(Filter2.class);
    filters.add(Filter1.class);
    filters.add(Filter5.class);
    Collections.sort(filters, ActivateComparator.COMPARATOR);
    StringBuilder builder = new StringBuilder();
    for (int i = 0; i < filters.size(); i++) {
        builder.append(filters.get(i).getSimpleName()).append("->");
    }
    System.out.println(builder.toString());
}

输出的日志是这样的:

技术图片

发现问题了没?

首先我很心机的控制了一下 list 的添加顺序:

技术图片

这样前三次比较就能构建这样的 Filter 链:

Filter4->Filter3->Filter2->Filter1->

然后,Filter5 进来后先和 Filter1 比,发现其 Order 为 0 比 Filter1 的 -1 大,于是比较结束,得到这样的Filter 链:

Filter4->Filter3->Filter2->Filter1->Filter5->

整个过程中,Filter5 与 Filter2 完全没有发生任何比较的操作,也就更不涉及到 Filter5 里面的 before 标签了:

技术图片

但是当我把 list 的添加顺序修改一下:

技术图片

咦,就正确了,你就说神不神奇?

神奇吧?

为啥呢?

去看看 Timsort 的原理吧。

技术图片

追根溯源

其实写到这里的我产生了一个疑问:

是谁,什么时候,引入了 after/before 机制?

因为这个机制我个人觉得出发点是挺好的,多一个配置的地方,把选择权留给用户。

但是在实际的使用中,却容易出现比较混乱的情况。

于是我看了一下提交记录:

技术图片

这个注解最早是梁飞(就是 Dubbo 项目要的开创者之一)写出来的,而设计之初没有 before 和 after,但是有一个 match 和 mismatch。

然后在写出这个注解一天之后的凌晨 1 点 54 分,提交了一个方法级别的匹配:

技术图片

这三个方法使用起来甚至比 before/after 更加复杂了。

于是一觉睡醒之后的 12:34 分,梁飞又删除了这三个配置:

技术图片

两个月之后的 2012 年 5 月 8 日,加入了 after 和 before 配置:

技术图片

然后就一直留在 Dubbo 源码里面,直到 6 年后的 2018 年 8 月 7 日,打上了不建议使用的注解:

技术图片

并提到了这个 issue:

https://github.com/apache/dubbo/issues/2180

技术图片

里面说:Dubbo 源码中没有使用 after 和 before,且排序是存在问题的。

于是这两个方法,在 2.7.0 版本之后,被标注为不建议使用,宣告了该方法的死亡。

我不知道 2012 年,梁飞为什么引入了这两个方法,我也曾想从他的代码提交记录上找到点蛛丝马迹,可惜没有。

但是,有了另外的一个想法:

当年梁飞引入这两个方法后,他写的比较器,是否考虑到了这样的情况呢?

于是我马上又看了比较器的代码提交记录:

org.apache.dubbo.common.extension.support.ActivateComparator

技术图片

并且把他的代码拷贝了出来,用同样的测试用例跑了一下:

技术图片

很遗憾,也有一样的问题。

或许,当年就不应该引入这两个方法。

大道至简,学 Spring 的 Order,就只有一个 Order:

技术图片

然后我又突然想了另外一个框架:SofaRPC。

SofaRPC 和 Dubbo 和 HSF 之间有着千丝万缕的爱恨情仇,于是我去瞅了一眼 SofaRPC 对应的地方:

com.alipay.sofa.rpc.ext.Extension

技术图片

用于排序的,也就只是保留了 order。

这样比较器的代码就很简单了:

com.alipay.sofa.rpc.common.struct.OrderedComparator

技术图片

另外,我顺便对比了一下梁飞最早写的比较器和现在最新的比较器的代码,功能完全一样,但是代码却差异较大:

技术图片

不得不说,经过几次重构之后,最新的比较器的可读性高了很多。

我追踪了一下这个类的提交记录,也就看着这个类的一步步演化,其实算是一个比较好的代码重构的例子。

有兴趣的自己去翻一翻。

好了,就到这了,打完收工。

技术图片

一个排序引发的BUG

标签:shadow   cal   stact   mat   测试用例   要求   monitor   16px   compareto   

原文地址:https://www.cnblogs.com/thisiswhy/p/14949480.html

(0)
(0)
   
举报
评论 一句话评论(0
登录后才能评论!
© 2014 mamicode.com 版权所有  联系我们:gaon5@hotmail.com
迷上了代码!