-
Notifications
You must be signed in to change notification settings - Fork 61
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
[DISPATCHER] Maintenance of smooth weight round robin #1396
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wycccccc 感謝更新,這次重構得很棒,一些意見請看一下
this.roundRobinLease = roundRobinLease; | ||
} | ||
|
||
public static PreArrangementSmoothRR of(int preLength, Duration roundRobinLease) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
這個類別不錯,將統一的邏輯拉出來
這邊可以改成 package-private
var ids = | ||
clusterInfo.nodes().stream().map(NodeInfo::id).collect(Collectors.toUnmodifiableSet()); | ||
// TODO: make ROUND_ROBIN_LENGTH configurable ??? | ||
IntStream.range(0, preLength) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preLength
可以用roundRobin.length
取代
return new PreArrangementSmoothRR(preLength, roundRobinLease); | ||
} | ||
|
||
synchronized void tryToUpdateRoundRobin( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tryToUpdate
就好,因為這隻類別已經知道是RR了
import org.astraea.common.admin.NodeInfo; | ||
import org.astraea.common.admin.ReplicaInfo; | ||
|
||
public class PreArrangementSmoothRR { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
可否改叫做RoundRobinKeeper
@@ -42,6 +42,30 @@ static <E> RoundRobin<E> smooth(Map<E, Double> scores) { | |||
*/ | |||
Optional<E> next(Set<E> availableTargets); | |||
|
|||
/** | |||
* Given initial key-score pair, it will output a preferred key with the highest current weight. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
這個註解很棒
}); | ||
} | ||
|
||
private double standardDeviationImperative(double avgMetrics, Map<E, Double> metrics) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
這段邏輯可否整合到Dispersion
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
已經整合過去了。
@wycccccc 標題現在要加上一些簡單的分類,到時候合併的時候請注意一下 |
好,之後會注意
瞭解,我再持續關注 上述問題都已處理完畢,再麻煩review。 |
@wycccccc 抱歉,又有一些衝突,可否在麻煩處理一下。等你這次處理完我們先完成這個 PR 再進行後續的修改 |
衝突已經修復,有問題我再及時處理,爭取今日睡前可以merge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 重構的不錯
resolve #1086
#1386
as titile, 這支pr一共做了如下幾件事: