alibaba / alibaba/Sentinel

SystemRuleManager checkBbr algorithm may cause a incorrect judgement | SystemRuleManager的checkBbr拥塞检测可能出现误判

Open
#3,212 0 comments 0 reactions 0 assignees View on GitHub
area/overload-control kind/question
Dominant language
Java
Stars
23.1k
Forks
8.1k
PR merge metrics
No merged PRs in 30d

Description

## Issue Description

Type: *feature request*

### Describe what feature you want
`SystemRuleManager.class` checkBbr algorithm may cause a incorrect judgement.
checkBbr() source code:
```java
public static void checkSystem(ResourceWrapper resourceWrapper, int count) throws BlockException {
.......
// load. BBR algorithm.
if (highestSystemLoadIsSet && getCurrentSystemAvgLoad() > highestSystemLoad) {
if (!checkBbr(currentThread)) {
throw new SystemBlockException(resourceWrapper.getName(), "load");
}
}
.......
}

private static boolean checkBbr(int currentThread) {
if (currentThread > 1 &&
currentThread > Constants.ENTRY_NODE.maxSuccessQps() * Constants.ENTRY_NODE.minRt() / 1000) {
return false;
}
return true;
}
```
In `currentThread > Constants.ENTRY_NODE.maxSuccessQps() * Constants.ENTRY_NODE.minRt() / 1000`,
We konw that ` Constants.ENTRY_NODE.maxSuccessQps() * Constants.ENTRY_NODE.minRt() / 1000` is equal to BDP(Bandwidth-Delay Product), but `currentThread` is the num of currnet threads, so they're not in the same unit.
So I assume `currentThread` is equal to `curThreadNum * 1000 / 1000`. That means RT of curThreads are set to 1second fixedly.
When the actual RT is bigger than 1s and BDP less than `currentThread` ,it will cause overload .

### Describe your initial design (if present)
I think it can use `Constants.ENTRY_NODE.avgRt()` instead of `1s` to improve it.
```java
if (currentThread > 1 &&
currentThread * Constants.ENTRY_NODE.avgRt() / 1000 >
Constants.ENTRY_NODE.maxSuccessQps() * Constants.ENTRY_NODE.minRt() / 1000) {
return false;
}
```
Am I on the right track?
If right, i'm willing to submit a PR.
Thank u

### Additional context

Add any other context or screenshots about the feature request here.

Contributor guide

Open the contributing guide

Research direction

Start with SystemRuleManager.checkSystem() and its private checkBbr() method, then inspect Constants.ENTRY_NODE.maxSuccessQps(), minRt(), and avgRt(). Verify whether currentThread and the BDP expression use compatible units, and assess the proposed avgRt-based calculation. Done means the BBR overload judgment is corrected or the unit concern is resolved with supporting validation.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend, performance
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.