From d3c639d0cacc5534bc581f03a20c7fb55474e786 Mon Sep 17 00:00:00 2001 From: hehechen Date: Fri, 16 Dec 2022 00:53:44 +0800 Subject: [PATCH] address comments Signed-off-by: hehechen --- dbms/src/Debug/dbgFuncMisc.cpp | 2 +- dbms/src/Flash/Coprocessor/DAGContext.h | 2 +- dbms/src/Flash/Mpp/MPPTask.cpp | 2 +- dbms/src/Flash/Mpp/MPPTaskId.cpp | 2 +- dbms/src/Flash/Mpp/MPPTaskId.h | 2 +- dbms/src/Flash/Mpp/MPPTaskManager.cpp | 2 +- dbms/src/Storages/Transaction/LearnerRead.cpp | 6 +++++- 7 files changed, 11 insertions(+), 7 deletions(-) diff --git a/dbms/src/Debug/dbgFuncMisc.cpp b/dbms/src/Debug/dbgFuncMisc.cpp index 42c9461ffae..b02d6f38a57 100644 --- a/dbms/src/Debug/dbgFuncMisc.cpp +++ b/dbms/src/Debug/dbgFuncMisc.cpp @@ -29,7 +29,7 @@ inline size_t getReadTSOForLog(const String & line) { std::regex rx(R"((0|[1-9][0-9]*))"); std::smatch m; - // Rely on that MPP task prefix "MPP" + // Rely on that MPP task prefix "MPP" auto pos = line.find(", start_ts:"); if (pos != std::string::npos && regex_search(line.cbegin() + pos, line.cend(), m, rx)) { diff --git a/dbms/src/Flash/Coprocessor/DAGContext.h b/dbms/src/Flash/Coprocessor/DAGContext.h index 2cd5ae8040a..2a42ecda41a 100644 --- a/dbms/src/Flash/Coprocessor/DAGContext.h +++ b/dbms/src/Flash/Coprocessor/DAGContext.h @@ -158,7 +158,7 @@ class DAGContext , flags(dag_request->flags()) , sql_mode(dag_request->sql_mode()) , mpp_task_meta(meta_) - , mpp_task_id(mpp_task_meta.start_ts(), mpp_task_meta.task_id(), mpp_task_meta.server_id(), mpp_task_meta.query_ts(), mpp_task_meta.local_query_id()) + , mpp_task_id(mpp_task_meta) , max_recorded_error_count(getMaxErrorCount(*dag_request)) , warnings(max_recorded_error_count) , warning_count(0) diff --git a/dbms/src/Flash/Mpp/MPPTask.cpp b/dbms/src/Flash/Mpp/MPPTask.cpp index 2452fa32b30..2e5888f0c32 100644 --- a/dbms/src/Flash/Mpp/MPPTask.cpp +++ b/dbms/src/Flash/Mpp/MPPTask.cpp @@ -54,7 +54,7 @@ extern const char force_no_local_region_for_mpp_task[]; MPPTask::MPPTask(const mpp::TaskMeta & meta_, const ContextPtr & context_) : meta(meta_) - , id(meta.start_ts(), meta.task_id(), meta.server_id(), meta.query_ts(), meta.local_query_id()) + , id(meta) , context(context_) , manager(context_->getTMTContext().getMPPTaskManager().get()) , schedule_entry(manager, id) diff --git a/dbms/src/Flash/Mpp/MPPTaskId.cpp b/dbms/src/Flash/Mpp/MPPTaskId.cpp index 0872cb6e88a..9f4a9ce3dfc 100644 --- a/dbms/src/Flash/Mpp/MPPTaskId.cpp +++ b/dbms/src/Flash/Mpp/MPPTaskId.cpp @@ -79,7 +79,7 @@ bool MPPQueryId::operator<=(const MPPQueryId & rid) const size_t MPPQueryIdHash::operator()(MPPQueryId const & mpp_query_id) const noexcept { - if (isOldVersion(mpp_query_id)) + if (unlikely(isOldVersion(mpp_query_id))) { return std::hash()(mpp_query_id.start_ts); } diff --git a/dbms/src/Flash/Mpp/MPPTaskId.h b/dbms/src/Flash/Mpp/MPPTaskId.h index 59045759c1a..4e9c9cce7f8 100644 --- a/dbms/src/Flash/Mpp/MPPTaskId.h +++ b/dbms/src/Flash/Mpp/MPPTaskId.h @@ -45,7 +45,7 @@ struct MPPQueryId bool operator<=(const MPPQueryId & rid) const; String toString() const { - return fmt::format("query_ts:{}, local_query_id:{}, server_id:{}, start_ts:{}", query_ts, local_query_id, server_id, start_ts); + return fmt::format("", query_ts, local_query_id, server_id, start_ts); } }; diff --git a/dbms/src/Flash/Mpp/MPPTaskManager.cpp b/dbms/src/Flash/Mpp/MPPTaskManager.cpp index 6f2eb552a2a..41b6e35b727 100644 --- a/dbms/src/Flash/Mpp/MPPTaskManager.cpp +++ b/dbms/src/Flash/Mpp/MPPTaskManager.cpp @@ -100,7 +100,7 @@ std::pair MPPTaskManager::findAsyncTunnel(const ::mpp::Est cv.notify_all(); } } - return {nullptr, fmt::format("Can't find task [{},{}] within 10 s.", id.query_id.toString(), id.task_id)}; + return {nullptr, fmt::format("Can't find task [{}] within 10 s.", id.toString())}; } } /// don't need to delete the alarm here because registerMPPTask will delete all the related alarm diff --git a/dbms/src/Storages/Transaction/LearnerRead.cpp b/dbms/src/Storages/Transaction/LearnerRead.cpp index dc4650ae238..bc9cde099fb 100644 --- a/dbms/src/Storages/Transaction/LearnerRead.cpp +++ b/dbms/src/Storages/Transaction/LearnerRead.cpp @@ -318,7 +318,11 @@ LearnerReadSnapshot doLearnerRead( else { // cache read-index to avoid useless overhead about retry. - mvcc_query_info.addReadIndexRes(region_id, resp.read_index()); + // resp.read_index() is 0 when stale read, skip it to avoid overwriting read_index res in last retry. + if (resp.read_index() != 0) + { + mvcc_query_info.addReadIndexRes(region_id, resp.read_index()); + } } }