address comments

Signed-off-by: hehechen <[email protected]>
pingcap · Dec 15, 2022 · d3c639d · d3c639d
1 parent ffd0afd
commit d3c639d
Show file tree

Hide file tree

Showing 7 changed files with 11 additions and 7 deletions.
diff --git 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<query:query_ts:1671076528883980489, local_query_id:42356295, server_id:3340035, start_ts:438062669858603008,start_ts:438062669858603008,task_id:42356296>"
+        // Rely on that MPP task prefix "MPP<query:query_ts:1671076528883980489, local_query_id:42356295, server_id:3340035, start_ts:438062669858603008,task_id:42356296>"
         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
@@ -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
@@ -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
@@ -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<UInt64>()(mpp_query_id.start_ts);
     }

diff --git 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:{}>", 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
@@ -100,7 +100,7 @@ std::pair<MPPTunnelPtr, String> 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
@@ -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());
+                }
             }
         }