Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

关于线程限流问题的讨论 #59

Open
manzhizhen opened this issue Aug 14, 2018 · 24 comments
Open

关于线程限流问题的讨论 #59

manzhizhen opened this issue Aug 14, 2018 · 24 comments
Labels
kind/discussion For further discussion

Comments

@manzhizhen
Copy link
Contributor

manzhizhen commented Aug 14, 2018

Issue Description

bug report

Describe what happened (or what feature you want)

sentinel采用类似于责任链的设计模式,将统计、限流、降级、监控等功能串起来,使每个环节自责更清晰(见DefaultSlotsChainBuilder),这种设计模式对于大多数关于“数量”的统计场景是没问题的,比如QPS、错误量等。但对于线程数限流(即并发限流)这样做是有问题的,详见:https://blog.csdn.net/manzhizhen/article/details/81413014。

在Sentinel中,当前服务对线程数加(请求进来)和减(请求执行完毕)的操作是在StatisticSlot中完成的:

@Override
public void entry(Context context, ResourceWrapper resourceWrapper, DefaultNode node, int count, Object... args) throws Throwable {

        // 注意: 其他代码省略
        fireEntry(context, resourceWrapper, node, count, args);
        **node.increaseThreadNum();**
}

@Override
public void exit(Context context, ResourceWrapper resourceWrapper, int count, Object... args) {
        DefaultNode node = (DefaultNode)context.getCurNode();
        // 注意: 其他代码省略
        **node.decreaseThreadNum();**
}

而线程数的限流操作是在另一个类来做的,例如SystemSlot

@Override
public void entry(Context context, ResourceWrapper resourceWrapper, DefaultNode node, int count, Object... args)  throws Throwable {
    **SystemRuleManager.checkSystem(resourceWrapper);**
    fireEntry(context, resourceWrapper, node, count, args);
}

其中SystemRuleManager.checkSystem的操作如下:
public static void checkSystem(ResourceWrapper resourceWrapper) throws BlockException {

    // 注意: 其他代码省略
    // total thread
    int currentThread = Constants.ENTRY_NODE == null ? 0 : Constants.ENTRY_NODE.curThreadNum();
    **if (currentThread > maxThread) {
        throw new SystemBlockException(resourceWrapper.getName(), "thread");
    }**
}

将统计和限流分开的这种方式,无法真正做到线程数量(也就是并发度)的精准控制,会有竞态条件产生,比较好的做法是用信号量来实现。

Describe what you expected to happen

How to reproduce it (as minimally and precisely as possible)

Tell us your environment

Anything else we need to know?

@manzhizhen manzhizhen changed the title 关于线程限流的一个问题的讨论 关于线程限流一个问题的讨论 Aug 14, 2018
@manzhizhen manzhizhen changed the title 关于线程限流一个问题的讨论 关于线程限流问题的讨论 Aug 14, 2018
@byamao1
Copy link

byamao1 commented Aug 15, 2018

我同意这个issue。源码中确实会存在这种可能。DefaultProcessorSlotChain中首先会进行例如FlowSlot.entry的检查,然后才会在StatisticSlot.entry进行统计数量。如果不使用同步机制,有可能会同时几个线程同时调用DefaultProcessorSlotChain超过线程上限。

@jialianglinjl
Copy link
Contributor

jialianglinjl commented Aug 15, 2018 via email

@sczyh30 sczyh30 added the kind/bug Category issues or prs related to bug. label Aug 15, 2018
@byamao1
Copy link

byamao1 commented Aug 15, 2018

我对工程中的FlowThreadDemo进行了修改:

  • 开启10个线程群(1个群25个线程)进行饱和测试
  • 加入CyclicBarrier保证线程群内能够同时触发SphU.entry
  • 在log处改造为if(oneSecondPass > 20),使得显示的log为超限情境
    测试结果来看,限流能力与methodBRunningTime有正比比关系:methodBRunningTime越小,能力越小。

下步我会结合效率和限流能力,试图找到一个平衡点方案。(有苛刻之处乃本人之误,如需测试代码可联系本人)

@jialianglinjl
Copy link
Contributor

jialianglinjl commented Aug 15, 2018 via email

@manzhizhen
Copy link
Contributor Author

