Skip to content
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

Open
wants to merge 38 commits into
base: main
Choose a base branch
from

Conversation

qoo332001
Copy link
Collaborator

@qoo332001 qoo332001 commented Apr 17, 2023

此PR新增MigrateTimeCost,用來估計一個搬移計畫的搬移時間,以及用來限制產生的搬移計畫

@chia7712
Copy link
Contributor

修改 MetricSensor#fetch,在fetch方法的參數加了identity,主要原因是因為要統計broker的流量

我想先討論這個點,請問這個資訊的添加是否為了某個特定的“用途”?如果是的話,我們可否找一個“沒那麼破壞性”的改法?例如可否把 identity 放到 MBeanClient 身上?

@qoo332001
Copy link
Collaborator Author

例如可否把 identity 放到 MBeanClient 身上?

這樣應該也可行,我改改看

Copy link
Contributor

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@qoo332001 感謝更新,有兩個比較大的建議請看一下

Copy link
Contributor

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@qoo332001 這個PR是否有對應的報告可以先看一下效果?

@chia7712
Copy link
Contributor

麻煩修正一下衝突喔

chia7712
chia7712 previously approved these changes May 22, 2023
Copy link
Contributor

@chia7712 chia7712 left a 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已經是過期的標題

chia7712
chia7712 previously approved these changes May 24, 2023
Copy link
Contributor

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@qoo332001
Copy link
Collaborator Author

@chia7712 我修正了一些實驗時發現的bug,並且修復了衝突,麻煩在看一下,感謝!

@qoo332001 qoo332001 changed the title [COST] add PartitionMigrateTimeCost and revise MetricSensor#fetch [COST] add PartitionMigrateTimeCost May 25, 2023
Copy link
Contributor

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@qoo332001 感謝貢獻,還有幾個建議請看一下

@qoo332001 qoo332001 changed the title [COST] add PartitionMigrateTimeCost [COST] add MigrateTimeCost May 27, 2023

public record MaxReplicationInRateBean(BeanObject beanObject) implements HasMaxRate {
@Override
public BeanObject beanObject() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

這就不用重載了

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants