Statistics: OperationId is generated by KQP (#7694)

ydb/core/kqp/gateway/actors/analyze_actor.cpp

@@ -2,6 +2,7 @@
 
 #include <ydb/core/base/path.h>
 #include <ydb/core/base/tablet_pipecache.h>
+#include <ydb/core/util/ulid.h>
 #include <ydb/library/actors/core/log.h>
 #include <ydb/library/services/services.pb.h>
 
@@ -15,6 +16,18 @@ enum {
 
 using TNavigate = NSchemeCache::TSchemeCacheNavigate;
 
+TString MakeOperationId() {
+    TULIDGenerator ulidGen;
+    return ulidGen.Next(TActivationContext::Now()).ToBinary();
+}
+
+TAnalyzeActor::TAnalyzeActor(TString tablePath, TVector<TString> columns, NThreading::TPromise<NYql::IKikimrGateway::TGenericResult> promise)
+    : TablePath(tablePath)
+    , Columns(columns) 
+    , Promise(promise)
+    , OperationId(MakeOperationId())
+{}
+
 void TAnalyzeActor::Bootstrap() {
     using TNavigate = NSchemeCache::TSchemeCacheNavigate;
     auto navigate = std::make_unique<TNavigate>();
@@ -34,7 +47,7 @@ void TAnalyzeActor::SendAnalyzeStatus() {
 
     auto getStatus = std::make_unique<NStat::TEvStatistics::TEvAnalyzeStatus>();
     auto& record = getStatus->Record;
-    PathIdFromPathId(PathId, record.MutablePathId());
+    record.SetOperationId(OperationId);
 
     Send(
         MakePipePerNodeCacheID(false),
@@ -43,9 +56,19 @@ void TAnalyzeActor::SendAnalyzeStatus() {
 }
 
 void TAnalyzeActor::Handle(NStat::TEvStatistics::TEvAnalyzeResponse::TPtr& ev, const TActorContext& ctx) {
-    Y_UNUSED(ev);
     Y_UNUSED(ctx);
 
+    const auto& record = ev->Get()->Record;
+    const TString operationId = record.GetOperationId();
+
+    if (operationId != OperationId) {
+        ALOG_CRIT(NKikimrServices::KQP_GATEWAY, 
+            "TAnalyzeActor, TEvAnalyzeResponse has operationId=" << operationId 
+            << " , but expected " << OperationId);
+    }
+
+
+    // TODO Don't send EvAnalyzeStatus, EvAnalyzeResponse is already here
     SendAnalyzeStatus();
 }
 
@@ -172,6 +195,7 @@ void TAnalyzeActor::SendStatisticsAggregatorAnalyze(const NSchemeCache::TSchemeC
 
     auto analyzeRequest = std::make_unique<NStat::TEvStatistics::TEvAnalyze>();
     auto& record = analyzeRequest->Record;
+    record.SetOperationId(OperationId);
     auto table = record.AddTables();
     
     PathIdFromPathId(PathId, table->MutablePathId());
@@ -199,6 +223,7 @@ void TAnalyzeActor::SendStatisticsAggregatorAnalyze(const NSchemeCache::TSchemeC
         *table->MutableColumnTags()->Add() = tagByColumnName[columnName];
     }
 
+    // TODO This request should be retried if StatisticsAggregator fails
     Send(
         MakePipePerNodeCacheID(false),
         new TEvPipeCache::TEvForward(analyzeRequest.release(), entry.DomainInfo->Params.GetStatisticsAggregator(), true),

ydb/core/kqp/gateway/actors/analyze_actor.h

@@ -20,11 +20,7 @@ struct TEvAnalyzePrivate {
 
 class TAnalyzeActor : public NActors::TActorBootstrapped<TAnalyzeActor> { 
 public:
-    TAnalyzeActor(TString tablePath, TVector<TString> columns, NThreading::TPromise<NYql::IKikimrGateway::TGenericResult> promise)
-        : TablePath(tablePath)
-        , Columns(columns) 
-        , Promise(promise)
-    {}
+    TAnalyzeActor(TString tablePath, TVector<TString> columns, NThreading::TPromise<NYql::IKikimrGateway::TGenericResult> promise);
 
     void Bootstrap();
 
@@ -61,6 +57,7 @@ private:
     // For Statistics Aggregator
     std::optional<ui64> StatisticsAggregatorId;
     TPathId PathId;
+    TString OperationId;
 };
 
 } // end of NKikimr::NKqp

ydb/core/protos/out/out.cpp

@@ -24,6 +24,7 @@
 #include <ydb/core/protos/flat_scheme_op.pb.h>
 #include <ydb/core/protos/subdomains.pb.h>
 #include <ydb/core/protos/data_events.pb.h>
+#include <ydb/core/protos/statistics.pb.h>
 
 #include <util/stream/output.h>
 
@@ -238,3 +239,7 @@ Y_DECLARE_OUT_SPEC(, NKikimrDataEvents::TEvWrite::TOperation::EOperationType, st
 Y_DECLARE_OUT_SPEC(, NKikimrDataEvents::TEvWrite::ETxMode, stream, value) {
     stream << NKikimrDataEvents::TEvWrite::ETxMode_Name(value);
 }
+
+Y_DECLARE_OUT_SPEC(, NKikimrStat::TEvAnalyzeStatusResponse_EStatus, stream, value) {
+    stream << NKikimrStat::TEvAnalyzeStatusResponse_EStatus_Name(value);
+}

ydb/core/protos/statistics.proto

@@ -81,24 +81,24 @@ message TTable {
 
 // KQP -> SA
 message TEvAnalyze {
-    optional uint64 Cookie = 1; // request cookie to match response item
+    optional bytes OperationId = 1; // unique identifier to match response item
     repeated TTable Tables = 2; // list of analyzed tables and columns
     repeated EColumnStatisticType Types = 3; // list of statistics types requested. Empty means asking for all available.
 }
 
 // SA -> KQP
 message TEvAnalyzeResponse {
-    optional uint64 Cookie = 1;
+    optional bytes OperationId = 1;
 }
 
 // KQP -> SA
 message TEvAnalyzeStatus {
-    optional NKikimrProto.TPathID PathId = 1;
+    optional bytes OperationId = 1; // unique identifier to match response item
 }
 
 // SA -> KQP
 message TEvAnalyzeStatusResponse {
-    optional NKikimrProto.TPathID PathId = 1;
+    optional bytes OperationId = 1;
 
     enum EStatus {
         STATUS_UNSPECIFIED = 0;

ydb/core/statistics/aggregator/aggregator_impl.cpp

@@ -433,17 +433,18 @@ void TStatisticsAggregator::Handle(TEvStatistics::TEvStatTableCreationResponse::
 }
 
 void TStatisticsAggregator::Handle(TEvStatistics::TEvAnalyzeStatus::TPtr& ev) {
-    auto& inRecord = ev->Get()->Record;
-    auto pathId = PathIdFromPathId(inRecord.GetPathId());
+    const auto& inRecord = ev->Get()->Record;
+    const TString operationId = inRecord.GetOperationId();
 
     auto response = std::make_unique<TEvStatistics::TEvAnalyzeStatusResponse>();
     auto& outRecord = response->Record;
+    outRecord.SetOperationId(operationId);
 
-    if (TraversalTableId.PathId == pathId) {
+    if (ForceTraversalOperationId == operationId) {
         outRecord.SetStatus(NKikimrStat::TEvAnalyzeStatusResponse::STATUS_IN_PROGRESS);
     } else {
         if (std::any_of(ForceTraversals.begin(), ForceTraversals.end(), 
-            [&pathId](const TForceTraversal& elem) { return elem.PathId == pathId;})) {
+            [&operationId](const TForceTraversal& elem) { return elem.OperationId == operationId;})) {
             outRecord.SetStatus(NKikimrStat::TEvAnalyzeStatusResponse::STATUS_ENQUEUED);
         } else {
             outRecord.SetStatus(NKikimrStat::TEvAnalyzeStatusResponse::STATUS_NO_OPERATION);
@@ -586,7 +587,6 @@ void TStatisticsAggregator::ScheduleNextTraversal(NIceDb::TNiceDb& db) {
         pathId = operation.PathId;
 
         ForceTraversalOperationId = operation.OperationId;
-        ForceTraversalCookie = operation.Cookie;
         ForceTraversalColumnTags = operation.ColumnTags;
         ForceTraversalTypes = operation.Types;
         ForceTraversalReplyToActorId = operation.ReplyToActorId;
@@ -678,22 +678,17 @@ void TStatisticsAggregator::PersistStartKey(NIceDb::TNiceDb& db) {
 
 void TStatisticsAggregator::PersistForceTraversal(NIceDb::TNiceDb& db) {
     PersistSysParam(db, Schema::SysParam_ForceTraversalOperationId, ToString(ForceTraversalOperationId));
-    PersistSysParam(db, Schema::SysParam_ForceTraversalCookie, ToString(ForceTraversalCookie));
+    PersistSysParam(db, Schema::SysParam_ForceTraversalCookie, ForceTraversalOperationId);
     PersistSysParam(db, Schema::SysParam_ForceTraversalColumnTags, ToString(ForceTraversalColumnTags));
     PersistSysParam(db, Schema::SysParam_ForceTraversalTypes, ToString(ForceTraversalTypes));
 }
 
-void TStatisticsAggregator::PersistNextForceTraversalOperationId(NIceDb::TNiceDb& db) {
-    PersistSysParam(db, Schema::SysParam_NextForceTraversalOperationId, ToString(NextForceTraversalOperationId));
-}
-
 void TStatisticsAggregator::PersistGlobalTraversalRound(NIceDb::TNiceDb& db) {
     PersistSysParam(db, Schema::SysParam_GlobalTraversalRound, ToString(GlobalTraversalRound));
 }
 
 void TStatisticsAggregator::ResetTraversalState(NIceDb::TNiceDb& db) {
-    ForceTraversalOperationId = 0;
-    ForceTraversalCookie = 0;
+    ForceTraversalOperationId.clear();
     TraversalTableId.PathId = TPathId();
     ForceTraversalColumnTags.clear();
     ForceTraversalTypes.clear();

ydb/core/statistics/aggregator/aggregator_impl.h

@@ -147,7 +147,6 @@ private:
     void PersistTraversal(NIceDb::TNiceDb& db);
     void PersistForceTraversal(NIceDb::TNiceDb& db);
     void PersistStartKey(NIceDb::TNiceDb& db);
-    void PersistNextForceTraversalOperationId(NIceDb::TNiceDb& db);    
     void PersistGlobalTraversalRound(NIceDb::TNiceDb& db);
 
     void ResetTraversalState(NIceDb::TNiceDb& db);
@@ -306,15 +305,13 @@ private:
 
 private: // stored in local db
     
-    ui64 ForceTraversalOperationId = 0;    
-    ui64 ForceTraversalCookie = 0;
+    TString ForceTraversalOperationId;
     TString ForceTraversalColumnTags;
     TString ForceTraversalTypes;
     TTableId TraversalTableId; 
     bool TraversalIsColumnTable = false;
     TSerializedCellVec TraversalStartKey;
     TInstant TraversalStartTime;
-    ui64 NextForceTraversalOperationId = 0;
 
     size_t GlobalTraversalRound = 1; 
 
@@ -327,8 +324,7 @@ private: // stored in local db
     TTraversalsByTime ScheduleTraversalsByTime;
 
     struct TForceTraversal {
-        ui64 OperationId = 0;
-        ui64 Cookie = 0;
+        TString OperationId;
         TPathId PathId;
         TString ColumnTags;
         TString Types;

ydb/core/statistics/aggregator/schema.h

@@ -50,7 +50,7 @@ struct TAggregatorSchema : NIceDb::Schema {
         struct OperationId    : Column<1, NScheme::NTypeIds::Uint64> {};
         struct OwnerId        : Column<2, NScheme::NTypeIds::Uint64> {};
         struct LocalPathId    : Column<3, NScheme::NTypeIds::Uint64> {};
-        struct Cookie         : Column<4, NScheme::NTypeIds::Uint64> {};
+        struct Cookie         : Column<4, NScheme::NTypeIds::String> {};
         struct ColumnTags     : Column<5, NScheme::NTypeIds::String> {};
         struct Types          : Column<6, NScheme::NTypeIds::String> {};
 
@@ -87,7 +87,7 @@ struct TAggregatorSchema : NIceDb::Schema {
     static constexpr ui64 SysParam_ForceTraversalColumnTags = 7;
     static constexpr ui64 SysParam_ForceTraversalTypes = 8;
     static constexpr ui64 SysParam_TraversalStartTime = 9;
-    static constexpr ui64 SysParam_NextForceTraversalOperationId = 10;
+    // deprecated 10
     static constexpr ui64 SysParam_TraversalIsColumnTable = 11;
     static constexpr ui64 SysParam_GlobalTraversalRound = 12;
 };

ydb/core/statistics/aggregator/tx_analyze_table.cpp

@@ -19,7 +19,7 @@ struct TStatisticsAggregator::TTxAnalyzeTable : public TTxBase {
     TTxType GetTxType() const override { return TXTYPE_ANALYZE_TABLE; }
 
     bool Execute(TTransactionContext& txc, const TActorContext&) override {
-        SA_LOG_D("[" << Self->TabletID() << "] TTxAnalyzeTable::Execute");
+        SA_LOG_D("[" << Self->TabletID() << "] TTxAnalyzeTable::Execute. ReplyToActorId " << ReplyToActorId << " , Record " << Record);
 
         if (!Self->EnableColumnStatistics) {
             return true;
@@ -27,27 +27,31 @@ struct TStatisticsAggregator::TTxAnalyzeTable : public TTxBase {
 
         NIceDb::TNiceDb db(txc.DB);
 
-        const ui64 cookie = Record.GetCookie();
+        const TString operationId = Record.GetOperationId();
         const TString types = JoinVectorIntoString(TVector<ui32>(Record.GetTypes().begin(), Record.GetTypes().end()), ",");
         
         for (const auto& table : Record.GetTables()) {
             const TPathId pathId = PathIdFromPathId(table.GetPathId());
             const TString columnTags = JoinVectorIntoString(TVector<ui32>{table.GetColumnTags().begin(),table.GetColumnTags().end()},",");
 
-            // drop request with the same cookie and path from this sender
-            if (std::any_of(Self->ForceTraversals.begin(), Self->ForceTraversals.end(), 
-                [this, &pathId, &cookie](const TForceTraversal& elem) { 
+            // check existing force traversal with the same cookie and path
+            auto forceTraversal = std::find_if(Self->ForceTraversals.begin(), Self->ForceTraversals.end(), 
+                [&pathId, &operationId](const TForceTraversal& elem) { 
                     return elem.PathId == pathId 
-                        && elem.Cookie == cookie
-                        && elem.ReplyToActorId == ReplyToActorId
-                    ;})) {
+                        && elem.OperationId == operationId;});
+
+            // update existing force traversal
+            if (forceTraversal != Self->ForceTraversals.end()) {
+                SA_LOG_D("[" << Self->TabletID() << "] TTxAnalyzeTable::Execute. Update existing force traversal. PathId " << pathId << " , ReplyToActorId " << ReplyToActorId);
+                forceTraversal->ReplyToActorId = ReplyToActorId;
                 return true;
             }
 
+            SA_LOG_D("[" << Self->TabletID() << "] TTxAnalyzeTable::Execute. Create new force traversal operation for pathId " << pathId);
+
             // create new force trasersal
             TForceTraversal operation {
-                .OperationId = Self->NextForceTraversalOperationId,
-                .Cookie = cookie,
+                .OperationId = operationId,
                 .PathId = pathId,
                 .ColumnTags = columnTags,
                 .Types = types,
@@ -66,8 +70,6 @@ struct TStatisticsAggregator::TTxAnalyzeTable : public TTxBase {
 */
         }
 
-        Self->PersistNextForceTraversalOperationId(db);
-
         return true;
     }
 
@@ -77,8 +79,6 @@ struct TStatisticsAggregator::TTxAnalyzeTable : public TTxBase {
 };
 
 void TStatisticsAggregator::Handle(TEvStatistics::TEvAnalyze::TPtr& ev) {
-    ++NextForceTraversalOperationId;
-
     Execute(new TTxAnalyzeTable(this, ev->Get()->Record, ev->Sender), TActivationContext::AsActorContext());
 }
 

ydb/core/statistics/aggregator/tx_finish_trasersal.cpp

@@ -5,15 +5,13 @@
 namespace NKikimr::NStat {
 
 struct TStatisticsAggregator::TTxFinishTraversal : public TTxBase {
-    ui64 OperationId;
-    ui64 Cookie;
+    TString OperationId;
     TPathId PathId;
     TActorId ReplyToActorId;
 
     TTxFinishTraversal(TSelf* self)
         : TTxBase(self)
         , OperationId(self->ForceTraversalOperationId)
-        , Cookie(self->ForceTraversalCookie)
         , PathId(self->TraversalTableId.PathId)
         , ReplyToActorId(self->ForceTraversalReplyToActorId)
     {}
@@ -43,12 +41,12 @@ struct TStatisticsAggregator::TTxFinishTraversal : public TTxBase {
         
         if (operationsRemain) {
             SA_LOG_D("[" << Self->TabletID() << "] TTxFinishTraversal::Complete. Don't send TEvAnalyzeResponse. " <<
-                "There are pending operations, Cookie " << Cookie << " , ActorId=" << ReplyToActorId);
+                "There are pending operations, OperationId " << OperationId << " , ActorId=" << ReplyToActorId);
         } else {
             SA_LOG_D("[" << Self->TabletID() << "] TTxFinishTraversal::Complete. " <<
-                "Send TEvAnalyzeResponse, Cookie=" << Cookie << ", ActorId=" << ReplyToActorId);
+                "Send TEvAnalyzeResponse, OperationId=" << OperationId << ", ActorId=" << ReplyToActorId);
             auto response = std::make_unique<TEvStatistics::TEvAnalyzeResponse>();
-            response->Record.SetCookie(Cookie);
+            response->Record.SetOperationId(OperationId);
             ctx.Send(ReplyToActorId, response.release());
         }
     }

ydb/core/statistics/aggregator/tx_init.cpp

@@ -55,15 +55,10 @@ struct TStatisticsAggregator::TTxInit : public TTxBase {
                         SA_LOG_D("[" << Self->TabletID() << "] Loaded traversal start key");
                         break;
                     case Schema::SysParam_ForceTraversalOperationId: {
-                        Self->ForceTraversalOperationId = FromString<ui64>(value);
+                        Self->ForceTraversalOperationId = value;
                         SA_LOG_D("[" << Self->TabletID() << "] Loaded traversal operation id: " << value);
                         break;
                     }  
-                    case Schema::SysParam_ForceTraversalCookie: {
-                        Self->ForceTraversalCookie = FromString<ui64>(value);
-                        SA_LOG_D("[" << Self->TabletID() << "] Loaded traversal cookie: " << value);
-                        break;
-                    }
                     case Schema::SysParam_TraversalTableOwnerId:
                         Self->TraversalTableId.PathId.OwnerId = FromString<ui64>(value);
                         SA_LOG_D("[" << Self->TabletID() << "] Loaded traversal table owner id: "
@@ -90,11 +85,6 @@ struct TStatisticsAggregator::TTxInit : public TTxBase {
                         SA_LOG_D("[" << Self->TabletID() << "] Loaded traversal start time: " << us);
                         break;
                     }
-                    case Schema::SysParam_NextForceTraversalOperationId: {
-                        Self->NextForceTraversalOperationId = FromString<ui64>(value);
-                        SA_LOG_D("[" << Self->TabletID() << "] Loaded next traversal operation id: " << value);
-                        break;
-                    }
                     case Schema::SysParam_TraversalIsColumnTable: {
                         Self->TraversalIsColumnTable = FromString<bool>(value);
                         SA_LOG_D("[" << Self->TabletID() << "] Loaded traversal IsColumnTable: " << value);
@@ -217,7 +207,7 @@ struct TStatisticsAggregator::TTxInit : public TTxBase {
                 ui64 operationId = rowset.GetValue<Schema::ForceTraversals::OperationId>();
                 ui64 ownerId = rowset.GetValue<Schema::ForceTraversals::OwnerId>();
                 ui64 localPathId = rowset.GetValue<Schema::ForceTraversals::LocalPathId>();
-                ui64 cookie = rowset.GetValue<Schema::ForceTraversals::Cookie>();
+                TString cookie = rowset.GetValue<Schema::ForceTraversals::Cookie>();
                 TString columnTags = rowset.GetValue<Schema::ForceTraversals::ColumnTags>();
                 TString types = rowset.GetValue<Schema::ForceTraversals::Types>();