apache / apache/shenyu

Refactor: Decouple load balance metrics collection from DividePlugin

Closed
#6,426 1 comment 0 reactions 0 assignees View on GitHub
type: bug
Dominant language
Java
Stars
8.8k
Forks
3.1k
Avg merge
7d 1h
Merged PRs (30d)
85

Description

### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current Behavior

Currently, DividePlugin contains strategy-specific callback logic for P2C and ShortestResponse load balance algorithms:
```java
if (ruleHandle.getLoadBalance().equals(P2C)) {
return chain.execute(exchange)
.doOnSuccess(e -> responseTrigger(upstream))
.doOnError(throwable -> responseTrigger(upstream));
} else if (ruleHandle.getLoadBalance().equals(SHORTEST_RESPONSE)) {
beginTime = System.currentTimeMillis();
return chain.execute(exchange)
.doOnSuccess(e -> successResponseTrigger(upstream));
}
return chain.execute(exchange);
```
This design has several issues:
1. Violates Single Responsibility Principle: DividePlugin's core responsibility is selecting an upstream and writing routing info into exchange. Metrics collection for specific load balance strategies should not be mixed in.
2. Violates Open/Closed Principle: Adding a new load balance strategy that requires runtime metrics (e.g., adaptive load balancing) would require modifying DividePlugin, rather than simply implementing a new LoadBalance class.
3. Thread safety issue: beginTime is an instance variable shared across concurrent requests, which is not thread-safe. It should be stored per-request (e.g., in exchange attributes).

### Expected Behavior

Add onSuccess and onError callback methods to the LoadBalance interface with default empty implementations:
```java
public interface LoadBalance {
Upstream select(List upstreamList, LoadBalanceData data);

default void onSuccess(Upstream upstream) {}

default void onError(Upstream upstream) {}
}

return chain.execute(exchange)
.doOnSuccess(e -> loadBalance.onSuccess(upstream))
.doOnError(t -> loadBalance.onError(upstream));
```

### Steps To Reproduce

_No response_

### Environment

```markdown
ShenYu version(s):latest
```

### Debug logs

_No response_

### Anything else?

i will do it

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with DividePlugin and the LoadBalance interface, then trace the P2C and SHORTEST_RESPONSE callback paths described in the issue. Check how beginTime is stored across concurrent requests and review the affected LoadBalance implementations. Done means strategy-specific callbacks are decoupled from DividePlugin, request state is not shared, and relevant gateway tests pass.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.