-
Notifications
You must be signed in to change notification settings - Fork 743
Balancing: add new limits, timeouts and sensors, fix blob on main by ingress, but not realy #9239
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
Balancing: add new limits, timeouts and sensors, fix blob on main by ingress, but not realy #9239
Conversation
robdrynkin
commented
Sep 13, 2024
- Not for changelog (changelog entry is not required)
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
|
||
| merger.Clear(); | ||
|
|
||
| if (SendOnMainParts.Size() >= MAX_TO_SEND_PER_EPOCH && TryDeleteParts.Size() >= MAX_TO_DELETE_PER_EPOCH) { |
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.
maybe || ?
if any of limits is exceeded actor should stop.
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.
No, thats not a mistake, I check TryDeleteParts.Size() < MAX_TO_DELETE_PER_EPOCH and SendOnMainParts.Size() < MAX_TO_SEND_PER_EPOCH above before pushing to queues. If I will check ||, there could be a situation when I will only send parts on main for a long time, without deletes
…ingress, but not realy (ydb-platform#9239)