fix data race when reporting issue counters KIKIMR-19711

3 files changed, 21 insertions, 13 deletions

diff --git a/ydb/core/kqp/counters/kqp_counters.cpp b/ydb/core/kqp/counters/kqp_counters.cpp
index a01abce3b1..4fdf572c9f 100644
--- a/ydb/core/kqp/counters/kqp_counters.cpp
+++ b/ydb/core/kqp/counters/kqp_counters.cpp
@@ -383,13 +383,16 @@ TString TKqpCountersBase::GetIssueName(ui32 issueCode) {
     return TStringBuilder() << "CODE:" << ToString(issueCode);
 }
 
-void TKqpCountersBase::ReportIssues(const Ydb::Issue::IssueMessage& issue) {
-    auto issueCounter = IssueCounters.FindPtr(issue.issue_code());
+void TKqpCountersBase::ReportIssues(
+    THashMap<ui32, ::NMonitoring::TDynamicCounters::TCounterPtr>& issueCounters,
+    const Ydb::Issue::IssueMessage& issue)
+{
+    auto issueCounter = issueCounters.FindPtr(issue.issue_code());
     if (!issueCounter) {
         auto counterName = TStringBuilder() << "Issues/" << GetIssueName(issue.issue_code());
         auto counter = KqpGroup->GetCounter(counterName , true);
 
-        auto result = IssueCounters.emplace(issue.issue_code(), counter);
+        auto result = issueCounters.emplace(issue.issue_code(), counter);
         issueCounter = &result.first->second;
     }
 
@@ -400,7 +403,7 @@ void TKqpCountersBase::ReportIssues(const Ydb::Issue::IssueMessage& issue) {
     }
 
     for (auto& childIssue : issue.issues()) {
-        ReportIssues(childIssue);
+        ReportIssues(issueCounters, childIssue);
     }
 }
 
@@ -980,10 +983,13 @@ void TKqpCounters::ReportResultsBytes(TKqpDbCountersPtr dbCounters, ui64 results
     }
 }
 
-void TKqpCounters::ReportIssues(TKqpDbCountersPtr dbCounters, const Ydb::Issue::IssueMessage& issue) {
-    TKqpCountersBase::ReportIssues(issue);
+void TKqpCounters::ReportIssues(TKqpDbCountersPtr dbCounters,
+    THashMap<ui32, ::NMonitoring::TDynamicCounters::TCounterPtr>& issueCounters,
+    const Ydb::Issue::IssueMessage& issue)
+{
+    TKqpCountersBase::ReportIssues(issueCounters, issue);
     if (dbCounters) {
-        dbCounters->ReportIssues(issue);
+        dbCounters->ReportIssues(issueCounters, issue);
     }
 }
 
diff --git a/ydb/core/kqp/counters/kqp_counters.h b/ydb/core/kqp/counters/kqp_counters.h
index c7d6918598..8a8e9da788 100644
--- a/ydb/core/kqp/counters/kqp_counters.h
+++ b/ydb/core/kqp/counters/kqp_counters.h
@@ -57,7 +57,8 @@ protected:
     void ReportResultsBytes(ui64 resultsSize);
 
     static TString GetIssueName(ui32 issueCode);
-    void ReportIssues(const Ydb::Issue::IssueMessage& issue);
+    void ReportIssues(THashMap<ui32, ::NMonitoring::TDynamicCounters::TCounterPtr>& issueCounters,
+        const Ydb::Issue::IssueMessage& issue);
 
     void ReportQueryLatency(NKikimrKqp::EQueryAction action, const TDuration& duration);
 
@@ -152,8 +153,6 @@ protected:
     ::NMonitoring::TDynamicCounters::TCounterPtr YdbResponseBytes;
     ::NMonitoring::TDynamicCounters::TCounterPtr QueryResultsBytes;
 
-    THashMap<ui32, ::NMonitoring::TDynamicCounters::TCounterPtr> IssueCounters;
-
     // Workers
     NMonitoring::THistogramPtr WorkerLifeSpan;
     NMonitoring::THistogramPtr QueriesPerWorker;
@@ -247,7 +246,6 @@ private:
 
 using TKqpDbCountersPtr = TIntrusivePtr<TKqpDbCounters>;
 
-
 class TKqpCounters : public TThrRefBase, public TKqpCountersBase {
 private:
     struct TTxByKindCounters {
@@ -278,7 +276,9 @@ public:
 
     void ReportResponseStatus(TKqpDbCountersPtr dbCounters, ui64 responseSize, Ydb::StatusIds::StatusCode ydbStatus);
     void ReportResultsBytes(TKqpDbCountersPtr dbCounters, ui64 resultsSize);
-    void ReportIssues(TKqpDbCountersPtr dbCounters, const Ydb::Issue::IssueMessage& issue);
+    void ReportIssues(TKqpDbCountersPtr dbCounters,
+        THashMap<ui32, ::NMonitoring::TDynamicCounters::TCounterPtr>& issueCounters,
+        const Ydb::Issue::IssueMessage& issue);
 
     void ReportQueryWithRangeScan(TKqpDbCountersPtr dbCounters);
     void ReportQueryWithFullScan(TKqpDbCountersPtr dbCounters);
diff --git a/ydb/core/kqp/session_actor/kqp_session_actor.cpp b/ydb/core/kqp/session_actor/kqp_session_actor.cpp
index a01d471991..bdcf88e4d7 100644
--- a/ydb/core/kqp/session_actor/kqp_session_actor.cpp
+++ b/ydb/core/kqp/session_actor/kqp_session_actor.cpp
@@ -1608,7 +1608,7 @@ public:
         
         Counters->ReportResponseStatus(Settings.DbCounters, record.ByteSize(), record.GetYdbStatus());
         for (auto& issue : record.GetResponse().GetQueryIssues()) {
-            Counters->ReportIssues(Settings.DbCounters, issue);
+            Counters->ReportIssues(Settings.DbCounters, CachedIssueCounters, issue);
         }
 
         Send(QueryState->Sender, QueryResponse.release(), 0, QueryState->ProxyRequestId);
@@ -2179,6 +2179,8 @@ private:
     TActorId Owner;
     TString SessionId;
 
+    // cached lookups to issue counters
+    THashMap<ui32, ::NMonitoring::TDynamicCounters::TCounterPtr> CachedIssueCounters;
     TInstant CreationTime;
     TIntrusivePtr<TKqpCounters> Counters;
     TIntrusivePtr<TKqpRequestCounters> RequestCounters;