aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorijon <ijon@ydb.tech>2024-01-31 12:03:39 +0300
committerGitHub <noreply@github.com>2024-01-31 12:03:39 +0300
commit8c77bc140c36d19e235725554b9e5ceaf141d304 (patch)
tree5a8541952376c442f4ebb1aaab9228adb87ec8a8
parent03d8b00cb82919ad02a176fc7b6d32f678b7698f (diff)
downloadydb-8c77bc140c36d19e235725554b9e5ceaf141d304.tar.gz
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
-rw-r--r--ydb/core/tx/schemeshard/schemeshard__operation_memory_changes.cpp21
-rw-r--r--ydb/core/tx/schemeshard/schemeshard__operation_memory_changes.h10
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);