wenzhenghu commented on code in PR #68694: URL: https://github.com/apache/doris/pull/68694#discussion_r4267294017
########## be/src/common/query_log_context.cpp: ########## @@ -0,0 +1,170 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include "common/query_log_context.h" + +#include <bthread/bthread.h> + +#include <atomic> +#include <charconv> +#include <mutex> +#include <ostream> + +#include "common/config.h" +#include "common/logging.h" +#include "gen_cpp/Types_types.h" +#include "gen_cpp/types.pb.h" + +namespace doris { +namespace { + +bthread_key_t query_log_key; +std::atomic<bool> query_log_initialized {false}; +thread_local QueryLogIdentity* pthread_query_log_identity = nullptr; + +QueryLogIdentity* get_query_log_identity() { + if (bthread_self() != 0) { + return static_cast<QueryLogIdentity*>(bthread_getspecific(query_log_key)); + } + return pthread_query_log_identity; +} + +void set_query_log_identity(QueryLogIdentity* identity) { + if (bthread_self() != 0) { + CHECK_EQ(0, bthread_setspecific(query_log_key, identity)); + } else { + pthread_query_log_identity = identity; + } +} + +// The caller provides space for 16 hex digits per half plus the separator. +char* format_log_id(char* buffer, uint64_t hi, uint64_t lo) { + auto first = std::to_chars(buffer, buffer + 16, hi, 16); + *first.ptr++ = '-'; + return std::to_chars(first.ptr, buffer + 33, lo, 16).ptr; +} + +void write_log_id(std::ostream& stream, uint64_t hi, uint64_t lo) { + char buffer[33]; + const auto* end = format_log_id(buffer, hi, lo); + stream.write(buffer, static_cast<std::streamsize>(end - buffer)); +} + +} // namespace + +QueryLogIdentity::QueryLogIdentity(const TUniqueId& query_id) + : query_hi(static_cast<uint64_t>(query_id.hi)), + query_lo(static_cast<uint64_t>(query_id.lo)) {} + +QueryLogIdentity::QueryLogIdentity(const PUniqueId& query_id) + : query_hi(static_cast<uint64_t>(query_id.hi())), + query_lo(static_cast<uint64_t>(query_id.lo())) {} + +QueryLogIdentity::QueryLogIdentity(const TUniqueId& query_id, const TUniqueId& instance_id) + : QueryLogIdentity(query_id) { + instance_hi = static_cast<uint64_t>(instance_id.hi); + instance_lo = static_cast<uint64_t>(instance_id.lo); +} + +QueryLogIdentity::QueryLogIdentity(const PUniqueId& query_id, const PUniqueId& instance_id) + : QueryLogIdentity(query_id) { + instance_hi = static_cast<uint64_t>(instance_id.hi()); + instance_lo = static_cast<uint64_t>(instance_id.lo()); +} + +void init_query_log_context() { + if (!config::sys_log_enable_query_id) { + return; + } + static std::once_flag once; + std::call_once(once, [] { + CHECK_EQ(0, bthread_key_create(&query_log_key, nullptr)); + query_log_initialized.store(true, std::memory_order_release); + }); +} + +QueryLogIdentity current_query_log_identity() { + if (!query_log_initialized.load(std::memory_order_acquire)) { + return {}; + } + auto* identity = get_query_log_identity(); + return identity == nullptr ? QueryLogIdentity {} : *identity; +} + +std::string query_id_log_suffix(const TUniqueId& query_id) { + const QueryLogIdentity query(query_id); + if (query.query_hi == 0 && query.query_lo == 0) { + return {}; + } + const auto current = current_query_log_identity(); + if (config::sys_log_enable_query_id && FLAGS_log_prefix && current.query_hi == query.query_hi && + current.query_lo == query.query_lo) { + return {}; + } + char buffer[36] = {' ', '['}; + auto* end = format_log_id(buffer + 2, query.query_hi, query.query_lo); + *end++ = ']'; + return {buffer, static_cast<size_t>(end - buffer)}; +} + +void ScopedQueryLogContext::reset(QueryLogIdentity identity) { + if (!query_log_initialized.load(std::memory_order_acquire)) { + return; + } + _identity = identity; + if (!_installed) { + _previous = get_query_log_identity(); + set_query_log_identity(&_identity); + _installed = true; + } +} + +ScopedQueryLogContext::~ScopedQueryLogContext() { Review Comment: Added `DCHECK(get_query_log_identity() == &_identity)` before restoring `_previous` for an installed scope in [e8c944b9cb66](https://github.com/apache/doris/commit/e8c944b9cb668c580b1854979a0225a04d80ef76). It checks LIFO ownership without adding a recovery branch, and leaves never-installed scopes unchanged. The ASAN query-log scope tests passed. ########## be/src/service/internal_service.h: ########## @@ -259,8 +261,8 @@ class PInternalService : public PBackendService { Status _exec_plan_fragment_impl(const std::string& s_request, PFragmentRequestVersion version, bool compact, - const std::function<void(RuntimeState*, Status*)>& cb = - std::function<void(RuntimeState*, Status*)>()); + const std::function<void(RuntimeState*, Status*)>& cb = {}, + ScopedQueryLogContext* query_log_scope = nullptr); Review Comment: I kept the private helper's caller-owned scope parameter. The RPC request contains serialized Thrift parameters, so the query ID is only known after decoding inside `_exec_plan_fragment_impl`. Keeping that scope alive in the wrapper lets its exception, status, and timing logs retain the decoded query's identity after the helper returns. Moving decoding into the RPC entry would expand the refactor beyond attribution. The parameter does not change the wire RPC interface and retains no query object. I left the per-fragment resets unchanged rather than combining this update with an RPC lifecycle refactor. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