如果有办法将线程限流的判断还有加减1放在一个地方(就可以会用信号量来做了),就可以规避这个问题,而且性能不会太差,但可能会违背了现有的操作链的设计方式,不妨尝试一下

@byamao1
Copy link

byamao1 commented Aug 16, 2018

用令牌的方式来限制资源被访问次数应该是可行的。与信号量差不多的思路(水平有限),只不过是利用原子类实现了令牌的个数限制。代码放在博客,有问题互相交流。

https://blog.csdn.net/byamao1/article/details/81747230

@manzhizhen
Copy link
Contributor Author

@byamao1 你这段代码也是有问题的,令牌桶其实很难实现并发控制的效果:https://blog.csdn.net/manzhizhen/article/details/81413014

@manzhizhen
Copy link
Contributor Author

@jialianglinjl @sczyh30 @byamao1 我想到一种方案,通过信号量+ThreadLocal来实现,我写完到时候各位看一下

@jialianglinjl
Copy link
Contributor

jialianglinjl commented Aug 20, 2018 via email

@byamao1
Copy link

byamao1 commented Aug 20, 2018

好的,方案看起来可待。

@ProgramLife
Copy link

是否考虑用volatile 去定义线程数。相较于锁,它的性能不差。

@byamao1
Copy link

byamao1 commented Aug 21, 2018

volatile 只能保证可见性,不能保证变量递增递减操作的原子性。

@ProgramLife
Copy link

感谢指正,我忽略了递增,递减并不是原子性操作,那我只能想到的也就是上面提到的信号量的方式,@byamao1

@byamao1
Copy link

byamao1 commented Sep 4, 2018

如果没有很好的方案,建议给两个策略:
1.严格精确的限流,但是牺牲并发性能
2.不精确的限流,保证并发性能

@project620
Copy link

一开始以为做成责任链,可以让用户像netty pipeline一样自定义handler,每个slot都是独立的,现在slotChain里的flowSolt和systemSolt依赖StatisticSlot的统计信息,这个问题,是否考虑让slot维护自己的信号量,而不是依赖Node上的线程数.

@byamao1
Copy link

byamao1 commented Sep 6, 2018

这么设计应该是有缘由的,需要coder澄清

@manzhizhen
Copy link
Contributor Author

@project620 @byamao1 感谢各位的讨论,不好意思现在才关注到各位的消息,确实,Sentinel责任链设计模式的应用让一个限流熔断系统的逻辑变得清晰,有迹可循,其实这次改动并不大,只是加强了线程限制的校验而已,原有功能和关联关系并没有发生变化(@project620 提到的FlowSolt是指?),原本SystemSolt中的实现也会依赖StatisticSlot的统计数据,否则限流无从下手的。具体可以参见这次改动: https://github.com/alibaba/Sentinel/pull/76/files

欢迎继续讨论!!!

@ro9er
Copy link
Contributor

ro9er commented Dec 5, 2018

@manzhizhen @byamao1 @sczyh30
之前这个实现我有点没看懂
其实这种情况出现很大程度是在StatisticSlot到其他限流Slot上的一个时间窗的问题。我这里想的一个大致思路是在Node中增加两个原子计数器(可以使Atomic也可以是LongAdder),一个ThreadLocal,一个原子计数器 A记录的是目前进到StatisticSlot的有多少个请求,一个原子计数B记录的是目前有多少个请求退出了StatisticSlot,两个都是不分时间并且做到一定的滚动防止溢出,然后一个ThreadLocal C记录的是当前这个请求在进入StatisticSlot时候的一个序号,其实就是A原子递增之后的一个值,类似于C = A.increamentAndGet。然后A和B之间的意义就是A - B 得到当前具体有多少个请求被累积在StatisticSlot到其他限流Slot之间,而B和C之间的意义在于C - B得到当前请求在累积的请求中的位置。
然后当StatisticSlot fireEntry之前先执行A累加和获取序号C工作,然后在下游限流判断的时候,把A B C纳入到判断中(我想的一个大致的计算方式是D = MAX(C - B, 0) 得到当前请求此时是积压的第几个请求),最终在回到statisticSlot的时候finally 执行B的累加。应该能在一定程度上解决这个问题。。但是还是无法保证完全精确。不过计算简单,也比较好理解,并且没有锁或者信号量控制,这个大家可以讨论一下。

