schemeshard: fix crash on concurrent alter-extsubdomains (#9201) (#10908

) Co-authored-by: ijon <ijon@ydb.tech>
ydb-platform · Oct 25, 2024 · a6b5e7e · a6b5e7e
1 parent a4c61b1
commit a6b5e7e
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 11 deletions.
diff --git a/ydb/core/tx/schemeshard/schemeshard__operation.cpp b/ydb/core/tx/schemeshard/schemeshard__operation.cpp
@@ -1263,7 +1263,7 @@ TVector<ISubOperation::TPtr> TOperation::ConstructParts(const TTxTransaction& tx
     case NKikimrSchemeOp::EOperationType::ESchemeOpCreateSubDomain:
         return {CreateSubDomain(NextPartId(), tx)};
     case NKikimrSchemeOp::EOperationType::ESchemeOpAlterSubDomain:
-        return {CreateCompatibleSubdomainAlter(context.SS, NextPartId(), tx)};
+        return CreateCompatibleSubdomainAlter(NextPartId(), tx, context);
     case NKikimrSchemeOp::EOperationType::ESchemeOpDropSubDomain:
         return {CreateDropSubdomain(NextPartId(), tx)};
     case NKikimrSchemeOp::EOperationType::ESchemeOpForceDropSubDomain:

diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_alter_extsubdomain.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_alter_extsubdomain.cpp
@@ -814,7 +814,7 @@ class TSyncHive: public TSubOperationState {
     }
 
     bool HandleReply(TEvHive::TEvUpdateDomainReply::TPtr& ev, TOperationContext& context) override {
-        const TTabletId hive = TTabletId(ev->Get()->Record.GetOrigin()); 
+        const TTabletId hive = TTabletId(ev->Get()->Record.GetOrigin());
 
         LOG_I(DebugHint() << "HandleReply TEvUpdateDomainReply"
             << ", from hive: " << hive);
@@ -1086,7 +1086,13 @@ ISubOperation::TPtr CreateAlterExtSubDomain(TOperationId id, TTxState::ETxState
 }
 
 TVector<ISubOperation::TPtr> CreateCompatibleAlterExtSubDomain(TOperationId id, const TTxTransaction& tx, TOperationContext& context) {
-    Y_ABORT_UNLESS(tx.GetOperationType() == NKikimrSchemeOp::ESchemeOpAlterExtSubDomain);
+    //NOTE: Accepting ESchemeOpAlterSubDomain operation for an ExtSubDomain is a special compatibility case
+    // for those old subdomains that at the time went through migration to a separate tenants.
+    // Console tablet holds records about types of the subdomains but they hadn't been updated
+    // at the migration time. So Console still thinks that old subdomains are plain subdomains
+    // whereas they had been migrated to the extsubdomains.
+    // This compatibility case should be upholded until Console records would be updated.
+    Y_ABORT_UNLESS(tx.GetOperationType() == NKikimrSchemeOp::ESchemeOpAlterExtSubDomain || tx.GetOperationType() == NKikimrSchemeOp::ESchemeOpAlterSubDomain);
 
     LOG_I("CreateCompatibleAlterExtSubDomain, opId " << id
         << ", feature flag EnableAlterDatabaseCreateHiveFirst " << context.SS->EnableAlterDatabaseCreateHiveFirst

diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_part.h b/ydb/core/tx/schemeshard/schemeshard__operation_part.h
@@ -493,7 +493,7 @@ ISubOperation::TPtr CreateAlterSubDomain(TOperationId id, const TTxTransaction&
 ISubOperation::TPtr CreateAlterSubDomain(TOperationId id, TTxState::ETxState state);
 
 ISubOperation::TPtr CreateCompatibleSubdomainDrop(TSchemeShard* ss, TOperationId id, const TTxTransaction& tx);
-ISubOperation::TPtr CreateCompatibleSubdomainAlter(TSchemeShard* ss, TOperationId id, const TTxTransaction& tx);
+TVector<ISubOperation::TPtr> CreateCompatibleSubdomainAlter(TOperationId id, const TTxTransaction& tx, TOperationContext& context);
 
 ISubOperation::TPtr CreateUpgradeSubDomain(TOperationId id, const TTxTransaction& tx);
 ISubOperation::TPtr CreateUpgradeSubDomain(TOperationId id, TTxState::ETxState state);
@@ -514,10 +514,10 @@ ISubOperation::TPtr CreateExtSubDomain(TOperationId id, TTxState::ETxState state
 
 // Alter
 TVector<ISubOperation::TPtr> CreateCompatibleAlterExtSubDomain(TOperationId nextId, const TTxTransaction& tx, TOperationContext& context);
-ISubOperation::TPtr CreateAlterExtSubDomain(TOperationId id, const TTxTransaction& tx);
 ISubOperation::TPtr CreateAlterExtSubDomain(TOperationId id, TTxState::ETxState state);
-ISubOperation::TPtr CreateAlterExtSubDomainCreateHive(TOperationId id, const TTxTransaction& tx);
 ISubOperation::TPtr CreateAlterExtSubDomainCreateHive(TOperationId id, TTxState::ETxState state);
+//NOTE: no variants to construct individual suboperations directly from TTxTransaction --
+// -- it should be possible only through CreateCompatibleAlterExtSubDomain
 
 // Drop
 ISubOperation::TPtr CreateForceDropExtSubDomain(TOperationId id, const TTxTransaction& tx);

diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_upgrade_subdomain.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_upgrade_subdomain.cpp
@@ -1530,13 +1530,13 @@ ISubOperation::TPtr CreateCompatibleSubdomainDrop(TSchemeShard* ss, TOperationId
     return CreateForceDropSubDomain(id, tx);
 }
 
-ISubOperation::TPtr CreateCompatibleSubdomainAlter(TSchemeShard* ss, TOperationId id, const TTxTransaction& tx) {
+TVector<ISubOperation::TPtr> CreateCompatibleSubdomainAlter(TOperationId id, const TTxTransaction& tx, TOperationContext& context) {
     const auto& info = tx.GetSubDomain();
 
     const TString& parentPathStr = tx.GetWorkingDir();
     const TString& name = info.GetName();
 
-    TPath path = TPath::Resolve(parentPathStr, ss).Dive(name);
+    TPath path = TPath::Resolve(parentPathStr, context.SS).Dive(name);
 
     {
         TPath::TChecker checks = path.Check();
@@ -1546,15 +1546,16 @@ ISubOperation::TPtr CreateCompatibleSubdomainAlter(TSchemeShard* ss, TOperationI
             .NotDeleted();
 
         if (!checks) {
-            return CreateAlterSubDomain(id, tx);
+            return {CreateAlterSubDomain(id, tx)};
         }
     }
 
     if (path.Base()->IsExternalSubDomainRoot()) {
-        return CreateAlterExtSubDomain(id, tx);
+        // plain subdomains don't have subdomain/tenant hives so only single operation should be returned here
+        return CreateCompatibleAlterExtSubDomain(id, tx, context);
     }
 
-    return CreateAlterSubDomain(id, tx);
+    return {CreateAlterSubDomain(id, tx)};
 }
 
 }