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");
}**
}将统计和限流分开的这种方式，无法真正做到线程数量（也就是并发度）的精准控制，会有竞态条件产生，比较好的做法是用信号量来实现。
23条答案
按热度按时间mefy6pfw1#
volatile 只能保证可见性，不能保证变量递增递减操作的原子性。
9udxz4iz2#
@ro9er 一个比较好的设计是把统计信号量的结果作为统计数据的一部分保存在统计结果对象里面，然后向下传递，最后交给其他的Slot来使用，比如判断什么的。
kkbh8khc3#
@manzhizhen 我刚仔细看了下。。懂你的代码的意思了。你的代码实际上是用了一个全局的信号量去控制并发线程数，在StatisticSlot中获取到了限流结果，然后将限流结果和全局信号量引用放到了threadlocal里面，延迟到了systemRuleSlot中进行判断。功能上肯定没问题，不过就是看责任划分上，实际上已经做到了尽可能的划分了，不过如果要精确统计就无法做到责任的完全划分，这个就看作者和本身开发点的一个考虑了。对于并发线程这个场景你的方案应该覆盖到了。
mccptt674#
@manzhizhen 这个也不仅仅是性能的问题，还有在于责任的隔离，你的做法我个人理解是在统计的Slot里面把限流的逻辑也加进去了，不知道我想的有没有问题，可以讲解一下，如果我理解错了的话还希望能够指正。
6qftjkof5#
@ro9er@byamao1 信号量是AQS最直接也是最轻量级的实现，对于信号量，几千并发的控制性能应该不在话下，一般的单台业务服务器的并发不太可能达到这个量级（假设一个服务平均耗时能稳定在10ms，那么1千并发约等于10万QPS）。
zaqlnxep6#
精确统计 和 高并发 两者应该不能同时兼顾。即使使用CAS也会因为对共享区的竞争而降低并发能力；使用LongAdder这样的高并发类就会牺牲精确性。
envsm3lx7#
@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的累加。应该能在一定程度上解决这个问题。。但是还是无法保证完全精确。不过计算简单，也比较好理解，并且没有锁或者信号量控制，这个大家可以讨论一下。
idfiyjo88#
@project620@byamao1 感谢各位的讨论，不好意思现在才关注到各位的消息，确实，Sentinel责任链设计模式的应用让一个限流熔断系统的逻辑变得清晰，有迹可循，其实这次改动并不大，只是加强了线程限制的校验而已，原有功能和关联关系并没有发生变化（@project620 提到的FlowSolt是指？），原本SystemSolt中的实现也会依赖StatisticSlot的统计数据，否则限流无从下手的。具体可以参见这次改动： https://github.com/alibaba/Sentinel/pull/76/files
欢迎继续讨论！！！
nlejzf6q9#
这么设计应该是有缘由的，需要coder澄清
pw136qt210#
一开始以为做成责任链，可以让用户像netty pipeline一样自定义handler，每个slot都是独立的，现在slotChain里的flowSolt和systemSolt依赖StatisticSlot的统计信息，这个问题，是否考虑让slot维护自己的信号量，而不是依赖Node上的线程数.
ibps3vxo11#
如果没有很好的方案，建议给两个策略：
1.严格精确的限流，但是牺牲并发性能
2.不精确的限流，保证并发性能
fumotvh312#
感谢指正，我忽略了递增，递减并不是原子性操作，那我只能想到的也就是上面提到的信号量的方式，@byamao1
shstlldc13#
我同意这个issue。源码中确实会存在这种可能。DefaultProcessorSlotChain中首先会进行例如FlowSlot.entry的检查，然后才会在StatisticSlot.entry进行统计数量。如果不使用同步机制，有可能会同时几个线程同时调用DefaultProcessorSlotChain超过线程上限。
kx5bkwkv14#
是否考虑用volatile 去定义线程数。相较于锁，它的性能不差。
krcsximq15#
好的，方案看起来可待。