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

[NBS] make new volume counters to fix UsedQuota strange values #2704

Merged
merged 17 commits into from
Jan 20, 2025

Conversation

vladstepanyuk
Copy link
Contributor

@vladstepanyuk vladstepanyuk commented Dec 16, 2024

#2703

На диске стоят ограничения в 5000 Iops и 89 mbps на чтение\запись
когда пишем 5000 iops и 20 mb метрика UsedQuota показывает корректные 100 процентов диска
image
новые метрики:
image
Если же мы будем писать 2500 iops и 10 mb
то UsedQuota показывает невалидные значения в районе нуля
image
Новые же метрики показывают корректные значения в районе 50 процентов
image

У тротлера есть бюджет и на каждую io операцию из этого бюджета вычитается цена операции, при этом бюджет восстанавливается с постоянной скоростью, так вот предыдущий вариант раз в секунду спрашивал текущий бюджет тротлера и высчитывал метрику как (MaxBudget - CurrentBudget) / MaxBudget. Допустим, за первые пол секунуды секунды мы потратили половину бюджета, затем пол секунды ничего не писали и не читали, бюджет успел восстановиться до максимального и в итоге в конце секунды получаем метрику 0%, хотя за секунду мы израсходовали половину бюджета. Т.е. предыдущий вариант показывал не использованную за секунду квоту, а сколько нам не хватает бюджета до максимального уровня в данный момент, и, если мы не использовали бюджет на 100%, он успевал полностью восстановиться и итоговая квота получалась в районе нуля

Copy link
Contributor

Hi! Thank you for contributing!
The tests on this PR will run after a maintainer adds an ok-to-test label to this PR manually. Thank you for your patience!

@komarevtsev-d komarevtsev-d self-requested a review December 16, 2024 06:08
@komarevtsev-d komarevtsev-d added ok-to-test Label to approve test launch for external members blockstore Add this label to run only cloud/blockstore build and tests on PR labels Dec 16, 2024
@github-actions github-actions bot removed the ok-to-test Label to approve test launch for external members label Dec 16, 2024
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 75ed07a.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
3555 3555 0 0 0 0

@komarevtsev-d komarevtsev-d added the ok-to-test Label to approve test launch for external members label Dec 16, 2024
@github-actions github-actions bot removed the ok-to-test Label to approve test launch for external members label Dec 16, 2024
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 0bb682e.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
3555 3555 0 0 0 0

@komarevtsev-d komarevtsev-d added ok-to-test Label to approve test launch for external members large-tests Launch large tests for PR labels Dec 17, 2024
@github-actions github-actions bot removed the ok-to-test Label to approve test launch for external members label Dec 17, 2024
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 314d94d.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
3748 3748 0 0 0 0

@komarevtsev-d komarevtsev-d added the ok-to-test Label to approve test launch for external members label Dec 18, 2024
@github-actions github-actions bot removed the ok-to-test Label to approve test launch for external members label Dec 18, 2024
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit ed436ec.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
3748 3748 0 0 0 0

komarevtsev-d
komarevtsev-d previously approved these changes Dec 19, 2024
Copy link
Collaborator

@qkrorlqr qkrorlqr left a comment

Choose a reason for hiding this comment

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

Отдельно по iops и bandwidth считать неправильно, потому что троттлер работает не так - он троттлит не независимо по iops и по bandwidth, а считает стоимость каждого запроса и троттлит по стоимости. За секунду суммарная стоимость запросов, проходящих через троттлер, не может быть больше 1. В общем, метрика должна показывать стоимость идущего через троттлер потока запросов, умноженную на 100 (чтоб получились проценты)

Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit e285ba4.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
3774 3774 0 0 0 0

@komarevtsev-d komarevtsev-d added the ok-to-test Label to approve test launch for external members label Jan 18, 2025
@github-actions github-actions bot removed the ok-to-test Label to approve test launch for external members label Jan 18, 2025
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit da56ebe.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
3774 3774 0 0 0 0

@komarevtsev-d komarevtsev-d added the ok-to-test Label to approve test launch for external members label Jan 20, 2025
@github-actions github-actions bot removed the ok-to-test Label to approve test launch for external members label Jan 20, 2025
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit eb46a19.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
3774 3774 0 0 0 0

@komarevtsev-d komarevtsev-d merged commit f013641 into ydb-platform:main Jan 20, 2025
17 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blockstore Add this label to run only cloud/blockstore build and tests on PR large-tests Launch large tests for PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants