-
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] Implement feedback for network ingress cost #1637
Merged
harryteng9527
merged 17 commits into
opensource4you:main
from
harryteng9527:impl-feedback-net-ingress
Apr 14, 2023
Merged
[COST] Implement feedback for network ingress cost #1637
harryteng9527
merged 17 commits into
opensource4you:main
from
harryteng9527:impl-feedback-net-ingress
Apr 14, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chia7712
reviewed
Apr 6, 2023
chia7712
reviewed
Apr 6, 2023
common/src/main/java/org/astraea/common/cost/NetworkIngressCost.java
Outdated
Show resolved
Hide resolved
chia7712
reviewed
Apr 7, 2023
common/src/main/java/org/astraea/common/cost/NetworkIngressCost.java
Outdated
Show resolved
Hide resolved
chia7712
reviewed
Apr 12, 2023
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.
@harryteng9527 感謝更新,還有幾個建議請看一下
common/src/main/java/org/astraea/common/cost/NetworkIngressCost.java
Outdated
Show resolved
Hide resolved
common/src/main/java/org/astraea/common/cost/NetworkIngressCost.java
Outdated
Show resolved
Hide resolved
chia7712
reviewed
Apr 12, 2023
chia7712
reviewed
Apr 13, 2023
common/src/main/java/org/astraea/common/cost/NetworkIngressCost.java
Outdated
Show resolved
Hide resolved
chia7712
approved these changes
Apr 13, 2023
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
resolve #1629 (comment)
此 PR 為 NetworkIngressCost 實作 incompatibility,讓此 cost 能反饋不建議放在一起的 partitions,供未來 assignor 作為判斷依據
目前決定 incompatibility 的依據
利用
traffic interval
及upper bound
判斷,依照同個節點的 partitions 來計算,若 partition 處於不同節點,則不管流量大小都適合放在一起Traffic interval
為 NetworkIngressCost 可以設置的參數,此參數只會作用在輸入流量低於
upper bound
的 partitions,若輸入流量大於upper bound
可以參考下方 upper bound 的介紹輸入流量低於
upper bound
的 partition 計算 incompatibility 時,利用traffic interval
設置的流量來判斷 partitions 適不適合與之放在一起,當 partition 彼此的流量差異超過traffic interval
及不適合放在一起。反之,適合放在一起Upper bound
為一個流量值,當 partition 的流量超過
upper bound
,低於 upper bound 的 partitions 就不適合與之放在一起。 每個節點的 upper bound 可能不會相同如何計算
upper bound
計算 incompatibility 的範例
若節點 A 目前有 3 個 partitions:p-0, p-1, p-2,流量分別為 10MB, 30MB, 50MB, upper bound 為 16MB, traffic interval 為 10MB
p-0 的流入流量低於 upper bound (16 MB),所以不適合與 6 MB 以下、26 MB以上的 partition(s) 放在一起
p-1, p-2 的流入流量高於 upper bound,所以不適合與低於 upper bound 的 partition(s) 放在一起