@byamao1
Copy link

byamao1 commented Dec 7, 2018

精确统计 和 高并发 两者应该不能同时兼顾。即使使用CAS也会因为对共享区的竞争而降低并发能力;使用LongAdder这样的高并发类就会牺牲精确性。

@manzhizhen
Copy link
Contributor Author

manzhizhen commented Dec 7, 2018

@ro9er @byamao1 信号量是AQS最直接也是最轻量级的实现,对于信号量,几千并发的控制性能应该不在话下,一般的单台业务服务器的并发不太可能达到这个量级(假设一个服务平均耗时能稳定在10ms,那么1千并发约等于10万QPS)。

@ro9er
Copy link
Contributor

ro9er commented Dec 7, 2018

@manzhizhen 这个也不仅仅是性能的问题,还有在于责任的隔离,你的做法我个人理解是在统计的Slot里面把限流的逻辑也加进去了,不知道我想的有没有问题,可以讲解一下,如果我理解错了的话还希望能够指正。

@ro9er
Copy link
Contributor

ro9er commented Dec 7, 2018

@manzhizhen 我刚仔细看了下。。懂你的代码的意思了。你的代码实际上是用了一个全局的信号量去控制并发线程数,在StatisticSlot中获取到了限流结果,然后将限流结果和全局信号量引用放到了threadlocal里面,延迟到了systemRuleSlot中进行判断。功能上肯定没问题,不过就是看责任划分上,实际上已经做到了尽可能的划分了,不过如果要精确统计就无法做到责任的完全划分,这个就看作者和本身开发点的一个考虑了。对于并发线程这个场景你的方案应该覆盖到了。

@manzhizhen
Copy link
Contributor Author

@ro9er 一个比较好的设计是把统计信号量的结果作为统计数据的一部分保存在统计结果对象里面,然后向下传递,最后交给其他的Slot来使用,比如判断什么的。

@sczyh30 sczyh30 added kind/discussion For further discussion and removed kind/bug Category issues or prs related to bug. labels Dec 27, 2018
@WuJingLearn
Copy link

private static void initFlowQpsRule() {
List rules = new ArrayList();
FlowRule rule1 = new FlowRule();
rule1.setResource("hello");
rule1.setCount(4);
rule1.setGrade(RuleConstant.FLOW_GRADE_THREAD);
rule1.setLimitApp("default");
rules.add(rule1);
FlowRuleManager.loadRules(rules);
}

public static void main(String[] args) {
initFlowQpsRule();

    for (int i = 0; i < 100; i++) {
        new Thread(()->{
            entry();
        }).start();
    }
}

public static void entry() {
Entry methodA = null;
try {
methodA = SphU.entry("hello");
System.out.println(Thread.currentThread().getName() + "访问到系统资源");
Thread.sleep(2000);
System.out.println(Thread.currentThread().getName() + "退出");
} catch (BlockException e1) {
System.out.println(Thread.currentThread() + "被限流了");
} catch (Exception e2) {
} finally {
if (methodA != null) {
methodA.exit();
}
}
}

如上文:设置了并发阈值为4,开启100个线程同时访问。结果都访问到资源,没有被限流
Thread-16访问到系统资源
Thread-97访问到系统资源
Thread-7访问到系统资源
Thread-48访问到系统资源
Thread-78访问到系统资源
Thread-17访问到系统资源
Thread-73访问到系统资源
Thread-96访问到系统资源
Thread-23访问到系统资源
Thread-98访问到系统资源
Thread-13访问到系统资源
Thread-12访问到系统资源
Thread-5访问到系统资源
Thread-74访问到系统资源
Thread-19访问到系统资源
Thread-90访问到系统资源
Thread-53访问到系统资源
Thread-50访问到系统资源
Thread-71访问到系统资源
Thread-0访问到系统资源
Thread-34访问到系统资源
Thread-1访问到系统资源
Thread-93访问到系统资源

你好,sentinel的并发线程数维度的限流,我的使用姿势是有问题吗。设置的阈值时4,但是100个线程都可以访问成功

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/discussion For further discussion
Projects
None yet
Development

No branches or pull requests

8 participants