schemeshard: fix copy-tables memory hog (#1442)

Export launches copy-tables operation, copy-tables invokes multiple
copy table suboperations, each copy-table suboperation grabs and
saves aside metadata of table's subdomain (to be able to restore
original subdomain state in case IgniteOperation() fails).

Straightforward implementation of TMemChanges resulted in that
the same and only subdomain being copied multiple times.

Which for massive export actions with many-many tables copied
is both cpu intensive and, more importantly, requires excessive
amount of memory (up to several observed OOM events).

This fix changes TMemChanges to store only one subdomain state
version per operation. Which is correct and appropriate for
copy-tables and all other (sub)operations.

KIKIMR-20242

2 files changed, 19 insertions, 12 deletions

diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_memory_changes.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_memory_changes.cpp
index 925576a38d..49b5d6904b 100644
--- a/ydb/core/tx/schemeshard/schemeshard__operation_memory_changes.cpp
+++ b/ydb/core/tx/schemeshard/schemeshard__operation_memory_changes.cpp
@@ -47,7 +47,13 @@ void TMemoryChanges::GrabShard(TSchemeShard *ss, const TShardIdx &shardId) {
 }
 
 void TMemoryChanges::GrabDomain(TSchemeShard* ss, const TPathId& pathId) {
-    Grab<TSubDomainInfo>(pathId, ss->SubDomains, SubDomains);
+    // Copy TSubDomainInfo from ss->SubDomains to local SubDomains.
+    // Make sure that copy will be made only when needed.
+    const auto found = ss->SubDomains.find(pathId);
+    Y_ABORT_UNLESS(found != ss->SubDomains.end());
+    if (!SubDomains.contains(pathId)) {
+        SubDomains.emplace(pathId, MakeIntrusive<TSubDomainInfo>(*found->second));
+    }
 }
 
 void TMemoryChanges::GrabNewIndex(TSchemeShard* ss, const TPathId& pathId) {
@@ -178,15 +184,12 @@ void TMemoryChanges::UnDo(TSchemeShard* ss) {
         Shards.pop();
     }
 
-    while (SubDomains) {
-        const auto& [id, elem] = SubDomains.top();
-        if (elem) {
-            ss->SubDomains[id] = elem;
-        } else {
-            ss->SubDomains.erase(id);
-        }
-        SubDomains.pop();
+    // Restore ss->SubDomains entries to saved copies of TSubDomainInfo objects.
+    // No copy, simple pointer replacement.
+    for (const auto& [id, elem] : SubDomains) {
+        ss->SubDomains[id] = elem;
     }
+    SubDomains.clear();
 
     while (TxStates) {
         const auto& [id, elem] = TxStates.top();
diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_memory_changes.h b/ydb/core/tx/schemeshard/schemeshard__operation_memory_changes.h
index 702edd9197..35291e43f2 100644
--- a/ydb/core/tx/schemeshard/schemeshard__operation_memory_changes.h
+++ b/ydb/core/tx/schemeshard/schemeshard__operation_memory_changes.h
@@ -33,8 +33,12 @@ class TMemoryChanges: public TSimpleRefCount<TMemoryChanges> {
     using TShardState = std::pair<TShardIdx, THolder<TShardInfo>>;
     TStack<TShardState> Shards;
 
-    using TSubDomainState = std::pair<TPathId, TSubDomainInfo::TPtr>;
-    TStack<TSubDomainState> SubDomains;
+    // Actually, any single subdomain should not be grabbed at more than one version
+    // per transaction/operation.
+    // And transaction/operation could not work on more than one subdomain.
+    // But just to be on the safe side (migrated paths, anyone?) we allow several
+    // subdomains to be grabbed.
+    THashMap<TPathId, TSubDomainInfo::TPtr> SubDomains;
 
     using TTxState = std::pair<TOperationId, THolder<TTxState>>;
     TStack<TTxState> TxStates;
@@ -78,7 +82,7 @@ public:
     void GrabExternalTable(TSchemeShard* ss, const TPathId& pathId);
 
     void GrabExternalDataSource(TSchemeShard* ss, const TPathId& pathId);
-    
+
     void GrabNewView(TSchemeShard* ss, const TPathId& pathId);
     void GrabView(TSchemeShard* ss, const TPathId& pathId);