YQ-4919 enable_streaming_queries_counters feature flag #29604
Conversation
|
⚪ ⚪ 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 |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
🟢 |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new feature flag EnableStreamingQueriesCounters to conditionally enable performance counters for streaming queries. When disabled, the system creates empty counter groups to avoid collecting metrics overhead.
- Added
EnableStreamingQueriesCountersfeature flag in protobuf definitions (defaulting to false) - Implemented conditional counter creation logic across multiple components (checkpoint coordinator, row dispatcher, topic session)
- Updated checkpoint storage service counter subgroup naming for clarity
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ydb/core/protos/feature_flags.proto | Adds the EnableStreamingQueriesCounters feature flag definition (field 218, default false) |
| ydb/tests/fq/streaming/conftest.py | Enables the feature flag in streaming query tests |
| ydb/core/kqp/proxy_service/kqp_proxy_service.cpp | Renames counter subgroup from "storage_service" to "checkpoints_storage_service" for clarity |
| ydb/core/kqp/executer_actor/kqp_data_executer.cpp | Conditionally creates path-specific counter subgroups for checkpoint coordinator based on feature flag |
| ydb/core/fq/libs/row_dispatcher/topic_session.cpp | Implements conditional counter creation for topic and query-specific metrics based on feature flag |
| ydb/core/fq/libs/row_dispatcher/row_dispatcher.cpp | Adds feature flag check for query statistics and StartSession counter creation; includes debug code that should be removed |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| TAggQueryStat(const TString& queryId, const ::NMonitoring::TDynamicCounterPtr& counters, const NYql::NPq::NProto::TDqPqTopicSource& sourceParams, bool enableStreamingQueriesCounters) | ||
| : QueryId(queryId) | ||
| , SubGroup(counters) { | ||
| Cerr << "enableStreamingQueriesCounters " << enableStreamingQueriesCounters <<Endl; |
There was a problem hiding this comment.
Debug logging statement using Cerr should be removed before merging. This appears to be leftover debug code that will produce unwanted console output in production.
| Cerr << "enableStreamingQueriesCounters " << enableStreamingQueriesCounters <<Endl; |
| TAggQueryStat(const TString& queryId, const ::NMonitoring::TDynamicCounterPtr& counters, const NYql::NPq::NProto::TDqPqTopicSource& sourceParams, bool enableStreamingQueriesCounters) | ||
| : QueryId(queryId) | ||
| , SubGroup(counters) { | ||
| Cerr << "enableStreamingQueriesCounters " << enableStreamingQueriesCounters <<Endl; |
There was a problem hiding this comment.
Missing space before <<Endl. Should be << Endl for consistency with the rest of the statement.
| Cerr << "enableStreamingQueriesCounters " << enableStreamingQueriesCounters <<Endl; | |
| Cerr << "enableStreamingQueriesCounters " << enableStreamingQueriesCounters << Endl; |
|
⚪
🟢
*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 |
1 participant
Changelog entry
...
Changelog category
Description for reviewers
...