-
Notifications
You must be signed in to change notification settings - Fork 60
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] add MigrateTimeCost
#1665
base: main
Are you sure you want to change the base?
[COST] add MigrateTimeCost
#1665
Conversation
common/src/main/java/org/astraea/common/cost/MigrationCost.java
Outdated
Show resolved
Hide resolved
我想先討論這個點,請問這個資訊的添加是否為了某個特定的“用途”?如果是的話,我們可否找一個“沒那麼破壞性”的改法?例如可否把 identity 放到 MBeanClient 身上? |
這樣應該也可行,我改改看 |
common/src/main/java/org/astraea/common/metrics/MBeanClient.java
Outdated
Show resolved
Hide resolved
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 感謝更新,有兩個比較大的建議請看一下
common/src/main/java/org/astraea/common/cost/PartitionMigrateTimeCost.java
Outdated
Show resolved
Hide resolved
common/src/main/java/org/astraea/common/cost/MigrationCost.java
Outdated
Show resolved
Hide resolved
common/src/main/java/org/astraea/common/cost/PartitionMigrateTimeCost.java
Outdated
Show resolved
Hide resolved
…artitionMigrateTimeCost
common/src/main/java/org/astraea/common/metrics/broker/HasRate.java
Outdated
Show resolved
Hide resolved
common/src/main/java/org/astraea/common/cost/PartitionMigrateTimeCost.java
Outdated
Show resolved
Hide resolved
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 這個PR是否有對應的報告可以先看一下效果?
common/src/main/java/org/astraea/common/cost/MigrationCost.java
Outdated
Show resolved
Hide resolved
common/src/main/java/org/astraea/common/cost/PartitionMigrateTimeCost.java
Outdated
Show resolved
Hide resolved
麻煩修正一下衝突喔 |
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
合併的時候注意一下題目,reivse MetricSensor#fetch
已經是過期的標題
common/src/main/java/org/astraea/common/cost/PartitionMigrateTimeCost.java
Outdated
Show resolved
Hide resolved
common/src/main/java/org/astraea/common/cost/PartitionMigrateTimeCost.java
Outdated
Show resolved
Hide resolved
…artitionMigrateTimeCost
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
@chia7712 我修正了一些實驗時發現的bug,並且修復了衝突,麻煩在看一下,感謝! |
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 感謝貢獻,還有幾個建議請看一下
common/src/main/java/org/astraea/common/cost/PartitionMigrateTimeCost.java
Outdated
Show resolved
Hide resolved
common/src/main/java/org/astraea/common/cost/MigrationCost.java
Outdated
Show resolved
Hide resolved
common/src/main/java/org/astraea/common/cost/MigrationCost.java
Outdated
Show resolved
Hide resolved
common/src/main/java/org/astraea/common/cost/PartitionMigrateTimeCost.java
Outdated
Show resolved
Hide resolved
MigrateTimeCost
common/src/main/java/org/astraea/common/cost/MigrateTimeCost.java
Outdated
Show resolved
Hide resolved
common/src/main/java/org/astraea/common/cost/MigrationCost.java
Outdated
Show resolved
Hide resolved
…artitionMigrateTimeCost
|
||
public record MaxReplicationInRateBean(BeanObject beanObject) implements HasMaxRate { | ||
@Override | ||
public BeanObject beanObject() { |
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.
這就不用重載了
此PR新增
MigrateTimeCost
,用來估計一個搬移計畫的搬移時間,以及用來限制產生的搬移計畫