prestable-24-4: enable feature flag EnableAlterDatabaseCreateHiveFirst

ydb/core/protos/feature_flags.proto

@@ -94,7 +94,7 @@ message TFeatureFlags {
     optional bool EnableDataShardGenericReadSets = 81 [default = false];
     // enable alter database operation to create subdomain's system tablets
     // directly in subdomain's hive
-    optional bool EnableAlterDatabaseCreateHiveFirst = 82 [default = false];
+    optional bool EnableAlterDatabaseCreateHiveFirst = 82 [default = true];
     reserved 83; // EnableKqpDataQuerySourceRead
     optional bool EnableSmallDiskOptimization = 84 [default = true];
     optional bool EnableDataShardVolatileTransactions = 85 [default = true];

ydb/core/tx/coordinator/coordinator_ut.cpp

@@ -8,6 +8,21 @@
 #include <library/cpp/testing/unittest/registar.h>
 #include <library/cpp/threading/future/async.h>
 
+// ad-hoc test parametrization support: only for single boolean flag
+// taken from ydb/core/ut/common/kqp_ut_common.h:Y_UNIT_TEST_TWIN
+//TODO: introduce general support for test parametrization?
+#define Y_UNIT_TEST_FLAG(N, OPT)                                                                                   \
+    template<bool OPT> void N(NUnitTest::TTestContext&);                                                           \
+    struct TTestRegistration##N {                                                                                  \
+        TTestRegistration##N() {                                                                                   \
+            TCurrentTest::AddTest(#N "-" #OPT "-false", static_cast<void (*)(NUnitTest::TTestContext&)>(&N<false>), false); \
+            TCurrentTest::AddTest(#N "-" #OPT "-true", static_cast<void (*)(NUnitTest::TTestContext&)>(&N<true>), false);   \
+        }                                                                                                          \
+    };                                                                                                             \
+    static TTestRegistration##N testRegistration##N;                                                               \
+    template<bool OPT>                                                                                             \
+    void N(NUnitTest::TTestContext&)
+
 namespace NKikimr::NFlatTxCoordinator::NTest {
 
     using namespace Tests;
@@ -379,7 +394,7 @@ namespace NKikimr::NFlatTxCoordinator::NTest {
             Ydb::Cms::CreateDatabaseRequest,
             Ydb::Cms::CreateDatabaseResponse>;
 
-        Y_UNIT_TEST(RestoreTenantConfiguration) {
+        Y_UNIT_TEST_FLAG(RestoreTenantConfiguration, AlterDatabaseCreateHiveFirst) {
             struct TCoordinatorHooks : public ICoordinatorHooks {
                 bool AllowPersistConfig_ = true;
                 std::unordered_map<ui64, NKikimrSubDomains::TProcessingParams> PersistConfig_;
@@ -397,7 +412,8 @@ namespace NKikimr::NFlatTxCoordinator::NTest {
                 .SetNodeCount(1)
                 .SetDynamicNodeCount(4)
                 .SetUseRealThreads(false)
-                .AddStoragePoolType("ssd");
+                .AddStoragePoolType("ssd")
+                .SetEnableAlterDatabaseCreateHiveFirst(AlterDatabaseCreateHiveFirst);
 
             Tests::TServer::TPtr server = new TServer(serverSettings);
 
@@ -414,6 +430,8 @@ namespace NKikimr::NFlatTxCoordinator::NTest {
             hooks.AllowPersistConfig_ = false;
             hooks.PersistConfig_.clear();
 
+            const ui32 ExpectedProcessingParamsVersion = 2 + (AlterDatabaseCreateHiveFirst ? 1 : 0);
+
             auto createDatabase = [&]() {
                 Ydb::Cms::CreateDatabaseRequest request;
                 request.set_path("/Root/db1");
@@ -445,8 +463,8 @@ namespace NKikimr::NFlatTxCoordinator::NTest {
             UNIT_ASSERT_C(hooks.PersistConfig_.size() > 0, "Expected coordinators to attempt to persist configs");
             std::vector<ui64> coordinators;
             for (auto& pr : hooks.PersistConfig_) {
-                UNIT_ASSERT_C(pr.second.GetVersion() == 2,
-                    "Expected tenant coordinator to have a version 2 config:\n" << pr.second.DebugString());
+                UNIT_ASSERT_C(pr.second.GetVersion() == ExpectedProcessingParamsVersion,
+                    "Expected tenant coordinator to have a version " << ExpectedProcessingParamsVersion << " config:\n" << pr.second.DebugString());
                 coordinators.push_back(pr.first);
             }
 
@@ -460,8 +478,8 @@ namespace NKikimr::NFlatTxCoordinator::NTest {
             runtime.SimulateSleep(TDuration::MilliSeconds(50));
             UNIT_ASSERT_C(hooks.PersistConfig_.size() == coordinators.size(), "Expected all coordinators to persist restored config");
             for (auto& pr : hooks.PersistConfig_) {
-                UNIT_ASSERT_C(pr.second.GetVersion() == 2,
-                    "Expected tenant coordinator to restore a version 2 config:\n" << pr.second.DebugString());
+                UNIT_ASSERT_C(pr.second.GetVersion() == ExpectedProcessingParamsVersion,
+                    "Expected tenant coordinator to restore a version " << ExpectedProcessingParamsVersion << " config:\n" << pr.second.DebugString());
             }
 
             // runtime.SetLogPriority(NKikimrServices::TABLET_MAIN, NActors::NLog::PRI_DEBUG);

ydb/core/tx/schemeshard/ut_serverless/ut_serverless.cpp

@@ -13,9 +13,9 @@ Y_UNIT_TEST_SUITE(TSchemeShardServerLess) {
     Y_UNIT_TEST(Fake) {
     }
 
-    Y_UNIT_TEST(BaseCase) {
+    Y_UNIT_TEST_FLAG(BaseCase, AlterDatabaseCreateHiveFirst) {
         TTestBasicRuntime runtime;
-        TTestEnv env(runtime);
+        TTestEnv env(runtime, TTestEnvOptions().EnableAlterDatabaseCreateHiveFirst(AlterDatabaseCreateHiveFirst));
         ui64 txId = 100;
 
         TestCreateExtSubDomain(runtime, ++txId,  "/MyRoot",
@@ -106,7 +106,17 @@ Y_UNIT_TEST_SUITE(TSchemeShardServerLess) {
                             NLs::PathsInsideDomain(1),
                             NLs::ShardsInsideDomain(0)});
 
-        ui64 sharedHiveTablets = TTestTxConfig::FakeHiveTablets + NKikimr::TFakeHiveState::TABLETS_PER_CHILD_HIVE;
+        // Check that shards of ServerLess0 db are gone after its deletion.
+        //
+        // SharedDB contains:
+        //  - 3 of its own shards: SchemeShard, Coordinator, Mediator
+        //  - 4 of ServerLess0's shards: SchemeShard, Coordinator, Mediator, 1 x DataShard of the table0
+        //
+
+        //NOTE: AlterDatabaseCreateHiveFirst create system tablets in a tenant hive, otherwise they are created in the root hive
+        ui64 sharedHiveTablets = TTestTxConfig::FakeHiveTablets + (AlterDatabaseCreateHiveFirst ? TFakeHiveState::TABLETS_PER_CHILD_HIVE : 1)
+            + 3  // shards of SharedDB
+        ;
         env.TestWaitTabletDeletion(runtime, xrange(sharedHiveTablets, sharedHiveTablets + 4), sharedHive);
     }
 
@@ -299,7 +309,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardServerLess) {
         UNIT_ASSERT(sharedDbSchemeShard != 0
                     && sharedDbSchemeShard != (ui64)-1
                     && sharedDbSchemeShard != TTestTxConfig::SchemeShard);
-                    
+
         TestDescribeResult(DescribePath(runtime, sharedDbSchemeShard, "/MyRoot/SharedDB"),
                            {NLs::PathExist,
                             NLs::ServerlessComputeResourcesMode(EServerlessComputeResourcesModeUnspecified)});
@@ -308,7 +318,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardServerLess) {
             R"(
                 ResourcesDomainKey {
                     SchemeShard: %lu
-                    PathId: 2 
+                    PathId: 2
                 }
                 Name: "ServerLess0"
             )",
@@ -348,7 +358,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardServerLess) {
         TestDescribeResult(DescribePath(runtime, tenantSchemeShard, "/MyRoot/ServerLess0"),
                            {NLs::PathExist,
                             NLs::ServerlessComputeResourcesMode(EServerlessComputeResourcesModeShared)});
-        
+
         auto checkServerlessComputeResourcesMode = [&](EServerlessComputeResourcesMode serverlessComputeResourcesMode) {
             TString alterData = Sprintf(
                 R"(
@@ -406,7 +416,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardServerLess) {
             R"(
                 ResourcesDomainKey {
                     SchemeShard: %lu
-                    PathId: 2 
+                    PathId: 2
                 }
                 Name: "ServerLess0"
             )",
@@ -487,7 +497,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardServerLess) {
             R"(
                 ResourcesDomainKey {
                     SchemeShard: %lu
-                    PathId: 2 
+                    PathId: 2
                 }
                 Name: "ServerLess0"
             )",

ydb/core/tx/schemeshard/ut_serverless/ya.make

@@ -6,6 +6,8 @@ TIMEOUT(600)
 
 SIZE(MEDIUM)
 
+TIMEOUT(120)
+
 PEERDIR(
     library/cpp/getopt
     library/cpp/regex/pcre