-
Notifications
You must be signed in to change notification settings - Fork 740
add S3 client knobs + split in‑flight limits #29541
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 S3 client knobs + split in‑flight limits #29541
Conversation
|
⚪ |
|
🟢 |
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.
Pull request overview
This PR refactors the configuration of in-flight interval limits by splitting the single MaxInFlightIntervalsOnRequest field into separate blob and S3 limits, and adds S3 client configuration knobs for better control over S3 operations.
- Deprecates
MaxInFlightIntervalsOnRequestand replaces it withBlobMaxInFlightIntervalsOnRequestfor blob storage operations - Introduces
S3MaxInFlightIntervalsOnRequestto separately control S3 operation limits - Adds
TS3Clientconfiguration message withExecutorThreadsCountandMaxConnectionsCountsettings for S3 client tuning
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| ydb/core/protos/config.proto | Deprecates old field, adds new TS3Client message and separate blob/S3 in-flight limit fields |
| ydb/core/tx/columnshard/engines/reader/simple_reader/iterator/collections/abstract.cpp | Updates to use new BlobMaxInFlightIntervalsOnRequest field name |
| ydb/core/tx/columnshard/engines/reader/plain_reader/iterator/scanner.cpp | Updates to use new BlobMaxInFlightIntervalsOnRequest field name |
| ydb/core/tx/columnshard/blobs_action/tier/storage.cpp | Implements logic to apply S3Client defaults and S3MaxInFlightIntervalsOnRequest limits to S3 settings |
| ydb/core/kqp/ut/common/kqp_ut_common.h | Updates test configuration to use new BlobMaxInFlightIntervalsOnRequest field name |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
⚪
🟢
*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 |
ydb/core/protos/config.proto
Outdated
| } | ||
| } | ||
| optional TTablesStorageLayoutPolicy TablesStorageLayoutPolicy = 1; | ||
| message TS3Client { |
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.
А почему существующий конфиг для s3 не подошел? Может его взять как есть?
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.
NKikimrSchemeOp::TS3Settings
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.
Done
|
⚪ ⚪
*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 |
|
⚪ |
|
⚪
🟢
*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 |
| , Compression(compression) { | ||
| } | ||
|
|
||
| explicit TTierConfig(const TTierProto& config) |
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.
Так это же не будет работать? Кто этот конструктор вызовет?
|
⚪ |
|
⚪ |
|
⚪ ⚪ 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 |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
Co-authored-by: Matveev Sergei <xyligansereja@yandex-team.ru>
Co-authored-by: Matveev Sergei <xyligansereja@yandex-team.ru>
Co-authored-by: Matveev Sergei <xyligansereja@yandex-team.ru>
Co-authored-by: Matveev Sergei <xyligansereja@yandex-team.ru>
Changelog entry
...
Changelog category
Description for reviewers
...