lower priority for tablet metrics #28554
Conversation
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪ Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
🟢 |
There was a problem hiding this comment.
Pull Request Overview
This PR lowers the priority of tablet metrics events in the Hive system to prevent them from blocking more urgent requests like viewer/healthcheck queries. The PR addresses a performance issue where high volumes of metrics updates could make the Hive appear unresponsive.
- Assigns a lower priority (100) to
EvTabletMetricsevents compared to default events (50) - Adds comments explaining the priority scheme
- Updates a unit test to account for metrics processing delays
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| ydb/core/mind/hive/hive_impl.cpp | Modified GetEventPriority() to assign lower priority to tablet metrics events with explanatory comments |
| ydb/core/mind/hive/hive_ut.cpp | Added event dispatch delay in TestNotEnoughResources to allow metrics to be processed before proceeding |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| default: | ||
| return 50; | ||
| // update metrics - there can be a lot of these, but they have no urgency | ||
| case TEvHive::EvTabletMetrics: | ||
| return 100; |
There was a problem hiding this comment.
The case TEvHive::EvTabletMetrics: is placed after the default case, making it unreachable. This case will never execute, and tablet metrics will always receive the default priority of 50 instead of the intended 100. Move the case TEvHive::EvTabletMetrics: block before the default case.
| default: | |
| return 50; | |
| // update metrics - there can be a lot of these, but they have no urgency | |
| case TEvHive::EvTabletMetrics: | |
| return 100; | |
| // update metrics - there can be a lot of these, but they have no urgency | |
| case TEvHive::EvTabletMetrics: | |
| return 100; | |
| default: | |
| return 50; |
| @@ -3145,14 +3145,19 @@ void THive::RequestPoolsInformation() { | |||
| } | |||
|
|
|||
| ui32 THive::GetEventPriority(IEventHandle* ev) { | |||
There was a problem hiding this comment.
minor: может тогда в настройки унести, чтобы иметь возможность управления без перекомпиляции?
|
@vporyadke привяжи issue |
2 participants
Changelog entry
...
Changelog category
Description for reviewers
...