From ff7d3e587835cd8c16d6eae9eea552decf8d65d1 Mon Sep 17 00:00:00 2001 From: Mingyu Chen Date: Sat, 29 Dec 2018 16:22:38 +0800 Subject: [PATCH] Unify the print method of TUniqueId (#487) --- be/src/runtime/data_spliter.cpp | 3 +- be/src/runtime/data_stream_mgr.cpp | 4 +-- be/src/runtime/etl_job_mgr.cpp | 2 +- be/src/runtime/export_task_mgr.cpp | 1 + be/src/runtime/initial_reservations.cc | 2 +- be/src/runtime/plan_fragment_executor.cpp | 8 ++--- be/src/runtime/result_sink.cpp | 4 +-- be/src/runtime/runtime_state.cpp | 2 +- be/src/runtime/tmp_file_mgr.cc | 2 +- be/src/service/internal_service.cpp | 5 +-- be/src/util/debug_util.cpp | 35 ------------------- be/src/util/debug_util.h | 7 ---- be/src/util/uid_util.cpp | 35 +++++++++++++++++++ be/src/util/uid_util.h | 13 +++++-- be/test/util/uid_util_test.cpp | 8 ++--- .../java/org/apache/doris/qe/Coordinator.java | 6 ++-- .../org/apache/doris/qe/QeProcessorImpl.java | 16 ++++++--- 17 files changed, 83 insertions(+), 70 deletions(-) diff --git a/be/src/runtime/data_spliter.cpp b/be/src/runtime/data_spliter.cpp index eb12836705..7edd7411df 100644 --- a/be/src/runtime/data_spliter.cpp +++ b/be/src/runtime/data_spliter.cpp @@ -31,6 +31,7 @@ #include "runtime/dpp_sink.h" #include "runtime/load_path_mgr.h" #include "runtime/mem_tracker.h" +#include "util/uid_util.h" #include "util/runtime_profile.h" #include "util/file_utils.h" #include "gen_cpp/DataSinks_types.h" @@ -86,7 +87,7 @@ Status DataSpliter::from_thrift( Status DataSpliter::prepare(RuntimeState* state) { std::stringstream title; - title << "DataSplitSink (dst_id=" << state->fragment_instance_id() << ")"; + title << "DataSplitSink (dst_fragment_instance_id=" << print_id(state->fragment_instance_id()) << ")"; RETURN_IF_ERROR(DataSink::prepare(state)); RETURN_IF_ERROR(Expr::prepare( _partition_expr_ctxs, state, _row_desc, _expr_mem_tracker.get())); diff --git a/be/src/runtime/data_stream_mgr.cpp b/be/src/runtime/data_stream_mgr.cpp index 215a69018e..0c75fbb04a 100644 --- a/be/src/runtime/data_stream_mgr.cpp +++ b/be/src/runtime/data_stream_mgr.cpp @@ -26,7 +26,7 @@ #include "runtime/data_stream_recvr.h" #include "runtime/raw_value.h" #include "runtime/runtime_state.h" -#include "util/debug_util.h" +#include "util/uid_util.h" #include "gen_cpp/types.pb.h" // PUniqueId #include "gen_cpp/BackendService.h" @@ -98,7 +98,7 @@ Status DataStreamMgr::add_data( const PRowBatch& pb_batch, int32_t sender_id, int be_number, int64_t packet_seq, ::google::protobuf::Closure** done) { - VLOG_ROW << "add_data(): finst_id=" << print_id(finst_id) + VLOG_ROW << "add_data(): fragment_instance_id=" << print_id(finst_id) << " node=" << node_id; TUniqueId t_finst_id; t_finst_id.hi = finst_id.hi(); diff --git a/be/src/runtime/etl_job_mgr.cpp b/be/src/runtime/etl_job_mgr.cpp index 4c3321a415..37edbe8371 100644 --- a/be/src/runtime/etl_job_mgr.cpp +++ b/be/src/runtime/etl_job_mgr.cpp @@ -23,7 +23,7 @@ #include "gen_cpp/Status_types.h" #include "gen_cpp/Types_types.h" #include "service/backend_options.h" -#include "util/debug_util.h" +#include "util/uid_util.h" #include "runtime/exec_env.h" #include "runtime/plan_fragment_executor.h" #include "runtime/fragment_mgr.h" diff --git a/be/src/runtime/export_task_mgr.cpp b/be/src/runtime/export_task_mgr.cpp index 1f2ec9cad9..95a99a1931 100644 --- a/be/src/runtime/export_task_mgr.cpp +++ b/be/src/runtime/export_task_mgr.cpp @@ -21,6 +21,7 @@ #include "runtime/fragment_mgr.h" #include "runtime/plan_fragment_executor.h" #include "runtime/runtime_state.h" +#include "util/uid_util.h" #include "gen_cpp/FrontendService.h" #include "gen_cpp/BackendService.h" diff --git a/be/src/runtime/initial_reservations.cc b/be/src/runtime/initial_reservations.cc index 1af1546152..90606184f1 100644 --- a/be/src/runtime/initial_reservations.cc +++ b/be/src/runtime/initial_reservations.cc @@ -26,7 +26,7 @@ #include "common/object_pool.h" #include "runtime/exec_env.h" #include "runtime/mem_tracker.h" -#include "util/debug_util.h" +#include "util/uid_util.h" #include "util/pretty_printer.h" #include "common/names.h" diff --git a/be/src/runtime/plan_fragment_executor.cpp b/be/src/runtime/plan_fragment_executor.cpp index 401b825460..d216f9acf8 100644 --- a/be/src/runtime/plan_fragment_executor.cpp +++ b/be/src/runtime/plan_fragment_executor.cpp @@ -37,7 +37,7 @@ #include "runtime/row_batch.h" #include "runtime/mem_tracker.h" #include "util/cpu_info.h" -#include "util/debug_util.h" +#include "util/uid_util.h" #include "util/container_util.hpp" #include "util/parse_util.h" #include "util/pretty_printer.h" @@ -70,7 +70,7 @@ Status PlanFragmentExecutor::prepare(const TExecPlanFragmentParams& request) { _query_id = params.query_id; LOG(INFO) << "Prepare(): query_id=" << print_id(_query_id) - << " instance_id=" << print_id(params.fragment_instance_id) + << " fragment_instance_id=" << print_id(params.fragment_instance_id) << " backend_num=" << request.backend_num; // VLOG(2) << "request:\n" << apache::thrift::ThriftDebugString(request); @@ -254,7 +254,7 @@ void PlanFragmentExecutor::print_volume_ids( } Status PlanFragmentExecutor::open() { - LOG(INFO) << "Open(): instance_id=" << _runtime_state->fragment_instance_id(); + LOG(INFO) << "Open(): fragment_instance_id=" << print_id(_runtime_state->fragment_instance_id()); // we need to start the profile-reporting thread before calling Open(), since it // may block @@ -493,7 +493,7 @@ void PlanFragmentExecutor::update_status(const Status& status) { } void PlanFragmentExecutor::cancel() { - LOG(INFO) << "cancel(): instance_id=" << _runtime_state->fragment_instance_id(); + LOG(INFO) << "cancel(): fragment_instance_id=" << print_id(_runtime_state->fragment_instance_id()); DCHECK(_prepared); _runtime_state->set_is_cancelled(true); _runtime_state->exec_env()->stream_mgr()->cancel(_runtime_state->fragment_instance_id()); diff --git a/be/src/runtime/result_sink.cpp b/be/src/runtime/result_sink.cpp index 8f6ef91dd1..b5eb2c978b 100644 --- a/be/src/runtime/result_sink.cpp +++ b/be/src/runtime/result_sink.cpp @@ -18,7 +18,7 @@ #include "runtime/result_sink.h" #include "common/config.h" -#include "util/debug_util.h" +#include "util/uid_util.h" #include "exprs/expr.h" #include "runtime/row_batch.h" #include "runtime/runtime_state.h" @@ -53,7 +53,7 @@ Status ResultSink::prepare_exprs(RuntimeState* state) { Status ResultSink::prepare(RuntimeState* state) { RETURN_IF_ERROR(DataSink::prepare(state)); std::stringstream title; - title << "DataBufferSender (dst_id=" << state->fragment_instance_id() << ")"; + title << "DataBufferSender (dst_fragment_instance_id=" << print_id(state->fragment_instance_id()) << ")"; // create profile _profile = state->obj_pool()->add(new RuntimeProfile(state->obj_pool(), title.str())); // prepare output_expr diff --git a/be/src/runtime/runtime_state.cpp b/be/src/runtime/runtime_state.cpp index ac5ca6dbc9..18e6b952d2 100644 --- a/be/src/runtime/runtime_state.cpp +++ b/be/src/runtime/runtime_state.cpp @@ -36,7 +36,7 @@ #include "runtime/load_path_mgr.h" #include "util/cpu_info.h" #include "util/mem_info.h" -#include "util/debug_util.h" +#include "util/uid_util.h" #include "util/disk_info.h" #include "util/file_utils.h" #include "util/pretty_printer.h" diff --git a/be/src/runtime/tmp_file_mgr.cc b/be/src/runtime/tmp_file_mgr.cc index 7d03ee0702..51aae7ea66 100644 --- a/be/src/runtime/tmp_file_mgr.cc +++ b/be/src/runtime/tmp_file_mgr.cc @@ -28,7 +28,7 @@ // #include #include "olap/olap_engine.h" -#include "util/debug_util.h" +#include "util/uid_util.h" #include "util/disk_info.h" #include "util/filesystem_util.h" #include "runtime/exec_env.h" diff --git a/be/src/service/internal_service.cpp b/be/src/service/internal_service.cpp index da3881235d..3b7080ccc9 100644 --- a/be/src/service/internal_service.cpp +++ b/be/src/service/internal_service.cpp @@ -24,6 +24,7 @@ #include "runtime/data_stream_mgr.h" #include "runtime/fragment_mgr.h" #include "service/brpc.h" +#include "util/uid_util.h" #include "util/thrift_util.h" #include "runtime/buffer_control_block.h" #include "runtime/result_buffer_mgr.h" @@ -150,7 +151,7 @@ Status PInternalServiceImpl::_exec_plan_fragment(brpc::Controller* cntl) { uint32_t len = ser_request.size(); RETURN_IF_ERROR(deserialize_thrift_msg(buf, &len, false, &t_request)); } - LOG(INFO) << "exec plan fragment, finst_id=" << t_request.params.fragment_instance_id + LOG(INFO) << "exec plan fragment, fragment_instance_id=" << print_id(t_request.params.fragment_instance_id) << ", coord=" << t_request.coord << ", backend=" << t_request.backend_num; return _exec_env->fragment_mgr()->exec_plan_fragment(t_request); } @@ -165,7 +166,7 @@ void PInternalServiceImpl::cancel_plan_fragment( TUniqueId tid; tid.__set_hi(request->finst_id().hi()); tid.__set_lo(request->finst_id().lo()); - LOG(INFO) << "cancel framgent, finst_id=" << tid; + LOG(INFO) << "cancel framgent, fragment_instance_id=" << print_id(tid); auto st = _exec_env->fragment_mgr()->cancel(tid); if (!st.ok()) { LOG(WARNING) << "cancel plan fragment failed, errmsg=" << st.get_error_msg(); diff --git a/be/src/util/debug_util.cpp b/be/src/util/debug_util.cpp index 54c92d6d2d..29cc714feb 100644 --- a/be/src/util/debug_util.cpp +++ b/be/src/util/debug_util.cpp @@ -78,41 +78,6 @@ THRIFT_ENUM_PRINT_FN(QueryState); THRIFT_ENUM_PRINT_FN(TMetricKind); THRIFT_ENUM_PRINT_FN(TUnit); -std::string print_id(const TUniqueId& id) { - std::stringstream out; - out << std::hex << id.hi << ":" << id.lo; - return out.str(); -} - -std::string print_id(const PUniqueId& id) { - std::stringstream out; - out << std::hex << id.hi() << ":" << id.lo(); - return out.str(); -} - -bool parse_id(const std::string& s, TUniqueId* id) { - DCHECK(id != NULL); - - const char* hi_part = s.c_str(); - char* colon = const_cast(strchr(hi_part, ':')); - - if (colon == NULL) { - return false; - } - - const char* lo_part = colon + 1; - *colon = '\0'; - - char* error_hi = NULL; - char* error_lo = NULL; - id->hi = strtoul(hi_part, &error_hi, 16); - id->lo = strtoul(lo_part, &error_lo, 16); - - bool valid = *error_hi == '\0' && *error_lo == '\0'; - *colon = ':'; - return valid; -} - std::string print_plan_node_type(const TPlanNodeType::type& type) { std::map::const_iterator i; i = _TPlanNodeType_VALUES_TO_NAMES.find(type); diff --git a/be/src/util/debug_util.h b/be/src/util/debug_util.h index a9ba08cc79..2a86222086 100644 --- a/be/src/util/debug_util.h +++ b/be/src/util/debug_util.h @@ -34,19 +34,12 @@ namespace doris { class PUniqueId; -std::string print_id(const TUniqueId& id); -std::string print_id(const PUniqueId& id); std::string print_plan_node_type(const TPlanNodeType::type& type); std::string print_tstmt_type(const TStmtType::type& type); std::string print_query_state(const QueryState::type& type); std::string PrintTUnit(const TUnit::type& type); std::string PrintTMetricKind(const TMetricKind::type& type); -// Parse 's' into a TUniqueId object. The format of s needs to be the output format -// from PrintId. (:) -// Returns true if parse succeeded. -bool parse_id(const std::string& s, TUniqueId* id); - // Returns a string " (build )" // If compact == false, this string is appended: "\nBuilt on " // This is used to set gflags build version diff --git a/be/src/util/uid_util.cpp b/be/src/util/uid_util.cpp index 2955ab3cae..dcbef4ca51 100644 --- a/be/src/util/uid_util.cpp +++ b/be/src/util/uid_util.cpp @@ -24,4 +24,39 @@ std::ostream& operator<<(std::ostream& os, const UniqueId& uid) { return os; } +std::string print_id(const TUniqueId& id) { + std::stringstream out; + out << std::hex << id.hi << ":" << id.lo; + return out.str(); +} + +std::string print_id(const PUniqueId& id) { + std::stringstream out; + out << std::hex << id.hi() << ":" << id.lo(); + return out.str(); +} + +bool parse_id(const std::string& s, TUniqueId* id) { + DCHECK(id != NULL); + + const char* hi_part = s.c_str(); + char* colon = const_cast(strchr(hi_part, ':')); + + if (colon == NULL) { + return false; + } + + const char* lo_part = colon + 1; + *colon = '\0'; + + char* error_hi = NULL; + char* error_lo = NULL; + id->hi = strtoul(hi_part, &error_hi, 16); + id->lo = strtoul(lo_part, &error_lo, 16); + + bool valid = *error_hi == '\0' && *error_lo == '\0'; + *colon = ':'; + return valid; +} + } diff --git a/be/src/util/uid_util.h b/be/src/util/uid_util.h index 59bc07f1d0..92e2fec1cf 100644 --- a/be/src/util/uid_util.h +++ b/be/src/util/uid_util.h @@ -27,7 +27,7 @@ #include "gen_cpp/Types_types.h" // for TUniqueId #include "gen_cpp/types.pb.h" // for PUniqueId -#include "util/debug_util.h" +// #include "util/debug_util.h" #include "util/hash_util.hpp" namespace doris { @@ -35,7 +35,7 @@ namespace doris { // convert int to a hex format string, buf must enough to hold coverted hex string template inline void to_hex(T val, char* buf) { - static const char* digits = "0123456789ABCDEF"; + static const char* digits = "0123456789abcdef"; for (int i = 0; i < 2 * sizeof(T); ++i) { buf[2 * sizeof(T) - 1 - i] = digits[val & 0x0F]; val >>= 4; @@ -111,6 +111,15 @@ inline TUniqueId generate_uuid() { std::ostream& operator<<(std::ostream& os, const UniqueId& uid); +std::string print_id(const TUniqueId& id); +std::string print_id(const PUniqueId& id); + +// Parse 's' into a TUniqueId object. The format of s needs to be the output format +// from PrintId. (:) +// Returns true if parse succeeded. +bool parse_id(const std::string& s, TUniqueId* id); + + } // namespace doris namespace std { diff --git a/be/test/util/uid_util_test.cpp b/be/test/util/uid_util_test.cpp index a90dc97b70..de00b77639 100644 --- a/be/test/util/uid_util_test.cpp +++ b/be/test/util/uid_util_test.cpp @@ -37,7 +37,7 @@ TEST_F(UidUtilTest, UniqueId) { { UniqueId id(123456789, 987654321); std::string hex_str = id.to_string(); - ASSERT_STREQ("00000000075BCD15:000000003ADE68B1", hex_str.c_str()); + ASSERT_STREQ("00000000075bcd15:000000003ade68b1", hex_str.c_str()); } { PUniqueId puid; @@ -45,7 +45,7 @@ TEST_F(UidUtilTest, UniqueId) { puid.set_lo(98765432123456789); UniqueId id(puid); std::string hex_str = id.to_string(); - ASSERT_STREQ("002BDC546291F4B1:015EE2A321CE7D15", hex_str.c_str()); + ASSERT_STREQ("002bdc546291f4b1:015ee2a321ce7d15", hex_str.c_str()); } { TUniqueId tuid; @@ -53,7 +53,7 @@ TEST_F(UidUtilTest, UniqueId) { tuid.__set_lo(98765432123456789); UniqueId id(tuid); std::string hex_str = id.to_string(); - ASSERT_STREQ("002BDC546291F4B1:015EE2A321CE7D15", hex_str.c_str()); + ASSERT_STREQ("002bdc546291f4b1:015ee2a321ce7d15", hex_str.c_str()); } { TUniqueId tuid; @@ -61,7 +61,7 @@ TEST_F(UidUtilTest, UniqueId) { tuid.__set_lo(98765432123456789); std::stringstream ss; ss << UniqueId(tuid); - ASSERT_STREQ("002BDC546291F4B1:015EE2A321CE7D15", ss.str().c_str()); + ASSERT_STREQ("002bdc546291f4b1:015ee2a321ce7d15", ss.str().c_str()); } } diff --git a/fe/src/main/java/org/apache/doris/qe/Coordinator.java b/fe/src/main/java/org/apache/doris/qe/Coordinator.java index cfbe7b9302..7e232ad25f 100644 --- a/fe/src/main/java/org/apache/doris/qe/Coordinator.java +++ b/fe/src/main/java/org/apache/doris/qe/Coordinator.java @@ -349,11 +349,13 @@ public class Coordinator { // A call to Exec() must precede all other member function calls. public void exec() throws Exception { if (!scanNodes.isEmpty()) { - LOG.debug("debug: in Coordinator::exec. planNode: {}", scanNodes.get(0).treeToThrift()); + LOG.debug("debug: in Coordinator::exec. query id: {}, planNode: {}", + DebugUtil.printId(queryId), scanNodes.get(0).treeToThrift()); } if (!fragments.isEmpty()) { - LOG.debug("debug: in Coordinator::exec. fragment: {}", fragments.get(0).toThrift()); + LOG.debug("debug: in Coordinator::exec. query id: {}, fragment: {}", + DebugUtil.printId(queryId), fragments.get(0).toThrift()); } // prepare information diff --git a/fe/src/main/java/org/apache/doris/qe/QeProcessorImpl.java b/fe/src/main/java/org/apache/doris/qe/QeProcessorImpl.java index 1af14eb07d..cf8c361589 100644 --- a/fe/src/main/java/org/apache/doris/qe/QeProcessorImpl.java +++ b/fe/src/main/java/org/apache/doris/qe/QeProcessorImpl.java @@ -20,8 +20,14 @@ package org.apache.doris.qe; import org.apache.doris.common.UserException; import org.apache.doris.common.util.DebugUtil; -import org.apache.doris.thrift.*; +import org.apache.doris.thrift.TReportExecStatusParams; +import org.apache.doris.thrift.TReportExecStatusResult; +import org.apache.doris.thrift.TStatus; +import org.apache.doris.thrift.TStatusCode; +import org.apache.doris.thrift.TUniqueId; + import com.google.common.collect.Maps; + import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -49,7 +55,7 @@ public final class QeProcessorImpl implements QeProcessor { @Override public void registerQuery(TUniqueId queryId, QueryInfo info) throws UserException { - LOG.info("register query id = " + queryId.toString()); + LOG.info("register query id = " + DebugUtil.printId(queryId)); final QueryInfo result = coordinatorMap.putIfAbsent(queryId, info); if (result != null) { throw new UserException("queryId " + queryId + " already exists"); @@ -58,7 +64,7 @@ public final class QeProcessorImpl implements QeProcessor { @Override public void unregisterQuery(TUniqueId queryId) { - LOG.info("deregister query id = " + queryId.toString()); + LOG.info("deregister query id = " + DebugUtil.printId(queryId)); coordinatorMap.remove(queryId); } @@ -87,8 +93,8 @@ public final class QeProcessorImpl implements QeProcessor { @Override public TReportExecStatusResult reportExecStatus(TReportExecStatusParams params) { - LOG.info("ReportExecStatus(): instance_id=" + params.fragment_instance_id.toString() - + "queryID=" + params.query_id.toString() + " params=" + params); + LOG.info("ReportExecStatus(): fragment_instance_id=" + DebugUtil.printId(params.fragment_instance_id) + + ", query id=" + DebugUtil.printId(params.query_id) + " params=" + params); final TReportExecStatusResult result = new TReportExecStatusResult(); final QueryInfo info = coordinatorMap.get(params.query_id);