increase and make configurable some slj params #29319
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR makes stream lookup parameters configurable and adjustable, specifically introducing two new configurable settings: MaxRowsProcessingStreamLookup and MaxTotalBytesQuotaStreamLookup. These replace the hardcoded MAX_IN_FLIGHT_LIMIT constant.
Key changes:
- Adds two new configurable parameters for stream lookup: max rows processing and max total bytes quota
- Removes hardcoded
MAX_IN_FLIGHT_LIMITconstant and makes the limit configurable via settings - Adds counters to track bytes quota in flight and quota exceeded events
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ydb/core/protos/table_service_config.proto | Adds new proto fields for configurable stream lookup parameters |
| ydb/core/kqp/runtime/kqp_stream_lookup_worker.h | Updates IsOverloaded signature to accept max rows parameter |
| ydb/core/kqp/runtime/kqp_stream_lookup_worker.cpp | Removes hardcoded limit, updates implementations to use configurable parameter |
| ydb/core/kqp/runtime/kqp_stream_lookup_actor.cpp | Implements quota tracking logic and uses new configurable parameters |
| ydb/core/kqp/runtime/kqp_read_iterator_common.h | Declares new settings fields and accessor functions with default values |
| ydb/core/kqp/runtime/kqp_read_iterator_common.cpp | Implements accessor functions for new settings |
| ydb/core/kqp/runtime/kqp_buffer_lookup_actor.cpp | Comments out assertion for overload check |
| ydb/core/kqp/node_service/kqp_node_service.cpp | Wires configuration from proto to runtime settings |
| ydb/core/kqp/counters/kqp_counters.h | Declares new counter pointers |
| ydb/core/kqp/counters/kqp_counters.cpp | Initializes new counters for quota tracking |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| optional uint32 MaxTotalRetries = 7; | ||
|
|
||
| optional uint64 MaxRowsProcessingStreamLookup = 9; | ||
| optional uint64 MaxTotalBytesQuotaStreamLookup = 10q; |
There was a problem hiding this comment.
Syntax error: field number has invalid suffix 'q'. The field number should be '10' not '10q'.
| optional uint64 MaxTotalBytesQuotaStreamLookup = 10q; | |
| optional uint64 MaxTotalBytesQuotaStreamLookup = 10; |
| ptr->MaxRowsProcessingStreamLookup = settings.GetMaxRowsProcessingStreamLookup(); | ||
| ptr->MaxTotalBytesQuotaStreamLookup = settings.GetMaxTotalBytesQuotaStreamLookup(); |
There was a problem hiding this comment.
Missing validation for the new settings. If these fields are not set in the proto, GetMaxRowsProcessingStreamLookup() and GetMaxTotalBytesQuotaStreamLookup() will return 0, overwriting the default values defined in TIteratorReadBackoffSettings (65536 and 2560MB respectively). Consider using conditional assignment like if (settings.HasMaxRowsProcessingStreamLookup()) similar to the pattern used for other optional settings above (lines 451-456).
| ptr->MaxRowsProcessingStreamLookup = settings.GetMaxRowsProcessingStreamLookup(); | |
| ptr->MaxTotalBytesQuotaStreamLookup = settings.GetMaxTotalBytesQuotaStreamLookup(); | |
| if (settings.HasMaxRowsProcessingStreamLookup()) { | |
| ptr->MaxRowsProcessingStreamLookup = settings.GetMaxRowsProcessingStreamLookup(); | |
| } | |
| if (settings.HasMaxTotalBytesQuotaStreamLookup()) { | |
| ptr->MaxTotalBytesQuotaStreamLookup = settings.GetMaxTotalBytesQuotaStreamLookup(); | |
| } |
|
🟢 |
gridnevvvit
force-pushed
the
increase-and-make-configurable-some-slj-params
branch
from
df333ce to
8c0ad25
Compare
|
⚪ |
|
⚪ |
gridnevvvit
force-pushed
the
increase-and-make-configurable-some-slj-params
branch
from
8c0ad25 to
0875318
Compare
|
⚪
🟢
*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
⚪ Ya make output | Test bloat | 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 |
gridnevvvit
force-pushed
the
increase-and-make-configurable-some-slj-params
branch
2 times, most recently
from
403fd31 to
a19df1c
Compare
gridnevvvit
force-pushed
the
increase-and-make-configurable-some-slj-params
branch
2 times, most recently
from
2e98193 to
8250f33
Compare
|
⚪ ⚪ 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 |
2 participants
Changelog entry
Changelog category
Description for reviewers
...