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

Add parameters WriteMixedBlobThreshold and WriteMixedBlobThresholdSSD #2910

Closed
wants to merge 1 commit into from

Conversation

agalibin
Copy link
Collaborator

@agalibin agalibin commented Jan 23, 2025

Добавляет параметры WriteMixedBlobThreshold и WriteMixedBlobThresholdSSD. Это размер запроса в байтах.
С размером меньше WriteMixedBlobThreshold(SSD) будут записаны в fresh канал.
Данные с размером от WriteMixedBlobThreshold(SSD) до WriteBlobThreshold(SSD) будут записаны в mixed канал.
С размером больше WriteBlobThreshold(SSD) будут записаны в merged канал.
По умолчанию выставлены 0, что рассматривается, как фича отключена и надо работать по-старому.
Это первый шаг, далее будет добавлена логика для Compaction.

#2875

@agalibin agalibin added large-tests Launch large tests for PR blockstore Add this label to run only cloud/blockstore build and tests on PR labels Jan 23, 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 0b753a1.

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

@SvartMetal SvartMetal self-requested a review January 23, 2025 16:32
@agalibin agalibin force-pushed the users/agalibin/nbs-2875 branch 2 times, most recently from 4cdefba to 1c4608e Compare January 23, 2025 17:24

if (requestSize < writeBlobThreshold) {
if (requestSize < (writeMixedBlobThreshold ? writeMixedBlobThreshold
Copy link
Collaborator

Choose a reason for hiding this comment

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

я предлагаю организовать код так:
if (requestSize > writeBlobThreshold) {
пишем merged
} else if (requestSize > writeMixedBlobThreshold) {
пишем mixed
} else {
fresh
}

Copy link
Contributor

Note

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

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

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

 as minimum request size in bytes to write to the mixed channel.
@agalibin agalibin force-pushed the users/agalibin/nbs-2875 branch from 1c4608e to 39144a1 Compare January 24, 2025 07:57
Copy link
Contributor

Note

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

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

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

optional uint32 WriteMixedBlobThreshold = 403;

// Overrides WriteMixedBlobThreshold for SSD volumes.
optional uint32 WriteMixedBlobThresholdSSD = 404;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@agalibin @EvgeniyKozev нужны доказательства того что эти правки улучшают какой-то пользовательский сценарий

@agalibin
Copy link
Collaborator Author

agalibin commented Jan 27, 2025

in pending state: need to implement performance tests before and after.
After discussion we decided to implement this feature with another way so I'll close PR.

@agalibin agalibin closed this Jan 28, 2025
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.

[NBS] Add threshold to split Mixed and Merged channels by requests size.
3 participants