From c9a507d62c28125d45e3bd636247aabdebdc6845 Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Thu, 7 Nov 2024 15:07:09 +0300 Subject: [PATCH 01/10] Initial commit --- .../grpc_services/query/rpc_execute_query.cpp | 7 +- .../grpc_services/rpc_execute_data_query.cpp | 6 +- ydb/core/kqp/common/events/query.h | 4 +- ydb/core/kqp/common/kqp_event_impl.cpp | 6 +- ydb/core/kqp/ut/query/kqp_query_ut.cpp | 66 +++++++++++++++++++ ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp | 49 ++++++++++++++ ydb/public/api/protos/ydb_query.proto | 5 ++ ydb/public/api/protos/ydb_table.proto | 3 + .../ydb_cli/commands/ydb_service_table.cpp | 24 +++++++ .../lib/ydb_cli/commands/ydb_service_table.h | 1 + .../sdk/cpp/client/ydb_query/client.cpp | 5 ++ ydb/public/sdk/cpp/client/ydb_query/client.h | 14 +++- .../cpp/client/ydb_query/impl/exec_query.cpp | 20 ++++-- ydb/public/sdk/cpp/client/ydb_query/query.h | 1 + .../cpp/client/ydb_table/impl/table_client.h | 9 ++- ydb/public/sdk/cpp/client/ydb_table/table.cpp | 9 ++- ydb/public/sdk/cpp/client/ydb_table/table.h | 7 +- 17 files changed, 220 insertions(+), 16 deletions(-) diff --git a/ydb/core/grpc_services/query/rpc_execute_query.cpp b/ydb/core/grpc_services/query/rpc_execute_query.cpp index 1730d30a78f0..c55a0835e0fe 100644 --- a/ydb/core/grpc_services/query/rpc_execute_query.cpp +++ b/ydb/core/grpc_services/query/rpc_execute_query.cpp @@ -262,6 +262,8 @@ class TExecuteQueryRPC : public TActorBootstrapped { .SetSupportStreamTrailingResult(true) .SetOutputChunkMaxSize(req->response_part_limit_bytes()); + assert(req->Getcollect_full_diagnostics()); + auto ev = MakeHolder( QueryAction, queryType, @@ -276,7 +278,8 @@ class TExecuteQueryRPC : public TActorBootstrapped { cachePolicy, nullptr, // operationParams settings, - req->pool_id()); + req->pool_id(), + req->Getcollect_full_diagnostics()); if (!ctx.Send(NKqp::MakeKqpProxyID(ctx.SelfID.NodeId()), ev.Release(), 0, 0, Span_.GetTraceId())) { NYql::TIssues issues; @@ -394,6 +397,8 @@ class TExecuteQueryRPC : public TActorBootstrapped { hasTrailingMessage = true; response.mutable_tx_meta()->set_id(kqpResponse.GetTxMeta().id()); } + assert(!kqpResponse.GetQueryDiagnostics().empty()); + response.set_query_full_diagnostics(kqpResponse.GetQueryDiagnostics()); } if (hasTrailingMessage) { diff --git a/ydb/core/grpc_services/rpc_execute_data_query.cpp b/ydb/core/grpc_services/rpc_execute_data_query.cpp index 6152ae5f14cb..ea8d304be48a 100644 --- a/ydb/core/grpc_services/rpc_execute_data_query.cpp +++ b/ydb/core/grpc_services/rpc_execute_data_query.cpp @@ -145,7 +145,10 @@ class TExecuteDataQueryRPC : public TRpcKqpRequestActorparameters(), req->collect_stats(), req->has_query_cache_policy() ? &req->query_cache_policy() : nullptr, - req->has_operation_params() ? &req->operation_params() : nullptr); + req->has_operation_params() ? &req->operation_params() : nullptr, + NKqp::NPrivateEvents::TQueryRequestSettings(), + "", + req->Getcollect_full_diagnostics()); ReportCostInfo_ = req->operation_params().report_cost_info() == Ydb::FeatureFlag::ENABLED; @@ -203,6 +206,7 @@ class TExecuteDataQueryRPC : public TRpcKqpRequestActorinsert({queryParameter.GetName(), parameterType}); } } + queryResult->set_query_full_diagnostics(kqpResponse.GetQueryDiagnostics()); } catch (const std::exception& ex) { NYql::TIssues issues; issues.AddIssue(NYql::ExceptionToIssue(ex)); diff --git a/ydb/core/kqp/common/events/query.h b/ydb/core/kqp/common/events/query.h index 19aad90211f2..a246a4a0d939 100644 --- a/ydb/core/kqp/common/events/query.h +++ b/ydb/core/kqp/common/events/query.h @@ -68,7 +68,8 @@ struct TEvQueryRequest: public NActors::TEventLocalSetUsePublicResponseDataFormat(true); @@ -395,6 +396,7 @@ struct TEvQueryRequest: public NActors::TEventLocal UserRequestContext; TDuration ProgressStatsPeriod; std::optional PoolConfig; + bool CollectFullDiagnostics = false; }; struct TEvDataQueryStreamPart: public TEventPBGetDatabaseName().GetOrElse(""))) @@ -35,6 +36,7 @@ TEvKqp::TEvQueryRequest::TEvQueryRequest( , QueryCachePolicy(queryCachePolicy) , HasOperationParams(operationParams) , QuerySettings(querySettings) + , CollectFullDiagnostics(collectFullDiagnostics) { if (HasOperationParams) { OperationTimeout = GetDuration(operationParams->operation_timeout()); @@ -107,6 +109,8 @@ void TEvKqp::TEvQueryRequest::PrepareRemote() const { Record.MutableRequest()->SetIsInternalCall(RequestCtx->IsInternalCall()); Record.MutableRequest()->SetOutputChunkMaxSize(QuerySettings.OutputChunkMaxSize); + Record.MutableRequest()->SetCollectDiagnostics(CollectFullDiagnostics); + RequestCtx.reset(); } } diff --git a/ydb/core/kqp/ut/query/kqp_query_ut.cpp b/ydb/core/kqp/ut/query/kqp_query_ut.cpp index 14750787df5f..d79c598bdf52 100644 --- a/ydb/core/kqp/ut/query/kqp_query_ut.cpp +++ b/ydb/core/kqp/ut/query/kqp_query_ut.cpp @@ -179,6 +179,72 @@ Y_UNIT_TEST_SUITE(KqpQuery) { UNIT_ASSERT_VALUES_EQUAL(counters.RecompileRequestGet()->Val(), 1); } + Y_UNIT_TEST(ExecuteDataQueryCollectFullDiagnostics) { + auto setting = NKikimrKqp::TKqpSetting(); + auto serverSettings = TKikimrSettings() + .SetKqpSettings({setting}); + + TKikimrRunner kikimr(serverSettings); + auto db = kikimr.GetTableClient(); + auto session = db.CreateSession().GetValueSync().GetSession(); + + { + UNIT_ASSERT(session.ExecuteSchemeQuery(R"( + CREATE TABLE `/Root/TestTable` ( + Key Uint64, + Value String, + PRIMARY KEY (Key) + ); + )").GetValueSync().IsSuccess()); + } + + { + const TString query(Q1_(R"( + SELECT * FROM `/Root/TestTable`; + )")); + + { + auto settings = TExecDataQuerySettings(); + settings.CollectFullDiagnostics(true); + + auto result = session.ExecuteDataQuery(query, TTxControl::BeginTx().CommitTx(), settings).ExtractValueSync(); + + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString().c_str()); + + UNIT_ASSERT_C(!result.GetDiagnostics().empty(), "Query result diagnostics is empty"); + + TStringStream in; + in << result.GetDiagnostics(); + NJson::TJsonValue value; + ReadJsonTree(&in, &value); + + UNIT_ASSERT_C(value.IsMap(), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_id"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("version"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_text"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_parameter_types"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("table_metadata"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value["table_metadata"].IsArray(), "Incorrect Diagnostics: table_metadata type should be an array"); + UNIT_ASSERT_C(value.Has("created_at"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_syntax"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_database"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_cluster"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_plan"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_type"), "Incorrect Diagnostics"); + } + + { + auto settings = TExecDataQuerySettings(); + + auto result = session.ExecuteDataQuery(query, TTxControl::BeginTx().CommitTx(), settings).ExtractValueSync(); + + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString().c_str()); + + UNIT_ASSERT_C(result.GetDiagnostics().empty(), "Query result diagnostics should be empty, but it's not"); + } + } + } + Y_UNIT_TEST(QueryCachePermissionsLoss) { TKikimrRunner kikimr; auto db = kikimr.GetTableClient(); diff --git a/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp b/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp index bd3080432739..43db2da0da60 100644 --- a/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp +++ b/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp @@ -272,6 +272,55 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { } } + Y_UNIT_TEST(ExecuteCollectFullDiagnostics) { + auto kikimr = DefaultKikimrRunner(); + auto db = kikimr.GetQueryClient(); + + { + TExecuteQuerySettings settings; + settings.CollectFullDiagnostics(true); + + auto result = db.ExecuteQuery(R"( + SELECT Key, Value2 FROM TwoShard WHERE Value2 > 0; + )", TTxControl::BeginTx().CommitTx(), settings).ExtractValueSync(); + + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + UNIT_ASSERT_C(!result.GetDiagnostics().empty(), "Query result diagnostics is empty"); + + TStringStream in; + in << result.GetDiagnostics(); + NJson::TJsonValue value; + ReadJsonTree(&in, &value); + + UNIT_ASSERT_C(value.IsMap(), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_id"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("version"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_text"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_parameter_types"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("table_metadata"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value["table_metadata"].IsArray(), "Incorrect Diagnostics: table_metadata type should be an array"); + UNIT_ASSERT_C(value.Has("created_at"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_syntax"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_database"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_cluster"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_plan"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_type"), "Incorrect Diagnostics"); + } + + { + TExecuteQuerySettings settings; + settings.CollectFullDiagnostics(true); + + auto result = db.ExecuteQuery(R"( + SELECT Key, Value2 FROM TwoShard WHERE Value2 > 0; + )", TTxControl::BeginTx().CommitTx(), settings).ExtractValueSync(); + + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString().c_str()); + + UNIT_ASSERT_C(result.GetDiagnostics().empty(), "Query result diagnostics should be empty, but it's not"); + } + } + void CheckQueryResult(TExecuteQueryResult result) { UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); UNIT_ASSERT_VALUES_EQUAL(result.GetResultSets().size(), 1); diff --git a/ydb/public/api/protos/ydb_query.proto b/ydb/public/api/protos/ydb_query.proto index b1fabe26d5d7..1473817f872e 100644 --- a/ydb/public/api/protos/ydb_query.proto +++ b/ydb/public/api/protos/ydb_query.proto @@ -172,6 +172,8 @@ message ExecuteQueryRequest { int64 response_part_limit_bytes = 9 [(Ydb.value) = "[0; 33554432]"]; string pool_id = 10; // Workload manager pool id + + bool collect_full_diagnostics = 11; } message ResultSetMeta { @@ -191,6 +193,9 @@ message ExecuteQueryResponsePart { Ydb.TableStats.QueryStats exec_stats = 5; TransactionMeta tx_meta = 6; + + // Full query diagnostics + string query_full_diagnostics = 7; } message ExecuteScriptRequest { diff --git a/ydb/public/api/protos/ydb_table.proto b/ydb/public/api/protos/ydb_table.proto index 172c97ceb3d0..a83a44f8f4b1 100644 --- a/ydb/public/api/protos/ydb_table.proto +++ b/ydb/public/api/protos/ydb_table.proto @@ -942,6 +942,7 @@ message ExecuteDataQueryRequest { QueryCachePolicy query_cache_policy = 5; Ydb.Operations.OperationParams operation_params = 6; QueryStatsCollection.Mode collect_stats = 7; + bool collect_full_diagnostics = 8; } message ExecuteDataQueryResponse { @@ -984,6 +985,8 @@ message ExecuteQueryResult { QueryMeta query_meta = 3; // Query execution statistics Ydb.TableStats.QueryStats query_stats = 4; + // Full query diagnostics + string query_full_diagnostics = 5; } // Explain data query diff --git a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp index 0200655e536a..a27f6d8ae622 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp @@ -365,6 +365,8 @@ void TCommandExecuteQuery::Config(TConfig& config) { config.Opts->AddLongOption('q', "query", "Text of query to execute").RequiredArgument("[String]").StoreResult(&Query); config.Opts->AddLongOption('f', "file", "Path to file with query text to execute") .RequiredArgument("PATH").StoreResult(&QueryFile); + config.Opts->AddLongOption("collect-diagnostics", "Collects diagnostics and saves it to file") + .StoreTrue(&CollectFullDiagnostics); AddOutputFormats(config, { EDataFormat::Pretty, @@ -432,6 +434,9 @@ int TCommandExecuteQuery::ExecuteDataQuery(TConfig& config) { NTable::TExecDataQuerySettings settings; settings.KeepInQueryCache(true); settings.CollectQueryStats(ParseQueryStatsModeOrThrow(CollectStatsMode, defaultStatsMode)); + if (CollectFullDiagnostics) { + settings.CollectFullDiagnostics(true); + } NTable::TTxSettings txSettings; if (TxMode) { @@ -516,6 +521,11 @@ void TCommandExecuteQuery::PrintDataQueryResponse(NTable::TDataQueryResult& resu { Cout << Endl << "Flame graph is available for full or profile stats only" << Endl; } + if (CollectFullDiagnostics) + { + TFileOutput file(TStringBuilder() << "diagnostics_" << TGUID::Create().AsGuidString() << ".txt"); + file << result.GetDiagnostics(); + } } int TCommandExecuteQuery::ExecuteSchemeQuery(TConfig& config) { @@ -558,6 +568,9 @@ namespace { if (timeout.has_value()) { settings.ClientTimeout(*timeout); } + if (CollectFullDiagnostics) { + settings.CollectFullDiagnostics(true); + } return settings; } else if constexpr (std::is_same_v) { const auto defaultStatsMode = basicStats @@ -568,6 +581,9 @@ namespace { if (timeout.has_value()) { settings.ClientTimeout(*timeout); } + if (CollectFullDiagnostics) { + settings.CollectFullDiagnostics(true); + } return settings; } Y_UNREACHABLE(); @@ -753,6 +769,8 @@ bool TCommandExecuteQuery::PrintQueryResponse(TIterator& result) { fullStats = queryStats.GetPlan(); } } + + if () } } // TResultSetPrinter destructor should be called before printing stats @@ -767,6 +785,12 @@ bool TCommandExecuteQuery::PrintQueryResponse(TIterator& result) { queryPlanPrinter.Print(*fullStats); } + if (CollectFullDiagnostics) + { + TFileOutput file(TStringBuilder() << "diagnostics_" << TGUID::Create().AsGuidString() << ".txt"); + file << result.GetDiagnostics(); + } + PrintFlameGraph(fullStats); if (IsInterrupted()) { diff --git a/ydb/public/lib/ydb_cli/commands/ydb_service_table.h b/ydb/public/lib/ydb_cli/commands/ydb_service_table.h index a0716f7dc322..70d3fcf1f006 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_service_table.h +++ b/ydb/public/lib/ydb_cli/commands/ydb_service_table.h @@ -123,6 +123,7 @@ class TCommandExecuteQuery : public TTableCommand, TCommandQueryBase, TCommandWi TString TxMode; TString QueryType; bool BasicStats = false; + bool CollectFullDiagnostics = false; }; class TCommandExplain : public TTableCommand, public TCommandWithOutput, TCommandQueryBase, TInterruptibleCommand { diff --git a/ydb/public/sdk/cpp/client/ydb_query/client.cpp b/ydb/public/sdk/cpp/client/ydb_query/client.cpp index 331f17ec429c..24b111371f07 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/client.cpp +++ b/ydb/public/sdk/cpp/client/ydb_query/client.cpp @@ -723,4 +723,9 @@ TResultSetParser TExecuteQueryResult::GetResultSetParser(size_t resultIndex) con return TResultSetParser(GetResultSet(resultIndex)); } +const TString& TExecuteQueryResult::GetDiagnostics() const { + CheckStatusOk("TExecuteQueryResult::GetDiagnostics"); + return Diagnostics_; +} + } // namespace NYdb::NQuery diff --git a/ydb/public/sdk/cpp/client/ydb_query/client.h b/ydb/public/sdk/cpp/client/ydb_query/client.h index a459c4b3981a..5c31967b0ff7 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/client.h +++ b/ydb/public/sdk/cpp/client/ydb_query/client.h @@ -213,23 +213,26 @@ class TExecuteQueryPart : public TStreamPartStatus { ui64 GetResultSetIndex() const { return ResultSetIndex_; } const TResultSet& GetResultSet() const { return *ResultSet_; } TResultSet ExtractResultSet() { return std::move(*ResultSet_); } + const TString& GetDiagnostics() const { return Diagnostics_; } const TMaybe& GetStats() const { return Stats_; } const TMaybe& GetTransaction() const { return Transaction_; } - TExecuteQueryPart(TStatus&& status, TMaybe&& queryStats, TMaybe&& tx) + TExecuteQueryPart(TStatus&& status, TMaybe&& queryStats, TMaybe&& tx, TString&& diagnostics) : TStreamPartStatus(std::move(status)) , Stats_(std::move(queryStats)) , Transaction_(std::move(tx)) + , Diagnostics_(std::move(diagnostics)) {} TExecuteQueryPart(TStatus&& status, TResultSet&& resultSet, i64 resultSetIndex, - TMaybe&& queryStats, TMaybe&& tx) + TMaybe&& queryStats, TMaybe&& tx, TString&& diagnostics) : TStreamPartStatus(std::move(status)) , ResultSet_(std::move(resultSet)) , ResultSetIndex_(resultSetIndex) , Stats_(std::move(queryStats)) , Transaction_(std::move(tx)) + , Diagnostics_(std::move(diagnostics)) {} private: @@ -237,6 +240,7 @@ class TExecuteQueryPart : public TStreamPartStatus { i64 ResultSetIndex_ = 0; TMaybe Stats_; TMaybe Transaction_; + TString Diagnostics_; }; class TExecuteQueryResult : public TStatus { @@ -249,22 +253,26 @@ class TExecuteQueryResult : public TStatus { TMaybe GetTransaction() const {return Transaction_; } + const TString& GetDiagnostics() const; + TExecuteQueryResult(TStatus&& status) : TStatus(std::move(status)) {} TExecuteQueryResult(TStatus&& status, TVector&& resultSets, - TMaybe&& stats, TMaybe&& tx) + TMaybe&& stats, TMaybe&& tx, TString&& diagnostics) : TStatus(std::move(status)) , ResultSets_(std::move(resultSets)) , Stats_(std::move(stats)) , Transaction_(std::move(tx)) + , Diagnostics_(std::move(diagnostics)) {} private: TVector ResultSets_; TMaybe Stats_; TMaybe Transaction_; + TString Diagnostics_; }; } // namespace NYdb::NQuery diff --git a/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp b/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp index 93b91c5ac2e1..fe5958a9f43d 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp +++ b/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp @@ -67,13 +67,14 @@ class TExecuteQueryIterator::TReaderImpl { auto readCb = [self, promise](TGRpcStatus&& grpcStatus) mutable { if (!grpcStatus.Ok()) { self->Finished_ = true; - promise.SetValue({TStatus(TPlainStatus(grpcStatus, self->Endpoint_)), {}, {}}); + promise.SetValue({TStatus(TPlainStatus(grpcStatus, self->Endpoint_)), {}, {}, ""}); } else { NYql::TIssues issues; NYql::IssuesFromMessage(self->Response_.issues(), issues); EStatus clientStatus = static_cast(self->Response_.status()); TPlainStatus plainStatus{clientStatus, std::move(issues), self->Endpoint_, {}}; TStatus status{std::move(plainStatus)}; + TString diagnostics; TMaybe stats; TMaybe tx; @@ -85,16 +86,19 @@ class TExecuteQueryIterator::TReaderImpl { tx = TTransaction(self->Session_.GetRef(), self->Response_.tx_meta().id()); } + diagnostics = self->Response_.query_full_diagnostics(); + if (self->Response_.has_result_set()) { promise.SetValue({ std::move(status), TResultSet(std::move(*self->Response_.mutable_result_set())), self->Response_.result_set_index(), std::move(stats), - std::move(tx) + std::move(tx), + std::move(diagnostics) }); } else { - promise.SetValue({std::move(status), std::move(stats), std::move(tx)}); + promise.SetValue({std::move(status), std::move(stats), std::move(tx), std::move(diagnostics)}); } } }; @@ -145,6 +149,7 @@ struct TExecuteQueryBuffer : public TThrRefBase, TNonCopyable { TVector ResultSets_; TMaybe Stats_; TMaybe Tx_; + TString Diagnostics_; void Next() { TPtr self(this); @@ -178,10 +183,11 @@ struct TExecuteQueryBuffer : public TThrRefBase, TNonCopyable { TStatus(EStatus::SUCCESS, NYql::TIssues(std::move(issues))), std::move(resultSets), std::move(stats), - std::move(tx) + std::move(tx), + {} )); } else { - self->Promise_.SetValue(TExecuteQueryResult(std::move(part), {}, std::move(stats), {})); + self->Promise_.SetValue(TExecuteQueryResult(std::move(part), {}, std::move(stats), {}, {})); } return; @@ -210,6 +216,8 @@ struct TExecuteQueryBuffer : public TThrRefBase, TNonCopyable { self->Tx_ = tx; } + self->Diagnostics_ = part.GetDiagnostics(); + self->Next(); }); } @@ -240,6 +248,8 @@ TFuture> StreamExecuteQueryIm request.set_response_part_limit_bytes(*settings.OutputChunkMaxSize_); } + request.set_collect_full_diagnostics(settings.CollectFullDiagnostics_); + if (txControl.HasTx()) { auto requestTxControl = request.mutable_tx_control(); requestTxControl->set_commit_tx(txControl.CommitTx_); diff --git a/ydb/public/sdk/cpp/client/ydb_query/query.h b/ydb/public/sdk/cpp/client/ydb_query/query.h index 3ea49527ae77..e35da4695ac8 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/query.h +++ b/ydb/public/sdk/cpp/client/ydb_query/query.h @@ -74,6 +74,7 @@ struct TExecuteQuerySettings : public TRequestSettings { FLUENT_SETTING_DEFAULT(EStatsMode, StatsMode, EStatsMode::None); FLUENT_SETTING_OPTIONAL(bool, ConcurrentResultSets); FLUENT_SETTING(TString, ResourcePool); + FLUENT_SETTING_DEFAULT(bool, CollectFullDiagnostics, false); }; struct TBeginTxSettings : public TRequestSettings {}; diff --git a/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.h b/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.h index da681f0d959a..42baeeff1d93 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.h +++ b/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.h @@ -188,7 +188,8 @@ class TTableClient::TImpl: public TClientImplCommon, public txControl.Tx_, Nothing(), false, - Nothing())); + Nothing(), + {})); } return ExecuteDataQueryInternal(session, query, txControl, params, settings, fromCache); @@ -213,6 +214,7 @@ class TTableClient::TImpl: public TClientImplCommon, public } request.set_collect_stats(GetStatsCollectionMode(settings.CollectQueryStats_)); + request.set_collect_full_diagnostics(settings.CollectFullDiagnostics_); SetQuery(query, request.mutable_query()); CollectQuerySize(query, QuerySizeHistogram); @@ -240,6 +242,7 @@ class TTableClient::TImpl: public TClientImplCommon, public TMaybe tx; TMaybe dataQuery; TMaybe queryStats; + TString diagnostics; auto queryText = GetQueryText(query); if (any) { @@ -264,6 +267,8 @@ class TTableClient::TImpl: public TClientImplCommon, public if (result.has_query_stats()) { queryStats = TQueryStats(result.query_stats()); } + + diagnostics = result.query_full_diagnostics(); } if (keepInCache && dataQuery && queryText) { @@ -271,7 +276,7 @@ class TTableClient::TImpl: public TClientImplCommon, public } TDataQueryResult dataQueryResult(TStatus(std::move(status)), - std::move(res), tx, dataQuery, fromCache, queryStats); + std::move(res), tx, dataQuery, fromCache, queryStats, std::move(diagnostics)); delete sessionPtr; tx.Clear(); diff --git a/ydb/public/sdk/cpp/client/ydb_table/table.cpp b/ydb/public/sdk/cpp/client/ydb_table/table.cpp index 03ddeb2990a8..b9845ea22a5e 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/table.cpp +++ b/ydb/public/sdk/cpp/client/ydb_table/table.cpp @@ -2156,13 +2156,15 @@ TTableDescription TDescribeTableResult::GetTableDescription() const { //////////////////////////////////////////////////////////////////////////////// TDataQueryResult::TDataQueryResult(TStatus&& status, TVector&& resultSets, - const TMaybe& transaction, const TMaybe& dataQuery, bool fromCache, const TMaybe &queryStats) + const TMaybe& transaction, const TMaybe& dataQuery, bool fromCache, const TMaybe &queryStats, + TString&& diagnostics) : TStatus(std::move(status)) , Transaction_(transaction) , ResultSets_(std::move(resultSets)) , DataQuery_(dataQuery) , FromCache_(fromCache) , QueryStats_(queryStats) + , Diagnostics_(std::move(diagnostics)) {} const TVector& TDataQueryResult::GetResultSets() const { @@ -2209,6 +2211,11 @@ const TString TDataQueryResult::GetQueryPlan() const { } } +const TString& TDataQueryResult::GetDiagnostics() const { + CheckStatusOk("TDataQueryResult::GetDiagnostics"); + return Diagnostics_; +} + //////////////////////////////////////////////////////////////////////////////// TBeginTransactionResult::TBeginTransactionResult(TStatus&& status, TTransaction transaction) diff --git a/ydb/public/sdk/cpp/client/ydb_table/table.h b/ydb/public/sdk/cpp/client/ydb_table/table.h index ef0060886e9d..8e3cd604ab09 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/table.h +++ b/ydb/public/sdk/cpp/client/ydb_table/table.h @@ -1713,6 +1713,8 @@ struct TExecDataQuerySettings : public TOperationRequestSettings {}; @@ -2015,7 +2017,7 @@ class TDescribeTableResult : public NScheme::TDescribePathResult { class TDataQueryResult : public TStatus { public: TDataQueryResult(TStatus&& status, TVector&& resultSets, const TMaybe& transaction, - const TMaybe& dataQuery, bool fromCache, const TMaybe& queryStats); + const TMaybe& dataQuery, bool fromCache, const TMaybe& queryStats, TString&& diagnostics); const TVector& GetResultSets() const; TVector ExtractResultSets() &&; @@ -2032,12 +2034,15 @@ class TDataQueryResult : public TStatus { const TString GetQueryPlan() const; + const TString& GetDiagnostics() const; + private: TMaybe Transaction_; TVector ResultSets_; TMaybe DataQuery_; bool FromCache_; TMaybe QueryStats_; + TString Diagnostics_; }; class TReadTableSnapshot { From 4dd148cc75d9d1fb8376b232b67614337dff6cc6 Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Thu, 7 Nov 2024 20:42:24 +0300 Subject: [PATCH 02/10] Fixes --- ydb/core/grpc_services/query/rpc_execute_query.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/ydb/core/grpc_services/query/rpc_execute_query.cpp b/ydb/core/grpc_services/query/rpc_execute_query.cpp index c55a0835e0fe..b10352f2a7cf 100644 --- a/ydb/core/grpc_services/query/rpc_execute_query.cpp +++ b/ydb/core/grpc_services/query/rpc_execute_query.cpp @@ -262,8 +262,6 @@ class TExecuteQueryRPC : public TActorBootstrapped { .SetSupportStreamTrailingResult(true) .SetOutputChunkMaxSize(req->response_part_limit_bytes()); - assert(req->Getcollect_full_diagnostics()); - auto ev = MakeHolder( QueryAction, queryType, @@ -397,7 +395,6 @@ class TExecuteQueryRPC : public TActorBootstrapped { hasTrailingMessage = true; response.mutable_tx_meta()->set_id(kqpResponse.GetTxMeta().id()); } - assert(!kqpResponse.GetQueryDiagnostics().empty()); response.set_query_full_diagnostics(kqpResponse.GetQueryDiagnostics()); } From 8d7f55273539235b191926edd60ba62378bed94e Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Thu, 14 Nov 2024 07:53:25 +0300 Subject: [PATCH 03/10] Fixes --- ydb/core/kqp/common/events/query.h | 6 +++--- ydb/core/kqp/common/kqp_event_impl.cpp | 4 +++- ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp | 4 +++- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/ydb/core/kqp/common/events/query.h b/ydb/core/kqp/common/events/query.h index a246a4a0d939..a50cda93f0d1 100644 --- a/ydb/core/kqp/common/events/query.h +++ b/ydb/core/kqp/common/events/query.h @@ -69,7 +69,7 @@ struct TEvQueryRequest: public NActors::TEventLocal collectFullDiagnostics = std::nullopt); TEvQueryRequest() { Record.MutableRequest()->SetUsePublicResponseDataFormat(true); @@ -283,7 +283,7 @@ struct TEvQueryRequest: public NActors::TEventLocal UserRequestContext; TDuration ProgressStatsPeriod; std::optional PoolConfig; - bool CollectFullDiagnostics = false; + std::optional CollectFullDiagnostics = std::nullopt; }; struct TEvDataQueryStreamPart: public TEventPBSetIsInternalCall(RequestCtx->IsInternalCall()); Record.MutableRequest()->SetOutputChunkMaxSize(QuerySettings.OutputChunkMaxSize); - Record.MutableRequest()->SetCollectDiagnostics(CollectFullDiagnostics); + if (CollectFullDiagnostics.has_value()) { + Record.MutableRequest()->SetCollectDiagnostics(CollectFullDiagnostics.value()); + } RequestCtx.reset(); } diff --git a/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp b/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp index fe5958a9f43d..dcdd894fa5f3 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp +++ b/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp @@ -169,10 +169,12 @@ struct TExecuteQueryBuffer : public TThrRefBase, TNonCopyable { TVector issues; TVector resultProtos; TMaybe tx; + TString diagnostics; std::swap(self->Issues_, issues); std::swap(self->ResultSets_, resultProtos); std::swap(self->Tx_, tx); + std::swap(self->Diagnostics_, diagnostics); TVector resultSets; for (auto& proto : resultProtos) { @@ -184,7 +186,7 @@ struct TExecuteQueryBuffer : public TThrRefBase, TNonCopyable { std::move(resultSets), std::move(stats), std::move(tx), - {} + std::move(diagnostics) )); } else { self->Promise_.SetValue(TExecuteQueryResult(std::move(part), {}, std::move(stats), {}, {})); From 4e106ad04e9b9fa8b9a7f7b2022d1a4076b849c9 Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Thu, 14 Nov 2024 08:04:36 +0300 Subject: [PATCH 04/10] Fixes --- ydb/core/kqp/common/kqp_event_impl.cpp | 2 +- .../lib/ydb_cli/commands/ydb_service_table.cpp | 13 +++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/ydb/core/kqp/common/kqp_event_impl.cpp b/ydb/core/kqp/common/kqp_event_impl.cpp index e564cadd1ace..4d23e043a45d 100644 --- a/ydb/core/kqp/common/kqp_event_impl.cpp +++ b/ydb/core/kqp/common/kqp_event_impl.cpp @@ -20,7 +20,7 @@ TEvKqp::TEvQueryRequest::TEvQueryRequest( const ::Ydb::Operations::OperationParams* operationParams, const TQueryRequestSettings& querySettings, const TString& poolId, - bool collectFullDiagnostics) + std::optional collectFullDiagnostics) : RequestCtx(ctx) , RequestActorId(requestActorId) , Database(CanonizePath(ctx->GetDatabaseName().GetOrElse(""))) diff --git a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp index a27f6d8ae622..87aabc1e0401 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp @@ -558,7 +558,7 @@ namespace { NQuery::TExecuteQuerySettings>; template - auto GetSettings(const TString& collectStatsMode, const bool basicStats, std::optional timeout) { + auto GetSettings(const TString& collectStatsMode, const bool basicStats, std::optional timeout, bool collectFullDiagnostics) { if constexpr (std::is_same_v) { const auto defaultStatsMode = basicStats ? NTable::ECollectQueryStatsMode::Basic @@ -568,7 +568,7 @@ namespace { if (timeout.has_value()) { settings.ClientTimeout(*timeout); } - if (CollectFullDiagnostics) { + if (collectFullDiagnostics) { settings.CollectFullDiagnostics(true); } return settings; @@ -581,7 +581,7 @@ namespace { if (timeout.has_value()) { settings.ClientTimeout(*timeout); } - if (CollectFullDiagnostics) { + if (collectFullDiagnostics) { settings.CollectFullDiagnostics(true); } return settings; @@ -690,7 +690,7 @@ int TCommandExecuteQuery::ExecuteQueryImpl(TConfig& config) { if (OperationTimeout) { optTimeout = TDuration::MilliSeconds(FromString(OperationTimeout)); } - const auto settings = GetSettings(CollectStatsMode, BasicStats, optTimeout); + const auto settings = GetSettings(CollectStatsMode, BasicStats, optTimeout, CollectFullDiagnostics); TAsyncPartIterator asyncResult; SetInterruptHandlers(); @@ -748,6 +748,7 @@ template bool TCommandExecuteQuery::PrintQueryResponse(TIterator& result) { TMaybe stats; TMaybe fullStats; + TString diagnostics; { TResultSetPrinter printer(OutputFormat, &IsInterrupted); @@ -770,7 +771,7 @@ bool TCommandExecuteQuery::PrintQueryResponse(TIterator& result) { } } - if () + diagnostics = streamPart.GetDiagnostics(); } } // TResultSetPrinter destructor should be called before printing stats @@ -788,7 +789,7 @@ bool TCommandExecuteQuery::PrintQueryResponse(TIterator& result) { if (CollectFullDiagnostics) { TFileOutput file(TStringBuilder() << "diagnostics_" << TGUID::Create().AsGuidString() << ".txt"); - file << result.GetDiagnostics(); + file << diagnostics; } PrintFlameGraph(fullStats); From 2ce92915cca323e501b1362ec82e33a1d7eeaba0 Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Fri, 22 Nov 2024 10:44:34 +0300 Subject: [PATCH 05/10] Fixes --- ydb/public/lib/ydb_cli/commands/ydb_sql.cpp | 15 +++++++++++++++ ydb/public/lib/ydb_cli/commands/ydb_sql.h | 1 + 2 files changed, 16 insertions(+) diff --git a/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp b/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp index abfd8377149c..4c47890ca26d 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -43,6 +44,8 @@ void TCommandSql::Config(TConfig& config) { config.Opts->AddLongOption("syntax", "Query syntax [yql, pg]") .RequiredArgument("[String]").DefaultValue("yql").StoreResult(&Syntax) .Hidden(); + config.Opts->AddLongOption("collect-diagnostics", "Collects diagnostics and saves it to file") + .StoreTrue(&CollectFullDiagnostics); AddOutputFormats(config, { EDataFormat::Pretty, @@ -146,6 +149,10 @@ int TCommandSql::RunCommand(TConfig& config) { throw TMisuseException() << "Unknow syntax option \"" << Syntax << "\""; } + if (CollectFullDiagnostics) { + settings.CollectFullDiagnostics(true); + } + if (!Parameters.empty() || InputParamStream) { // Execute query with parameters THolder paramBuilder; @@ -183,6 +190,7 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { TMaybe stats; TMaybe plan; TMaybe ast; + TString diagnostics; { TResultSetPrinter printer(OutputFormat, &IsInterrupted); @@ -205,6 +213,8 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { plan = queryStats.GetPlan(); } } + + diagnostics = streamPart.GetDiagnostics(); } } // TResultSetPrinter destructor should be called before printing stats @@ -235,6 +245,11 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { queryPlanPrinter.Print(*plan); } + if (CollectFullDiagnostics) { + TFileOutput file(TStringBuilder() << "diagnostics_" << TGUID::Create().AsGuidString() << ".txt"); + file << diagnostics; + } + if (IsInterrupted()) { Cerr << "" << Endl; return EXIT_FAILURE; diff --git a/ydb/public/lib/ydb_cli/commands/ydb_sql.h b/ydb/public/lib/ydb_cli/commands/ydb_sql.h index 19dccfd01e61..0e8df3af0ff4 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_sql.h +++ b/ydb/public/lib/ydb_cli/commands/ydb_sql.h @@ -34,6 +34,7 @@ class TCommandSql : public TYdbCommand, public TCommandWithOutput, public TComma bool ExplainMode = false; bool ExplainAnalyzeMode = false; bool ExplainAst = false; + bool CollectDiagnostics = false; }; } From dfe426dae350fed975e59136e63b9d7a74311498 Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Thu, 12 Dec 2024 15:55:01 +0300 Subject: [PATCH 06/10] Fixes --- ydb/public/lib/ydb_cli/commands/ydb_sql.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ydb/public/lib/ydb_cli/commands/ydb_sql.h b/ydb/public/lib/ydb_cli/commands/ydb_sql.h index 0e8df3af0ff4..277ecce3d81e 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_sql.h +++ b/ydb/public/lib/ydb_cli/commands/ydb_sql.h @@ -34,7 +34,7 @@ class TCommandSql : public TYdbCommand, public TCommandWithOutput, public TComma bool ExplainMode = false; bool ExplainAnalyzeMode = false; bool ExplainAst = false; - bool CollectDiagnostics = false; + bool CollectFullDiagnostics = false; }; } From 6d7e7a572e9cb65a34848153f63207ab90e3a7d4 Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Mon, 16 Dec 2024 07:05:09 +0300 Subject: [PATCH 07/10] Fixes --- ydb/core/grpc_services/query/rpc_execute_query.cpp | 6 +++--- ydb/core/grpc_services/rpc_execute_data_query.cpp | 4 +--- ydb/core/kqp/common/events/query.h | 12 ++++++++---- ydb/core/kqp/common/kqp_event_impl.cpp | 8 ++------ 4 files changed, 14 insertions(+), 16 deletions(-) diff --git a/ydb/core/grpc_services/query/rpc_execute_query.cpp b/ydb/core/grpc_services/query/rpc_execute_query.cpp index b10352f2a7cf..4006c0f51d3b 100644 --- a/ydb/core/grpc_services/query/rpc_execute_query.cpp +++ b/ydb/core/grpc_services/query/rpc_execute_query.cpp @@ -260,7 +260,8 @@ class TExecuteQueryRPC : public TActorBootstrapped { .SetUseCancelAfter(false) .SetSyntax(syntax) .SetSupportStreamTrailingResult(true) - .SetOutputChunkMaxSize(req->response_part_limit_bytes()); + .SetOutputChunkMaxSize(req->response_part_limit_bytes()) + .SetCollectFullDiagnostics(req->Getcollect_full_diagnostics()); auto ev = MakeHolder( QueryAction, @@ -276,8 +277,7 @@ class TExecuteQueryRPC : public TActorBootstrapped { cachePolicy, nullptr, // operationParams settings, - req->pool_id(), - req->Getcollect_full_diagnostics()); + req->pool_id()); if (!ctx.Send(NKqp::MakeKqpProxyID(ctx.SelfID.NodeId()), ev.Release(), 0, 0, Span_.GetTraceId())) { NYql::TIssues issues; diff --git a/ydb/core/grpc_services/rpc_execute_data_query.cpp b/ydb/core/grpc_services/rpc_execute_data_query.cpp index ea8d304be48a..c1d0a393d9e0 100644 --- a/ydb/core/grpc_services/rpc_execute_data_query.cpp +++ b/ydb/core/grpc_services/rpc_execute_data_query.cpp @@ -146,9 +146,7 @@ class TExecuteDataQueryRPC : public TRpcKqpRequestActorcollect_stats(), req->has_query_cache_policy() ? &req->query_cache_policy() : nullptr, req->has_operation_params() ? &req->operation_params() : nullptr, - NKqp::NPrivateEvents::TQueryRequestSettings(), - "", - req->Getcollect_full_diagnostics()); + NKqp::NPrivateEvents::TQueryRequestSettings().SetCollectFullDiagnostics(req->Getcollect_full_diagnostics())); ReportCostInfo_ = req->operation_params().report_cost_info() == Ydb::FeatureFlag::ENABLED; diff --git a/ydb/core/kqp/common/events/query.h b/ydb/core/kqp/common/events/query.h index a50cda93f0d1..cab2acd0e3ac 100644 --- a/ydb/core/kqp/common/events/query.h +++ b/ydb/core/kqp/common/events/query.h @@ -45,11 +45,17 @@ struct TQueryRequestSettings { return *this; } + TQueryRequestSettings& SetCollectFullDiagnostics(bool flag) { + CollectFullDiagnostics = flag; + return *this; + } + ui64 OutputChunkMaxSize = 0; bool KeepSession = false; bool UseCancelAfter = true; ::Ydb::Query::Syntax Syntax = Ydb::Query::Syntax::SYNTAX_UNSPECIFIED; bool SupportsStreamTrailingResult = false; + bool CollectFullDiagnostics = false; }; struct TEvQueryRequest: public NActors::TEventLocal { @@ -68,8 +74,7 @@ struct TEvQueryRequest: public NActors::TEventLocal collectFullDiagnostics = std::nullopt); + const TString& poolId = ""); TEvQueryRequest() { Record.MutableRequest()->SetUsePublicResponseDataFormat(true); @@ -283,7 +288,7 @@ struct TEvQueryRequest: public NActors::TEventLocal UserRequestContext; TDuration ProgressStatsPeriod; std::optional PoolConfig; - std::optional CollectFullDiagnostics = std::nullopt; }; struct TEvDataQueryStreamPart: public TEventPB collectFullDiagnostics) + const TString& poolId) : RequestCtx(ctx) , RequestActorId(requestActorId) , Database(CanonizePath(ctx->GetDatabaseName().GetOrElse(""))) @@ -36,7 +35,6 @@ TEvKqp::TEvQueryRequest::TEvQueryRequest( , QueryCachePolicy(queryCachePolicy) , HasOperationParams(operationParams) , QuerySettings(querySettings) - , CollectFullDiagnostics(collectFullDiagnostics) { if (HasOperationParams) { OperationTimeout = GetDuration(operationParams->operation_timeout()); @@ -109,9 +107,7 @@ void TEvKqp::TEvQueryRequest::PrepareRemote() const { Record.MutableRequest()->SetIsInternalCall(RequestCtx->IsInternalCall()); Record.MutableRequest()->SetOutputChunkMaxSize(QuerySettings.OutputChunkMaxSize); - if (CollectFullDiagnostics.has_value()) { - Record.MutableRequest()->SetCollectDiagnostics(CollectFullDiagnostics.value()); - } + Record.MutableRequest()->SetCollectDiagnostics(QuerySettings.CollectFullDiagnostics); RequestCtx.reset(); } From 78040a4094443ad63eadf3f7a05ef2be4242fe9e Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Wed, 18 Dec 2024 07:56:03 +0300 Subject: [PATCH 08/10] Fixes --- ydb/core/grpc_services/rpc_stream_execute_scan_query.cpp | 3 ++- ydb/core/kqp/common/events/query.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/ydb/core/grpc_services/rpc_stream_execute_scan_query.cpp b/ydb/core/grpc_services/rpc_stream_execute_scan_query.cpp index c37dc9b06e64..c72e1ea30f02 100644 --- a/ydb/core/grpc_services/rpc_stream_execute_scan_query.cpp +++ b/ydb/core/grpc_services/rpc_stream_execute_scan_query.cpp @@ -225,7 +225,8 @@ class TStreamExecuteScanQueryRPC : public TActorBootstrappedparameters(), req->collect_stats(), nullptr, // query_cache_policy - nullptr + nullptr, + NKqp::NPrivateEvents::TQueryRequestSettings().SetCollectFullDiagnostics(req->Getcollect_full_diagnostics()) ); ev->Record.MutableRequest()->SetCollectDiagnostics(req->Getcollect_full_diagnostics()); diff --git a/ydb/core/kqp/common/events/query.h b/ydb/core/kqp/common/events/query.h index cab2acd0e3ac..8c52c47b200b 100644 --- a/ydb/core/kqp/common/events/query.h +++ b/ydb/core/kqp/common/events/query.h @@ -288,7 +288,7 @@ struct TEvQueryRequest: public NActors::TEventLocalGetCollectDiagnostics(); } ui32 CalculateSerializedSize() const override { From cc58b9a1fb54fcc68c822c5520102d703a6bf53d Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Mon, 23 Dec 2024 14:05:16 +0300 Subject: [PATCH 09/10] Fixes --- .../grpc_services/query/rpc_execute_query.cpp | 28 +++++++++++++++-- .../grpc_services/rpc_execute_data_query.cpp | 17 ++++++++-- .../rpc_stream_execute_scan_query.cpp | 31 ++++++++++++++++--- ydb/core/kqp/common/compilation/events.h | 12 +++---- ydb/core/kqp/common/events/query.h | 8 +---- ydb/core/kqp/common/kqp_event_impl.cpp | 2 -- .../kqp/compile_service/kqp_compile_actor.cpp | 12 ++++--- .../compile_service/kqp_compile_service.cpp | 14 ++++----- .../kqp/compile_service/kqp_compile_service.h | 1 + .../kqp/session_actor/kqp_query_state.cpp | 9 ++---- ydb/core/kqp/ut/olap/kqp_olap_ut.cpp | 4 +-- ydb/core/kqp/ut/query/kqp_query_ut.cpp | 2 +- ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp | 4 +-- ydb/public/api/protos/ydb_query.proto | 5 --- ydb/public/api/protos/ydb_query_stats.proto | 1 + ydb/public/api/protos/ydb_table.proto | 11 ++----- .../sdk/cpp/client/ydb_query/client.cpp | 5 --- ydb/public/sdk/cpp/client/ydb_query/client.h | 14 ++------- .../cpp/client/ydb_query/impl/exec_query.cpp | 21 +++---------- ydb/public/sdk/cpp/client/ydb_query/query.h | 1 - ydb/public/sdk/cpp/client/ydb_query/stats.cpp | 10 ++++++ ydb/public/sdk/cpp/client/ydb_query/stats.h | 1 + .../sdk/cpp/client/ydb_table/impl/readers.cpp | 7 ++--- .../client/ydb_table/impl/table_client.cpp | 1 - .../cpp/client/ydb_table/impl/table_client.h | 9 ++---- .../cpp/client/ydb_table/query_stats/stats.h | 2 +- ydb/public/sdk/cpp/client/ydb_table/table.cpp | 13 ++++---- ydb/public/sdk/cpp/client/ydb_table/table.h | 21 +++---------- 28 files changed, 135 insertions(+), 131 deletions(-) diff --git a/ydb/core/grpc_services/query/rpc_execute_query.cpp b/ydb/core/grpc_services/query/rpc_execute_query.cpp index 4006c0f51d3b..10d38dfba818 100644 --- a/ydb/core/grpc_services/query/rpc_execute_query.cpp +++ b/ydb/core/grpc_services/query/rpc_execute_query.cpp @@ -175,6 +175,25 @@ bool NeedReportAst(const Ydb::Query::ExecuteQueryRequest& req) { } } +bool NeedCollectDiagnostics(const Ydb::Query::ExecuteQueryRequest& req) { + switch (req.exec_mode()) { + case Ydb::Query::EXEC_MODE_EXPLAIN: + return true; + + case Ydb::Query::EXEC_MODE_EXECUTE: + switch (req.stats_mode()) { + case Ydb::Query::StatsMode::STATS_MODE_FULL: + case Ydb::Query::StatsMode::STATS_MODE_PROFILE: + return true; + default: + return false; + } + + default: + return false; + } +} + class TExecuteQueryRPC : public TActorBootstrapped { public: static constexpr NKikimrServices::TActivity::EType ActorActivityType() { @@ -260,8 +279,7 @@ class TExecuteQueryRPC : public TActorBootstrapped { .SetUseCancelAfter(false) .SetSyntax(syntax) .SetSupportStreamTrailingResult(true) - .SetOutputChunkMaxSize(req->response_part_limit_bytes()) - .SetCollectFullDiagnostics(req->Getcollect_full_diagnostics()); + .SetOutputChunkMaxSize(req->response_part_limit_bytes()); auto ev = MakeHolder( QueryAction, @@ -279,6 +297,8 @@ class TExecuteQueryRPC : public TActorBootstrapped { settings, req->pool_id()); + ev->Record.MutableRequest()->SetCollectDiagnostics(NeedCollectDiagnostics(*req)); + if (!ctx.Send(NKqp::MakeKqpProxyID(ctx.SelfID.NodeId()), ev.Release(), 0, 0, Span_.GetTraceId())) { NYql::TIssues issues; issues.AddIssue(MakeIssue(NKikimrIssues::TIssuesIds::DEFAULT_ERROR, "Internal error")); @@ -376,6 +396,9 @@ class TExecuteQueryRPC : public TActorBootstrapped { if (NeedReportAst(*Request_->GetProtoRequest())) { response.mutable_exec_stats()->set_query_ast(kqpResponse.GetQueryAst()); } + if (NeedCollectDiagnostics(*Request_->GetProtoRequest())) { + response.mutable_exec_stats()->set_query_diagnostics(kqpResponse.GetQueryDiagnostics()); + } } if (record.GetYdbStatus() == Ydb::StatusIds::SUCCESS) { @@ -395,7 +418,6 @@ class TExecuteQueryRPC : public TActorBootstrapped { hasTrailingMessage = true; response.mutable_tx_meta()->set_id(kqpResponse.GetTxMeta().id()); } - response.set_query_full_diagnostics(kqpResponse.GetQueryDiagnostics()); } if (hasTrailingMessage) { diff --git a/ydb/core/grpc_services/rpc_execute_data_query.cpp b/ydb/core/grpc_services/rpc_execute_data_query.cpp index c1d0a393d9e0..473ab06e8e08 100644 --- a/ydb/core/grpc_services/rpc_execute_data_query.cpp +++ b/ydb/core/grpc_services/rpc_execute_data_query.cpp @@ -27,6 +27,16 @@ using namespace Ydb; using namespace Ydb::Table; using namespace NKqp; +bool NeedCollectDiagnostics(const Ydb::Table::ExecuteDataQueryRequest& req) { + switch (req.collect_stats()) { + case Ydb::Table::QueryStatsCollection::STATS_COLLECTION_FULL: + case Ydb::Table::QueryStatsCollection::STATS_COLLECTION_PROFILE: + return true; + default: + return false; + } +} + using TEvExecuteDataQueryRequest = TGrpcRequestOperationCall; @@ -145,8 +155,9 @@ class TExecuteDataQueryRPC : public TRpcKqpRequestActorparameters(), req->collect_stats(), req->has_query_cache_policy() ? &req->query_cache_policy() : nullptr, - req->has_operation_params() ? &req->operation_params() : nullptr, - NKqp::NPrivateEvents::TQueryRequestSettings().SetCollectFullDiagnostics(req->Getcollect_full_diagnostics())); + req->has_operation_params() ? &req->operation_params() : nullptr); + + ev->Record.MutableRequest()->SetCollectDiagnostics(NeedCollectDiagnostics(*req)); ReportCostInfo_ = req->operation_params().report_cost_info() == Ydb::FeatureFlag::ENABLED; @@ -167,6 +178,7 @@ class TExecuteDataQueryRPC : public TRpcKqpRequestActormutable_query_stats(), from); to->mutable_query_stats()->set_query_ast(from.GetQueryAst()); + to->mutable_query_stats()->set_query_diagnostics(from.GetQueryDiagnostics()); return; } } @@ -204,7 +216,6 @@ class TExecuteDataQueryRPC : public TRpcKqpRequestActorinsert({queryParameter.GetName(), parameterType}); } } - queryResult->set_query_full_diagnostics(kqpResponse.GetQueryDiagnostics()); } catch (const std::exception& ex) { NYql::TIssues issues; issues.AddIssue(NYql::ExceptionToIssue(ex)); diff --git a/ydb/core/grpc_services/rpc_stream_execute_scan_query.cpp b/ydb/core/grpc_services/rpc_stream_execute_scan_query.cpp index c72e1ea30f02..addf6b5384d7 100644 --- a/ydb/core/grpc_services/rpc_stream_execute_scan_query.cpp +++ b/ydb/core/grpc_services/rpc_stream_execute_scan_query.cpp @@ -93,6 +93,27 @@ bool NeedReportPlan(const Ydb::Table::ExecuteScanQueryRequest& req) { } } +bool NeedCollectDiagnostics(const Ydb::Table::ExecuteScanQueryRequest& req) { + switch (req.mode()) { + case ExecuteScanQueryRequest_Mode_MODE_EXPLAIN: + return true; + + case ExecuteScanQueryRequest_Mode_MODE_EXEC: + switch (req.collect_stats()) { + case Ydb::Table::QueryStatsCollection::STATS_COLLECTION_FULL: + case Ydb::Table::QueryStatsCollection::STATS_COLLECTION_PROFILE: + return true; + default: + break; + } + + return false; + + default: + return false; + } +} + bool CheckRequest(const Ydb::Table::ExecuteScanQueryRequest& req, TParseRequestError& error) { switch (req.mode()) { @@ -225,11 +246,10 @@ class TStreamExecuteScanQueryRPC : public TActorBootstrappedparameters(), req->collect_stats(), nullptr, // query_cache_policy - nullptr, - NKqp::NPrivateEvents::TQueryRequestSettings().SetCollectFullDiagnostics(req->Getcollect_full_diagnostics()) + nullptr ); - ev->Record.MutableRequest()->SetCollectDiagnostics(req->Getcollect_full_diagnostics()); + ev->Record.MutableRequest()->SetCollectDiagnostics(NeedCollectDiagnostics(*req)); if (!ctx.Send(NKqp::MakeKqpProxyID(ctx.SelfID.NodeId()), ev.Release())) { NYql::TIssues issues; @@ -292,6 +312,7 @@ class TStreamExecuteScanQueryRPC : public TActorBootstrappedGetProtoRequest()); bool reportPlan = reportStats && NeedReportPlan(*Request_->GetProtoRequest()); + bool collectDiagnostics = NeedCollectDiagnostics(*Request_->GetProtoRequest()); if (reportStats) { if (kqpResponse.HasQueryStats()) { @@ -309,7 +330,9 @@ class TStreamExecuteScanQueryRPC : public TActorBootstrappedmutable_query_stats()->set_query_ast(kqpResponse.GetQueryAst()); } - response.mutable_result()->set_query_full_diagnostics(kqpResponse.GetQueryDiagnostics()); + if (collectDiagnostics) { + response.mutable_result()->mutable_query_stats()->set_query_diagnostics(kqpResponse.GetQueryDiagnostics()); + } Y_PROTOBUF_SUPPRESS_NODISCARD response.SerializeToString(&out); Request_->SendSerializedResult(std::move(out), record.GetYdbStatus()); diff --git a/ydb/core/kqp/common/compilation/events.h b/ydb/core/kqp/common/compilation/events.h index 9ed2d03e8735..1799c6800647 100644 --- a/ydb/core/kqp/common/compilation/events.h +++ b/ydb/core/kqp/common/compilation/events.h @@ -16,7 +16,7 @@ namespace NKikimr::NKqp::NPrivateEvents { struct TEvCompileRequest: public TEventLocal { TEvCompileRequest(const TIntrusiveConstPtr& userToken, const TString& clientAddress, const TMaybe& uid, - TMaybe&& query, bool keepInCache, bool isQueryActionPrepare, bool perStatementResult, TInstant deadline, + TMaybe&& query, bool keepInCache, NKikimrKqp::EQueryAction queryAction, bool perStatementResult, TInstant deadline, TKqpDbCountersPtr dbCounters, const TGUCSettings::TPtr& gUCSettings, const TMaybe& applicationName, std::shared_ptr> intrestedInResult, const TIntrusivePtr& userRequestContext, NLWTrace::TOrbit orbit = {}, TKqpTempTablesState::TConstPtr tempTablesState = nullptr, bool collectDiagnostics = false, TMaybe queryAst = Nothing(), @@ -26,7 +26,7 @@ struct TEvCompileRequest: public TEventLocal Uid; TMaybe Query; bool KeepInCache = false; - bool IsQueryActionPrepare = false; + NKikimrKqp::EQueryAction QueryAction; bool PerStatementResult = false; // it is allowed for local event to use absolute time (TInstant) instead of time interval (TDuration) TInstant Deadline; @@ -76,7 +76,7 @@ struct TEvCompileRequest: public TEventLocal { TEvRecompileRequest(const TIntrusiveConstPtr& userToken, const TString& clientAddress, const TString& uid, - const TMaybe& query, bool isQueryActionPrepare, TInstant deadline, + const TMaybe& query, NKikimrKqp::EQueryAction queryAction, TInstant deadline, TKqpDbCountersPtr dbCounters, const TGUCSettings::TPtr& gUCSettings, const TMaybe& applicationName, std::shared_ptr> intrestedInResult, const TIntrusivePtr& userRequestContext, NLWTrace::TOrbit orbit = {}, TKqpTempTablesState::TConstPtr tempTablesState = nullptr, TMaybe queryAst = Nothing(), @@ -85,7 +85,7 @@ struct TEvRecompileRequest: public TEventLocal Query; - bool IsQueryActionPrepare = false; + NKikimrKqp::EQueryAction QueryAction; TInstant Deadline; TKqpDbCountersPtr DbCounters; diff --git a/ydb/core/kqp/common/events/query.h b/ydb/core/kqp/common/events/query.h index 8c52c47b200b..618aa74de9f0 100644 --- a/ydb/core/kqp/common/events/query.h +++ b/ydb/core/kqp/common/events/query.h @@ -45,17 +45,11 @@ struct TQueryRequestSettings { return *this; } - TQueryRequestSettings& SetCollectFullDiagnostics(bool flag) { - CollectFullDiagnostics = flag; - return *this; - } - ui64 OutputChunkMaxSize = 0; bool KeepSession = false; bool UseCancelAfter = true; ::Ydb::Query::Syntax Syntax = Ydb::Query::Syntax::SYNTAX_UNSPECIFIED; bool SupportsStreamTrailingResult = false; - bool CollectFullDiagnostics = false; }; struct TEvQueryRequest: public NActors::TEventLocal { @@ -288,7 +282,7 @@ struct TEvQueryRequest: public NActors::TEventLocalGetCollectDiagnostics(); + return Record.MutableRequest()->GetCollectDiagnostics(); } ui32 CalculateSerializedSize() const override { diff --git a/ydb/core/kqp/common/kqp_event_impl.cpp b/ydb/core/kqp/common/kqp_event_impl.cpp index 9eac3b93a645..b6f11ca523c3 100644 --- a/ydb/core/kqp/common/kqp_event_impl.cpp +++ b/ydb/core/kqp/common/kqp_event_impl.cpp @@ -107,8 +107,6 @@ void TEvKqp::TEvQueryRequest::PrepareRemote() const { Record.MutableRequest()->SetIsInternalCall(RequestCtx->IsInternalCall()); Record.MutableRequest()->SetOutputChunkMaxSize(QuerySettings.OutputChunkMaxSize); - Record.MutableRequest()->SetCollectDiagnostics(QuerySettings.CollectFullDiagnostics); - RequestCtx.reset(); } } diff --git a/ydb/core/kqp/compile_service/kqp_compile_actor.cpp b/ydb/core/kqp/compile_service/kqp_compile_actor.cpp index d072d9e642c0..ac64fec2056b 100644 --- a/ydb/core/kqp/compile_service/kqp_compile_actor.cpp +++ b/ydb/core/kqp/compile_service/kqp_compile_actor.cpp @@ -52,7 +52,7 @@ class TKqpCompileActor : public TActorBootstrapped { TKqpDbCountersPtr dbCounters, std::optional federatedQuerySetup, const TIntrusivePtr& userRequestContext, NWilson::TTraceId traceId, TKqpTempTablesState::TConstPtr tempTablesState, bool collectFullDiagnostics, - bool perStatementResult, + bool perStatementResult, NKikimrKqp::EQueryAction queryAction, ECompileActorAction compileAction, TMaybe queryAst, NYql::TExprContext* splitCtx, NYql::TExprNode::TPtr splitExpr) @@ -75,6 +75,7 @@ class TKqpCompileActor : public TActorBootstrapped { , CompileActorSpan(TWilsonKqp::CompileActor, std::move(traceId), "CompileActor") , TempTablesState(std::move(tempTablesState)) , CollectFullDiagnostics(collectFullDiagnostics) + , QueryAction(queryAction) , CompileAction(compileAction) , QueryAst(std::move(queryAst)) , SplitCtx(splitCtx) @@ -365,7 +366,9 @@ class TKqpCompileActor : public TActorBootstrapped { replayMessage.InsertValue("query_syntax", ToString(Config->_KqpYqlSyntaxVersion.Get().GetRef())); replayMessage.InsertValue("query_database", QueryId.Database); replayMessage.InsertValue("query_cluster", QueryId.Cluster); - replayMessage.InsertValue("query_plan", queryPlan); + if (QueryAction == NKikimrKqp::QUERY_ACTION_EXPLAIN) { + replayMessage.InsertValue("query_plan", queryPlan); + } replayMessage.InsertValue("query_type", ToString(QueryId.Settings.QueryType)); if (CollectFullDiagnostics) { @@ -613,6 +616,7 @@ class TKqpCompileActor : public TActorBootstrapped { bool CollectFullDiagnostics; bool PerStatementResult; + NKikimrKqp::EQueryAction QueryAction; ECompileActorAction CompileAction; TMaybe QueryAst; @@ -665,7 +669,7 @@ IActor* CreateKqpCompileActor(const TActorId& owner, const TKqpSettings::TConstP const TString& uid, const TKqpQueryId& query, const TIntrusiveConstPtr& userToken, const TString& clientAddress, std::optional federatedQuerySetup, TKqpDbCountersPtr dbCounters, const TGUCSettings::TPtr& gUCSettings, const TMaybe& applicationName, const TIntrusivePtr& userRequestContext, - NWilson::TTraceId traceId, TKqpTempTablesState::TConstPtr tempTablesState, + NWilson::TTraceId traceId, TKqpTempTablesState::TConstPtr tempTablesState, NKikimrKqp::EQueryAction queryAction, ECompileActorAction compileAction, TMaybe queryAst, bool collectFullDiagnostics, bool perStatementResult, NYql::TExprContext* splitCtx, NYql::TExprNode::TPtr splitExpr) { @@ -674,7 +678,7 @@ IActor* CreateKqpCompileActor(const TActorId& owner, const TKqpSettings::TConstP uid, query, userToken, clientAddress, dbCounters, federatedQuerySetup, userRequestContext, std::move(traceId), std::move(tempTablesState), collectFullDiagnostics, - perStatementResult, compileAction, std::move(queryAst), + perStatementResult, queryAction, compileAction, std::move(queryAst), splitCtx, splitExpr); } diff --git a/ydb/core/kqp/compile_service/kqp_compile_service.cpp b/ydb/core/kqp/compile_service/kqp_compile_service.cpp index 01d32434aa7a..c3f1a4e5809e 100644 --- a/ydb/core/kqp/compile_service/kqp_compile_service.cpp +++ b/ydb/core/kqp/compile_service/kqp_compile_service.cpp @@ -29,17 +29,17 @@ using namespace NYql; struct TKqpCompileSettings { - TKqpCompileSettings(bool keepInCache, bool isQueryActionPrepare, bool perStatementResult, + TKqpCompileSettings(bool keepInCache, NKikimrKqp::EQueryAction queryAction, bool perStatementResult, const TInstant& deadline, ECompileActorAction action = ECompileActorAction::COMPILE) : KeepInCache(keepInCache) - , IsQueryActionPrepare(isQueryActionPrepare) + , QueryAction(queryAction) , PerStatementResult(perStatementResult) , Deadline(deadline) , Action(action) {} bool KeepInCache; - bool IsQueryActionPrepare; + NKikimrKqp::EQueryAction QueryAction; bool PerStatementResult; TInstant Deadline; ECompileActorAction Action; @@ -464,7 +464,7 @@ class TKqpCompileService : public TActorBootstrapped { TKqpCompileSettings compileSettings( request.KeepInCache, - request.IsQueryActionPrepare, + request.QueryAction, request.PerStatementResult, request.Deadline, ev->Get()->Split @@ -532,7 +532,7 @@ class TKqpCompileService : public TActorBootstrapped { TKqpCompileSettings compileSettings( true, - request.IsQueryActionPrepare, + request.QueryAction, false, request.Deadline, ev->Get()->Split @@ -626,7 +626,7 @@ class TKqpCompileService : public TActorBootstrapped { try { if (compileResult->Status == Ydb::StatusIds::SUCCESS) { if (!hasTempTablesNameClashes) { - UpdateQueryCache(ctx, compileResult, keepInCache, compileRequest.CompileSettings.IsQueryActionPrepare, isPerStatementExecution); + UpdateQueryCache(ctx, compileResult, keepInCache, compileRequest.CompileSettings.QueryAction == NKikimrKqp::QUERY_ACTION_PREPARE, isPerStatementExecution); } if (ev->Get()->ReplayMessage && !QueryReplayBackend->IsNull()) { @@ -849,7 +849,7 @@ class TKqpCompileService : public TActorBootstrapped { void StartCompilation(TKqpCompileRequest&& request, const TActorContext& ctx) { auto compileActor = CreateKqpCompileActor(ctx.SelfID, KqpSettings, TableServiceConfig, QueryServiceConfig, ModuleResolverState, Counters, request.Uid, request.Query, request.UserToken, request.ClientAddress, FederatedQuerySetup, request.DbCounters, request.GUCSettings, request.ApplicationName, request.UserRequestContext, - request.CompileServiceSpan.GetTraceId(), request.TempTablesState, request.CompileSettings.Action, std::move(request.QueryAst), CollectDiagnostics, + request.CompileServiceSpan.GetTraceId(), request.TempTablesState, request.CompileSettings.QueryAction, request.CompileSettings.Action, std::move(request.QueryAst), CollectDiagnostics, request.CompileSettings.PerStatementResult, request.SplitCtx, request.SplitExpr); auto compileActorId = ctx.ExecutorThread.RegisterActor(compileActor, TMailboxType::HTSwap, AppData(ctx)->UserPoolId); diff --git a/ydb/core/kqp/compile_service/kqp_compile_service.h b/ydb/core/kqp/compile_service/kqp_compile_service.h index 853ba5a35910..9031019ff4a6 100644 --- a/ydb/core/kqp/compile_service/kqp_compile_service.h +++ b/ydb/core/kqp/compile_service/kqp_compile_service.h @@ -166,6 +166,7 @@ IActor* CreateKqpCompileActor(const TActorId& owner, const TKqpSettings::TConstP TKqpDbCountersPtr dbCounters, const TGUCSettings::TPtr& gUCSettings, const TMaybe& applicationName, const TIntrusivePtr& userRequestContext, NWilson::TTraceId traceId = {}, TKqpTempTablesState::TConstPtr tempTablesState = nullptr, + NKikimrKqp::EQueryAction queryAction = NKikimrKqp::QUERY_ACTION_EXECUTE, ECompileActorAction compileAction = ECompileActorAction::COMPILE, TMaybe queryAst = {}, bool collectFullDiagnostics = false, diff --git a/ydb/core/kqp/session_actor/kqp_query_state.cpp b/ydb/core/kqp/session_actor/kqp_query_state.cpp index 4c02f46b151a..1f05419948bd 100644 --- a/ydb/core/kqp/session_actor/kqp_query_state.cpp +++ b/ydb/core/kqp/session_actor/kqp_query_state.cpp @@ -300,8 +300,6 @@ std::unique_ptr TKqpQueryState::BuildCompileRequest(s compileDeadline = Min(compileDeadline, QueryDeadlines.CancelAt); } - bool isQueryActionPrepare = GetAction() == NKikimrKqp::QUERY_ACTION_PREPARE; - TMaybe statementAst; if (!Statements.empty()) { YQL_ENSURE(CurrentStatementId < Statements.size()); @@ -309,7 +307,7 @@ std::unique_ptr TKqpQueryState::BuildCompileRequest(s } return std::make_unique(UserToken, ClientAddress, uid, std::move(query), keepInCache, - isQueryActionPrepare, perStatementResult, compileDeadline, DbCounters, gUCSettingsPtr, ApplicationName, std::move(cookie), + GetAction(), perStatementResult, compileDeadline, DbCounters, gUCSettingsPtr, ApplicationName, std::move(cookie), UserRequestContext, std::move(Orbit), TempTablesState, GetCollectDiagnostics(), statementAst); } @@ -344,12 +342,11 @@ std::unique_ptr TKqpQueryState::BuildReCompileReque } auto compileDeadline = QueryDeadlines.TimeoutAt; - bool isQueryActionPrepare = GetAction() == NKikimrKqp::QUERY_ACTION_PREPARE; if (QueryDeadlines.CancelAt) { compileDeadline = Min(compileDeadline, QueryDeadlines.CancelAt); } - return std::make_unique(UserToken, ClientAddress, CompileResult->Uid, query, isQueryActionPrepare, + return std::make_unique(UserToken, ClientAddress, CompileResult->Uid, query, GetAction(), compileDeadline, DbCounters, gUCSettingsPtr, ApplicationName, std::move(cookie), UserRequestContext, std::move(Orbit), TempTablesState, CompileResult->QueryAst); } @@ -392,7 +389,7 @@ std::unique_ptr TKqpQueryState::BuildCompileSplittedR } return std::make_unique(UserToken, ClientAddress, uid, std::move(query), false, - false, perStatementResult, compileDeadline, DbCounters, gUCSettingsPtr, ApplicationName, std::move(cookie), + NKikimrKqp::QUERY_ACTION_EXECUTE, perStatementResult, compileDeadline, DbCounters, gUCSettingsPtr, ApplicationName, std::move(cookie), UserRequestContext, std::move(Orbit), TempTablesState, GetCollectDiagnostics(), statementAst, false, SplittedCtx.get(), SplittedExprs.at(NextSplittedExpr)); } diff --git a/ydb/core/kqp/ut/olap/kqp_olap_ut.cpp b/ydb/core/kqp/ut/olap/kqp_olap_ut.cpp index 1e70d62d8b18..1df7fab8e8d9 100644 --- a/ydb/core/kqp/ut/olap/kqp_olap_ut.cpp +++ b/ydb/core/kqp/ut/olap/kqp_olap_ut.cpp @@ -308,7 +308,7 @@ Y_UNIT_TEST_SUITE(KqpOlap) { { TStreamExecScanQuerySettings settings; - settings.CollectQueryStats(ECollectQueryStatsMode::Full); + settings.CollectQueryStats(ECollectQueryStatsMode::Basic); auto it = client.StreamExecuteScanQuery(R"( --!syntax_v1 SELECT `resource_id`, `timestamp` @@ -325,7 +325,7 @@ Y_UNIT_TEST_SUITE(KqpOlap) { { TStreamExecScanQuerySettings settings; settings.CollectQueryStats(ECollectQueryStatsMode::Full); - settings.CollectFullDiagnostics(true); + auto it = client.StreamExecuteScanQuery(R"( --!syntax_v1 SELECT `resource_id`, `timestamp` diff --git a/ydb/core/kqp/ut/query/kqp_query_ut.cpp b/ydb/core/kqp/ut/query/kqp_query_ut.cpp index d79c598bdf52..04e3f3fab959 100644 --- a/ydb/core/kqp/ut/query/kqp_query_ut.cpp +++ b/ydb/core/kqp/ut/query/kqp_query_ut.cpp @@ -205,7 +205,7 @@ Y_UNIT_TEST_SUITE(KqpQuery) { { auto settings = TExecDataQuerySettings(); - settings.CollectFullDiagnostics(true); + settings.CollectQueryStats(ECollectQueryStatsMode::Full); auto result = session.ExecuteDataQuery(query, TTxControl::BeginTx().CommitTx(), settings).ExtractValueSync(); diff --git a/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp b/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp index 43db2da0da60..233e33ff75ac 100644 --- a/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp +++ b/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp @@ -278,7 +278,7 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { { TExecuteQuerySettings settings; - settings.CollectFullDiagnostics(true); + settings.StatsMode(EStatsMode::Full); auto result = db.ExecuteQuery(R"( SELECT Key, Value2 FROM TwoShard WHERE Value2 > 0; @@ -309,7 +309,7 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { { TExecuteQuerySettings settings; - settings.CollectFullDiagnostics(true); + settings.StatsMode(EStatsMode::Basic); auto result = db.ExecuteQuery(R"( SELECT Key, Value2 FROM TwoShard WHERE Value2 > 0; diff --git a/ydb/public/api/protos/ydb_query.proto b/ydb/public/api/protos/ydb_query.proto index 1473817f872e..b1fabe26d5d7 100644 --- a/ydb/public/api/protos/ydb_query.proto +++ b/ydb/public/api/protos/ydb_query.proto @@ -172,8 +172,6 @@ message ExecuteQueryRequest { int64 response_part_limit_bytes = 9 [(Ydb.value) = "[0; 33554432]"]; string pool_id = 10; // Workload manager pool id - - bool collect_full_diagnostics = 11; } message ResultSetMeta { @@ -193,9 +191,6 @@ message ExecuteQueryResponsePart { Ydb.TableStats.QueryStats exec_stats = 5; TransactionMeta tx_meta = 6; - - // Full query diagnostics - string query_full_diagnostics = 7; } message ExecuteScriptRequest { diff --git a/ydb/public/api/protos/ydb_query_stats.proto b/ydb/public/api/protos/ydb_query_stats.proto index 300d5d9837c0..d1827b71a378 100644 --- a/ydb/public/api/protos/ydb_query_stats.proto +++ b/ydb/public/api/protos/ydb_query_stats.proto @@ -43,4 +43,5 @@ message QueryStats { string query_ast = 5; uint64 total_duration_us = 6; uint64 total_cpu_time_us = 7; + string query_diagnostics = 8; } diff --git a/ydb/public/api/protos/ydb_table.proto b/ydb/public/api/protos/ydb_table.proto index a83a44f8f4b1..0c2939e48cc8 100644 --- a/ydb/public/api/protos/ydb_table.proto +++ b/ydb/public/api/protos/ydb_table.proto @@ -942,7 +942,6 @@ message ExecuteDataQueryRequest { QueryCachePolicy query_cache_policy = 5; Ydb.Operations.OperationParams operation_params = 6; QueryStatsCollection.Mode collect_stats = 7; - bool collect_full_diagnostics = 8; } message ExecuteDataQueryResponse { @@ -985,8 +984,6 @@ message ExecuteQueryResult { QueryMeta query_meta = 3; // Query execution statistics Ydb.TableStats.QueryStats query_stats = 4; - // Full query diagnostics - string query_full_diagnostics = 5; } // Explain data query @@ -1291,9 +1288,7 @@ message ExecuteScanQueryRequest { Mode mode = 6; reserved 7; // report_progress QueryStatsCollection.Mode collect_stats = 8; - // works only in mode: MODE_EXPLAIN, - // collects additional diagnostics about query compilation, including query plan and scheme - bool collect_full_diagnostics = 9; + reserved 9; // collect_full_diagnostics } message ExecuteScanQueryPartialResponse { @@ -1309,7 +1304,5 @@ message ExecuteScanQueryPartialResult { reserved 4; // query_progress reserved 5; // query_plan Ydb.TableStats.QueryStats query_stats = 6; - // works only in mode: MODE_EXPLAIN, - // collects additional diagnostics about query compilation, including query plan and scheme - string query_full_diagnostics = 7; + reserved 7; // query_full_diagnostics } diff --git a/ydb/public/sdk/cpp/client/ydb_query/client.cpp b/ydb/public/sdk/cpp/client/ydb_query/client.cpp index 24b111371f07..331f17ec429c 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/client.cpp +++ b/ydb/public/sdk/cpp/client/ydb_query/client.cpp @@ -723,9 +723,4 @@ TResultSetParser TExecuteQueryResult::GetResultSetParser(size_t resultIndex) con return TResultSetParser(GetResultSet(resultIndex)); } -const TString& TExecuteQueryResult::GetDiagnostics() const { - CheckStatusOk("TExecuteQueryResult::GetDiagnostics"); - return Diagnostics_; -} - } // namespace NYdb::NQuery diff --git a/ydb/public/sdk/cpp/client/ydb_query/client.h b/ydb/public/sdk/cpp/client/ydb_query/client.h index 5c31967b0ff7..a459c4b3981a 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/client.h +++ b/ydb/public/sdk/cpp/client/ydb_query/client.h @@ -213,26 +213,23 @@ class TExecuteQueryPart : public TStreamPartStatus { ui64 GetResultSetIndex() const { return ResultSetIndex_; } const TResultSet& GetResultSet() const { return *ResultSet_; } TResultSet ExtractResultSet() { return std::move(*ResultSet_); } - const TString& GetDiagnostics() const { return Diagnostics_; } const TMaybe& GetStats() const { return Stats_; } const TMaybe& GetTransaction() const { return Transaction_; } - TExecuteQueryPart(TStatus&& status, TMaybe&& queryStats, TMaybe&& tx, TString&& diagnostics) + TExecuteQueryPart(TStatus&& status, TMaybe&& queryStats, TMaybe&& tx) : TStreamPartStatus(std::move(status)) , Stats_(std::move(queryStats)) , Transaction_(std::move(tx)) - , Diagnostics_(std::move(diagnostics)) {} TExecuteQueryPart(TStatus&& status, TResultSet&& resultSet, i64 resultSetIndex, - TMaybe&& queryStats, TMaybe&& tx, TString&& diagnostics) + TMaybe&& queryStats, TMaybe&& tx) : TStreamPartStatus(std::move(status)) , ResultSet_(std::move(resultSet)) , ResultSetIndex_(resultSetIndex) , Stats_(std::move(queryStats)) , Transaction_(std::move(tx)) - , Diagnostics_(std::move(diagnostics)) {} private: @@ -240,7 +237,6 @@ class TExecuteQueryPart : public TStreamPartStatus { i64 ResultSetIndex_ = 0; TMaybe Stats_; TMaybe Transaction_; - TString Diagnostics_; }; class TExecuteQueryResult : public TStatus { @@ -253,26 +249,22 @@ class TExecuteQueryResult : public TStatus { TMaybe GetTransaction() const {return Transaction_; } - const TString& GetDiagnostics() const; - TExecuteQueryResult(TStatus&& status) : TStatus(std::move(status)) {} TExecuteQueryResult(TStatus&& status, TVector&& resultSets, - TMaybe&& stats, TMaybe&& tx, TString&& diagnostics) + TMaybe&& stats, TMaybe&& tx) : TStatus(std::move(status)) , ResultSets_(std::move(resultSets)) , Stats_(std::move(stats)) , Transaction_(std::move(tx)) - , Diagnostics_(std::move(diagnostics)) {} private: TVector ResultSets_; TMaybe Stats_; TMaybe Transaction_; - TString Diagnostics_; }; } // namespace NYdb::NQuery diff --git a/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp b/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp index dcdd894fa5f3..3a2e597f3f6b 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp +++ b/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp @@ -67,14 +67,13 @@ class TExecuteQueryIterator::TReaderImpl { auto readCb = [self, promise](TGRpcStatus&& grpcStatus) mutable { if (!grpcStatus.Ok()) { self->Finished_ = true; - promise.SetValue({TStatus(TPlainStatus(grpcStatus, self->Endpoint_)), {}, {}, ""}); + promise.SetValue({TStatus(TPlainStatus(grpcStatus, self->Endpoint_)), {}, {}}); } else { NYql::TIssues issues; NYql::IssuesFromMessage(self->Response_.issues(), issues); EStatus clientStatus = static_cast(self->Response_.status()); TPlainStatus plainStatus{clientStatus, std::move(issues), self->Endpoint_, {}}; TStatus status{std::move(plainStatus)}; - TString diagnostics; TMaybe stats; TMaybe tx; @@ -86,19 +85,16 @@ class TExecuteQueryIterator::TReaderImpl { tx = TTransaction(self->Session_.GetRef(), self->Response_.tx_meta().id()); } - diagnostics = self->Response_.query_full_diagnostics(); - if (self->Response_.has_result_set()) { promise.SetValue({ std::move(status), TResultSet(std::move(*self->Response_.mutable_result_set())), self->Response_.result_set_index(), std::move(stats), - std::move(tx), - std::move(diagnostics) + std::move(tx) }); } else { - promise.SetValue({std::move(status), std::move(stats), std::move(tx), std::move(diagnostics)}); + promise.SetValue({std::move(status), std::move(stats), std::move(tx)}); } } }; @@ -169,12 +165,10 @@ struct TExecuteQueryBuffer : public TThrRefBase, TNonCopyable { TVector issues; TVector resultProtos; TMaybe tx; - TString diagnostics; std::swap(self->Issues_, issues); std::swap(self->ResultSets_, resultProtos); std::swap(self->Tx_, tx); - std::swap(self->Diagnostics_, diagnostics); TVector resultSets; for (auto& proto : resultProtos) { @@ -185,11 +179,10 @@ struct TExecuteQueryBuffer : public TThrRefBase, TNonCopyable { TStatus(EStatus::SUCCESS, NYql::TIssues(std::move(issues))), std::move(resultSets), std::move(stats), - std::move(tx), - std::move(diagnostics) + std::move(tx) )); } else { - self->Promise_.SetValue(TExecuteQueryResult(std::move(part), {}, std::move(stats), {}, {})); + self->Promise_.SetValue(TExecuteQueryResult(std::move(part), {}, std::move(stats), {})); } return; @@ -218,8 +211,6 @@ struct TExecuteQueryBuffer : public TThrRefBase, TNonCopyable { self->Tx_ = tx; } - self->Diagnostics_ = part.GetDiagnostics(); - self->Next(); }); } @@ -250,8 +241,6 @@ TFuture> StreamExecuteQueryIm request.set_response_part_limit_bytes(*settings.OutputChunkMaxSize_); } - request.set_collect_full_diagnostics(settings.CollectFullDiagnostics_); - if (txControl.HasTx()) { auto requestTxControl = request.mutable_tx_control(); requestTxControl->set_commit_tx(txControl.CommitTx_); diff --git a/ydb/public/sdk/cpp/client/ydb_query/query.h b/ydb/public/sdk/cpp/client/ydb_query/query.h index e35da4695ac8..3ea49527ae77 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/query.h +++ b/ydb/public/sdk/cpp/client/ydb_query/query.h @@ -74,7 +74,6 @@ struct TExecuteQuerySettings : public TRequestSettings { FLUENT_SETTING_DEFAULT(EStatsMode, StatsMode, EStatsMode::None); FLUENT_SETTING_OPTIONAL(bool, ConcurrentResultSets); FLUENT_SETTING(TString, ResourcePool); - FLUENT_SETTING_DEFAULT(bool, CollectFullDiagnostics, false); }; struct TBeginTxSettings : public TRequestSettings {}; diff --git a/ydb/public/sdk/cpp/client/ydb_query/stats.cpp b/ydb/public/sdk/cpp/client/ydb_query/stats.cpp index c007547d4e84..76f895c6b9a8 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/stats.cpp +++ b/ydb/public/sdk/cpp/client/ydb_query/stats.cpp @@ -56,6 +56,16 @@ TMaybe TExecStats::GetAst() const { return proto.query_ast(); } +TMaybe TExecStats::GetDiagnostics() const { + auto proto = Impl_->Proto; + + if (proto.query_diagnostics().empty()) { + return {}; + } + + return proto.query_diagnostics(); +} + TDuration TExecStats::GetTotalDuration() const { return TDuration::MicroSeconds(Impl_->Proto.total_duration_us()); } diff --git a/ydb/public/sdk/cpp/client/ydb_query/stats.h b/ydb/public/sdk/cpp/client/ydb_query/stats.h index 15c8a15a5134..6115214f1861 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/stats.h +++ b/ydb/public/sdk/cpp/client/ydb_query/stats.h @@ -31,6 +31,7 @@ class TExecStats { TMaybe GetPlan() const; TMaybe GetAst() const; + TMaybe GetDiagnostics() const; TDuration GetTotalDuration() const; TDuration GetTotalCpuTime() const; diff --git a/ydb/public/sdk/cpp/client/ydb_table/impl/readers.cpp b/ydb/public/sdk/cpp/client/ydb_table/impl/readers.cpp index 11a8d4d0b176..721337dbde4b 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/impl/readers.cpp +++ b/ydb/public/sdk/cpp/client/ydb_table/impl/readers.cpp @@ -82,19 +82,16 @@ TAsyncScanQueryPart TScanQueryPartIterator::TReaderImpl::ReadNext(std::shared_pt TPlainStatus plainStatus{clientStatus, std::move(issues), self->Endpoint_, {}}; TStatus status{std::move(plainStatus)}; TMaybe queryStats; - TMaybe diagnostics; if (self->Response_.result().has_query_stats()) { queryStats = TQueryStats(self->Response_.result().query_stats()); } - diagnostics = self->Response_.result().query_full_diagnostics(); - if (self->Response_.result().has_result_set()) { promise.SetValue({std::move(status), - TResultSet(std::move(*self->Response_.mutable_result()->mutable_result_set())), queryStats, diagnostics}); + TResultSet(std::move(*self->Response_.mutable_result()->mutable_result_set())), queryStats}); } else { - promise.SetValue({std::move(status), queryStats, diagnostics}); + promise.SetValue({std::move(status), queryStats}); } } }; diff --git a/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.cpp b/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.cpp index 38a84818661d..0449591a27e8 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.cpp +++ b/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.cpp @@ -1017,7 +1017,6 @@ TFuture> TT } request.set_collect_stats(GetStatsCollectionMode(settings.CollectQueryStats_)); - request.set_collect_full_diagnostics(settings.CollectFullDiagnostics_); auto promise = NewPromise>(); diff --git a/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.h b/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.h index 42baeeff1d93..da681f0d959a 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.h +++ b/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.h @@ -188,8 +188,7 @@ class TTableClient::TImpl: public TClientImplCommon, public txControl.Tx_, Nothing(), false, - Nothing(), - {})); + Nothing())); } return ExecuteDataQueryInternal(session, query, txControl, params, settings, fromCache); @@ -214,7 +213,6 @@ class TTableClient::TImpl: public TClientImplCommon, public } request.set_collect_stats(GetStatsCollectionMode(settings.CollectQueryStats_)); - request.set_collect_full_diagnostics(settings.CollectFullDiagnostics_); SetQuery(query, request.mutable_query()); CollectQuerySize(query, QuerySizeHistogram); @@ -242,7 +240,6 @@ class TTableClient::TImpl: public TClientImplCommon, public TMaybe tx; TMaybe dataQuery; TMaybe queryStats; - TString diagnostics; auto queryText = GetQueryText(query); if (any) { @@ -267,8 +264,6 @@ class TTableClient::TImpl: public TClientImplCommon, public if (result.has_query_stats()) { queryStats = TQueryStats(result.query_stats()); } - - diagnostics = result.query_full_diagnostics(); } if (keepInCache && dataQuery && queryText) { @@ -276,7 +271,7 @@ class TTableClient::TImpl: public TClientImplCommon, public } TDataQueryResult dataQueryResult(TStatus(std::move(status)), - std::move(res), tx, dataQuery, fromCache, queryStats, std::move(diagnostics)); + std::move(res), tx, dataQuery, fromCache, queryStats); delete sessionPtr; tx.Clear(); diff --git a/ydb/public/sdk/cpp/client/ydb_table/query_stats/stats.h b/ydb/public/sdk/cpp/client/ydb_table/query_stats/stats.h index 29a7c87fa1f8..fab414153512 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/query_stats/stats.h +++ b/ydb/public/sdk/cpp/client/ydb_table/query_stats/stats.h @@ -35,7 +35,7 @@ namespace NTable { enum class ECollectQueryStatsMode { None = 0, // Stats collection is disabled Basic = 1, // Aggregated stats of reads, updates and deletes per table - Full = 2, // Add per-stage execution profile and query plan on top of Basic mode + Full = 2, // Add per-stage execution profile and diagnostics on top of Basic mode Profile = 3 // Detailed execution stats including stats for individual tasks and channels }; diff --git a/ydb/public/sdk/cpp/client/ydb_table/table.cpp b/ydb/public/sdk/cpp/client/ydb_table/table.cpp index b9845ea22a5e..a5217478be59 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/table.cpp +++ b/ydb/public/sdk/cpp/client/ydb_table/table.cpp @@ -2156,15 +2156,13 @@ TTableDescription TDescribeTableResult::GetTableDescription() const { //////////////////////////////////////////////////////////////////////////////// TDataQueryResult::TDataQueryResult(TStatus&& status, TVector&& resultSets, - const TMaybe& transaction, const TMaybe& dataQuery, bool fromCache, const TMaybe &queryStats, - TString&& diagnostics) + const TMaybe& transaction, const TMaybe& dataQuery, bool fromCache, const TMaybe &queryStats) : TStatus(std::move(status)) , Transaction_(transaction) , ResultSets_(std::move(resultSets)) , DataQuery_(dataQuery) , FromCache_(fromCache) , QueryStats_(queryStats) - , Diagnostics_(std::move(diagnostics)) {} const TVector& TDataQueryResult::GetResultSets() const { @@ -2211,9 +2209,12 @@ const TString TDataQueryResult::GetQueryPlan() const { } } -const TString& TDataQueryResult::GetDiagnostics() const { - CheckStatusOk("TDataQueryResult::GetDiagnostics"); - return Diagnostics_; +const TString TDataQueryResult::GetDiagnostics() const { + if (QueryStats_.Defined()) { + return NYdb::TProtoAccessor::GetProto(*QueryStats_.Get()).query_diagnostics(); + } else { + return ""; + } } //////////////////////////////////////////////////////////////////////////////// diff --git a/ydb/public/sdk/cpp/client/ydb_table/table.h b/ydb/public/sdk/cpp/client/ydb_table/table.h index 8e3cd604ab09..72bda2f9a22d 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/table.h +++ b/ydb/public/sdk/cpp/client/ydb_table/table.h @@ -1164,9 +1164,6 @@ struct TStreamExecScanQuerySettings : public TRequestSettings {}; @@ -2017,7 +2012,7 @@ class TDescribeTableResult : public NScheme::TDescribePathResult { class TDataQueryResult : public TStatus { public: TDataQueryResult(TStatus&& status, TVector&& resultSets, const TMaybe& transaction, - const TMaybe& dataQuery, bool fromCache, const TMaybe& queryStats, TString&& diagnostics); + const TMaybe& dataQuery, bool fromCache, const TMaybe& queryStats); const TVector& GetResultSets() const; TVector ExtractResultSets() &&; @@ -2034,7 +2029,7 @@ class TDataQueryResult : public TStatus { const TString GetQueryPlan() const; - const TString& GetDiagnostics() const; + const TString GetDiagnostics() const; private: TMaybe Transaction_; @@ -2042,7 +2037,6 @@ class TDataQueryResult : public TStatus { TMaybe DataQuery_; bool FromCache_; TMaybe QueryStats_; - TString Diagnostics_; }; class TReadTableSnapshot { @@ -2109,31 +2103,24 @@ class TScanQueryPart : public TStreamPartStatus { const TQueryStats& GetQueryStats() const { return *QueryStats_; } TQueryStats ExtractQueryStats() { return std::move(*QueryStats_); } - bool HasDiagnostics() const { return Diagnostics_.Defined(); } - const TString& GetDiagnostics() const { return *Diagnostics_; } - TString&& ExtractDiagnostics() { return std::move(*Diagnostics_); } - TScanQueryPart(TStatus&& status) : TStreamPartStatus(std::move(status)) {} - TScanQueryPart(TStatus&& status, const TMaybe& queryStats, const TMaybe& diagnostics) + TScanQueryPart(TStatus&& status, const TMaybe& queryStats) : TStreamPartStatus(std::move(status)) , QueryStats_(queryStats) - , Diagnostics_(diagnostics) {} - TScanQueryPart(TStatus&& status, TResultSet&& resultSet, const TMaybe& queryStats, const TMaybe& diagnostics) + TScanQueryPart(TStatus&& status, TResultSet&& resultSet, const TMaybe& queryStats) : TStreamPartStatus(std::move(status)) , ResultSet_(std::move(resultSet)) , QueryStats_(queryStats) - , Diagnostics_(diagnostics) {} private: TMaybe ResultSet_; TMaybe QueryStats_; - TMaybe Diagnostics_; }; using TAsyncScanQueryPart = NThreading::TFuture; From c706d98cdfe06ec5953045ec3b9c2159273e6f6f Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Fri, 27 Dec 2024 13:35:04 +0300 Subject: [PATCH 10/10] Fixes --- ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp | 88 ++++++++++++++++++- .../ydb_cli/commands/ydb_service_table.cpp | 39 +------- .../lib/ydb_cli/commands/ydb_service_table.h | 1 - ydb/public/lib/ydb_cli/commands/ydb_sql.cpp | 45 +++++++--- ydb/public/lib/ydb_cli/commands/ydb_sql.h | 2 +- ydb/public/sdk/cpp/client/ydb_query/stats.cpp | 1 + 6 files changed, 120 insertions(+), 56 deletions(-) diff --git a/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp b/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp index 233e33ff75ac..d5a8736efe0f 100644 --- a/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp +++ b/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp @@ -285,10 +285,11 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { )", TTxControl::BeginTx().CommitTx(), settings).ExtractValueSync(); UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); - UNIT_ASSERT_C(!result.GetDiagnostics().empty(), "Query result diagnostics is empty"); + auto& stats = NYdb::TProtoAccessor::GetProto(*result.GetStats()); + UNIT_ASSERT_C(!stats.query_diagnostics().empty(), "Query result diagnostics is empty"); TStringStream in; - in << result.GetDiagnostics(); + in << stats.query_diagnostics(); NJson::TJsonValue value; ReadJsonTree(&in, &value); @@ -303,7 +304,7 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { UNIT_ASSERT_C(value.Has("query_syntax"), "Incorrect Diagnostics"); UNIT_ASSERT_C(value.Has("query_database"), "Incorrect Diagnostics"); UNIT_ASSERT_C(value.Has("query_cluster"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_plan"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(!value.Has("query_plan"), "Incorrect Diagnostics"); UNIT_ASSERT_C(value.Has("query_type"), "Incorrect Diagnostics"); } @@ -317,7 +318,86 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString().c_str()); - UNIT_ASSERT_C(result.GetDiagnostics().empty(), "Query result diagnostics should be empty, but it's not"); + auto& stats = NYdb::TProtoAccessor::GetProto(*result.GetStats()); + UNIT_ASSERT_C(stats.query_diagnostics().empty(), "Query result diagnostics should be empty, but it's not"); + } + } + + Y_UNIT_TEST(StreamExecuteCollectFullDiagnostics) { + auto kikimr = DefaultKikimrRunner(); + auto db = kikimr.GetQueryClient(); + + { + TExecuteQuerySettings settings; + settings.StatsMode(EStatsMode::Full); + + auto it = db.StreamExecuteQuery(R"( + SELECT Key, Value2 FROM TwoShard WHERE Value2 > 0; + )", TTxControl::BeginTx().CommitTx(), settings).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(it.GetStatus(), EStatus::SUCCESS, it.GetIssues().ToString()); + + TString statsString; + for (;;) { + auto streamPart = it.ReadNext().GetValueSync(); + if (!streamPart.IsSuccess()) { + UNIT_ASSERT_C(streamPart.EOS(), streamPart.GetIssues().ToString()); + break; + } + + const auto& execStats = streamPart.GetStats(); + if (execStats.Defined()) { + auto& stats = NYdb::TProtoAccessor::GetProto(*execStats); + statsString = stats.query_diagnostics(); + } + } + + UNIT_ASSERT_C(!statsString.empty(), "Query result diagnostics is empty"); + + TStringStream in; + in << statsString; + NJson::TJsonValue value; + ReadJsonTree(&in, &value); + + UNIT_ASSERT_C(value.IsMap(), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_id"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("version"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_text"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_parameter_types"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("table_metadata"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value["table_metadata"].IsArray(), "Incorrect Diagnostics: table_metadata type should be an array"); + UNIT_ASSERT_C(value.Has("created_at"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_syntax"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_database"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_cluster"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(!value.Has("query_plan"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_type"), "Incorrect Diagnostics"); + } + + { + TExecuteQuerySettings settings; + settings.StatsMode(EStatsMode::Basic); + + auto it = db.StreamExecuteQuery(R"( + SELECT Key, Value2 FROM TwoShard WHERE Value2 > 0; + )", TTxControl::BeginTx().CommitTx(), settings).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(it.GetStatus(), EStatus::SUCCESS, it.GetIssues().ToString()); + + TString statsString; + for (;;) { + auto streamPart = it.ReadNext().GetValueSync(); + if (!streamPart.IsSuccess()) { + UNIT_ASSERT_C(streamPart.EOS(), streamPart.GetIssues().ToString()); + break; + } + + const auto& execStats = streamPart.GetStats(); + if (execStats.Defined()) { + auto& stats = NYdb::TProtoAccessor::GetProto(*execStats); + statsString = stats.query_diagnostics(); + } + } + + UNIT_ASSERT_C(statsString.empty(), "Query result diagnostics should be empty, but it's not"); } } diff --git a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp index 87aabc1e0401..53cf75945f18 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp @@ -365,8 +365,6 @@ void TCommandExecuteQuery::Config(TConfig& config) { config.Opts->AddLongOption('q', "query", "Text of query to execute").RequiredArgument("[String]").StoreResult(&Query); config.Opts->AddLongOption('f', "file", "Path to file with query text to execute") .RequiredArgument("PATH").StoreResult(&QueryFile); - config.Opts->AddLongOption("collect-diagnostics", "Collects diagnostics and saves it to file") - .StoreTrue(&CollectFullDiagnostics); AddOutputFormats(config, { EDataFormat::Pretty, @@ -434,9 +432,6 @@ int TCommandExecuteQuery::ExecuteDataQuery(TConfig& config) { NTable::TExecDataQuerySettings settings; settings.KeepInQueryCache(true); settings.CollectQueryStats(ParseQueryStatsModeOrThrow(CollectStatsMode, defaultStatsMode)); - if (CollectFullDiagnostics) { - settings.CollectFullDiagnostics(true); - } NTable::TTxSettings txSettings; if (TxMode) { @@ -521,11 +516,6 @@ void TCommandExecuteQuery::PrintDataQueryResponse(NTable::TDataQueryResult& resu { Cout << Endl << "Flame graph is available for full or profile stats only" << Endl; } - if (CollectFullDiagnostics) - { - TFileOutput file(TStringBuilder() << "diagnostics_" << TGUID::Create().AsGuidString() << ".txt"); - file << result.GetDiagnostics(); - } } int TCommandExecuteQuery::ExecuteSchemeQuery(TConfig& config) { @@ -558,7 +548,7 @@ namespace { NQuery::TExecuteQuerySettings>; template - auto GetSettings(const TString& collectStatsMode, const bool basicStats, std::optional timeout, bool collectFullDiagnostics) { + auto GetSettings(const TString& collectStatsMode, const bool basicStats, std::optional timeout) { if constexpr (std::is_same_v) { const auto defaultStatsMode = basicStats ? NTable::ECollectQueryStatsMode::Basic @@ -568,9 +558,6 @@ namespace { if (timeout.has_value()) { settings.ClientTimeout(*timeout); } - if (collectFullDiagnostics) { - settings.CollectFullDiagnostics(true); - } return settings; } else if constexpr (std::is_same_v) { const auto defaultStatsMode = basicStats @@ -581,9 +568,6 @@ namespace { if (timeout.has_value()) { settings.ClientTimeout(*timeout); } - if (collectFullDiagnostics) { - settings.CollectFullDiagnostics(true); - } return settings; } Y_UNREACHABLE(); @@ -690,7 +674,7 @@ int TCommandExecuteQuery::ExecuteQueryImpl(TConfig& config) { if (OperationTimeout) { optTimeout = TDuration::MilliSeconds(FromString(OperationTimeout)); } - const auto settings = GetSettings(CollectStatsMode, BasicStats, optTimeout, CollectFullDiagnostics); + const auto settings = GetSettings(CollectStatsMode, BasicStats, optTimeout); TAsyncPartIterator asyncResult; SetInterruptHandlers(); @@ -770,8 +754,6 @@ bool TCommandExecuteQuery::PrintQueryResponse(TIterator& result) { fullStats = queryStats.GetPlan(); } } - - diagnostics = streamPart.GetDiagnostics(); } } // TResultSetPrinter destructor should be called before printing stats @@ -786,12 +768,6 @@ bool TCommandExecuteQuery::PrintQueryResponse(TIterator& result) { queryPlanPrinter.Print(*fullStats); } - if (CollectFullDiagnostics) - { - TFileOutput file(TStringBuilder() << "diagnostics_" << TGUID::Create().AsGuidString() << ".txt"); - file << diagnostics; - } - PrintFlameGraph(fullStats); if (IsInterrupted()) { @@ -897,10 +873,6 @@ int TCommandExplain::Run(TConfig& config) { settings.Explain(true); } - if (CollectFullDiagnostics) { - settings.CollectFullDiagnostics(true); - } - auto result = client.StreamExecuteScanQuery(Query, settings).GetValueSync(); ThrowOnError(result); @@ -917,13 +889,6 @@ int TCommandExplain::Run(TConfig& config) { planJson = proto.query_plan(); ast = proto.query_ast(); } - if (tablePart.HasDiagnostics()) { - diagnostics = tablePart.ExtractDiagnostics(); - } - } - - if (CollectFullDiagnostics) { - SaveDiagnosticsToFile(diagnostics); } if (IsInterrupted()) { diff --git a/ydb/public/lib/ydb_cli/commands/ydb_service_table.h b/ydb/public/lib/ydb_cli/commands/ydb_service_table.h index 70d3fcf1f006..a0716f7dc322 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_service_table.h +++ b/ydb/public/lib/ydb_cli/commands/ydb_service_table.h @@ -123,7 +123,6 @@ class TCommandExecuteQuery : public TTableCommand, TCommandQueryBase, TCommandWi TString TxMode; TString QueryType; bool BasicStats = false; - bool CollectFullDiagnostics = false; }; class TCommandExplain : public TTableCommand, public TCommandWithOutput, TCommandQueryBase, TInterruptibleCommand { diff --git a/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp b/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp index 4c47890ca26d..704577770162 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp @@ -1,6 +1,8 @@ #include "ydb_sql.h" #include +#include +#include #include #include #include @@ -9,7 +11,6 @@ #include #include #include -#include #include #include @@ -41,11 +42,11 @@ void TCommandSql::Config(TConfig& config) { .StoreTrue(&ExplainAnalyzeMode); config.Opts->AddLongOption("stats", "Execution statistics collection mode [none, basic, full, profile]") .RequiredArgument("[String]").StoreResult(&CollectStatsMode); + config.Opts->AddLongOption("diagnostics-file", "Path to file where the diagnostics will be saved.") + .RequiredArgument("[String]").StoreResult(&DiagnosticsFile); config.Opts->AddLongOption("syntax", "Query syntax [yql, pg]") .RequiredArgument("[String]").DefaultValue("yql").StoreResult(&Syntax) .Hidden(); - config.Opts->AddLongOption("collect-diagnostics", "Collects diagnostics and saves it to file") - .StoreTrue(&CollectFullDiagnostics); AddOutputFormats(config, { EDataFormat::Pretty, @@ -149,10 +150,6 @@ int TCommandSql::RunCommand(TConfig& config) { throw TMisuseException() << "Unknow syntax option \"" << Syntax << "\""; } - if (CollectFullDiagnostics) { - settings.CollectFullDiagnostics(true); - } - if (!Parameters.empty() || InputParamStream) { // Execute query with parameters THolder paramBuilder; @@ -190,7 +187,7 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { TMaybe stats; TMaybe plan; TMaybe ast; - TString diagnostics; + TMaybe diagnostics; { TResultSetPrinter printer(OutputFormat, &IsInterrupted); @@ -212,9 +209,8 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { if (queryStats.GetPlan()) { plan = queryStats.GetPlan(); } + diagnostics = queryStats.GetDiagnostics(); } - - diagnostics = streamPart.GetDiagnostics(); } } // TResultSetPrinter destructor should be called before printing stats @@ -232,6 +228,10 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { Cout << Endl << "Statistics:" << Endl << *stats; } + if (diagnostics) { + Cout << Endl << "Diagnostics:" << Endl << NJson::PrettifyJson(*diagnostics, true) << Endl;; + } + if (plan) { if (!ExplainMode && !ExplainAnalyzeMode && (OutputFormat == EDataFormat::Default || OutputFormat == EDataFormat::Pretty)) { @@ -245,9 +245,28 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { queryPlanPrinter.Print(*plan); } - if (CollectFullDiagnostics) { - TFileOutput file(TStringBuilder() << "diagnostics_" << TGUID::Create().AsGuidString() << ".txt"); - file << diagnostics; + if (!DiagnosticsFile.empty()) { + TFileOutput file(DiagnosticsFile); + + NJson::TJsonValue diagnosticsJson(NJson::JSON_MAP); + + if (stats) { + diagnosticsJson.InsertValue("stats", *stats); + } + if (ast) { + diagnosticsJson.InsertValue("ast", *ast); + } + if (plan) { + NJson::TJsonValue planJson; + NJson::ReadJsonTree(*plan, &planJson, true); + diagnosticsJson.InsertValue("plan", planJson); + } + if (diagnostics) { + NJson::TJsonValue metaJson; + NJson::ReadJsonTree(*diagnostics, &metaJson, true); + diagnosticsJson.InsertValue("meta", metaJson); + } + file << NJson::PrettifyJson(NJson::WriteJson(diagnosticsJson, true), false); } if (IsInterrupted()) { diff --git a/ydb/public/lib/ydb_cli/commands/ydb_sql.h b/ydb/public/lib/ydb_cli/commands/ydb_sql.h index 277ecce3d81e..ce6eb2780808 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_sql.h +++ b/ydb/public/lib/ydb_cli/commands/ydb_sql.h @@ -28,13 +28,13 @@ class TCommandSql : public TYdbCommand, public TCommandWithOutput, public TComma int PrintResponse(NQuery::TExecuteQueryIterator& result); TString CollectStatsMode; + TString DiagnosticsFile; TString Query; TString QueryFile; TString Syntax; bool ExplainMode = false; bool ExplainAnalyzeMode = false; bool ExplainAst = false; - bool CollectFullDiagnostics = false; }; } diff --git a/ydb/public/sdk/cpp/client/ydb_query/stats.cpp b/ydb/public/sdk/cpp/client/ydb_query/stats.cpp index 76f895c6b9a8..c2dcdf09a2c6 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/stats.cpp +++ b/ydb/public/sdk/cpp/client/ydb_query/stats.cpp @@ -29,6 +29,7 @@ TString TExecStats::ToString(bool withPlan) const { if (!withPlan) { proto.clear_query_plan(); proto.clear_query_ast(); + proto.clear_query_diagnostics(); } TString res;