From 412a7a3c1026d734edf9d397cc4e454647089d4e Mon Sep 17 00:00:00 2001 From: ijon Date: Thu, 29 Feb 2024 15:59:47 +0300 Subject: [PATCH] schemeboard: pass describe-result as an opaque payload Cherry-pick 3819aed from main (#2083). Change type of `{TEvUpdate,TEvNotify}.DescribeSchemeResult` from transparent `TEvDescribeSchemeResult` to opaque `bytes` and support that throughout Populator, Replica, Subscriber actors. Properly typed TEvDescribeSchemeResult induce additional overhead to automatically serialize and deserialize this message when transfering over the wire. This performance cost is usually either negligible or imperceptible. But in specific situations, particularly when rapidly updating partitioning information for tables with huge number of shards, this overhead could lead to significant issues. Schemeboard replicas could get overloaded and become unresponsive to further requests. This is problematic, especially considering the schemeboard subsystem's critical role in servicing all databases within a cluster, making it a SPOF. The core realization is that the schemeboard components do not require the full content of a TEvDescribeSchemeResult message to operate efficiently. Instead, only a limited set of fields (path, path-id, version and info about subdomain/database) is required for processing. And a whole TEvDescribeSchemeResult could be passed through as an opaque payload. Type change from TEvDescribeSchemeResult to bytes without changing field number is a safe move. Actual value of the field remains unchanged at the wire protocol level. Thus, older implementations will interpret the payload as a TEvDescribeSchemeResult message and proceed with deserialization as usual. And newer implementations will recognize the data as a binary blob and will deserialize it explicitly only when necessary. KIKIMR-14948 --- ydb/core/protos/flat_tx_scheme.proto | 3 +- ydb/core/protos/scheme_board.proto | 82 +++++- .../datashard_ut_change_exchange.cpp | 43 +-- ydb/core/tx/scheme_board/cache_ut.cpp | 5 + ydb/core/tx/scheme_board/events.cpp | 152 ++++++++++ ydb/core/tx/scheme_board/events.h | 126 +++----- ydb/core/tx/scheme_board/helpers.cpp | 99 +++++-- ydb/core/tx/scheme_board/helpers.h | 21 +- ydb/core/tx/scheme_board/load_test.cpp | 14 +- .../scheme_board/opaque_path_description.cpp | 41 +++ .../tx/scheme_board/opaque_path_description.h | 46 +++ ydb/core/tx/scheme_board/populator.cpp | 96 +++---- ydb/core/tx/scheme_board/populator.h | 2 +- ydb/core/tx/scheme_board/replica.cpp | 268 +++++++++--------- ydb/core/tx/scheme_board/replica_ut.cpp | 49 ++-- ydb/core/tx/scheme_board/subscriber.cpp | 100 +++++-- ydb/core/tx/scheme_board/subscriber_ut.cpp | 2 +- ydb/core/tx/scheme_board/ut_helpers.cpp | 5 +- ydb/core/tx/scheme_board/ya.make | 2 + ydb/core/tx/schemeshard/schemeshard.h | 7 +- .../schemeshard__init_populator.cpp | 4 +- .../schemeshard_path_describer.cpp | 8 +- ydb/core/tx/tx_proxy/describe.cpp | 6 +- ydb/core/viewer/json_describe.h | 2 - ydb/core/viewer/protos/viewer.proto | 2 +- 25 files changed, 786 insertions(+), 399 deletions(-) create mode 100644 ydb/core/tx/scheme_board/events.cpp create mode 100644 ydb/core/tx/scheme_board/opaque_path_description.cpp create mode 100644 ydb/core/tx/scheme_board/opaque_path_description.h diff --git a/ydb/core/protos/flat_tx_scheme.proto b/ydb/core/protos/flat_tx_scheme.proto index 0ee8122cd057..694ee37abd00 100644 --- a/ydb/core/protos/flat_tx_scheme.proto +++ b/ydb/core/protos/flat_tx_scheme.proto @@ -106,12 +106,13 @@ message TEvDescribeSchemeResult { optional string Reason = 2; optional string Path = 3; optional NKikimrSchemeOp.TPathDescription PathDescription = 4; - optional fixed64 PathOwner = 5; + optional fixed64 DEPRECATED_PathOwner = 5; // replaced by PathOwnerId optional fixed64 PathId = 6; optional string LastExistedPrefixPath = 7; optional fixed64 LastExistedPrefixPathId = 8; optional NKikimrSchemeOp.TPathDescription LastExistedPrefixDescription = 9; + optional fixed64 PathOwnerId = 10; } diff --git a/ydb/core/protos/scheme_board.proto b/ydb/core/protos/scheme_board.proto index f063144f5d03..a663fb8e5a59 100644 --- a/ydb/core/protos/scheme_board.proto +++ b/ydb/core/protos/scheme_board.proto @@ -1,4 +1,4 @@ -import "ydb/core/protos/flat_tx_scheme.proto"; +import "ydb/core/scheme/protos/pathid.proto"; package NKikimrSchemeBoard; option java_package = "ru.yandex.kikimr.proto"; @@ -13,22 +13,76 @@ message TEvHandshake { optional uint64 Generation = 2; } -// here and below -// Owner is the tablet id of schemeshard witch holds the records -// LocalPathId is a second part of TPathId -// PathOwnerId is a first part of TPathId +// Here and below. +// Owner is the tablet id of schemeshard which holds the records. +// (PathOwnerId, LocalPathId) constitute TPathId of the object. +// TEvUpdate.DescribeSchemeResultSerialized is a NKikimrScheme.TEvDescribeSchemeResult +// in the form of opaque payload. +// Originally, that field existed as a properly typed TEvDescribeSchemeResult message. +// However, that induce additional overhead to serialize and deserialize this message +// when transferring over wire. +// This performance cost is usually either negligible or imperceptible. +// But in specific situations, particularly when rapidly updating partitioning information +// for tables with huge number of shards, this overhead could lead to significant issues. +// Schemeboard replicas could get overloaded and become unresponsive to further requests. +// This is problematic, especially considering the schemeboard subsystem's critical role +// in servicing all databases within a cluster, making it a Single Point of Failure (SPOF). +// +// The core realization is that the schemeboard components do not require the full content of +// a TEvDescribeSchemeResult message to operate efficiently. Instead, only a limited set of +// fields (path, path-id, version and info about subdomain/database) is required for processing. +// And a whole TEvDescribeSchemeResult could be passed through as an opaque payload. +// +// Type change from TEvDescribeSchemeResult to (repeated) bytes without changing field number +// is a safe move. Actual value of the field remains unchanged at the wire-format level. +// Thus, older implementations will interpret the payload as a TEvDescribeSchemeResult message +// and proceed with deserialization as usual. And newer implementations will recognize the data +// as a binary blob and will deserialize it explicitly only when necessary. +// +// Note that the `repeated` label for the `DescribeSchemeResultSerialized` field is essential +// to remain backward-compatible with the previous implementation. This is because even if +// DescribeSchemeResult previously was labeled `optional` but actual value used at +// the wire-format level was (and is) a pack of TEvDescribeSchemeResult messages. +// Automerge of consecutive messages for the same field is a prominent feature of the protobuf. +// Schemeshard use that feature to supply full TEvDescribeSchemeResult as a sequence of +// partially filled TEvDescribeSchemeResult's. +// +// - Path +// - PathOwnerId, LocalPathId +// - PathDirEntryPathVersion +// - PathSubdomainPathId +// - PathAbandonedTenantsSchemeShards +// are taken from the original TEvDescribeSchemeResult (one way or another). +// message TEvUpdate { optional uint64 Owner = 1; optional uint64 Generation = 2; optional TLocalPathIdRange DeletedLocalPathIds = 3; - optional string Path = 4; - optional uint64 LocalPathId = 5; + + optional string Path = 4; // extracted from DescribeSchemeResult.Path + optional uint64 LocalPathId = 5; // extracted from DescribeSchemeResult.PathId + optional bool IsDeletion = 6 [default = false]; - optional NKikimrScheme.TEvDescribeSchemeResult DescribeSchemeResult = 7; + + repeated bytes DescribeSchemeResultSerialized = 7; + optional bool NeedAck = 8 [default = false]; - optional uint64 PathOwnerId = 9; + + optional uint64 PathOwnerId = 9; // extracted from DescribeSchemeResult.PathOwnerId, DescribeSchemeResult.PathDescription.Self.SchemeshardId in order of presence + optional TLocalPathIdRange MigratedLocalPathIds = 10; + + // Explicit values extracted from DescribeSchemeResultSerialized + + // DescribeSchemeResult.PathDescription.Self.PathVersion + optional uint64 PathDirEntryPathVersion = 11; + + // DescribeSchemeResult.PathDescription.DomainDescription.DomainKey + optional NKikimrProto.TPathID PathSubdomainPathId = 13; + + // DescribeSchemeResult.PathDescription.AbandonedTenantsSchemeShards + repeated uint64 PathAbandonedTenantsSchemeShards = 14; } message TEvUpdateAck { @@ -65,6 +119,7 @@ message TEvUnsubscribe { optional uint64 LocalPathId = 3; } + // See comments for TEvUpdate. message TEvNotify { optional string Path = 1; // and/or @@ -72,9 +127,14 @@ message TEvNotify { optional uint64 LocalPathId = 3; // common fields optional bool IsDeletion = 4 [default = false]; - optional NKikimrScheme.TEvDescribeSchemeResult DescribeSchemeResult = 5; - optional uint64 Version = 6; + + optional bytes DescribeSchemeResultSerialized = 5; + + optional uint64 Version = 6; // same as TEvUpdate.PathDirEntryPathVersion optional bool Strong = 7 [default = false]; + + optional NKikimrProto.TPathID PathSubdomainPathId = 8; + repeated uint64 PathAbandonedTenantsSchemeShards = 9; } message TEvNotifyAck { diff --git a/ydb/core/tx/datashard/datashard_ut_change_exchange.cpp b/ydb/core/tx/datashard/datashard_ut_change_exchange.cpp index 601a8c579ec3..9ec786dada11 100644 --- a/ydb/core/tx/datashard/datashard_ut_change_exchange.cpp +++ b/ydb/core/tx/datashard/datashard_ut_change_exchange.cpp @@ -789,7 +789,7 @@ Y_UNIT_TEST_SUITE(Cdc) { static THolder MakeClient(const NYdb::TDriver& driver, const TString& database) { return MakeHolder(driver, NYdb::TCommonClientSettings().Database(database)); } - }; + }; class TTestTopicEnv: public TTestEnv { public: @@ -798,7 +798,7 @@ Y_UNIT_TEST_SUITE(Cdc) { static THolder MakeClient(const NYdb::TDriver& driver, const TString& database) { return MakeHolder(driver, NYdb::NTopic::TTopicClientSettings().Database(database)); } - }; + }; TShardedTableOptions SimpleTable() { return TShardedTableOptions() @@ -1344,7 +1344,7 @@ Y_UNIT_TEST_SUITE(Cdc) { (3, 30); )", R"( DELETE FROM `/Root/Table` WHERE key = 1; - )"}, { + )"}, { R"({"update":{},"key":[1]})", R"({"update":{},"key":[2]})", R"({"update":{},"key":[3]})", @@ -1360,7 +1360,7 @@ Y_UNIT_TEST_SUITE(Cdc) { (3, 30); )", R"( DELETE FROM `/Root/Table` WHERE key = 1; - )"}, { + )"}, { {DebeziumBody("u", nullptr, nullptr), {{"__key", R"({"payload":{"key":1}})"}}}, {DebeziumBody("u", nullptr, nullptr), {{"__key", R"({"payload":{"key":2}})"}}}, {DebeziumBody("u", nullptr, nullptr), {{"__key", R"({"payload":{"key":3}})"}}}, @@ -1376,7 +1376,7 @@ Y_UNIT_TEST_SUITE(Cdc) { (3, 30); )", R"( DELETE FROM `/Root/Table` WHERE key = 1; - )"}, { + )"}, { R"({"update":{"value":10},"key":[1]})", R"({"update":{"value":20},"key":[2]})", R"({"update":{"value":30},"key":[3]})", @@ -1397,7 +1397,7 @@ Y_UNIT_TEST_SUITE(Cdc) { (3, 300); )", R"( DELETE FROM `/Root/Table` WHERE key = 1; - )"}, { + )"}, { R"({"update":{},"newImage":{"value":10},"key":[1]})", R"({"update":{},"newImage":{"value":20},"key":[2]})", R"({"update":{},"newImage":{"value":30},"key":[3]})", @@ -1421,7 +1421,7 @@ Y_UNIT_TEST_SUITE(Cdc) { (3, 300); )", R"( DELETE FROM `/Root/Table` WHERE key = 1; - )"}, { + )"}, { {DebeziumBody("c", nullptr, R"({"key":1,"value":10})"), {{"__key", R"({"payload":{"key":1}})"}}}, {DebeziumBody("c", nullptr, R"({"key":2,"value":20})"), {{"__key", R"({"payload":{"key":2}})"}}}, {DebeziumBody("c", nullptr, R"({"key":3,"value":30})"), {{"__key", R"({"payload":{"key":3}})"}}}, @@ -1445,7 +1445,7 @@ Y_UNIT_TEST_SUITE(Cdc) { (3, 300); )", R"( DELETE FROM `/Root/Table` WHERE key = 1; - )"}, { + )"}, { {DebeziumBody("u", nullptr, nullptr), {{"__key", R"({"payload":{"key":1}})"}}}, {DebeziumBody("u", nullptr, nullptr), {{"__key", R"({"payload":{"key":2}})"}}}, {DebeziumBody("u", nullptr, nullptr), {{"__key", R"({"payload":{"key":3}})"}}}, @@ -1456,7 +1456,7 @@ Y_UNIT_TEST_SUITE(Cdc) { }); } - Y_UNIT_TEST(NewImageLogDebezium) { + Y_UNIT_TEST(NewImageLogDebezium) { TopicRunner::Read(SimpleTable(), NewImage(NKikimrSchemeOp::ECdcStreamFormatDebeziumJson), {R"( UPSERT INTO `/Root/Table` (key, value) VALUES (1, 10), @@ -1469,7 +1469,7 @@ Y_UNIT_TEST_SUITE(Cdc) { (3, 300); )", R"( DELETE FROM `/Root/Table` WHERE key = 1; - )"}, { + )"}, { {DebeziumBody("u", nullptr, R"({"key":1,"value":10})"), {{"__key", R"({"payload":{"key":1}})"}}}, {DebeziumBody("u", nullptr, R"({"key":2,"value":20})"), {{"__key", R"({"payload":{"key":2}})"}}}, {DebeziumBody("u", nullptr, R"({"key":3,"value":30})"), {{"__key", R"({"payload":{"key":3}})"}}}, @@ -1486,7 +1486,7 @@ Y_UNIT_TEST_SUITE(Cdc) { (1, 10), (2, 20), (3, 30); - )"}, { + )"}, { R"({"update":{},"key":[1],"ts":"***"})", R"({"update":{},"key":[2],"ts":"***"})", R"({"update":{},"key":[3],"ts":"***"})", @@ -1512,7 +1512,7 @@ Y_UNIT_TEST_SUITE(Cdc) { UPSERT INTO `/Root/Table` (__Hash, id_shard, id_sort, __RowData) VALUES ( 1, "10", "100", JsonDocument('{"M":{"color":{"S":"pink"},"weight":{"N":"4.5"}}}') ); - )"}, { + )"}, { WriteJson(NJson::TJsonMap({ {"awsRegion", ""}, {"dynamodb", NJson::TJsonMap({ @@ -1541,7 +1541,7 @@ Y_UNIT_TEST_SUITE(Cdc) { ); )", R"( DELETE FROM `/Root/Table` WHERE __Hash = 1; - )"}, { + )"}, { WriteJson(NJson::TJsonMap({ {"awsRegion", ""}, {"dynamodb", NJson::TJsonMap({ @@ -1639,7 +1639,7 @@ Y_UNIT_TEST_SUITE(Cdc) { (1, 0.0%s/0.0%s), (2, 1.0%s/0.0%s), (3, -1.0%s/0.0%s); - )", s, s, s, s, s, s)}, { + )", s, s, s, s, s, s)}, { R"({"update":{"value":"nan"},"key":[1]})", R"({"update":{"value":"inf"},"key":[2]})", R"({"update":{"value":"-inf"},"key":[3]})", @@ -1674,7 +1674,7 @@ Y_UNIT_TEST_SUITE(Cdc) { TopicRunner::Read(table, KeysOnly(NKikimrSchemeOp::ECdcStreamFormatDebeziumJson), {Sprintf(R"( UPSERT INTO `/Root/Table` (key, value) VALUES ("%s", 1); - )", key.c_str())}, { + )", key.c_str())}, { {DebeziumBody("u", nullptr, nullptr), {{"__key", Sprintf(R"({"payload":{"key":"%s"}})", key.c_str())}}}, }); } @@ -2043,7 +2043,7 @@ Y_UNIT_TEST_SUITE(Cdc) { ExecSQL(env.GetServer(), env.GetEdgeActor(), R"( UPSERT INTO `/Root/TableAux` (key, value) VALUES (1, 10); - )"); + )"); SetSplitMergePartCountLimit(&runtime, -1); const auto tabletIds = GetTableShards(env.GetServer(), env.GetEdgeActor(), "/Root/Table"); @@ -2292,7 +2292,7 @@ Y_UNIT_TEST_SUITE(Cdc) { auto tabletIds = GetTableShards(env.GetServer(), env.GetEdgeActor(), "/Root/Table"); UNIT_ASSERT_VALUES_EQUAL(tabletIds.size(), 1); - WaitTxNotification(env.GetServer(), env.GetEdgeActor(), + WaitTxNotification(env.GetServer(), env.GetEdgeActor(), AsyncSplitTable(env.GetServer(), env.GetEdgeActor(), "/Root/Table", tabletIds.at(0), 4)); // execute on old partitions @@ -2376,7 +2376,8 @@ Y_UNIT_TEST_SUITE(Cdc) { case TSchemeBoardEvents::EvUpdate: if (auto* msg = ev->Get()) { - const auto desc = msg->GetRecord().GetDescribeSchemeResult(); + NKikimrScheme::TEvDescribeSchemeResult desc; + Y_ABORT_UNLESS(ParseFromStringNoSizeLimit(desc, *msg->GetRecord().GetDescribeSchemeResultSerialized().begin())); if (desc.GetPath() == "/Root/Table/Stream" && desc.GetPathDescription().GetSelf().GetCreateFinished()) { delayed.emplace_back(ev.Release()); return TTestActorRuntime::EEventAction::DROP; @@ -2446,7 +2447,7 @@ Y_UNIT_TEST_SUITE(Cdc) { ExecSQL(env.GetServer(), env.GetEdgeActor(), R"( UPSERT INTO `/Root/Table` (key, value) VALUES (1, 10); - )"); + )"); SetSplitMergePartCountLimit(&runtime, -1); const auto tabletIds = GetTableShards(env.GetServer(), env.GetEdgeActor(), "/Root/Table"); @@ -3266,7 +3267,7 @@ Y_UNIT_TEST_SUITE(Cdc) { auto tabletIds = GetTableShards(env.GetServer(), env.GetEdgeActor(), "/Root/Table"); UNIT_ASSERT_VALUES_EQUAL(tabletIds.size(), 1); - WaitTxNotification(env.GetServer(), env.GetEdgeActor(), + WaitTxNotification(env.GetServer(), env.GetEdgeActor(), AsyncSplitTable(env.GetServer(), env.GetEdgeActor(), "/Root/Table", tabletIds.at(0), 4)); // merge @@ -3298,7 +3299,7 @@ template <> void Out>(IOutputStream& output, const std::pair& x) { output << x.first << ":" << x.second; } - + void AppendToString(TString& dst, const std::pair& x) { TStringOutput output(dst); output << x; diff --git a/ydb/core/tx/scheme_board/cache_ut.cpp b/ydb/core/tx/scheme_board/cache_ut.cpp index 745fdf839b2a..7fc37dffbcb9 100644 --- a/ydb/core/tx/scheme_board/cache_ut.cpp +++ b/ydb/core/tx/scheme_board/cache_ut.cpp @@ -37,6 +37,11 @@ class TCacheTest: public TTestWithSchemeshard { " Kind: \"pool-kind-1\" " "} " " Name: \"Root\" "); + + // Context->SetLogPriority(NKikimrServices::SCHEME_BOARD_REPLICA, NLog::PRI_DEBUG); + // Context->SetLogPriority(NKikimrServices::SCHEME_BOARD_SUBSCRIBER, NLog::PRI_DEBUG); + // Context->SetLogPriority(NKikimrServices::TX_PROXY_SCHEME_CACHE, NLog::PRI_DEBUG); + // Context->SetLogPriority(NKikimrServices::FLAT_TX_SCHEMESHARD, NLog::PRI_DEBUG); } UNIT_TEST_SUITE(TCacheTest); diff --git a/ydb/core/tx/scheme_board/events.cpp b/ydb/core/tx/scheme_board/events.cpp new file mode 100644 index 000000000000..ba721e3790b6 --- /dev/null +++ b/ydb/core/tx/scheme_board/events.cpp @@ -0,0 +1,152 @@ +#include + +#include "helpers.h" +#include "opaque_path_description.h" +#include "events.h" + + +namespace NKikimr { + +using namespace NSchemeBoard; + +namespace { + +//NOTE: MakeOpaquePathDescription moves out DescribeSchemeResultSerialized from TEvUpdate message. +// It cannot be used twice on the same message. +// +// This code would be much simpler without backward compatibility support. +// Consider removing compatibility support at version stable-25-1. +TOpaquePathDescription MakeOpaquePathDescription(NKikimrSchemeBoard::TEvUpdate& update) { + // PathSubdomainPathId's absence is a marker that input message was sent + // from the older populator implementation + + // Move out elements out of the repeated field. + // Mark field as empty to prevent subsequent extraction attempts + TString data; + auto* field = update.MutableDescribeSchemeResultSerialized(); + if (field->size() == 1) { + data = std::move(*field->begin()); + } else { + data = JoinRange("", field->begin(), field->end()); + } + field->Clear(); + + if (update.HasPathSubdomainPathId()) { + return TOpaquePathDescription{ + .DescribeSchemeResultSerialized = std::move(data), + //NOTE: unsuccessful describe results cannot be here, by design + .Status = NKikimrScheme::StatusSuccess, + .PathId = TPathId(update.GetPathOwnerId(), update.GetLocalPathId()), + .Path = update.GetPath(), + .PathVersion = update.GetPathDirEntryPathVersion(), + .SubdomainPathId = PathIdFromPathId(update.GetPathSubdomainPathId()), + .PathAbandonedTenantsSchemeShards = TSet( + update.GetPathAbandonedTenantsSchemeShards().begin(), + update.GetPathAbandonedTenantsSchemeShards().end() + ) + }; + + } else { + google::protobuf::Arena arena; + const auto* proto = DeserializeDescribeSchemeResult(data, &arena); + + return TOpaquePathDescription{ + .DescribeSchemeResultSerialized = std::move(data), + //NOTE: unsuccessful describe results cannot be here, by design + .Status = NKikimrScheme::StatusSuccess, + .PathId = NSchemeBoard::GetPathId(*proto), + .Path = proto->GetPath(), + .PathVersion = NSchemeBoard::GetPathVersion(*proto), + .SubdomainPathId = NSchemeBoard::GetDomainId(*proto), + .PathAbandonedTenantsSchemeShards = NSchemeBoard::GetAbandonedSchemeShardIds(*proto) + }; + } +} + +} // anonymous namespace + +// TSchemeBoardEvents::TEvUpdate +// + +TOpaquePathDescription TSchemeBoardEvents::TEvUpdate::ExtractPathDescription() { + return MakeOpaquePathDescription(Record); +} + +// TSchemeBoardEvents::TEvUpdateBuilder +// + +TSchemeBoardEvents::TEvUpdateBuilder::TEvUpdateBuilder(const ui64 owner, const ui64 generation) { + Record.SetOwner(owner); + Record.SetGeneration(generation); +} + +TSchemeBoardEvents::TEvUpdateBuilder::TEvUpdateBuilder(const ui64 owner, const ui64 generation, const TPathId& pathId) { + Record.SetOwner(owner); + Record.SetGeneration(generation); + Record.SetPathOwnerId(pathId.OwnerId); + Record.SetLocalPathId(pathId.LocalPathId); + Record.SetIsDeletion(true); +} + +TSchemeBoardEvents::TEvUpdateBuilder::TEvUpdateBuilder( + const ui64 owner, + const ui64 generation, + const TOpaquePathDescription& pathDescription, + const bool isDeletion +) { + Record.SetOwner(owner); + Record.SetGeneration(generation); + Record.SetIsDeletion(isDeletion); + + Record.SetPath(pathDescription.Path); + Record.SetPathOwnerId(pathDescription.PathId.OwnerId); + Record.SetLocalPathId(pathDescription.PathId.LocalPathId); + + Record.SetPathDirEntryPathVersion(pathDescription.PathVersion); + PathIdFromPathId(pathDescription.SubdomainPathId, Record.MutablePathSubdomainPathId()); + + Record.MutablePathAbandonedTenantsSchemeShards()->Assign( + pathDescription.PathAbandonedTenantsSchemeShards.begin(), + pathDescription.PathAbandonedTenantsSchemeShards.end() + ); +} + +void TSchemeBoardEvents::TEvUpdateBuilder::SetDescribeSchemeResultSerialized(const TString& serialized) { + Record.AddDescribeSchemeResultSerialized(serialized); +} + +void TSchemeBoardEvents::TEvUpdateBuilder::SetDescribeSchemeResultSerialized(TString&& serialized) { + Record.AddDescribeSchemeResultSerialized(std::move(serialized)); +} + +// TSchemeBoardEvents::TEvNotifyBuilder +// + +TSchemeBoardEvents::TEvNotifyBuilder::TEvNotifyBuilder(const TString& path, const bool isDeletion /*= false*/) { + Record.SetPath(path); + Record.SetIsDeletion(isDeletion); +} + +TSchemeBoardEvents::TEvNotifyBuilder::TEvNotifyBuilder(const TPathId& pathId, const bool isDeletion /*= false*/) { + Record.SetPathOwnerId(pathId.OwnerId); + Record.SetLocalPathId(pathId.LocalPathId); + Record.SetIsDeletion(isDeletion); +} + +TSchemeBoardEvents::TEvNotifyBuilder::TEvNotifyBuilder(const TString& path, const TPathId& pathId, const bool isDeletion /*= false*/) { + Record.SetPath(path); + Record.SetPathOwnerId(pathId.OwnerId); + Record.SetLocalPathId(pathId.LocalPathId); + Record.SetIsDeletion(isDeletion); +} + +void TSchemeBoardEvents::TEvNotifyBuilder::SetPathDescription(const TOpaquePathDescription& pathDescription) { + Record.SetDescribeSchemeResultSerialized(pathDescription.DescribeSchemeResultSerialized); + PathIdFromPathId(pathDescription.SubdomainPathId, Record.MutablePathSubdomainPathId()); + Record.MutablePathAbandonedTenantsSchemeShards()->Assign( + pathDescription.PathAbandonedTenantsSchemeShards.begin(), + pathDescription.PathAbandonedTenantsSchemeShards.end() + ); +} + +} // NKikimr diff --git a/ydb/core/tx/scheme_board/events.h b/ydb/core/tx/scheme_board/events.h index 8e686e8b92d3..0c00d939b0da 100644 --- a/ydb/core/tx/scheme_board/events.h +++ b/ydb/core/tx/scheme_board/events.h @@ -2,7 +2,7 @@ #include "defs.h" #include "helpers.h" -#include "two_part_description.h" +#include "opaque_path_description.h" #include #include @@ -14,6 +14,8 @@ namespace NKikimr { +using namespace NSchemeBoard; + struct TSchemeBoardEvents { using TDescribeSchemeResult = NKikimrScheme::TEvDescribeSchemeResult; @@ -114,7 +116,7 @@ struct TSchemeBoardEvents { const TLocalPathId DeletedPathBegin = 0; // The points are inclusive const TLocalPathId DeletedPathEnd = 0; // [DeletedPathBegin; DeletedPathEnd] const TLocalPathId MigratedPathId = InvalidLocalPathId; - NSchemeBoard::TTwoPartDescription Description; + TOpaquePathDescription Description; TEvDescribeResult() = default; @@ -132,7 +134,7 @@ struct TSchemeBoardEvents { explicit TEvDescribeResult( TLocalPathId deletedPathBegin, TLocalPathId deletedPathEnd, - const NSchemeBoard::TTwoPartDescription& description) + const TOpaquePathDescription& description) : Commit(false) , DeletedPathBegin(deletedPathBegin) , DeletedPathEnd(deletedPathEnd) @@ -159,16 +161,24 @@ struct TSchemeBoardEvents { } bool HasDescription() const { - return Description; + return !Description.IsEmpty(); } TString ToString() const override { - return TStringBuilder() << ToStringHeader() << " {" + auto builder = TStringBuilder() << ToStringHeader() << " {" << " Commit: " << (Commit ? "true" : "false") << " DeletedPathBegin: " << DeletedPathBegin << " DeletedPathEnd: " << DeletedPathEnd - << " msg: " << Description.Record.ShortDebugString() - << " }"; + ; + if (HasDescription()) { + builder << " { Path: " << Description.Path + << " PathId: " << Description.PathId + << " PathVersion: " << Description.PathVersion + << " }" + ; + } + builder << " }"; + return builder; } }; @@ -219,6 +229,9 @@ struct TSchemeBoardEvents { struct TEvUpdate: public TEventPreSerializedPB { TEvUpdate() = default; + TString GetPath() const { + return Record.GetPath(); + } TPathId GetPathId() const { if (!Record.HasLocalPathId()) { return TPathId(); @@ -230,6 +243,8 @@ struct TSchemeBoardEvents { ); } + TOpaquePathDescription ExtractPathDescription(); + TString ToString() const override { return PrintOwnerGeneration(this, Record); } @@ -240,58 +255,17 @@ struct TSchemeBoardEvents { TEvUpdateBuilder() = default; - explicit TEvUpdateBuilder(const ui64 owner, const ui64 generation) { - Record.SetOwner(owner); - Record.SetGeneration(generation); - } - - explicit TEvUpdateBuilder(const ui64 owner, const ui64 generation, const TPathId& pathId) { - Record.SetOwner(owner); - Record.SetGeneration(generation); - Record.SetPathOwnerId(pathId.OwnerId); - Record.SetLocalPathId(pathId.LocalPathId); - Record.SetIsDeletion(true); - } - + explicit TEvUpdateBuilder(const ui64 owner, const ui64 generation); + explicit TEvUpdateBuilder(const ui64 owner, const ui64 generation, const TPathId& pathId); explicit TEvUpdateBuilder( const ui64 owner, const ui64 generation, - const TDescribeSchemeResult& describeSchemeResult, + const TOpaquePathDescription& pathDescription, const bool isDeletion = false - ) { - Record.SetOwner(owner); - Record.SetGeneration(generation); - if (describeSchemeResult.HasPath()) { - Record.SetPath(describeSchemeResult.GetPath()); - } - if (describeSchemeResult.HasPathDescription() - && describeSchemeResult.GetPathDescription().HasSelf()) { - Record.SetPathOwnerId(describeSchemeResult.GetPathDescription().GetSelf().GetSchemeshardId()); - } - Record.SetLocalPathId(describeSchemeResult.GetPathId()); - if (describeSchemeResult.HasPathOwnerId()) { - Record.SetPathOwnerId(describeSchemeResult.GetPathOwnerId()); - } - Record.SetIsDeletion(isDeletion); - } - - void SetDescribeSchemeResult(TString preSerialized) { - PreSerializedData = NSchemeBoard::PreSerializedProtoField( - std::move(preSerialized), Record.kDescribeSchemeResultFieldNumber); - } + ); - void SetDescribeSchemeResult(TDescribeSchemeResult describeSchemeResult) { - *Record.MutableDescribeSchemeResult() = std::move(describeSchemeResult); - } - - void SetDescribeSchemeResult(TString preSerialized, TDescribeSchemeResult describeSchemeResult) { - SetDescribeSchemeResult(std::move(preSerialized)); - SetDescribeSchemeResult(std::move(describeSchemeResult)); - } - - void SetDescribeSchemeResult(NSchemeBoard::TTwoPartDescription twoPart) { - SetDescribeSchemeResult(std::move(twoPart.PreSerialized), std::move(twoPart.Record)); - } + void SetDescribeSchemeResultSerialized(const TString& serialized); + void SetDescribeSchemeResultSerialized(TString&& serialized); }; struct TEvUpdateAck: public TEventPB { @@ -398,6 +372,21 @@ struct TSchemeBoardEvents { } }; + template + static TStringBuilder& PrintPathVersion(TStringBuilder& out, const T& record) { + return out + << " Version: " << NSchemeBoard::GetPathVersion(record) + ; + } + + template <> + TString PrintPath(const IEventBase* ev, const NKikimrSchemeBoard::TEvNotify& record) { + auto out = TStringBuilder() << ev->ToStringHeader() << " {"; + PrintPath(out, record); + PrintPathVersion(out, record); + return out << " }"; + } + struct TEvNotify: public TEventPreSerializedPB { TEvNotify() = default; @@ -411,32 +400,11 @@ struct TSchemeBoardEvents { TEvNotifyBuilder() = default; - explicit TEvNotifyBuilder(const TString& path, const bool isDeletion = false) { - Record.SetPath(path); - Record.SetIsDeletion(isDeletion); - } + explicit TEvNotifyBuilder(const TString& path, const bool isDeletion = false); + explicit TEvNotifyBuilder(const TPathId& pathId, const bool isDeletion = false); + explicit TEvNotifyBuilder(const TString& path, const TPathId& pathId, const bool isDeletion = false); - explicit TEvNotifyBuilder(const TPathId& pathId, const bool isDeletion = false) { - Record.SetPathOwnerId(pathId.OwnerId); - Record.SetLocalPathId(pathId.LocalPathId); - Record.SetIsDeletion(isDeletion); - } - - explicit TEvNotifyBuilder( - const TString& path, - const TPathId& pathId, - const bool isDeletion = false - ) { - Record.SetPath(path); - Record.SetPathOwnerId(pathId.OwnerId); - Record.SetLocalPathId(pathId.LocalPathId); - Record.SetIsDeletion(isDeletion); - } - - void SetDescribeSchemeResult(const TString& preSerialized) { - PreSerializedData = NSchemeBoard::PreSerializedProtoField( - preSerialized, Record.kDescribeSchemeResultFieldNumber); - } + void SetPathDescription(const TOpaquePathDescription& pathDescription); }; struct TEvNotifyAck: public TEventPB { diff --git a/ydb/core/tx/scheme_board/helpers.cpp b/ydb/core/tx/scheme_board/helpers.cpp index 7505f1c39f28..89cbaf614cc2 100644 --- a/ydb/core/tx/scheme_board/helpers.cpp +++ b/ydb/core/tx/scheme_board/helpers.cpp @@ -1,13 +1,10 @@ #include "helpers.h" -#include -#include -#include +#include -#include -#include +#include -#include +#include namespace NKikimr { namespace NSchemeBoard { @@ -16,6 +13,9 @@ TActorId MakeInterconnectProxyId(const ui32 nodeId) { return TActivationContext::InterconnectProxy(nodeId); } +// NKikimrScheme::TEvDescribeSchemeResult +// + ui64 GetPathVersion(const NKikimrScheme::TEvDescribeSchemeResult& record) { if (!record.HasPathDescription()) { return 0; @@ -34,6 +34,10 @@ ui64 GetPathVersion(const NKikimrScheme::TEvDescribeSchemeResult& record) { return self.GetPathVersion(); } +// Object path-id is determined from TDescribeSchemeResult with backward compatibility support. +// DescribeSchemeResult.PathOwnerId should be used. +// DescribeSchemeResult.PathDescription.Self.SchemeshardId is deprecated. +// DescribeSchemeResult.PathOwnerId takes preference. TPathId GetPathId(const NKikimrScheme::TEvDescribeSchemeResult &record) { if (record.HasPathId() && record.HasPathOwnerId()) { return TPathId(record.GetPathOwnerId(), record.GetPathId()); @@ -81,6 +85,40 @@ TSet GetAbandonedSchemeShardIds(const NKikimrScheme::TEvDescribeSchemeResu ); } +// NKikimrSchemeBoard::TEvUpdate +// + +ui64 GetPathVersion(const NKikimrSchemeBoard::TEvUpdate& record) { + return record.GetPathDirEntryPathVersion(); +} + +TSet GetAbandonedSchemeShardIds(const NKikimrSchemeBoard::TEvUpdate& record) { + return TSet( + record.GetPathAbandonedTenantsSchemeShards().begin(), + record.GetPathAbandonedTenantsSchemeShards().end() + ); +} + +// NKikimrSchemeBoard::TEvNotify +// + +ui64 GetPathVersion(const NKikimrSchemeBoard::TEvNotify& record) { + return record.GetVersion(); +} + +NSchemeBoard::TDomainId GetDomainId(const NKikimrSchemeBoard::TEvNotify& record) { + return PathIdFromPathId(record.GetPathSubdomainPathId()); +} + +TSet GetAbandonedSchemeShardIds(const NKikimrSchemeBoard::TEvNotify& record) { + return TSet( + record.GetPathAbandonedTenantsSchemeShards().begin(), + record.GetPathAbandonedTenantsSchemeShards().end() + ); +} + +// Assorted methods +// TIntrusivePtr SerializeEvent(IEventBase* ev) { TAllocChunkSerializer serializer; Y_ABORT_UNLESS(ev->SerializeToArcadiaStream(&serializer)); @@ -96,21 +134,44 @@ void MultiSend(const TVector& recipients, const TActorId& sende } } -TString PreSerializedProtoField(TString data, int fieldNo) { - using CodedOutputStream = google::protobuf::io::CodedOutputStream; - using StringOutputStream = google::protobuf::io::StringOutputStream; - using WireFormat = google::protobuf::internal::WireFormatLite; +// Work with TEvDescribeSchemeResult and its parts +// - TString key; - { - StringOutputStream stream(&key); - CodedOutputStream output(&stream); - WireFormat::WriteTag(fieldNo, WireFormat::WireType::WIRETYPE_LENGTH_DELIMITED, &output); - output.WriteVarint32(data.size()); - } +TString SerializeDescribeSchemeResult(const NKikimrScheme::TEvDescribeSchemeResult& proto) { + TString serialized; + Y_PROTOBUF_SUPPRESS_NODISCARD proto.SerializeToString(&serialized); + return serialized; +} + +TString SerializeDescribeSchemeResult(const TString& preSerializedPart, const NKikimrScheme::TEvDescribeSchemeResult& protoPart) { + return Join("", preSerializedPart, SerializeDescribeSchemeResult(protoPart)); +} + +NKikimrScheme::TEvDescribeSchemeResult DeserializeDescribeSchemeResult(const TString& serialized) { + NKikimrScheme::TEvDescribeSchemeResult result; + Y_ABORT_UNLESS(ParseFromStringNoSizeLimit(result, serialized)); + return result; +} + +NKikimrScheme::TEvDescribeSchemeResult* DeserializeDescribeSchemeResult(const TString& serialized, google::protobuf::Arena* arena) { + auto* proto = google::protobuf::Arena::CreateMessage(arena); + Y_ABORT_UNLESS(ParseFromStringNoSizeLimit(*proto, serialized)); + return proto; +} + +TString JsonFromDescribeSchemeResult(const TString& serialized) { + using namespace google::protobuf::util; + + google::protobuf::Arena arena; + const auto* proto = DeserializeDescribeSchemeResult(serialized, &arena); + + JsonPrintOptions opts; + opts.preserve_proto_field_names = true; + + TString json; + MessageToJsonString(*proto, &json, opts); - data.prepend(key); - return data; + return json; } } // NSchemeBoard diff --git a/ydb/core/tx/scheme_board/helpers.h b/ydb/core/tx/scheme_board/helpers.h index f9c5e9c378b1..a00fb1f44354 100644 --- a/ydb/core/tx/scheme_board/helpers.h +++ b/ydb/core/tx/scheme_board/helpers.h @@ -4,6 +4,7 @@ #include #include +#include #include @@ -34,12 +35,20 @@ namespace NSchemeBoard { using TDomainId = TPathId; TActorId MakeInterconnectProxyId(const ui32 nodeId); + +// Extract metainfo directly from DescribeSchemeResult message ui64 GetPathVersion(const NKikimrScheme::TEvDescribeSchemeResult& record); TPathId GetPathId(const NKikimrScheme::TEvDescribeSchemeResult& record); TDomainId GetDomainId(const NKikimrScheme::TEvDescribeSchemeResult& record); -TSet GetAbandonedSchemeShardIds(const NKikimrScheme::TEvDescribeSchemeResult &record); +TSet GetAbandonedSchemeShardIds(const NKikimrScheme::TEvDescribeSchemeResult& record); + +// Extract metainfo from TEvUpdate message +ui64 GetPathVersion(const NKikimrSchemeBoard::TEvUpdate& record); -TIntrusivePtr SerializeEvent(IEventBase* ev); +// Extract metainfo from TEvNotify message +ui64 GetPathVersion(const NKikimrSchemeBoard::TEvNotify& record); +NSchemeBoard::TDomainId GetDomainId(const NKikimrSchemeBoard::TEvNotify& record); +TSet GetAbandonedSchemeShardIds(const NKikimrSchemeBoard::TEvNotify& record); void MultiSend(const TVector& recipients, const TActorId& sender, TAutoPtr ev, ui32 flags = 0, ui64 cookie = 0); @@ -48,8 +57,12 @@ void MultiSend(const TVector& recipients, const TActorId& sende MultiSend(recipients, sender, static_cast(ev.Release()), flags, cookie); } -// Only string or embed message fields are supported (it used wire type = 2 internally) -TString PreSerializedProtoField(TString data, int fieldNo); +// Work with TEvDescribeSchemeResult and its parts +TString SerializeDescribeSchemeResult(const NKikimrScheme::TEvDescribeSchemeResult& proto); +TString SerializeDescribeSchemeResult(const TString& preSerializedPart, const NKikimrScheme::TEvDescribeSchemeResult& protoPart); +NKikimrScheme::TEvDescribeSchemeResult DeserializeDescribeSchemeResult(const TString& serialized); +NKikimrScheme::TEvDescribeSchemeResult* DeserializeDescribeSchemeResult(const TString& serialized, google::protobuf::Arena* arena); +TString JsonFromDescribeSchemeResult(const TString& serialized); } // NSchemeBoard } // NKikimr diff --git a/ydb/core/tx/scheme_board/load_test.cpp b/ydb/core/tx/scheme_board/load_test.cpp index 1e2b0f6a88f3..48f3999cc7c4 100644 --- a/ydb/core/tx/scheme_board/load_test.cpp +++ b/ydb/core/tx/scheme_board/load_test.cpp @@ -57,14 +57,14 @@ class TLoadProducer: public TActorBootstrapped { TDescription& dirDesc = dirDescTwoPart.Record; dirDesc.SetStatus(NKikimrScheme::StatusSuccess); - dirDesc.SetPathOwner(owner); + dirDesc.SetPathOwnerId(owner); dirDesc.SetPathId(nextPathId++); dirDesc.SetPath(dirPath); auto& dirSelf = *dirDesc.MutablePathDescription()->MutableSelf(); dirSelf.SetName(dirName); dirSelf.SetPathId(dirDesc.GetPathId()); - dirSelf.SetSchemeshardId(dirDesc.GetPathOwner()); + dirSelf.SetSchemeshardId(dirDesc.GetPathOwnerId()); dirSelf.SetPathType(NKikimrSchemeOp::EPathTypeDir); dirSelf.SetCreateFinished(true); dirSelf.SetCreateTxId(1); @@ -84,7 +84,7 @@ class TLoadProducer: public TActorBootstrapped { TDescription& objDesc = objDescTwoPart.Record; objDesc.SetStatus(NKikimrScheme::StatusSuccess); - objDesc.SetPathOwner(owner); + objDesc.SetPathOwnerId(owner); objDesc.SetPathId(nextPathId++); objDesc.SetPath(objPath); @@ -94,7 +94,7 @@ class TLoadProducer: public TActorBootstrapped { dirChildren.Add()->CopyFrom(objSelf); objSelf.SetPathId(objDesc.GetPathId()); - objSelf.SetSchemeshardId(objDesc.GetPathOwner()); + objSelf.SetSchemeshardId(objDesc.GetPathOwnerId()); objSelf.SetCreateFinished(true); objSelf.SetCreateTxId(1); objSelf.SetCreateStep(1); @@ -194,7 +194,7 @@ class TLoadProducer: public TActorBootstrapped { Descriptions = GenerateDescriptions(Owner, Config, NextPathId); Populator = Register(CreateSchemeBoardPopulator( - Owner, Max(), ssId, Descriptions, NextPathId + Owner, Max(), ssId, std::vector>(Descriptions.begin(), Descriptions.end()), NextPathId )); TPathId pathId(Owner, NextPathId - 1); @@ -236,7 +236,7 @@ class TLoadProducer: public TActorBootstrapped { Send(Subscriber, new TEvents::TEvPoisonPill()); Subscriber = TActorId(); - const TInstant ts = TInstant::FromValue(GetPathVersion(msg->DescribeSchemeResult)); + const TInstant ts = TInstant::FromValue(NSchemeBoard::GetPathVersion(msg->DescribeSchemeResult)); *SyncDuration = (TlsActivationContext->Now() - ts).MilliSeconds(); Test(); @@ -352,7 +352,7 @@ class TLoadConsumer: public TActorBootstrapped { void Handle(TSchemeBoardEvents::TEvNotifyUpdate::TPtr& ev) { const auto* msg = ev->Get(); - const TInstant ts = TInstant::FromValue(GetPathVersion(msg->DescribeSchemeResult)); + const TInstant ts = TInstant::FromValue(NSchemeBoard::GetPathVersion(msg->DescribeSchemeResult)); if (IsDir(msg->DescribeSchemeResult)) { LatencyDir->Collect((TlsActivationContext->Now() - ts).MilliSeconds()); } else { diff --git a/ydb/core/tx/scheme_board/opaque_path_description.cpp b/ydb/core/tx/scheme_board/opaque_path_description.cpp new file mode 100644 index 000000000000..a78a0dc58877 --- /dev/null +++ b/ydb/core/tx/scheme_board/opaque_path_description.cpp @@ -0,0 +1,41 @@ +#include "helpers.h" +#include "two_part_description.h" +#include "opaque_path_description.h" + +namespace NKikimr::NSchemeBoard { + +bool TOpaquePathDescription::IsEmpty() const { + return DescribeSchemeResultSerialized.empty(); +} + +TString TOpaquePathDescription::ToString() const { + // fields are reordered to be more useful to people reading logs + return TStringBuilder() << "{" + << "Status " << Status + << ", Path " << Path + << ", PathId " << PathId + << ", PathVersion " << PathVersion + << ", SubdomainPathId " << SubdomainPathId + << ", PathAbandonedTenantsSchemeShards size " << PathAbandonedTenantsSchemeShards.size() + << ", DescribeSchemeResultSerialized size " << DescribeSchemeResultSerialized.size() + << "}" + ; +} + +TOpaquePathDescription MakeOpaquePathDescription(const TString& preSerializedPart, const NKikimrScheme::TEvDescribeSchemeResult& protoPart) { + return TOpaquePathDescription{ + .DescribeSchemeResultSerialized = SerializeDescribeSchemeResult(preSerializedPart, protoPart), + .Status = protoPart.GetStatus(), + .PathId = GetPathId(protoPart), + .Path = protoPart.GetPath(), + .PathVersion = GetPathVersion(protoPart), + .SubdomainPathId = GetDomainId(protoPart), + .PathAbandonedTenantsSchemeShards = GetAbandonedSchemeShardIds(protoPart), + }; +} + +TOpaquePathDescription MakeOpaquePathDescription(const TTwoPartDescription& twoPartDescription) { + return MakeOpaquePathDescription(twoPartDescription.PreSerialized, twoPartDescription.Record); +} + +} // NKikimr::NSchemeBoard diff --git a/ydb/core/tx/scheme_board/opaque_path_description.h b/ydb/core/tx/scheme_board/opaque_path_description.h new file mode 100644 index 000000000000..384d7c2ac25e --- /dev/null +++ b/ydb/core/tx/scheme_board/opaque_path_description.h @@ -0,0 +1,46 @@ +#pragma once + +#include +#include + +#include +#include + + +namespace NKikimr::NSchemeBoard { + +struct TOpaquePathDescription { + // Serialized TEvDescribeSchemeResult + TString DescribeSchemeResultSerialized; + + // Explicit values extracted from original TEvDescribeSchemeResult + + // TEvDescribeSchemeResult.Status, invalid status value by default + NKikimrScheme::EStatus Status = NKikimrScheme::EStatus_MAX; + + // TEvDescribeSchemeResult.(PathOwnerId,PathId) or TEvDescribeSchemeResult.PathDescription.Self.(SchemeshardId,PathId) in order of presence + TPathId PathId; + + // TEvDescribeSchemeResult.Path + TString Path; + + // TEvDescribeSchemeResult.PathDescription.Self.PathVersion + ui64 PathVersion = 0; + + // TEvDescribeSchemeResult.PathDescription.DomainDescription.DomainKey + TPathId SubdomainPathId; + + // TEvDescribeSchemeResult.PathDescription.AbandonedTenantsSchemeShards + TSet PathAbandonedTenantsSchemeShards; + + bool IsEmpty() const; + + TString ToString() const; +}; + +struct TTwoPartDescription; + +TOpaquePathDescription MakeOpaquePathDescription(const TString& preSerializedPart, const NKikimrScheme::TEvDescribeSchemeResult& protoPart); +TOpaquePathDescription MakeOpaquePathDescription(const TTwoPartDescription& twoPartDescription); + +} // NKikimr::NSchemeBoard diff --git a/ydb/core/tx/scheme_board/populator.cpp b/ydb/core/tx/scheme_board/populator.cpp index c0a25fc039dc..28c305aaee09 100644 --- a/ydb/core/tx/scheme_board/populator.cpp +++ b/ydb/core/tx/scheme_board/populator.cpp @@ -1,10 +1,9 @@ #include "events.h" #include "helpers.h" #include "monitorable_actor.h" +#include "opaque_path_description.h" #include "populator.h" -#include - #include #include #include @@ -64,7 +63,7 @@ class TReplicaPopulator: public TMonitorableActor { } auto update = msg->HasDescription() - ? MakeHolder(Owner, Generation, msg->Description.Record) + ? MakeHolder(Owner, Generation, msg->Description) : MakeHolder(Owner, Generation); if (msg->HasDeletedLocalPathIds()) { @@ -77,17 +76,11 @@ class TReplicaPopulator: public TMonitorableActor { } if (msg->HasDescription()) { - auto& record = msg->Description.Record; - - if (!record.HasStatus()) { - SBP_LOG_E("Ignore description without status"); - } else if (record.GetStatus() != NKikimrScheme::StatusSuccess) { - SBP_LOG_E("Ignore description" - << ": status# " << record.GetStatus() - << ", msg# " << record.ShortDebugString()); + if (msg->Description.Status != NKikimrScheme::StatusSuccess) { + SBP_LOG_E("Ignore description: " << msg->Description.ToString()); } else { - CurPathId = GetPathId(record); - update->SetDescribeSchemeResult(std::move(msg->Description)); + CurPathId = msg->Description.PathId; + update->SetDescribeSchemeResultSerialized(std::move(msg->Description.DescribeSchemeResultSerialized)); } } @@ -121,7 +114,7 @@ class TReplicaPopulator: public TMonitorableActor { void EnqueueUpdate(TSchemeBoardEvents::TEvUpdate::TPtr& ev, bool canSend = false) { const TPathId pathId = ev->Get()->GetPathId(); const auto& record = (static_cast(ev->Get()))->Record; - const ui64 version = record.GetIsDeletion() ? Max() : GetPathVersion(record.GetDescribeSchemeResult()); + const ui64 version = record.GetIsDeletion() ? Max() : NSchemeBoard::GetPathVersion(record); if (canSend && UpdatesInFlight.size() < BatchSizeLimit) { bool needSend = true; @@ -509,16 +502,16 @@ class TPopulator: public TMonitorableActor { auto it = Descriptions.find(pathId); Y_ABORT_UNLESS(it != Descriptions.end()); - const auto& record = it->second.Record; + const TOpaquePathDescription& desc = it->second; - TConstArrayRef replicas = SelectReplicas(pathId, record.GetPath()); + TConstArrayRef replicas = SelectReplicas(pathId, desc.Path); for (const auto& replica : replicas) { const TActorId* replicaPopulator = ReplicaToReplicaPopulator.FindPtr(replica); Y_ABORT_UNLESS(replicaPopulator != nullptr); - auto update = MakeHolder(Owner, Generation, record, isDeletion); + auto update = MakeHolder(Owner, Generation, desc, isDeletion); if (!isDeletion) { - update->SetDescribeSchemeResult(it->second); + update->SetDescribeSchemeResultSerialized(desc.DescribeSchemeResultSerialized); } update->Record.SetNeedAck(true); @@ -557,7 +550,8 @@ class TPopulator: public TMonitorableActor { } while (it != Descriptions.end()) { // skip irrelevant to the replica - if (it->second.Record.GetStatus() == NKikimrScheme::StatusPathDoesNotExist) { + const auto& desc = it->second; + if (desc.Status == NKikimrScheme::StatusPathDoesNotExist) { // KIKIMR-13173 // it is assumed that not deleted pathes present in Descriptions // but it might be, since we have the difference at path description and init population @@ -568,8 +562,7 @@ class TPopulator: public TMonitorableActor { continue; } - TPathId pathId(it->second.Record.GetPathOwnerId(), it->second.Record.GetPathId()); - TConstArrayRef replicas = SelectReplicas(pathId, it->second.Record.GetPath()); + TConstArrayRef replicas = SelectReplicas(desc.PathId, desc.Path); if (Find(replicas, replica) != replicas.end()) { break; } @@ -591,17 +584,15 @@ class TPopulator: public TMonitorableActor { } const auto& description = it->second; - const auto& record = description.Record; - auto pathId = TPathId(it->second.Record.GetPathOwnerId(), it->second.Record.GetPathId()); - if (pathId.OwnerId != Owner) { + if (description.PathId.OwnerId != Owner) { // this is an alien migrated migrated path from another owner, push it as a dot Send(replicaPopulator, new TSchemeBoardEvents::TEvDescribeResult(0, 0, description)); return; } TLocalPathId deletedBegin = startPathId.LocalPathId; - TLocalPathId deletedEnd = record.GetPathId() - 1; + TLocalPathId deletedEnd = description.PathId.LocalPathId - 1; if (startPathId.OwnerId != Owner) { deletedBegin = 1; @@ -613,7 +604,7 @@ class TPopulator: public TMonitorableActor { deletedEnd = 0; } - if (record.GetStatus() == NKikimrScheme::EStatus::StatusRedirectDomain) { + if (description.Status == NKikimrScheme::EStatus::StatusRedirectDomain) { // this path has been migrated to another owner Send(replicaPopulator, new TSchemeBoardEvents::TEvDescribeResult(deletedBegin, deletedEnd, it->first.LocalPathId)); return; @@ -633,11 +624,12 @@ class TPopulator: public TMonitorableActor { if (it == Descriptions.end()) { update = MakeHolder(Owner, Generation, pathId); } else { - update = MakeHolder(Owner, Generation, it->second.Record); - update->SetDescribeSchemeResult(it->second); + const auto& desc = it->second; + update = MakeHolder(Owner, Generation, desc); + update->SetDescribeSchemeResultSerialized(desc.DescribeSchemeResultSerialized); } - update->Record.SetNeedAck(true); + Send(ev->Sender, std::move(update)); } @@ -650,13 +642,19 @@ class TPopulator: public TMonitorableActor { } void Handle(NSchemeShard::TEvSchemeShard::TEvDescribeSchemeResult::TPtr& ev) { - SBP_LOG_D("Handle " << ev->Get()->ToString() - << ": sender# " << ev->Sender - << ", cookie# " << ev->Cookie); - + //NOTE: avoid using TEventPreSerializedPB::GetRecord() or TEventPreSerializedPB::ToString() + // that will cause full reconstruction of TEvDescribeSchemeResult from base stab + // and PreSerializedData auto* msg = static_cast(ev->Get()); auto& record = msg->Record; + SBP_LOG_D("Handle TEvSchemeShard::TEvDescribeSchemeResult { " << record.ShortDebugString() << " }" + << ": sender# " << ev->Sender + << ", cookie# " << ev->Cookie + << ", event size# " << msg->GetCachedByteSize() + << ", preserialized size# " << msg->PreSerializedData.size() + ); + if (!record.HasStatus()) { SBP_LOG_E("Description without status"); return; @@ -664,13 +662,15 @@ class TPopulator: public TMonitorableActor { const TPathId pathId = GetPathId(record); const bool isDeletion = record.GetStatus() == NKikimrScheme::StatusPathDoesNotExist; - const ui64 version = isDeletion ? Max() : GetPathVersion(record); + const ui64 version = isDeletion ? Max() : NSchemeBoard::GetPathVersion(record); SBP_LOG_N("Update description" << ": owner# " << Owner << ", pathId# " << pathId << ", cookie# " << ev->Cookie - << ", is deletion# " << (isDeletion ? "true" : "false")); + << ", is deletion# " << (isDeletion ? "true" : "false") + << ", version: " << (isDeletion ? 0 : version) + ); if (isDeletion) { if (!Descriptions.contains(pathId)) { @@ -684,7 +684,7 @@ class TPopulator: public TMonitorableActor { return; } } else { - Descriptions[pathId] = TTwoPartDescription(std::move(msg->PreSerializedData), std::move(record)); + Descriptions[pathId] = MakeOpaquePathDescription(msg->PreSerializedData, record); MaxPathId = Max(MaxPathId, pathId.NextId()); } @@ -819,22 +819,14 @@ class TPopulator: public TMonitorableActor { void Handle(TSchemeBoardMonEvents::TEvDescribeRequest::TPtr& ev) { const auto& record = ev->Get()->Record; - TTwoPartDescription* desc = nullptr; + TOpaquePathDescription* desc = nullptr; if (record.HasPathId()) { desc = Descriptions.FindPtr(TPathId(record.GetPathId().GetOwnerId(), record.GetPathId().GetLocalPathId())); } TString json; if (desc) { - NKikimrScheme::TEvDescribeSchemeResult fullProto; - Y_PROTOBUF_SUPPRESS_NODISCARD fullProto.ParseFromString(desc->PreSerialized); - fullProto.MergeFrom(desc->Record); - - using namespace google::protobuf::util; - - JsonPrintOptions opts; - opts.preserve_proto_field_names = true; - MessageToJsonString(fullProto, &json, opts); + json = JsonFromDescribeSchemeResult(desc->DescribeSchemeResultSerialized); } else { json = "{}"; } @@ -872,14 +864,16 @@ class TPopulator: public TMonitorableActor { const ui64 owner, const ui64 generation, const ui32 ssId, - TMap descriptions, + std::vector>&& twoPartDescriptions, const ui64 maxPathId) : Owner(owner) , Generation(generation) , StateStorageGroup(ssId) - , Descriptions(std::move(descriptions)) , MaxPathId(TPathId(owner, maxPathId)) { + for (const auto& [pathId, twoPart] : twoPartDescriptions) { + Descriptions.emplace(pathId, MakeOpaquePathDescription(twoPart)); + } } void Bootstrap() { @@ -933,7 +927,7 @@ class TPopulator: public TMonitorableActor { const ui64 Generation; const ui64 StateStorageGroup; - TMap Descriptions; + TMap Descriptions; TPathId MaxPathId; TDelayedUpdates DelayedUpdates; @@ -958,10 +952,10 @@ IActor* CreateSchemeBoardPopulator( const ui64 owner, const ui64 generation, const ui32 ssId, - TMap descriptions, + std::vector>&& twoPartDescriptions, const ui64 maxPathId ) { - return new NSchemeBoard::TPopulator(owner, generation, ssId, std::move(descriptions), maxPathId); + return new NSchemeBoard::TPopulator(owner, generation, ssId, std::move(twoPartDescriptions), maxPathId); } } // NKikimr diff --git a/ydb/core/tx/scheme_board/populator.h b/ydb/core/tx/scheme_board/populator.h index ff0b4cead4b0..d1b5cb9f7cae 100644 --- a/ydb/core/tx/scheme_board/populator.h +++ b/ydb/core/tx/scheme_board/populator.h @@ -13,7 +13,7 @@ IActor* CreateSchemeBoardPopulator( const ui64 owner, const ui64 generation, const ui32 schemeBoardSSId, - TMap descriptions, + std::vector>&& twoPartDescriptions, const ui64 maxPathId ); diff --git a/ydb/core/tx/scheme_board/replica.cpp b/ydb/core/tx/scheme_board/replica.cpp index 07b81862fc29..c1600b4bd644 100644 --- a/ydb/core/tx/scheme_board/replica.cpp +++ b/ydb/core/tx/scheme_board/replica.cpp @@ -2,11 +2,11 @@ #include "events.h" #include "helpers.h" #include "monitorable_actor.h" +#include "opaque_path_description.h" #include "replica.h" -#include +#include -#include #include #include @@ -192,24 +192,16 @@ class TReplica: public TMonitorableActor { MultiSend(subscribers, Owner->SelfId(), std::move(notify)); } - void CalculateResultSize() { - ResultSize = DescribeSchemeResult.ByteSizeLong(); - } - - size_t FullSize() const { - size_t size = ResultSize; - if (PreSerializedDescribeSchemeResult) { - size += PreSerializedDescribeSchemeResult->size(); - } - return size; - } - void TrackMemory() const { - NActors::NMemory::TLabel::Add(FullSize()); + NActors::NMemory::TLabel::Add( + PathDescription.DescribeSchemeResultSerialized.size() + ); } void UntrackMemory() const { - NActors::NMemory::TLabel::Sub(FullSize()); + NActors::NMemory::TLabel::Sub( + PathDescription.DescribeSchemeResultSerialized.size() + ); } void Move(TDescription&& other) { @@ -219,13 +211,10 @@ class TReplica: public TMonitorableActor { Owner = other.Owner; Path = std::move(other.Path); PathId = std::move(other.PathId); - DescribeSchemeResult = std::move(other.DescribeSchemeResult); - PreSerializedDescribeSchemeResult = std::move(other.PreSerializedDescribeSchemeResult); + PathDescription = std::move(other.PathDescription); ExplicitlyDeleted = other.ExplicitlyDeleted; Subscribers = std::move(other.Subscribers); - ResultSize = other.ResultSize; - other.ResultSize = 0; TrackNotify = other.TrackNotify; TrackMemory(); @@ -262,12 +251,11 @@ class TReplica: public TMonitorableActor { explicit TDescription( TReplica* owner, const TPathId& pathId, - TDescribeSchemeResult&& describeSchemeResult) + TOpaquePathDescription&& pathDescription) : Owner(owner) , PathId(pathId) - , DescribeSchemeResult(std::move(describeSchemeResult)) + , PathDescription(std::move(pathDescription)) { - CalculateResultSize(); TrackMemory(); } @@ -275,13 +263,12 @@ class TReplica: public TMonitorableActor { TReplica* owner, const TString& path, const TPathId& pathId, - TDescribeSchemeResult&& describeSchemeResult) + TOpaquePathDescription&& pathDescription) : Owner(owner) , Path(path) , PathId(pathId) - , DescribeSchemeResult(std::move(describeSchemeResult)) + , PathDescription(std::move(pathDescription)) { - CalculateResultSize(); TrackMemory(); } @@ -339,22 +326,18 @@ class TReplica: public TMonitorableActor { other.TrackNotify = false; if (*this > other) { - other.DescribeSchemeResult.Swap(&DescribeSchemeResult); - other.PreSerializedDescribeSchemeResult.Clear(); + // this desc is newer then the other + std::swap(other.PathDescription, PathDescription); other.ExplicitlyDeleted = ExplicitlyDeleted; other.Notify(); - DescribeSchemeResult.Swap(&other.DescribeSchemeResult); - other.PreSerializedDescribeSchemeResult.Clear(); + std::swap(PathDescription, other.PathDescription); } else if (*this < other) { - DescribeSchemeResult = std::move(other.DescribeSchemeResult); - PreSerializedDescribeSchemeResult.Clear(); + // this desc is older then the other + PathDescription = std::move(other.PathDescription); ExplicitlyDeleted = other.ExplicitlyDeleted; Notify(); } - CalculateResultSize(); - other.CalculateResultSize(); - TrackNotify = true; other.TrackNotify = true; TrackMemory(); @@ -365,15 +348,15 @@ class TReplica: public TMonitorableActor { return *this; } - const TDescribeSchemeResult& GetProto() const { - return DescribeSchemeResult; + TString GetDescribeSchemeResultSerialized() const { + return PathDescription.DescribeSchemeResultSerialized; } TString ToString() const { return TStringBuilder() << "{" << " Path# " << Path << " PathId# " << PathId - << " DescribeSchemeResult# " << DescribeSchemeResult.ShortDebugString() + << " PathDescription# " << PathDescription.ToString() << " ExplicitlyDeleted# " << (ExplicitlyDeleted ? "true" : "false") << " }"; } @@ -403,28 +386,34 @@ class TReplica: public TMonitorableActor { if (ExplicitlyDeleted) { return Max(); } - - return ::NKikimr::NSchemeBoard::GetPathVersion(DescribeSchemeResult); + return PathDescription.PathVersion; } TDomainId GetDomainId() const { - return IsFilled() ? ::NKikimr::NSchemeBoard::GetDomainId(DescribeSchemeResult) : TDomainId(); + if (IsEmpty()) { + return TDomainId(); + } + return PathDescription.SubdomainPathId; } TSet GetAbandonedSchemeShardIds() const { - return IsFilled() ? ::NKikimr::NSchemeBoard::GetAbandonedSchemeShardIds(DescribeSchemeResult) : TSet(); + if (IsEmpty()) { + return TSet(); + } + return PathDescription.PathAbandonedTenantsSchemeShards; } - bool IsFilled() const { - return DescribeSchemeResult.ByteSizeLong(); + bool IsEmpty() const { + return PathDescription.IsEmpty(); } void Clear() { ExplicitlyDeleted = true; UntrackMemory(); - TDescribeSchemeResult().Swap(&DescribeSchemeResult); - PreSerializedDescribeSchemeResult.Clear(); - ResultSize = 0; + { + TOpaquePathDescription empty; + std::swap(PathDescription, empty); + } TrackMemory(); Notify(); } @@ -432,7 +421,7 @@ class TReplica: public TMonitorableActor { THolder BuildNotify(bool forceStrong = false) const { THolder notify; - const bool isDeletion = !IsFilled(); + const bool isDeletion = IsEmpty(); if (!PathId) { Y_ABORT_UNLESS(isDeletion); @@ -445,23 +434,15 @@ class TReplica: public TMonitorableActor { } if (!isDeletion) { - if (!PreSerializedDescribeSchemeResult) { - TString serialized; - Y_PROTOBUF_SUPPRESS_NODISCARD DescribeSchemeResult.SerializeToString(&serialized); - if (TrackNotify) { - UntrackMemory(); - } - PreSerializedDescribeSchemeResult = std::move(serialized); - if (TrackNotify) { - TrackMemory(); - } + notify->SetPathDescription(PathDescription); + if (TrackNotify) { + TrackMemory(); } - - notify->SetDescribeSchemeResult(*PreSerializedDescribeSchemeResult); } notify->Record.SetVersion(GetVersion()); - if (IsFilled() || IsExplicitlyDeleted() || forceStrong) { + + if (!IsEmpty() || IsExplicitlyDeleted() || forceStrong) { notify->Record.SetStrong(true); } @@ -505,15 +486,14 @@ class TReplica: public TMonitorableActor { // data TString Path; TPathId PathId; - TDescribeSchemeResult DescribeSchemeResult; - mutable TMaybe PreSerializedDescribeSchemeResult; + TOpaquePathDescription PathDescription; + bool ExplicitlyDeleted = false; // subscribers THashMap Subscribers; // memory tracking - size_t ResultSize = 0; bool TrackNotify = true; }; // TDescription @@ -541,6 +521,7 @@ class TReplica: public TMonitorableActor { return false; } + // register empty entry by path OR pathId template TDescription& UpsertDescription(const TPath& path) { SBR_LOG_I("Upsert description" @@ -549,6 +530,7 @@ class TReplica: public TMonitorableActor { return Descriptions.Upsert(path, TDescription(this, path)); } + // register empty entry by path AND pathId both TDescription& UpsertDescription(const TString& path, const TPathId& pathId) { SBR_LOG_I("Upsert description" << ": path# " << path @@ -557,25 +539,25 @@ class TReplica: public TMonitorableActor { return Descriptions.Upsert(path, pathId, TDescription(this, path, pathId)); } - template - TDescription& UpsertDescription(const TPath& path, TDescription&& description) { + // upsert description only by pathId + TDescription& UpsertDescription(const TPathId& pathId, TOpaquePathDescription&& pathDescription) { SBR_LOG_I("Upsert description" - << ": path# " << path - << ", desc# " << description.ToLogString()); + << ": pathId# " << pathId + << ", pathDescription# " << pathDescription.ToString() + ); - return Descriptions.Upsert(path, std::move(description)); + return Descriptions.Upsert(pathId, TDescription(this, pathId, std::move(pathDescription))); } - TDescription& UpsertDescription( - const TString& path, - const TPathId& pathId, - TDescribeSchemeResult&& describeSchemeResult - ) { + // upsert description by path AND pathId both + TDescription& UpsertDescription(const TString& path, const TPathId& pathId, TOpaquePathDescription&& pathDescription) { SBR_LOG_I("Upsert description" << ": path# " << path - << ", pathId# " << pathId); + << ", pathId# " << pathId + << ", pathDescription# " << pathDescription.ToString() + ); - return Descriptions.Upsert(path, pathId, TDescription(this, path, pathId, std::move(describeSchemeResult))); + return Descriptions.Upsert(path, pathId, TDescription(this, path, pathId, std::move(pathDescription))); } void SoftDeleteDescription(const TPathId& pathId, bool createIfNotExists = false) { @@ -590,7 +572,7 @@ class TReplica: public TMonitorableActor { return; } - if (!desc->IsFilled()) { + if (desc->IsEmpty()) { return; } @@ -601,7 +583,7 @@ class TReplica: public TMonitorableActor { << ", pathId# " << pathId); if (TDescription* descByPath = Descriptions.FindPtr(path)) { - if (descByPath != desc && descByPath->IsFilled()) { + if (descByPath != desc && !descByPath->IsEmpty()) { if (descByPath->GetPathId().OwnerId != pathId.OwnerId) { auto curPathId = descByPath->GetPathId(); auto curDomainId = descByPath->GetDomainId(); @@ -803,51 +785,55 @@ class TReplica: public TMonitorableActor { void Handle(TSchemeBoardEvents::TEvUpdate::TPtr& ev) { SBR_LOG_D("Handle " << ev->Get()->ToString() << ": sender# " << ev->Sender - << ", cookie# " << ev->Cookie); - - auto& record = *ev->Get()->MutableRecord(); - const ui64 owner = record.GetOwner(); - const ui64 generation = record.GetGeneration(); - - const auto populatorIt = Populators.find(owner); - if (populatorIt == Populators.end()) { - SBR_LOG_E("Reject update from unknown populator" - << ": sender# " << ev->Sender - << ", owner# " << owner - << ", generation# " << generation); - return; - } - if (generation != populatorIt->second.PendingGeneration) { - SBR_LOG_E("Reject update from stale populator" - << ": sender# " << ev->Sender - << ", owner# " << owner - << ", generation# " << generation - << ", pending generation# " << populatorIt->second.PendingGeneration); - return; - } - - if (record.HasDeletedLocalPathIds()) { - const TPathId begin(owner, record.GetDeletedLocalPathIds().GetBegin()); - const TPathId end(owner, record.GetDeletedLocalPathIds().GetEnd()); - SoftDeleteDescriptions(begin, end); - } + << ", cookie# " << ev->Cookie + << ", event size# " << ev->Get()->GetCachedByteSize() + ); + + const TString& path = ev->Get()->GetPath(); + const TPathId& pathId = ev->Get()->GetPathId(); + + { + auto& record = *ev->Get()->MutableRecord(); + const ui64 owner = record.GetOwner(); + const ui64 generation = record.GetGeneration(); + + const auto populatorIt = Populators.find(owner); + if (populatorIt == Populators.end()) { + SBR_LOG_E("Reject update from unknown populator" + << ": sender# " << ev->Sender + << ", owner# " << owner + << ", generation# " << generation); + return; + } + if (generation != populatorIt->second.PendingGeneration) { + SBR_LOG_E("Reject update from stale populator" + << ": sender# " << ev->Sender + << ", owner# " << owner + << ", generation# " << generation + << ", pending generation# " << populatorIt->second.PendingGeneration); + return; + } - if (!record.HasLocalPathId()) { - return AckUpdate(ev); - } + if (record.HasDeletedLocalPathIds()) { + const TPathId begin(owner, record.GetDeletedLocalPathIds().GetBegin()); + const TPathId end(owner, record.GetDeletedLocalPathIds().GetEnd()); + SoftDeleteDescriptions(begin, end); + } - const TString& path = record.GetPath(); - const TPathId pathId = ev->Get()->GetPathId(); + if (!record.HasLocalPathId()) { + return AckUpdate(ev); + } - SBR_LOG_N("Update description" - << ": path# " << path - << ", pathId# " << pathId - << ", deletion# " << (record.GetIsDeletion() ? "true" : "false")); + SBR_LOG_N("Update description" + << ": path# " << path + << ", pathId# " << pathId + << ", deletion# " << (record.GetIsDeletion() ? "true" : "false")); - if (record.GetIsDeletion()) { - SoftDeleteDescription(pathId, true); - return AckUpdate(ev); - } + if (record.GetIsDeletion()) { + SoftDeleteDescription(pathId, true); + return AckUpdate(ev); + } + } if (TDescription* desc = Descriptions.FindPtr(pathId)) { if (desc->IsExplicitlyDeleted()) { @@ -859,20 +845,24 @@ class TReplica: public TMonitorableActor { } } + // TEvUpdate is partially consumed here, with DescribeSchemeResult blob being moved out. + // AckUpdate(ev) calls below are ok, AckUpdate() doesn't use DescribeSchemeResult. + TOpaquePathDescription pathDescription = ev->Get()->ExtractPathDescription(); + TDescription* desc = Descriptions.FindPtr(path); if (!desc) { - UpsertDescription(path, pathId, std::move(*record.MutableDescribeSchemeResult())); + UpsertDescription(path, pathId, std::move(pathDescription)); return AckUpdate(ev); } if (!desc->GetPathId()) { - UpsertDescription(path, pathId, std::move(*record.MutableDescribeSchemeResult())); + UpsertDescription(path, pathId, std::move(pathDescription)); return AckUpdate(ev); } auto curPathId = desc->GetPathId(); - if (curPathId.OwnerId == pathId.OwnerId || !desc->IsFilled()) { + if (curPathId.OwnerId == pathId.OwnerId || desc->IsEmpty()) { if (curPathId > pathId) { return AckUpdate(ev); } @@ -883,15 +873,15 @@ class TReplica: public TMonitorableActor { RelinkSubscribers(desc, path); } - UpsertDescription(path, pathId, std::move(*record.MutableDescribeSchemeResult())); + UpsertDescription(path, pathId, std::move(pathDescription)); return AckUpdate(ev); } - Y_VERIFY_S(desc->IsFilled(), "Description is not filled" + Y_VERIFY_S(!desc->IsEmpty(), "Description is not filled" << ": desc# " << desc->ToLogString()); auto curDomainId = desc->GetDomainId(); - auto domainId = GetDomainId(record.GetDescribeSchemeResult()); + const auto& domainId = pathDescription.SubdomainPathId; auto log = [&](const TString& message) { SBR_LOG_N("" << message @@ -904,36 +894,36 @@ class TReplica: public TMonitorableActor { if (curPathId == domainId) { // Update from TSS, GSS->TSS // it is only because we need to manage undo of upgrade subdomain, finally remove it - auto abandonedSchemeShards = desc->GetAbandonedSchemeShardIds(); + const auto& abandonedSchemeShards = desc->GetAbandonedSchemeShardIds(); if (abandonedSchemeShards.contains(pathId.OwnerId)) { // TSS is ignored, present GSS reverted it log("Replace GSS by TSS description is rejected, GSS implicitly knows that TSS has been reverted" ", but still inject description only by pathId for safe"); - UpsertDescription(pathId, TDescription(this, path, pathId, std::move(*record.MutableDescribeSchemeResult()))); + UpsertDescription(pathId, std::move(pathDescription)); return AckUpdate(ev); } log("Replace GSS by TSS description"); - // unlick GSS desc by path + // unlink GSS desc by path Descriptions.DeleteIndex(path); RelinkSubscribers(desc, path); - UpsertDescription(path, pathId, std::move(*record.MutableDescribeSchemeResult())); + UpsertDescription(path, pathId, std::move(pathDescription)); return AckUpdate(ev); } if (curDomainId == pathId) { // Update from GSS, TSS->GSS // it is only because we need to manage undo of upgrade subdomain, finally remove it - auto abandonedSchemeShards = GetAbandonedSchemeShardIds(record.GetDescribeSchemeResult()); + const auto& abandonedSchemeShards = pathDescription.PathAbandonedTenantsSchemeShards; if (abandonedSchemeShards.contains(curPathId.OwnerId)) { // GSS reverts TSS log("Replace TSS by GSS description, TSS was implicitly reverted by GSS"); - // unlick TSS desc by path + // unlink TSS desc by path Descriptions.DeleteIndex(path); RelinkSubscribers(desc, path); - UpsertDescription(path, pathId, std::move(*record.MutableDescribeSchemeResult())); + UpsertDescription(path, pathId, std::move(pathDescription)); return AckUpdate(ev); } log("Inject description only by pathId, it is update from GSS"); - UpsertDescription(pathId, TDescription(this, path, pathId, std::move(*record.MutableDescribeSchemeResult()))); + UpsertDescription(pathId, std::move(pathDescription)); return AckUpdate(ev); } @@ -950,13 +940,13 @@ class TReplica: public TMonitorableActor { RelinkSubscribers(desc, path); } - UpsertDescription(path, pathId, std::move(*record.MutableDescribeSchemeResult())); + UpsertDescription(path, pathId, std::move(pathDescription)); return AckUpdate(ev); } else if (curDomainId < domainId) { log("Update description by newest path with newer domainId"); Descriptions.DeleteIndex(path); RelinkSubscribers(desc, path); - UpsertDescription(path, pathId, std::move(*record.MutableDescribeSchemeResult())); + UpsertDescription(path, pathId, std::move(pathDescription)); return AckUpdate(ev); } else { log("Totally ignore description, path with obsolete domainId"); @@ -1147,7 +1137,7 @@ class TReplica: public TMonitorableActor { auto it = Subscribers.find(ev->Sender); if (it == Subscribers.end()) { - // for backward compatability + // for backward compatibility ui64 version = 0; if (record.HasPath()) { @@ -1237,11 +1227,7 @@ class TReplica: public TMonitorableActor { TString json; if (desc) { - using namespace google::protobuf::util; - - JsonPrintOptions opts; - opts.preserve_proto_field_names = true; - MessageToJsonString(desc->GetProto(), &json, opts); + json = JsonFromDescribeSchemeResult(desc->GetDescribeSchemeResultSerialized()); } else { json = "{}"; } diff --git a/ydb/core/tx/scheme_board/replica_ut.cpp b/ydb/core/tx/scheme_board/replica_ut.cpp index 44dead06581c..2a91c719ed86 100644 --- a/ydb/core/tx/scheme_board/replica_ut.cpp +++ b/ydb/core/tx/scheme_board/replica_ut.cpp @@ -678,13 +678,11 @@ void TReplicaCombinationTest::UpdatesCombinationsDomainRoot() { UNIT_ASSERT_VALUES_EQUAL(std::get<2>(winId), TPathId(ev->Get()->GetRecord().GetPathOwnerId(), ev->Get()->GetRecord().GetLocalPathId())); UNIT_ASSERT_VALUES_EQUAL(std::get<3>(winId), ev->Get()->GetRecord().GetVersion()); - UNIT_ASSERT(ev->Get()->GetRecord().HasDescribeSchemeResult()); - const NKikimrScheme::TEvDescribeSchemeResult& descr = ev->Get()->GetRecord().GetDescribeSchemeResult(); - auto& domainKey = descr.GetPathDescription().GetDomainDescription().GetDomainKey(); - UNIT_ASSERT_VALUES_EQUAL(std::get<0>(winId), TDomainId(domainKey.GetSchemeShard(), domainKey.GetPathId())); - } - + UNIT_ASSERT(ev->Get()->GetRecord().HasDescribeSchemeResultSerialized()); + UNIT_ASSERT(ev->Get()->GetRecord().HasPathSubdomainPathId()); + UNIT_ASSERT_VALUES_EQUAL(std::get<0>(winId), PathIdFromPathId(ev->Get()->GetRecord().GetPathSubdomainPathId())); + } } } } @@ -731,7 +729,7 @@ void TReplicaCombinationTest::MigratedPathRecreation() { << " DomainId: " << std::get<0>(winId) << " IsDeletion: " << std::get<1>(winId) << " PathId: " << std::get<2>(winId) - << " Verions: " << std::get<3>(winId) + << " Versions: " << std::get<3>(winId) << Endl; UNIT_ASSERT_VALUES_EQUAL("/root/db/dir_inside", ev->Get()->GetRecord().GetPath()); @@ -740,10 +738,10 @@ void TReplicaCombinationTest::MigratedPathRecreation() { UNIT_ASSERT_VALUES_EQUAL(std::get<2>(winId), TPathId(ev->Get()->GetRecord().GetPathOwnerId(), ev->Get()->GetRecord().GetLocalPathId())); UNIT_ASSERT_VALUES_EQUAL(std::get<3>(winId), ev->Get()->GetRecord().GetVersion()); - UNIT_ASSERT(ev->Get()->GetRecord().HasDescribeSchemeResult()); - const NKikimrScheme::TEvDescribeSchemeResult& descr = ev->Get()->GetRecord().GetDescribeSchemeResult(); - auto& domainKey = descr.GetPathDescription().GetDomainDescription().GetDomainKey(); - UNIT_ASSERT_VALUES_EQUAL(std::get<0>(winId), TDomainId(domainKey.GetSchemeShard(), domainKey.GetPathId())); + UNIT_ASSERT(ev->Get()->GetRecord().HasDescribeSchemeResultSerialized()); + UNIT_ASSERT(ev->Get()->GetRecord().HasPathSubdomainPathId()); + + UNIT_ASSERT_VALUES_EQUAL(std::get<0>(winId), PathIdFromPathId(ev->Get()->GetRecord().GetPathSubdomainPathId())); } void TReplicaCombinationTest::UpdatesCombinationsMigratedPath() { @@ -776,9 +774,6 @@ void TReplicaCombinationTest::UpdatesCombinationsMigratedPath() { auto finalSubscriber = Context->AllocateEdgeActor(); auto ev = Context->SubscribeReplica(replica, finalSubscriber, "/Root/Tenant/table_inside"); - const NKikimrScheme::TEvDescribeSchemeResult& descr = ev->Get()->GetRecord().GetDescribeSchemeResult(); - auto& domainKey = descr.GetPathDescription().GetDomainDescription().GetDomainKey(); - Cerr << "=========== Left ==" << argsLeft.GenerateDescribe().ShortDebugString() << "\n=========== Right ==" << argsRight.GenerateDescribe().ShortDebugString() << "\n=========== super id ==" @@ -792,7 +787,7 @@ void TReplicaCombinationTest::UpdatesCombinationsMigratedPath() { << " PathID: " << TPathId(ev->Get()->GetRecord().GetPathOwnerId(), ev->Get()->GetRecord().GetLocalPathId()) << " deleted: " << ev->Get()->GetRecord().GetIsDeletion() << " version: " << ev->Get()->GetRecord().GetVersion() - << " domainId: " << TDomainId(domainKey.GetSchemeShard(), domainKey.GetPathId()) + << " domainId: " << PathIdFromPathId(ev->Get()->GetRecord().GetPathSubdomainPathId()) << Endl; if (argsLeft.PathId == TPathId(gssID, gssLocalId) && !argsLeft.IsDeletion && argsLeft.DomainId == TPathId(gssID, 2) @@ -813,10 +808,10 @@ void TReplicaCombinationTest::UpdatesCombinationsMigratedPath() { UNIT_ASSERT_VALUES_EQUAL(std::get<2>(argsRight.GetSuperId()), TPathId(ev->Get()->GetRecord().GetPathOwnerId(), ev->Get()->GetRecord().GetLocalPathId())); UNIT_ASSERT_VALUES_EQUAL(std::get<3>(argsRight.GetSuperId()), ev->Get()->GetRecord().GetVersion()); - UNIT_ASSERT(ev->Get()->GetRecord().HasDescribeSchemeResult()); - const NKikimrScheme::TEvDescribeSchemeResult& descr = ev->Get()->GetRecord().GetDescribeSchemeResult(); - auto& domainKey = descr.GetPathDescription().GetDomainDescription().GetDomainKey(); - UNIT_ASSERT_VALUES_EQUAL(std::get<0>(argsRight.GetSuperId()), TDomainId(domainKey.GetSchemeShard(), domainKey.GetPathId())); + UNIT_ASSERT(ev->Get()->GetRecord().HasDescribeSchemeResultSerialized()); + UNIT_ASSERT(ev->Get()->GetRecord().HasPathSubdomainPathId()); + + UNIT_ASSERT_VALUES_EQUAL(std::get<0>(argsRight.GetSuperId()), PathIdFromPathId(ev->Get()->GetRecord().GetPathSubdomainPathId())); continue; } @@ -832,10 +827,10 @@ void TReplicaCombinationTest::UpdatesCombinationsMigratedPath() { UNIT_ASSERT_VALUES_EQUAL(std::get<2>(argsLeft.GetSuperId()), TPathId(ev->Get()->GetRecord().GetPathOwnerId(), ev->Get()->GetRecord().GetLocalPathId())); UNIT_ASSERT_VALUES_EQUAL(std::get<3>(argsLeft.GetSuperId()), ev->Get()->GetRecord().GetVersion()); - UNIT_ASSERT(ev->Get()->GetRecord().HasDescribeSchemeResult()); - const NKikimrScheme::TEvDescribeSchemeResult& descr = ev->Get()->GetRecord().GetDescribeSchemeResult(); - auto& domainKey = descr.GetPathDescription().GetDomainDescription().GetDomainKey(); - UNIT_ASSERT_VALUES_EQUAL(std::get<0>(argsLeft.GetSuperId()), TDomainId(domainKey.GetSchemeShard(), domainKey.GetPathId())); + UNIT_ASSERT(ev->Get()->GetRecord().HasDescribeSchemeResultSerialized()); + UNIT_ASSERT(ev->Get()->GetRecord().HasPathSubdomainPathId()); + + UNIT_ASSERT_VALUES_EQUAL(std::get<0>(argsLeft.GetSuperId()), PathIdFromPathId(ev->Get()->GetRecord().GetPathSubdomainPathId())); continue; } @@ -882,10 +877,10 @@ void TReplicaCombinationTest::UpdatesCombinationsMigratedPath() { UNIT_ASSERT_VALUES_EQUAL(std::get<2>(winId), TPathId(ev->Get()->GetRecord().GetPathOwnerId(), ev->Get()->GetRecord().GetLocalPathId())); UNIT_ASSERT_VALUES_EQUAL(std::get<3>(winId), ev->Get()->GetRecord().GetVersion()); - UNIT_ASSERT(ev->Get()->GetRecord().HasDescribeSchemeResult()); - const NKikimrScheme::TEvDescribeSchemeResult& descr = ev->Get()->GetRecord().GetDescribeSchemeResult(); - auto& domainKey = descr.GetPathDescription().GetDomainDescription().GetDomainKey(); - UNIT_ASSERT_VALUES_EQUAL(std::get<0>(winId), TDomainId(domainKey.GetSchemeShard(), domainKey.GetPathId())); + UNIT_ASSERT(ev->Get()->GetRecord().HasDescribeSchemeResultSerialized()); + UNIT_ASSERT(ev->Get()->GetRecord().HasPathSubdomainPathId()); + + UNIT_ASSERT_VALUES_EQUAL(std::get<0>(winId), PathIdFromPathId(ev->Get()->GetRecord().GetPathSubdomainPathId())); } } } diff --git a/ydb/core/tx/scheme_board/subscriber.cpp b/ydb/core/tx/scheme_board/subscriber.cpp index 76264b385a60..4fdeb8479ecc 100644 --- a/ydb/core/tx/scheme_board/subscriber.cpp +++ b/ydb/core/tx/scheme_board/subscriber.cpp @@ -127,6 +127,67 @@ namespace { } }; + // TNotifyResponse isolates changes of how NKikimrSchemeBoard::TEvNotify is filled + // by previous and recent replica implementation. + // + // TNotifyResponse wouldn't be even needed if not for backward compatibility support. + // Consider removing compatibility support at version stable-25-1. + struct TNotifyResponse { + NKikimrSchemeBoard::TEvNotify Notify; + TPathId SubdomainPathId; + TSet PathAbandonedTenantsSchemeShards; + TMaybe DescribeSchemeResult; + + static TNotifyResponse FromNotify(NKikimrSchemeBoard::TEvNotify&& record) { + // PathSubdomainPathId's absence is a marker that input message was sent + // from the older replica implementation + + if (record.HasPathSubdomainPathId()) { + // Sender implementation is as recent as ours. + // Just copy two fields from the notify message (this branch is practically a stub) + + auto subdomainPathId = PathIdFromPathId(record.GetPathSubdomainPathId()); + auto pathAbandonedTenantsSchemeShards = TSet( + record.GetPathAbandonedTenantsSchemeShards().begin(), + record.GetPathAbandonedTenantsSchemeShards().end() + ); + return TNotifyResponse{ + .Notify = std::move(record), + .SubdomainPathId = std::move(subdomainPathId), + .PathAbandonedTenantsSchemeShards = std::move(pathAbandonedTenantsSchemeShards), + }; + + } else { + // Sender implementation is older then ours. + // Extract two essential fields from the payload, hold deserialized proto, + // drop original payload to save on memory + + // Move DescribeSchemeResult blob out of the input message. + auto data = std::move(*record.MutableDescribeSchemeResultSerialized()); + record.ClearDescribeSchemeResultSerialized(); + + // it's inconvenient to use arena here + auto proto = DeserializeDescribeSchemeResult(data); + + return TNotifyResponse{ + .Notify = std::move(record), + .SubdomainPathId = NSchemeBoard::GetDomainId(proto), + .PathAbandonedTenantsSchemeShards = NSchemeBoard::GetAbandonedSchemeShardIds(proto), + .DescribeSchemeResult = std::move(proto), + }; + } + } + + NKikimrScheme::TEvDescribeSchemeResult GetDescribeSchemeResult() { + if (DescribeSchemeResult) { + return *DescribeSchemeResult; + } else { + // it's inconvenient to use arena here + return DeserializeDescribeSchemeResult(Notify.GetDescribeSchemeResultSerialized()); + } + } + }; + struct TState { bool Deleted = false; bool Strong = false; @@ -154,15 +215,17 @@ namespace { } public: - static TState FromNotify(const NKikimrSchemeBoard::TEvNotify& record) { + static TState FromNotify(const TNotifyResponse& notifyResponse) { + const auto& record = notifyResponse.Notify; + const TPathVersion& pathVersion = TPathVersion::FromNotify(record); if (!record.GetIsDeletion()) { return TState( - TPathVersion::FromNotify(record), - GetDomainId(record.GetDescribeSchemeResult()), - GetAbandonedSchemeShardIds(record.GetDescribeSchemeResult()) + pathVersion, + notifyResponse.SubdomainPathId, + notifyResponse.PathAbandonedTenantsSchemeShards ); } else { - return TState(record.GetStrong(), TPathVersion::FromNotify(record)); + return TState(record.GetStrong(), pathVersion); } } @@ -229,7 +292,7 @@ namespace { if (Version.PathId == other.DomainId) { // Update from TSS, GSS->TSS if (AbandonedSchemeShards.contains(other.Version.PathId.OwnerId)) { // TSS is ignored, present GSS reverted that TSS - reason = "Update was ignored, GSS implisytly banned that TSS"; + reason = "Update was ignored, GSS implicitly banned that TSS"; return false; } @@ -685,7 +748,7 @@ class TSubscriber: public TMonitorableActor { return it->second; } - NKikimrSchemeBoard::TEvNotify& SelectResponse() { + TNotifyResponse* SelectResponse() { Y_ABORT_UNLESS(IsMajorityReached()); auto newest = SelectStateImpl(); @@ -694,7 +757,7 @@ class TSubscriber: public TMonitorableActor { auto it = InitialResponses.find(newest->first); Y_ABORT_UNLESS(it != InitialResponses.end()); - return it->second; + return &it->second; } bool IsMajorityReached() const { @@ -723,23 +786,25 @@ class TSubscriber: public TMonitorableActor { } void Handle(TSchemeBoardEvents::TEvNotify::TPtr& ev) { - auto& record = *ev->Get()->MutableRecord(); - SBS_LOG_D("Handle " << ev->Get()->ToString() << ": sender# " << ev->Sender); - if (!IsValidNotification(Path, record)) { + if (!IsValidNotification(Path, ev->Get()->GetRecord())) { SBS_LOG_E("Suspicious " << ev->Get()->ToString() << ": sender# " << ev->Sender); return; } - States[ev->Sender] = TState::FromNotify(record); + // TEvNotify message is consumed here, can't be used after this point + TNotifyResponse notifyResponse = TNotifyResponse::FromNotify(std::move(*ev->Get()->MutableRecord())); + TNotifyResponse* selectedNotify = ¬ifyResponse; + + States[ev->Sender] = TState::FromNotify(notifyResponse); if (!IsMajorityReached()) { - InitialResponses[ev->Sender] = std::move(record); + InitialResponses[ev->Sender] = std::move(notifyResponse); if (IsMajorityReached()) { MaybeRunVersionSync(); - record = SelectResponse(); + selectedNotify = SelectResponse(); } else { return; } @@ -773,8 +838,11 @@ class TSubscriber: public TMonitorableActor { return; } + const auto& record = selectedNotify->Notify; + if (!record.GetIsDeletion()) { - this->Send(Owner, BuildNotify(record, std::move(*record.MutableDescribeSchemeResult()))); + NKikimrScheme::TEvDescribeSchemeResult proto = selectedNotify->GetDescribeSchemeResult(); + this->Send(Owner, BuildNotify(record, std::move(proto))); } else { this->Send(Owner, BuildNotify(record, record.GetStrong())); } @@ -1032,7 +1100,7 @@ class TSubscriber: public TMonitorableActor { TSet Proxies; TMap States; - TMap InitialResponses; + TMap InitialResponses; TMaybe State; ui64 DelayedSyncRequest; diff --git a/ydb/core/tx/scheme_board/subscriber_ut.cpp b/ydb/core/tx/scheme_board/subscriber_ut.cpp index 4a33a9f5ab00..ffaaf856ebd4 100644 --- a/ydb/core/tx/scheme_board/subscriber_ut.cpp +++ b/ydb/core/tx/scheme_board/subscriber_ut.cpp @@ -152,7 +152,7 @@ void TSubscriberTest::InvalidNotification() { // send notification directly to subscriber auto* notify = new TSchemeBoardEvents::TEvNotifyBuilder(TPathId(1, 1)); - notify->Record.MutableDescribeSchemeResult()->CopyFrom(GenerateDescribe("another/path", TPathId(1, 1))); + notify->SetPathDescription(MakeOpaquePathDescription("", GenerateDescribe("another/path", TPathId(1, 1)))); Context->Send(subscriber, edge, notify); size_t counter = Context->CountEdgeEvents(); diff --git a/ydb/core/tx/scheme_board/ut_helpers.cpp b/ydb/core/tx/scheme_board/ut_helpers.cpp index 762d8ddea12f..63bdd22c7174 100644 --- a/ydb/core/tx/scheme_board/ut_helpers.cpp +++ b/ydb/core/tx/scheme_board/ut_helpers.cpp @@ -31,10 +31,11 @@ TSchemeBoardEvents::TEvUpdate* GenerateUpdate( ui64 generation, bool isDeletion ) { - auto* update = new TSchemeBoardEvents::TEvUpdateBuilder(owner, generation, describe, isDeletion); + const auto& pathDescription = MakeOpaquePathDescription("", describe); + auto* update = new TSchemeBoardEvents::TEvUpdateBuilder(owner, generation, pathDescription, isDeletion); if (!isDeletion) { - update->SetDescribeSchemeResult(describe); + update->SetDescribeSchemeResultSerialized(std::move(pathDescription.DescribeSchemeResultSerialized)); } return update; diff --git a/ydb/core/tx/scheme_board/ya.make b/ydb/core/tx/scheme_board/ya.make index a04c3b5c32bd..22f66c217f09 100644 --- a/ydb/core/tx/scheme_board/ya.make +++ b/ydb/core/tx/scheme_board/ya.make @@ -13,6 +13,7 @@ PEERDIR( SRCS( cache.cpp + events.cpp helpers.cpp load_test.cpp monitoring.cpp @@ -20,6 +21,7 @@ SRCS( replica.cpp subscriber.cpp two_part_description.cpp + opaque_path_description.cpp ) GENERATE_ENUM_SERIALIZATION(subscriber.h) diff --git a/ydb/core/tx/schemeshard/schemeshard.h b/ydb/core/tx/schemeshard/schemeshard.h index d86d17ffe9ad..f8aeb16ea5bf 100644 --- a/ydb/core/tx/schemeshard/schemeshard.h +++ b/ydb/core/tx/schemeshard/schemeshard.h @@ -331,10 +331,9 @@ struct TEvSchemeShard { EvDescribeSchemeResult> { TEvDescribeSchemeResult() = default; - TEvDescribeSchemeResult(const TString& path, ui64 pathOwner, TPathId pathId) + TEvDescribeSchemeResult(const TString& path, TPathId pathId) { Record.SetPath(path); - Record.SetPathOwner(pathOwner); Record.SetPathId(pathId.LocalPathId); Record.SetPathOwnerId(pathId.OwnerId); } @@ -345,8 +344,8 @@ struct TEvSchemeShard { TEvDescribeSchemeResultBuilder() = default; - TEvDescribeSchemeResultBuilder(const TString& path, ui64 pathOwner, TPathId pathId) - : TEvDescribeSchemeResult(path, pathOwner, pathId) + TEvDescribeSchemeResultBuilder(const TString& path, TPathId pathId) + : TEvDescribeSchemeResult(path, pathId) { } }; diff --git a/ydb/core/tx/schemeshard/schemeshard__init_populator.cpp b/ydb/core/tx/schemeshard/schemeshard__init_populator.cpp index aeda8d0aa0cf..9824caa2f528 100644 --- a/ydb/core/tx/schemeshard/schemeshard__init_populator.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__init_populator.cpp @@ -13,7 +13,7 @@ using namespace NTabletFlatExecutor; struct TSchemeShard::TTxInitPopulator : public TTransactionBase { using TDescription = NSchemeBoard::TTwoPartDescription; - TMap Descriptions; + std::vector> Descriptions; TSideEffects::TPublications DelayedPublications; explicit TTxInitPopulator(TSelf *self, TSideEffects::TPublications&& publications) @@ -49,7 +49,7 @@ struct TSchemeShard::TTxInitPopulator : public TTransactionBase { auto result = DescribePath(Self, ctx, pathId); TDescription description(std::move(result->PreSerializedData), std::move(result->Record)); - Descriptions.emplace(pathId, std::move(description)); + Descriptions.emplace_back(pathId, std::move(description)); } return true; diff --git a/ydb/core/tx/schemeshard/schemeshard_path_describer.cpp b/ydb/core/tx/schemeshard/schemeshard_path_describer.cpp index 943844615294..234706bba62c 100644 --- a/ydb/core/tx/schemeshard/schemeshard_path_describer.cpp +++ b/ydb/core/tx/schemeshard/schemeshard_path_describer.cpp @@ -944,11 +944,7 @@ THolder TPathDescriber::Describe pathStr = path.PathString(); } - Result.Reset(new TEvSchemeShard::TEvDescribeSchemeResultBuilder( - pathStr, - Self->TabletID(), - pathId - )); + Result.Reset(new TEvSchemeShard::TEvDescribeSchemeResultBuilder(pathStr, pathId)); Result->Record.SetStatus(checks.GetStatus()); Result->Record.SetReason(checks.GetError()); @@ -963,7 +959,7 @@ THolder TPathDescriber::Describe } } - Result = MakeHolder(pathStr, Self->TabletID(), pathId); + Result = MakeHolder(pathStr, pathId); auto descr = Result->Record.MutablePathDescription()->MutableSelf(); FillPathDescr(descr, path); diff --git a/ydb/core/tx/tx_proxy/describe.cpp b/ydb/core/tx/tx_proxy/describe.cpp index aed5e9c91d8c..980cebb18c8c 100644 --- a/ydb/core/tx/tx_proxy/describe.cpp +++ b/ydb/core/tx/tx_proxy/describe.cpp @@ -83,7 +83,7 @@ class TDescribeReq : public TActor { auto schemeShardId = entry.DomainInfo->DomainKey.OwnerId; auto result = MakeHolder( - path, schemeShardId, TPathId()); + path, TPathId()); auto* pathDescription = result->Record.MutablePathDescription(); auto* self = pathDescription->MutableSelf(); @@ -151,7 +151,7 @@ class TDescribeReq : public TActor { auto schemeShardId = entry.DomainInfo->DomainKey.OwnerId; auto result = MakeHolder( - path, schemeShardId, TPathId()); + path, TPathId()); auto* pathDescription = result->Record.MutablePathDescription(); auto* self = pathDescription->MutableSelf(); @@ -237,7 +237,7 @@ void TDescribeReq::Handle(TEvTxProxyReq::TEvNavigateScheme::TPtr &ev, const TAct if (record.GetDescribePath().GetPath() == "/") { // Special handling for enumerating roots TAutoPtr result = - new NSchemeShard::TEvSchemeShard::TEvDescribeSchemeResultBuilder("/", NSchemeShard::RootSchemeShardId, TPathId(NSchemeShard::RootSchemeShardId, NSchemeShard::RootPathId)); + new NSchemeShard::TEvSchemeShard::TEvDescribeSchemeResultBuilder("/", TPathId(NSchemeShard::RootSchemeShardId, NSchemeShard::RootPathId)); auto descr = result->Record.MutablePathDescription(); FillRootDescr(descr->MutableSelf(), "/", NSchemeShard::RootSchemeShardId); for (const auto& domain : domainsInfo->Domains) { diff --git a/ydb/core/viewer/json_describe.h b/ydb/core/viewer/json_describe.h index 36a03a8daca4..41de09e1c2d4 100644 --- a/ydb/core/viewer/json_describe.h +++ b/ydb/core/viewer/json_describe.h @@ -187,7 +187,6 @@ class TJsonDescribe : public TViewerPipeClient { result->SetReason(record.GetReason()); result->SetPath(record.GetPath()); result->MutablePathDescription()->CopyFrom(record.GetPathDescription()); - result->SetPathOwner(record.GetPathOwner()); result->SetPathId(record.GetPathId()); result->SetLastExistedPrefixPath(record.GetLastExistedPrefixPath()); result->SetLastExistedPrefixPathId(record.GetLastExistedPrefixPathId()); @@ -206,7 +205,6 @@ class TJsonDescribe : public TViewerPipeClient { TAutoPtr result(new NKikimrViewer::TEvDescribeSchemeInfo()); result->SetPath(path); - result->SetPathOwner(schemeShardId); result->SetPathId(pathId.LocalPathId); result->SetPathOwnerId(pathId.OwnerId); diff --git a/ydb/core/viewer/protos/viewer.proto b/ydb/core/viewer/protos/viewer.proto index 62218dde8ac5..61d0bc3ce413 100644 --- a/ydb/core/viewer/protos/viewer.proto +++ b/ydb/core/viewer/protos/viewer.proto @@ -561,7 +561,7 @@ message TEvDescribeSchemeInfo { optional string Reason = 2; optional string Path = 3; optional NKikimrSchemeOp.TPathDescription PathDescription = 4; - optional fixed64 PathOwner = 5; + optional fixed64 DEPRECATED_PathOwner = 5; // replaced by PathOwnerId optional fixed64 PathId = 6; optional string LastExistedPrefixPath = 7;