-
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
[COST] limit brokerCost to ReplicaLeaderCost #1780
base: main
Are you sure you want to change the base?
Conversation
@@ -32,13 +33,24 @@ public class ReplicaLeaderCost implements HasBrokerCost, HasClusterCost, HasMove | |||
private final Dispersion dispersion = Dispersion.normalizedStandardDeviation(); | |||
private final Configuration config; | |||
public static final String MAX_MIGRATE_LEADER_KEY = "max.migrated.leader.number"; | |||
static final String BROKER_COST_LIMIT_KEY = "max.broker.total.leader.number"; |
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.
請問這兩個參數的差異是什麼?是否有文件說明這兩個參數的用法?
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.
"max.migrated.leader.number"是在限制搬移計畫總共可以移動多少leader,"max.broker.total.leader.number"則是限制搬移後broker可以持有多少leader數量,已更新文件,麻煩在看一下,謝謝
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.
"max.broker.total.leader.number"則是限制搬移後broker可以持有多少leader數量
這個聽起來跟搬移的關係好像不大?move cost還是盡量集中在“搬移”這件事情上,這個動作應該做在 cluster cost 會比較好
另外一件事情則是限制“各個節點各自有多少個 leaders"好像不太容易評估和設定,如果目標是讓各個節點的 "leader數量平衡”那會比較合理
可否說明一下這個用法的出發點?
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.
另外一件事情則是限制“各個節點各自有多少個 leaders"好像不太容易評估和設定,如果目標是讓各個節點的 "leader數量平衡”那會比較合理
這個比較像是ClusterCost
的目的,這邊要改是MoveCost
可否說明一下這個用法的出發點?
主要是一開始是設定限制一個搬移計畫的leader移動總數,而這邊想要新增的是"可以限制個別broker可以移動leader的數量",可能之前沒有想清楚,我更新一下,更新之後應該會可以有兩種限制
- 限制一個搬移計畫可以移動的leader總數
- 限制一個搬移計畫中,每個broker可以移動的leader總數
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.
@qoo332001 方便線下討論一下嗎?我有在 slack 敲你
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.
可以
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.
根據剛剛討論的做了一些更新,麻煩再看一下,謝謝
…seReplicaLeaderCost
…seReplicaLeaderCost
return configuration.list(BROKER_COST_LIMIT_KEY, ",").stream() | ||
.collect( | ||
Collectors.toMap( | ||
idAndPath -> Integer.parseInt(idAndPath.split(":")[0]), |
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.
idAndPath
? 這個命名是不是怪怪的
此PR在
ReplicaLeaderCost
新增限制每個broker搬移過持中可以持有的leader數量