Skip to content

Conversation

@qoo332001
Copy link
Collaborator

@qoo332001 qoo332001 commented Apr 18, 2023

此PR:

  • WebService上修改MetricsSensor的設定方法
    • 將設定MetricsSensor的邏輯移出BalancerHandler
    • 設定MetricsSensor的邏輯改放到MetricSensorHandler(原為BeanHandler)的PostRequest中

需要這隻PR的原因:

  • 原來的BalancerHandler沒辦法長時間的收集metrics(在使用者執行PostRequest時才會收集metrics),這會導致一些需要長時間統計metrics的CostFunction(例如 [COST] add MigrateTimeCost #1665)無法收集足夠的metrics來計算分數
  • 此將WebService修改成平常就可以收集metrics,以及可以隨時選擇感興趣的指標(選擇CostFunction),如此變可以拉長指標的蒐集時間,並用長時間收集的metrics來計算CostFunction

修改前後差異:

  • 原本的BalancerHandler的流程如下:
    1. 使用者希望做負載平衡時打開WebService,此時MetricStore已經build,但因為沒有註冊MetricsSensor,因此不會撈取任何metrics
    2. 使用者送出PostRequest,此時成功註冊MetricsSensor並開始撈取metrics
    3. 等待撈到足夠的metrics後,CostFunction開始計算所需的分數
  • 修改後的MetricSensorHandlerBalancerHandler的流程如下:
    1. 使用者平常就會開著WebService,且同時透過送出MetricSensorHandler的PostRequest來選擇未來可能會想要做負載平衡的CostFunction,此時會同時註冊這些CostFunctionMetricsSensor並開始撈取metrics
    2. 當使用者要做負載平衡時,此時叢集已經收集了一段時間的metrics,此時呼叫BalancerHandler的PostRequest,可以使用這些統計一段時間的metrics來做負載平衡
    3. CostFunction開始計算所需的分數

等這隻改的差不多的時候會再補上測試以及文件

@chia7712
Copy link
Contributor

設定MetricsSensor的邏輯改放到MetricSensorHandler(原為BeanHandler)的PostRequest中

BeanHandler原本只是方便查找 metrics,跟這隻 PR 要做的事情差蠻多的。或許我們應該保持原本的邏輯並加入新的 handler

此將WebService修改成平常就可以收集metrics,以及可以隨時選擇感興趣的指標(選擇CostFunction),如此變可以拉長指標的蒐集時間,並用長時間收集的metrics來計算CostFunction

如果改成預設就將系統內可以找到的 cluster cost + move cost 掛載上去呢?這樣做應該可以簡化邏輯,不過效能面可能要想一下

@qoo332001
Copy link
Collaborator Author

BeanHandler原本只是方便查找 metrics,跟這隻 PR 要做的事情差蠻多的。或許我們應該保持原本的邏輯並加入新的 handler

ok

如果改成預設就將系統內可以找到的 cluster cost + move cost 掛載上去呢?

CostFunction是不是有可能撈取到非常多不需要的metrics? 當初就是怕有這樣的狀況,所以才會想要讓使用者用post來選擇未來可能會用到的CostFunction

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 感謝此功能,有些建議請看一下,另外記得更新文件

@chia7712
Copy link
Contributor

doc 是不是沒有更新?

@qoo332001 qoo332001 requested a review from chia7712 April 23, 2023 16:38
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 qoo332001 merged commit 63a34bd into opensource4you:main Apr 27, 2023
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