diff --git a/ydb/core/cms/api_adapters.cpp b/ydb/core/cms/api_adapters.cpp index 4ec7c2c7c11b..6badb3939950 100644 --- a/ydb/core/cms/api_adapters.cpp +++ b/ydb/core/cms/api_adapters.cpp @@ -378,6 +378,7 @@ class TCreateMaintenanceTask: public TPermissionResponseProcessor< cmsRequest.SetAvailabilityMode(ConvertAvailabilityMode(opts.availability_mode())); cmsRequest.SetPartialPermissionAllowed(true); cmsRequest.SetSchedule(true); + cmsRequest.SetPriority(opts.priority()); for (const auto& group : request.action_groups()) { Y_ABORT_UNLESS(group.actions().size() == 1); @@ -559,7 +560,8 @@ class TGetMaintenanceTask: public TAdapterActor< opts.set_task_uid(taskUid); opts.set_description(request.GetReason()); opts.set_availability_mode(ConvertAvailabilityMode(request.GetAvailabilityMode())); - + opts.set_priority(request.GetPriority()); + // pending actions for (const auto& action : request.GetActions()) { ConvertAction(action, *result.add_action_group_states()->add_action_states()); diff --git a/ydb/core/cms/cluster_info.cpp b/ydb/core/cms/cluster_info.cpp index ffeab7b70d2d..1bfa92b04e68 100644 --- a/ydb/core/cms/cluster_info.cpp +++ b/ydb/core/cms/cluster_info.cpp @@ -60,12 +60,12 @@ bool TLockableItem::IsLocked(TErrorInfo &error, TDuration defaultRetryTime, return true; } - if (!ScheduledLocks.empty() && ScheduledLocks.begin()->Order < DeactivatedLocksOrder) { + if (!ScheduledLocks.empty() && ScheduledLocks.begin()->Priority < DeactivatedLocksPriority) { error.Code = TStatus::DISALLOW_TEMP; - error.Reason = Sprintf("%s has scheduled action %s owned by %s (order %" PRIu64 " vs %" PRIu64 ")", + error.Reason = Sprintf("%s has scheduled action %s owned by %s (priority %" PRIi32 " vs %" PRIi32 ")", PrettyItemName().data(), ScheduledLocks.begin()->RequestId.data(), - ScheduledLocks.begin()->Owner.data(), ScheduledLocks.begin()->Order, - DeactivatedLocksOrder); + ScheduledLocks.begin()->Owner.data(), ScheduledLocks.begin()->Priority, + DeactivatedLocksPriority); error.Deadline = now + defaultRetryTime; return true; } @@ -113,12 +113,12 @@ void TLockableItem::RollbackLocks(ui64 point) void TLockableItem::ReactivateScheduledLocks() { - DeactivatedLocksOrder = Max(); + DeactivatedLocksPriority = Max(); } -void TLockableItem::DeactivateScheduledLocks(ui64 order) +void TLockableItem::DeactivateScheduledLocks(i32 priority) { - DeactivatedLocksOrder = order; + DeactivatedLocksPriority = priority; } void TLockableItem::RemoveScheduledLocks(const TString &requestId) @@ -650,8 +650,11 @@ void TClusterInfo::ApplyActionWithoutLog(const NKikimrCms::TAction &action) case TAction::REBOOT_HOST: if (auto nodes = NodePtrs(action.GetHost(), MakeServices(action))) { for (const auto node : nodes) { - for (auto &nodeGroup: node->NodeGroups) - nodeGroup->LockNode(node->NodeId); + for (auto &nodeGroup: node->NodeGroups) { + if (!nodeGroup->IsNodeLocked(node->NodeId)) { + nodeGroup->LockNode(node->NodeId); + } + } } } break; @@ -659,12 +662,18 @@ void TClusterInfo::ApplyActionWithoutLog(const NKikimrCms::TAction &action) for (const auto &device : action.GetDevices()) { if (HasPDisk(device)) { auto pdisk = &PDiskRef(device); - for (auto &nodeGroup: NodeRef(pdisk->NodeId).NodeGroups) - nodeGroup->LockNode(pdisk->NodeId); + for (auto &nodeGroup: NodeRef(pdisk->NodeId).NodeGroups) { + if (!nodeGroup->IsNodeLocked(pdisk->NodeId)) { + nodeGroup->LockNode(pdisk->NodeId); + } + } } else if (HasVDisk(device)) { auto vdisk = &VDiskRef(device); - for (auto &nodeGroup: NodeRef(vdisk->NodeId).NodeGroups) - nodeGroup->LockNode(vdisk->NodeId); + for (auto &nodeGroup: NodeRef(vdisk->NodeId).NodeGroups) { + if (!nodeGroup->IsNodeLocked(vdisk->NodeId)) { + nodeGroup->LockNode(vdisk->NodeId); + } + } } } break; @@ -756,7 +765,7 @@ ui64 TClusterInfo::AddLocks(const TPermissionInfo &permission, const TActorConte || permission.Action.GetType() == TAction::REBOOT_HOST || permission.Action.GetType() == TAction::REPLACE_DEVICES)) { item->State = RESTART; - lock = true;; + lock = true; } if (lock) { @@ -854,7 +863,7 @@ ui64 TClusterInfo::ScheduleActions(const TRequestInfo &request, const TActorCont auto items = FindLockedItems(action, ctx); for (auto item : items) - item->ScheduleLock({action, request.Owner, request.RequestId, request.Order}); + item->ScheduleLock({action, request.Owner, request.RequestId, request.Priority}); locks += items.size(); } @@ -868,10 +877,10 @@ void TClusterInfo::UnscheduleActions(const TString &requestId) entry.second->RemoveScheduledLocks(requestId); } -void TClusterInfo::DeactivateScheduledLocks(ui64 order) +void TClusterInfo::DeactivateScheduledLocks(i32 priority) { for (auto &entry : LockableItems) - entry.second->DeactivateScheduledLocks(order); + entry.second->DeactivateScheduledLocks(priority); } void TClusterInfo::ReactivateScheduledLocks() @@ -1020,8 +1029,11 @@ void TOperationLogManager::ApplyAction(const NKikimrCms::TAction &action, case NKikimrCms::TAction::REBOOT_HOST: if (auto nodes = clusterState->NodePtrs(action.GetHost(), MakeServices(action))) { for (const auto node : nodes) { - for (auto &nodeGroup: node->NodeGroups) - AddNodeLockOperation(node->NodeId, nodeGroup); + for (auto &nodeGroup: node->NodeGroups) { + if (!nodeGroup->IsNodeLocked(node->NodeId)) { + AddNodeLockOperation(node->NodeId, nodeGroup); + } + } } } break; @@ -1029,13 +1041,18 @@ void TOperationLogManager::ApplyAction(const NKikimrCms::TAction &action, for (const auto &device : action.GetDevices()) { if (clusterState->HasPDisk(device)) { auto pdisk = &clusterState->PDisk(device); - for (auto &nodeGroup: clusterState->NodeRef(pdisk->NodeId).NodeGroups) - AddNodeLockOperation(pdisk->NodeId, nodeGroup); - + for (auto &nodeGroup: clusterState->NodeRef(pdisk->NodeId).NodeGroups) { + if (!nodeGroup->IsNodeLocked(pdisk->NodeId)) { + AddNodeLockOperation(pdisk->NodeId, nodeGroup); + } + } } else if (clusterState->HasVDisk(device)) { auto vdisk = &clusterState->VDisk(device); - for (auto &nodeGroup: clusterState->NodeRef(vdisk->NodeId).NodeGroups) - AddNodeLockOperation(vdisk->NodeId, nodeGroup); + for (auto &nodeGroup: clusterState->NodeRef(vdisk->NodeId).NodeGroups) { + if (!nodeGroup->IsNodeLocked(vdisk->NodeId)) { + AddNodeLockOperation(vdisk->NodeId, nodeGroup); + } + } } } break; diff --git a/ydb/core/cms/cluster_info.h b/ydb/core/cms/cluster_info.h index 5ba7fddd126f..f9439ad46d15 100644 --- a/ydb/core/cms/cluster_info.h +++ b/ydb/core/cms/cluster_info.h @@ -97,11 +97,13 @@ struct TRequestInfo { request.SetPartialPermissionAllowed(Request.GetPartialPermissionAllowed()); request.SetReason(Request.GetReason()); request.SetAvailabilityMode(Request.GetAvailabilityMode()); + request.SetPriority(Priority); } TString RequestId; TString Owner; ui64 Order = 0; + i32 Priority = 0; NKikimrCms::TPermissionRequest Request; }; @@ -203,10 +205,10 @@ class TLockableItem : public TThrRefBase { }; struct TScheduledLock : TBaseLock { - TScheduledLock(const NKikimrCms::TAction &action, const TString &owner, const TString &requestId, ui64 order) + TScheduledLock(const NKikimrCms::TAction &action, const TString &owner, const TString &requestId, i32 priority) : TBaseLock(owner, action) , RequestId(requestId) - , Order(order) + , Priority(priority) { } @@ -217,7 +219,7 @@ class TLockableItem : public TThrRefBase { TScheduledLock &operator=(TScheduledLock &&other) = default; TString RequestId; - ui64 Order = 0; + i32 Priority = 0; }; struct TTemporaryLock : TBaseLock { @@ -268,7 +270,7 @@ class TLockableItem : public TThrRefBase { void ScheduleLock(TScheduledLock &&lock) { auto pos = LowerBound(ScheduledLocks.begin(), ScheduledLocks.end(), lock, [](auto &l, auto &r) { - return l.Order < r.Order; + return l.Priority < r.Priority; }); ScheduledLocks.insert(pos, lock); } @@ -278,7 +280,7 @@ class TLockableItem : public TThrRefBase { void RollbackLocks(ui64 point); - void DeactivateScheduledLocks(ui64 order); + void DeactivateScheduledLocks(i32 priority); void ReactivateScheduledLocks(); void RemoveScheduledLocks(const TString &requestId); @@ -296,7 +298,7 @@ class TLockableItem : public TThrRefBase { std::list ExternalLocks; std::list ScheduledLocks; TVector TempLocks; - ui64 DeactivatedLocksOrder = Max(); + i32 DeactivatedLocksPriority = Max(); THashSet Markers; }; @@ -667,7 +669,6 @@ class TClusterInfo : public TThrRefBase { TOperationLogManager LogManager; TOperationLogManager ScheduledLogManager; - void ApplyActionToOperationLog(const NKikimrCms::TAction &action); void ApplyActionWithoutLog(const NKikimrCms::TAction &action); void ApplyNodeLimits(ui32 clusterLimit, ui32 clusterRatioLimit, ui32 tenantLimit, ui32 tenantRatioLimit); @@ -912,7 +913,7 @@ class TClusterInfo : public TThrRefBase { ui64 AddTempLocks(const NKikimrCms::TAction &action, const TActorContext *ctx); ui64 ScheduleActions(const TRequestInfo &request, const TActorContext *ctx); void UnscheduleActions(const TString &requestId); - void DeactivateScheduledLocks(ui64 order); + void DeactivateScheduledLocks(i32 priority); void ReactivateScheduledLocks(); void RollbackLocks(ui64 point); diff --git a/ydb/core/cms/cluster_info_ut.cpp b/ydb/core/cms/cluster_info_ut.cpp index cfe643214261..9fc6a14cc9f7 100644 --- a/ydb/core/cms/cluster_info_ut.cpp +++ b/ydb/core/cms/cluster_info_ut.cpp @@ -199,33 +199,33 @@ void AddActions(TRequestInfo &request, const NKikimrCms::TAction &action, Ts... } template -TRequestInfo MakeRequest(const TString &id, const TString &owner, ui64 order, Ts... actions) +TRequestInfo MakeRequest(const TString &id, const TString &owner, i32 priority, Ts... actions) { TRequestInfo res; res.RequestId = id; res.Owner = owner; - res.Order = order; + res.Priority = priority; AddActions(res, actions...); return res; } template -void CheckScheduledLocks(I pos, I end, const TString &id, const TString &owner, ui64 order) +void CheckScheduledLocks(I pos, I end, const TString &id, const TString &owner, i32 priority) { UNIT_ASSERT(pos != end); UNIT_ASSERT_VALUES_EQUAL(pos->RequestId, id); UNIT_ASSERT_VALUES_EQUAL(pos->Owner, owner); - UNIT_ASSERT_VALUES_EQUAL(pos->Order, order); + UNIT_ASSERT_VALUES_EQUAL(pos->Priority, priority); UNIT_ASSERT(++pos == end); } template -void CheckScheduledLocks(I pos, I end, const TString &id, const TString &owner, ui64 order, Ts... locks) +void CheckScheduledLocks(I pos, I end, const TString &id, const TString &owner, i32 priority, Ts... locks) { UNIT_ASSERT(pos != end); UNIT_ASSERT_VALUES_EQUAL(pos->RequestId, id); UNIT_ASSERT_VALUES_EQUAL(pos->Owner, owner); - UNIT_ASSERT_VALUES_EQUAL(pos->Order, order); + UNIT_ASSERT_VALUES_EQUAL(pos->Priority, priority); CheckScheduledLocks(++pos, end, locks...); } @@ -442,7 +442,7 @@ Y_UNIT_TEST_SUITE(TClusterInfoTest) { "request-3", "user-3", 3, "request-4", "user-4", 4); - cluster->DeactivateScheduledLocks(request2.Order); + cluster->DeactivateScheduledLocks(request2.Priority); TErrorInfo error; UNIT_ASSERT(cluster->Node(1).IsLocked(error, TDuration(), Now(), TDuration())); diff --git a/ydb/core/cms/cms.cpp b/ydb/core/cms/cms.cpp index 12f543aa5cc8..9abf3ea981fc 100644 --- a/ydb/core/cms/cms.cpp +++ b/ydb/core/cms/cms.cpp @@ -50,6 +50,8 @@ void TCms::OnActivateExecutor(const TActorContext &ctx) return; } + EnableCMSRequestPriorities = AppData(ctx)->FeatureFlags.GetEnableCMSRequestPriorities(); + Executor()->RegisterExternalTabletCounters(TabletCountersPtr.Release()); State->CmsTabletId = TabletID(); @@ -244,13 +246,16 @@ void TCms::ProcessInitQueue(const TActorContext &ctx) void TCms::SubscribeForConfig(const TActorContext &ctx) { - NConsole::SubscribeViaConfigDispatcher(ctx, {(ui32)NKikimrConsole::TConfigItem::CmsConfigItem}, ctx.SelfID); + NConsole::SubscribeViaConfigDispatcher(ctx, {(ui32)NKikimrConsole::TConfigItem::CmsConfigItem, + (ui32)NKikimrConsole::TConfigItem::FeatureFlagsItem}, ctx.SelfID); } void TCms::AdjustInfo(TClusterInfoPtr &info, const TActorContext &ctx) const { for (const auto &entry : State->Permissions) info->AddLocks(entry.second, &ctx); + for (const auto &entry : State->ScheduledRequests) + info->ScheduleActions(entry.second, &ctx); for (const auto &entry : State->Notifications) info->AddExternalLocks(entry.second, &ctx); for (const auto &entry : State->HostMarkers) @@ -305,6 +310,7 @@ bool TCms::CheckPermissionRequest(const TPermissionRequest &request, scheduled.SetDuration(request.GetDuration()); scheduled.SetTenantPolicy(request.GetTenantPolicy()); scheduled.SetAvailabilityMode(request.GetAvailabilityMode()); + scheduled.SetPriority(request.GetPriority()); } LOG_INFO_S(ctx, NKikimrServices::CMS, @@ -1467,6 +1473,20 @@ void TCms::CheckAndEnqueueRequest(TEvCms::TEvPermissionRequest::TPtr &ev, const } } + if (rec.GetPriority() && !EnableCMSRequestPriorities) { + if (rec.GetUser() == WALLE_CMS_USER) { + rec.ClearPriority(); + } else { + return ReplyWithError( + ev, TStatus::WRONG_REQUEST, "Unsupported: feature flag EnableCMSRequestPriorities is off", ctx); + } + } + + if (-100 > rec.GetPriority() || rec.GetPriority() > 100) { + return ReplyWithError( + ev, TStatus::WRONG_REQUEST, "Priority value is out of range", ctx); + } + EnqueueRequest(ev.Release(), ctx); } @@ -1819,7 +1839,6 @@ void TCms::Handle(TEvCms::TEvPermissionRequest::TPtr &ev, TAutoPtr resp = new TEvCms::TEvPermissionResponse; TRequestInfo scheduled; auto &rec = ev->Get()->Record; - TString user = rec.GetUser(); auto requestStartTime = TInstant::Now(); @@ -1859,12 +1878,25 @@ void TCms::Handle(TEvCms::TEvPermissionRequest::TPtr &ev, } } + ClusterInfo->LogManager.PushRollbackPoint(); + const i32 priority = rec.GetPriority(); + for (const auto &[_, scheduledRequest] : State->ScheduledRequests) { + if (scheduledRequest.Priority < priority) { + for (const auto &action : scheduledRequest.Request.GetActions()) { + ClusterInfo->LogManager.ApplyAction(action, ClusterInfo); + } + } + } + ClusterInfo->DeactivateScheduledLocks(priority); bool ok = CheckPermissionRequest(rec, resp->Record, scheduled.Request, ctx); + ClusterInfo->ReactivateScheduledLocks(); + ClusterInfo->LogManager.RollbackOperations(); // Schedule request if required. if (rec.GetDryRun()) { Reply(ev, std::move(resp), ctx); } else { + TString user = rec.GetUser(); auto reqId = user + "-r-" + ToString(State->NextRequestId++); resp->Record.SetRequestId(reqId); @@ -1872,7 +1904,9 @@ void TCms::Handle(TEvCms::TEvPermissionRequest::TPtr &ev, if (scheduled.Request.ActionsSize() || scheduled.Request.GetEvictVDisks()) { scheduled.Owner = user; scheduled.Order = State->NextRequestId - 1; + scheduled.Priority = priority; scheduled.RequestId = reqId; + ClusterInfo->ScheduleActions(scheduled, &ctx); copy = new TRequestInfo(scheduled); State->ScheduledRequests.emplace(reqId, std::move(scheduled)); @@ -1923,8 +1957,19 @@ void TCms::Handle(TEvCms::TEvCheckRequest::TPtr &ev, const TActorContext &ctx) auto requestStartTime = TInstant::Now(); + ClusterInfo->LogManager.PushRollbackPoint(); + for (const auto &scheduled_request : State->ScheduledRequests) { + if (scheduled_request.second.Priority < request.Priority) { + for (const auto &action : scheduled_request.second.Request.GetActions()) + ClusterInfo->LogManager.ApplyAction(action, ClusterInfo); + } + } + + ClusterInfo->DeactivateScheduledLocks(request.Priority); request.Request.SetAvailabilityMode(rec.GetAvailabilityMode()); bool ok = CheckPermissionRequest(request.Request, resp->Record, scheduled.Request, ctx); + ClusterInfo->ReactivateScheduledLocks(); + ClusterInfo->LogManager.RollbackOperations(); // Schedule request if required. if (rec.GetDryRun()) { @@ -1932,14 +1977,19 @@ void TCms::Handle(TEvCms::TEvCheckRequest::TPtr &ev, const TActorContext &ctx) } else { TAutoPtr copy; auto order = request.Order; + auto priority = request.Priority; + ClusterInfo->UnscheduleActions(request.RequestId); State->ScheduledRequests.erase(it); if (scheduled.Request.ActionsSize() || scheduled.Request.GetEvictVDisks()) { scheduled.Owner = user; scheduled.Order = order; + scheduled.Priority = priority; scheduled.RequestId = rec.GetRequestId(); resp->Record.SetRequestId(scheduled.RequestId); + ClusterInfo->ScheduleActions(scheduled, &ctx); + copy = new TRequestInfo(scheduled); State->ScheduledRequests.emplace(rec.GetRequestId(), std::move(scheduled)); } else { @@ -2222,7 +2272,12 @@ void TCms::Handle(TEvCms::TEvGetSentinelStateRequest::TPtr &ev, const TActorCont void TCms::Handle(TEvConsole::TEvConfigNotificationRequest::TPtr &ev, const TActorContext &ctx) -{ +{ + const auto& appConfig = ev->Get()->Record.GetConfig(); + if (appConfig.HasFeatureFlags()) { + EnableCMSRequestPriorities = appConfig.GetFeatureFlags().GetEnableCMSRequestPriorities(); + } + if (ev->Get()->Record.HasLocal() && ev->Get()->Record.GetLocal()) { Execute(CreateTxUpdateConfig(ev), ctx); } else { diff --git a/ydb/core/cms/cms_impl.h b/ydb/core/cms/cms_impl.h index 3f588599edf0..e25dd4349ccf 100644 --- a/ydb/core/cms/cms_impl.h +++ b/ydb/core/cms/cms_impl.h @@ -464,6 +464,8 @@ class TCms : public TActor, public TTabletExecutedFlat { TInstant InfoCollectorStartTime; + bool EnableCMSRequestPriorities = false; + private: TString GenerateStat(); void GenerateNodeState(IOutputStream&); diff --git a/ydb/core/cms/cms_tx_load_state.cpp b/ydb/core/cms/cms_tx_load_state.cpp index 829ce9f94573..5afa362db709 100644 --- a/ydb/core/cms/cms_tx_load_state.cpp +++ b/ydb/core/cms/cms_tx_load_state.cpp @@ -83,12 +83,14 @@ class TCms::TTxLoadState : public TTransactionBase { TString id = requestRowset.GetValue(); TString owner = requestRowset.GetValue(); ui64 order = requestRowset.GetValue(); + i32 priority = requestRowset.GetValueOrDefault(); TString requestStr = requestRowset.GetValue(); TRequestInfo request; request.RequestId = id; request.Owner = owner; request.Order = order; + request.Priority = priority; google::protobuf::TextFormat::ParseFromString(requestStr, &request.Request); LOG_DEBUG(ctx, NKikimrServices::CMS, "Loaded request %s owned by %s: %s", diff --git a/ydb/core/cms/cms_tx_store_permissions.cpp b/ydb/core/cms/cms_tx_store_permissions.cpp index db625f2d1437..6a382b33fb88 100644 --- a/ydb/core/cms/cms_tx_store_permissions.cpp +++ b/ydb/core/cms/cms_tx_store_permissions.cpp @@ -82,18 +82,21 @@ class TCms::TTxStorePermissions : public TTransactionBase { if (Scheduled->Request.ActionsSize() || Scheduled->Request.GetEvictVDisks()) { ui64 order = Scheduled->Order; + i32 priority = Scheduled->Priority; TString requestStr; google::protobuf::TextFormat::PrintToString(Scheduled->Request, &requestStr); auto row = db.Table().Key(id); row.Update(NIceDb::TUpdate(owner), NIceDb::TUpdate(order), + NIceDb::TUpdate(priority), NIceDb::TUpdate(requestStr)); Self->AuditLog(ctx, TStringBuilder() << "Store request" << ": id# " << id << ", owner# " << owner << ", order# " << order + << ", priority# " << priority << ", body# " << requestStr); if (Scheduled->Request.GetEvictVDisks()) { diff --git a/ydb/core/cms/cms_ut.cpp b/ydb/core/cms/cms_ut.cpp index 03dde0cba74b..691f87a9190d 100644 --- a/ydb/core/cms/cms_ut.cpp +++ b/ydb/core/cms/cms_ut.cpp @@ -1807,6 +1807,260 @@ Y_UNIT_TEST_SUITE(TCmsTest) { // reject until prepared env.CheckRejectRequest("user", request3.GetRequestId()); } + + Y_UNIT_TEST(EmergencyDuringRollingRestart) + { + TCmsTestEnv env(TTestEnvOpts(8).WithEnableCMSRequestPriorities()); + + // Start rolling restart + auto rollingRestart = env.CheckPermissionRequest + ("user", true, false, true, true, -80, TStatus::ALLOW_PARTIAL, + MakeAction(TAction::RESTART_SERVICES, env.GetNodeId(0), 60000000, "storage"), + MakeAction(TAction::RESTART_SERVICES, env.GetNodeId(1), 60000000, "storage")); + UNIT_ASSERT_VALUES_EQUAL(rollingRestart.PermissionsSize(), 1); + + // Done with restarting first node + env.CheckDonePermission("user", rollingRestart.GetPermissions(0).GetId()); + + // Emergency request + auto emergency = env.CheckPermissionRequest + ("user", true, false, true, true, -100, TStatus::ALLOW, + MakeAction(TAction::RESTART_SERVICES, env.GetNodeId(1), 60000000, "storage")); + + // Rolling restart is blocked by emergency request + env.CheckRequest("user", rollingRestart.GetRequestId(), false, TStatus::DISALLOW_TEMP, 0); + + // Done with emergency request + env.CheckDonePermission("user", emergency.GetPermissions(0).GetId()); + + // Rolling restart can continue + env.CheckRequest("user", rollingRestart.GetRequestId(), false, TStatus::ALLOW, 1); + } + + Y_UNIT_TEST(ScheduledEmergencyDuringRollingRestart) + { + TCmsTestEnv env(TTestEnvOpts(8).WithEnableCMSRequestPriorities()); + + // Start rolling restart + auto rollingRestart = env.CheckPermissionRequest + ("user", true, false, true, true, -80, TStatus::ALLOW_PARTIAL, + MakeAction(TAction::RESTART_SERVICES, env.GetNodeId(0), 60000000, "storage"), + MakeAction(TAction::RESTART_SERVICES, env.GetNodeId(1), 60000000, "storage")); + UNIT_ASSERT_VALUES_EQUAL(rollingRestart.PermissionsSize(), 1); + + // Emergency request + auto emergency = env.CheckPermissionRequest + ("user", true, false, true, true, -100, TStatus::DISALLOW_TEMP, + MakeAction(TAction::RESTART_SERVICES, env.GetNodeId(1), 60000000, "storage")); + + // Done with restarting first node + env.CheckDonePermission("user", rollingRestart.GetPermissions(0).GetId()); + + // Rolling restart is blocked by emergency request + env.CheckRequest("user", rollingRestart.GetRequestId(), false, TStatus::DISALLOW_TEMP, 0); + + // Emergency request is not blocked + emergency = env.CheckRequest("user", emergency.GetRequestId(), false, TStatus::ALLOW, 1); + + // Done with emergency request + env.CheckDonePermission("user", emergency.GetPermissions(0).GetId()); + + // Rolling restart can continue + env.CheckRequest("user", rollingRestart.GetRequestId(), false, TStatus::ALLOW, 1); + } + + Y_UNIT_TEST(WalleRequestDuringRollingRestart) + { + TCmsTestEnv env(TTestEnvOpts(8).WithEnableCMSRequestPriorities()); + + // Start rolling restart + auto rollingRestart = env.CheckPermissionRequest + ("user", true, false, true, true, -80, TStatus::ALLOW_PARTIAL, + MakeAction(TAction::RESTART_SERVICES, env.GetNodeId(0), 60000000, "storage"), + MakeAction(TAction::RESTART_SERVICES, env.GetNodeId(1), 60000000, "storage")); + UNIT_ASSERT_VALUES_EQUAL(rollingRestart.PermissionsSize(), 1); + + // Done with restarting first node + env.CheckDonePermission("user", rollingRestart.GetPermissions(0).GetId()); + + // Wall-E task is blocked by rolling restart + env.CheckWalleCreateTask("task-1", "reboot", false, TStatus::DISALLOW_TEMP, env.GetNodeId(1)); + + // Rolling restart is not blocked + rollingRestart = env.CheckRequest("user", rollingRestart.GetRequestId(), false, TStatus::ALLOW, 1); + UNIT_ASSERT_VALUES_EQUAL(rollingRestart.PermissionsSize(), 1); + + // Done with restarting second node + env.CheckDonePermission("user", rollingRestart.GetPermissions(0).GetId()); + + // Wall-E task can continue + env.CheckWalleCheckTask("task-1", TStatus::ALLOW, env.GetNodeId(1)); + } + + Y_UNIT_TEST(ScheduledWalleRequestDuringRollingRestart) + { + TCmsTestEnv env(TTestEnvOpts(8).WithEnableCMSRequestPriorities()); + + // Start rolling restart + auto rollingRestart = env.CheckPermissionRequest + ("user", true, false, true, true, -80, TStatus::ALLOW_PARTIAL, + MakeAction(TAction::RESTART_SERVICES, env.GetNodeId(0), 60000000, "storage"), + MakeAction(TAction::RESTART_SERVICES, env.GetNodeId(1), 60000000, "storage")); + UNIT_ASSERT_VALUES_EQUAL(rollingRestart.PermissionsSize(), 1); + + // Wall-E task is blocked by rolling restart + env.CheckWalleCreateTask("task-1", "reboot", false, TStatus::DISALLOW_TEMP, env.GetNodeId(1)); + + // Done with restarting first node + env.CheckDonePermission("user", rollingRestart.GetPermissions(0).GetId()); + + // Wall-E task is stil blocked + env.CheckWalleCheckTask("task-1", TStatus::DISALLOW_TEMP, env.GetNodeId(1)); + + // Rolling restart is not blocked + rollingRestart = env.CheckRequest("user", rollingRestart.GetRequestId(), false, TStatus::ALLOW, 1); + UNIT_ASSERT_VALUES_EQUAL(rollingRestart.PermissionsSize(), 1); + + // Done with restarting second node + env.CheckDonePermission("user", rollingRestart.GetPermissions(0).GetId()); + + // Wall-E task can continue + env.CheckWalleCheckTask("task-1", TStatus::ALLOW, env.GetNodeId(1)); + } + + Y_UNIT_TEST(EnableCMSRequestPrioritiesFeatureFlag) + { + TCmsTestEnv env(8); + // Start rolling restart with specified priority + auto rollingRestart = env.CheckPermissionRequest + ("user", true, false, true, true, -80, TStatus::WRONG_REQUEST, + MakeAction(TAction::RESTART_SERVICES, env.GetNodeId(0), 60000000, "storage"), + MakeAction(TAction::RESTART_SERVICES, env.GetNodeId(1), 60000000, "storage")); + + const TString expectedReason = "Unsupported: feature flag EnableCMSRequestPriorities is off"; + UNIT_ASSERT_VALUES_EQUAL(rollingRestart.GetStatus().GetReason(), expectedReason); + } + + Y_UNIT_TEST(SamePriorityRequest) + { + TCmsTestEnv env(TTestEnvOpts(8).WithEnableCMSRequestPriorities()); + + // Start rolling restart + auto rollingRestart = env.CheckPermissionRequest + ("user", true, false, true, true, -80, TStatus::ALLOW_PARTIAL, + MakeAction(TAction::RESTART_SERVICES, env.GetNodeId(0), 60000000, "storage"), + MakeAction(TAction::RESTART_SERVICES, env.GetNodeId(1), 60000000, "storage")); + UNIT_ASSERT_VALUES_EQUAL(rollingRestart.PermissionsSize(), 1); + + // Issue same priority request + auto samePriorityRequest = env.CheckPermissionRequest + ("user", true, false, true, true, -80, TStatus::DISALLOW_TEMP, + MakeAction(TAction::RESTART_SERVICES, env.GetNodeId(1), 60000000, "storage")); + + // Done with restarting first node + env.CheckDonePermission("user", rollingRestart.GetPermissions(0).GetId()); + + // Rolling restart is not blocked by same priority request + rollingRestart = env.CheckRequest("user", rollingRestart.GetRequestId(), false, TStatus::ALLOW, 1); + UNIT_ASSERT_VALUES_EQUAL(rollingRestart.PermissionsSize(), 1); + + // Done with restarting second node + env.CheckDonePermission("user", rollingRestart.GetPermissions(0).GetId()); + + // Same priority can continue + env.CheckRequest("user", samePriorityRequest.GetRequestId(), false, TStatus::ALLOW, 1); + } + + Y_UNIT_TEST(SamePriorityRequest2) + { + TCmsTestEnv env(TTestEnvOpts(8).WithEnableCMSRequestPriorities()); + + // Start rolling restart + auto rollingRestart = env.CheckPermissionRequest + ("user", true, false, true, true, -80, TStatus::ALLOW_PARTIAL, + MakeAction(TAction::RESTART_SERVICES, env.GetNodeId(0), 60000000, "storage"), + MakeAction(TAction::RESTART_SERVICES, env.GetNodeId(1), 60000000, "storage")); + UNIT_ASSERT_VALUES_EQUAL(rollingRestart.PermissionsSize(), 1); + + // Issue same priority request + auto samePriorityRequest = env.CheckPermissionRequest + ("user", true, false, true, true, -80, TStatus::DISALLOW_TEMP, + MakeAction(TAction::RESTART_SERVICES, env.GetNodeId(1), 60000000, "storage")); + + // Done with restarting first node + env.CheckDonePermission("user", rollingRestart.GetPermissions(0).GetId()); + + // Request is not blocked by rolling restart of same priority + samePriorityRequest = env.CheckRequest("user", samePriorityRequest.GetRequestId(), false, TStatus::ALLOW, 1); + UNIT_ASSERT_VALUES_EQUAL(samePriorityRequest.PermissionsSize(), 1); + + // Done with same priority request permissions + env.CheckDonePermission("user", samePriorityRequest.GetPermissions(0).GetId()); + + // Rolling restart can continue + env.CheckRequest("user", rollingRestart.GetRequestId(), false, TStatus::ALLOW, 1); + } + + Y_UNIT_TEST(PriorityRange) + { + TCmsTestEnv env(TTestEnvOpts(8).WithEnableCMSRequestPriorities()); + + const TString expectedReason = "Priority value is out of range"; + + // Out of range priority + auto request = env.CheckPermissionRequest + ("user", true, false, true, true, -101, TStatus::WRONG_REQUEST, + MakeAction(TAction::RESTART_SERVICES, env.GetNodeId(0), 60000000, "storage"), + MakeAction(TAction::RESTART_SERVICES, env.GetNodeId(1), 60000000, "storage")); + UNIT_ASSERT_VALUES_EQUAL(request.GetStatus().GetReason(), expectedReason); + + // Out of range priority + request = env.CheckPermissionRequest + ("user", true, false, true, true, 101, TStatus::WRONG_REQUEST, + MakeAction(TAction::RESTART_SERVICES, env.GetNodeId(0), 60000000, "storage"), + MakeAction(TAction::RESTART_SERVICES, env.GetNodeId(1), 60000000, "storage")); + UNIT_ASSERT_VALUES_EQUAL(request.GetStatus().GetReason(), expectedReason); + } + + Y_UNIT_TEST(WalleTasksDifferentPriorities) + { + TCmsTestEnv env(TTestEnvOpts(8).WithEnableCMSRequestPriorities()); + + // Start rolling restart + auto rollingRestart = env.CheckPermissionRequest + ("user", true, false, true, true, -80, TStatus::ALLOW_PARTIAL, + MakeAction(TAction::RESTART_SERVICES, env.GetNodeId(0), 60000000, "storage"), + MakeAction(TAction::RESTART_SERVICES, env.GetNodeId(1), 60000000, "storage")); + UNIT_ASSERT_VALUES_EQUAL(rollingRestart.PermissionsSize(), 1); + + // Done with restarting first node + env.CheckDonePermission("user", rollingRestart.GetPermissions(0).GetId()); + + // Rolling restart is continue + rollingRestart = env.CheckRequest("user", rollingRestart.GetRequestId(), false, TStatus::ALLOW, 1); + UNIT_ASSERT_VALUES_EQUAL(rollingRestart.PermissionsSize(), 1); + + // Wall-E soft maintainance task is blocked by rolling restart + env.CheckWalleCreateTask("task-1", "temporary-unreachable", false, TStatus::DISALLOW_TEMP, env.GetNodeId(1)); + + // Wall-E reboot task is blocked by rolling restart + env.CheckWalleCreateTask("task-2", "reboot", false, TStatus::DISALLOW_TEMP, env.GetNodeId(1)); + + // Done with restarting second node + env.CheckDonePermission("user", rollingRestart.GetPermissions(0).GetId()); + + // Wall-E soft maintainance task is blocked by Wall-E reboot task + env.CheckWalleCheckTask("task-1", TStatus::DISALLOW_TEMP, env.GetNodeId(1)); + + // Wall-E reboot task can continue + env.CheckWalleCheckTask("task-2", TStatus::ALLOW, env.GetNodeId(1)); + + // Done with Wall-E reboot task + env.CheckWalleRemoveTask("task-2"); + + // Wall-E soft maintainance task can continue + env.CheckWalleCheckTask("task-1", TStatus::ALLOW, env.GetNodeId(1)); + } } } diff --git a/ydb/core/cms/cms_ut_common.cpp b/ydb/core/cms/cms_ut_common.cpp index ab8843330d19..aa9939b45994 100644 --- a/ydb/core/cms/cms_ut_common.cpp +++ b/ydb/core/cms/cms_ut_common.cpp @@ -485,6 +485,7 @@ static void SetupServices(TTestActorRuntime &runtime, const TTestEnvOpts &option NKikimrConfig::TAppConfig appConfig; appConfig.MutableBootstrapConfig()->CopyFrom(TFakeNodeWhiteboardService::BootstrapConfig); + appConfig.MutableFeatureFlags()->SetEnableCMSRequestPriorities(options.EnableCMSRequestPriorities); runtime.AddLocalService(MakeConfigsDispatcherID(runtime.GetNodeId(0)), TActorSetupCmd(CreateConfigsDispatcher(appConfig, {}), TMailboxType::Simple, 0), 0); diff --git a/ydb/core/cms/cms_ut_common.h b/ydb/core/cms/cms_ut_common.h index be43e21b74ab..8c400f4b5f9f 100644 --- a/ydb/core/cms/cms_ut_common.h +++ b/ydb/core/cms/cms_ut_common.h @@ -86,6 +86,7 @@ struct TTestEnvOpts { bool UseMirror3dcErasure; bool AdvanceCurrentTime; bool EnableSentinel; + bool EnableCMSRequestPriorities; TTestEnvOpts() = default; @@ -102,6 +103,7 @@ struct TTestEnvOpts { , UseMirror3dcErasure(false) , AdvanceCurrentTime(false) , EnableSentinel(false) + , EnableCMSRequestPriorities(false) { } @@ -114,6 +116,11 @@ struct TTestEnvOpts { EnableSentinel = false; return *this; } + + TTestEnvOpts& WithEnableCMSRequestPriorities() { + EnableCMSRequestPriorities = true; + return *this; + } }; class TCmsTestEnv : public TTestBasicRuntime { @@ -159,6 +166,7 @@ class TCmsTestEnv : public TTestBasicRuntime { bool defaultTenantPolicy, TDuration duration, NKikimrCms::EAvailabilityMode availabilityMode, + i32 priority, NKikimrCms::TStatus::ECode code, Ts... actions) { @@ -168,6 +176,7 @@ class TCmsTestEnv : public TTestBasicRuntime { if (duration) req->Record.SetDuration(duration.GetValue()); req->Record.SetAvailabilityMode(availabilityMode); + req->Record.SetPriority(priority); return CheckPermissionRequest(req, code); } @@ -184,7 +193,7 @@ class TCmsTestEnv : public TTestBasicRuntime { { return CheckPermissionRequest(user, partial, dry, schedule, defaultTenantPolicy, TDuration::Zero(), - availabilityMode, + availabilityMode, 0, code, actions...); } template @@ -199,7 +208,7 @@ class TCmsTestEnv : public TTestBasicRuntime { Ts... actions) { return CheckPermissionRequest(user, partial, dry, schedule, defaultTenantPolicy, - duration, NKikimrCms::MODE_MAX_AVAILABILITY, code, actions...); + duration, NKikimrCms::MODE_MAX_AVAILABILITY, 0, code, actions...); } template @@ -216,6 +225,21 @@ class TCmsTestEnv : public TTestBasicRuntime { NKikimrCms::MODE_MAX_AVAILABILITY, code, actions...); } + template + NKikimrCms::TPermissionResponse CheckPermissionRequest( + const TString &user, + bool partial, + bool dry, + bool schedule, + bool defaultTenantPolicy, + i32 priority, + NKikimrCms::TStatus::ECode code, + Ts... actions) + { + return CheckPermissionRequest(user, partial, dry, schedule, defaultTenantPolicy, + TDuration::Zero(), NKikimrCms::MODE_MAX_AVAILABILITY, priority, code, actions...); + } + NKikimrCms::TPermissionResponse CheckPermissionRequest(TAutoPtr req, NKikimrCms::TStatus::ECode code); diff --git a/ydb/core/cms/node_checkers.cpp b/ydb/core/cms/node_checkers.cpp index 75c9b3c91bac..51306ab3efb6 100644 --- a/ydb/core/cms/node_checkers.cpp +++ b/ydb/core/cms/node_checkers.cpp @@ -60,9 +60,7 @@ bool TNodesCounterBase::IsNodeLocked(ui32 nodeId) const { } void TNodesCounterBase::LockNode(ui32 nodeId) { - if (IsNodeLocked(nodeId)) { - return; - } + Y_ABORT_UNLESS(!IsNodeLocked(nodeId)); ++LockedNodesCount; if (NodeToState[nodeId] == NODE_STATE_DOWN) { @@ -74,10 +72,8 @@ void TNodesCounterBase::LockNode(ui32 nodeId) { } void TNodesCounterBase::UnlockNode(ui32 nodeId) { - if (!IsNodeLocked(nodeId)) { - return; - } - + Y_ABORT_UNLESS(IsNodeLocked(nodeId)); + --LockedNodesCount; if (NodeToState[nodeId] == NODE_STATE_RESTART) { NodeToState[nodeId] = NODE_STATE_DOWN; diff --git a/ydb/core/cms/scheme.h b/ydb/core/cms/scheme.h index 5bbd7dcc5645..35dcb10c6310 100644 --- a/ydb/core/cms/scheme.h +++ b/ydb/core/cms/scheme.h @@ -37,9 +37,10 @@ struct Schema : NIceDb::Schema { struct Owner : Column<2, NScheme::NTypeIds::Utf8> {}; struct Order : Column<3, NScheme::NTypeIds::Uint64> {}; struct Content : Column<4, NScheme::NTypeIds::Utf8> {}; + struct Priority : Column<5, NScheme::NTypeIds::Int32> {}; using TKey = TableKey; - using TColumns = TableColumns; + using TColumns = TableColumns; }; struct WalleTask : Table<4> { diff --git a/ydb/core/cms/walle.h b/ydb/core/cms/walle.h index bb12bd090ec2..d6e6cf9e2d6d 100644 --- a/ydb/core/cms/walle.h +++ b/ydb/core/cms/walle.h @@ -13,6 +13,9 @@ namespace NKikimr::NCms { constexpr const char *WALLE_CMS_USER = "Wall-E"; constexpr const char *WALLE_API_URL_PREFIX = "/api/walle/v11/"; +constexpr const i32 WALLE_DEFAULT_PRIORITY = 20; +constexpr const i32 WALLE_SOFT_MAINTAINANCE_PRIORITY = 50; + IActor *CreateWalleAdapter(TEvCms::TEvWalleCreateTaskRequest::TPtr &ev, TActorId cms); IActor *CreateWalleAdapter(TEvCms::TEvWalleListTasksRequest::TPtr &ev, const TCmsStatePtr state); IActor *CreateWalleAdapter(TEvCms::TEvWalleCheckTaskRequest::TPtr &ev, const TCmsStatePtr state, TActorId cms); diff --git a/ydb/core/cms/walle_create_task_adapter.cpp b/ydb/core/cms/walle_create_task_adapter.cpp index 83724568eace..744da6eb250e 100644 --- a/ydb/core/cms/walle_create_task_adapter.cpp +++ b/ydb/core/cms/walle_create_task_adapter.cpp @@ -121,8 +121,14 @@ class TWalleCreateTaskAdapter : public TActorBootstrappedRecord.SetUser(WALLE_CMS_USER); request->Record.SetSchedule(true); request->Record.SetDryRun(task.GetDryRun()); - - auto it = Actions.find(task.GetAction()); + const auto &action = task.GetAction(); + if (action == "temporary-unreachable") { + request->Record.SetPriority(WALLE_SOFT_MAINTAINANCE_PRIORITY); + } else { + request->Record.SetPriority(WALLE_DEFAULT_PRIORITY); + } + + auto it = Actions.find(action); Y_ABORT_UNLESS(it != Actions.end()); if (!it->second) { diff --git a/ydb/core/driver_lib/cli_utils/cli_cmds_cms.cpp b/ydb/core/driver_lib/cli_utils/cli_cmds_cms.cpp index 59ae3bf3d06f..3206ff8b21fa 100644 --- a/ydb/core/driver_lib/cli_utils/cli_cmds_cms.cpp +++ b/ydb/core/driver_lib/cli_utils/cli_cmds_cms.cpp @@ -412,6 +412,7 @@ class TClientCommandMakeRequest : public TClientCommandWithAction { ui32 Minutes; TString TenantPolicy; TString AvailabilityMode; + i32 Priority; TClientCommandMakeRequest(const TString &description, NKikimrCms::TAction::EType type, @@ -433,6 +434,7 @@ class TClientCommandMakeRequest : public TClientCommandWithAction { EvictVDisks = false; Hours = 0; Minutes = 0; + Priority = 0; config.Opts->AddLongOption("user", "User name").Required() .RequiredArgument("NAME").StoreResult(&User); @@ -453,6 +455,9 @@ class TClientCommandMakeRequest : public TClientCommandWithAction { .RequiredArgument("max|keep|force").DefaultValue("max").StoreResult(&AvailabilityMode); config.Opts->AddLongOption("evict-vdisks", "Evict vdisks before granting permission(s)") .NoArgument().SetFlag(&EvictVDisks); + config.Opts->AddLongOption("priority", "Request priority") + .RequiredArgument("NUM").StoreResult(&Priority); + } void Parse(TConfig& config) override @@ -495,6 +500,9 @@ class TClientCommandMakeRequest : public TClientCommandWithAction { auto duration = TDuration::Minutes(Minutes) + TDuration::Hours(Hours); rec.SetDuration(duration.GetValue()); } + if (Priority) { + rec.SetPriority(Priority); + } } }; diff --git a/ydb/core/protos/cms.proto b/ydb/core/protos/cms.proto index e8d9160f3332..9d0923d6897e 100644 --- a/ydb/core/protos/cms.proto +++ b/ydb/core/protos/cms.proto @@ -163,6 +163,7 @@ message TPermissionRequest { optional string MaintenanceTaskId = 10; // Evit vdisks before granting permission optional bool EvictVDisks = 11 [default = false]; + optional int32 Priority = 12; } enum EExtensionType { @@ -210,6 +211,7 @@ message TManageRequestResponse { optional bool PartialPermissionAllowed = 4; optional string Reason = 5; optional EAvailabilityMode AvailabilityMode = 6; + optional int32 Priority = 7; } optional TStatus Status = 1; diff --git a/ydb/core/protos/feature_flags.proto b/ydb/core/protos/feature_flags.proto index 52d1cf02c442..94cbb87867b6 100644 --- a/ydb/core/protos/feature_flags.proto +++ b/ydb/core/protos/feature_flags.proto @@ -128,4 +128,5 @@ message TFeatureFlags { optional bool EnableServerlessExclusiveDynamicNodes = 113 [default = false]; optional bool EnableAccessServiceBulkAuthorization = 114 [default = false]; optional bool EnableAddColumsWithDefaults = 115 [ default = false]; + optional bool EnableCMSRequestPriorities = 116 [default = false]; } diff --git a/ydb/core/testlib/basics/feature_flags.h b/ydb/core/testlib/basics/feature_flags.h index 22086cb95595..02dc841fa731 100644 --- a/ydb/core/testlib/basics/feature_flags.h +++ b/ydb/core/testlib/basics/feature_flags.h @@ -57,6 +57,7 @@ class TTestFeatureFlagsHolder { FEATURE_FLAG_SETTER(EnableServerlessExclusiveDynamicNodes) FEATURE_FLAG_SETTER(EnableAccessServiceBulkAuthorization) FEATURE_FLAG_SETTER(EnableAddColumsWithDefaults) + FEATURE_FLAG_SETTER(EnableCMSRequestPriorities) #undef FEATURE_FLAG_SETTER }; diff --git a/ydb/public/api/protos/draft/ydb_maintenance.proto b/ydb/public/api/protos/draft/ydb_maintenance.proto index 904fdc2ed331..610aeae86c04 100644 --- a/ydb/public/api/protos/draft/ydb_maintenance.proto +++ b/ydb/public/api/protos/draft/ydb_maintenance.proto @@ -85,6 +85,8 @@ message MaintenanceTaskOptions { // Availability mode. AvailabilityMode availability_mode = 3; bool dry_run = 4; + // Priority of the task. Lower value indicates higher priority. + int32 priority = 5 [(value) = "[-100; 100]"]; } // Used to describe the scope of a single action.