refactor 'ydb scheme rmdir' implementation (#16232)

Author: jepett0

ydb/apps/ydb/CHANGELOG.md

@@ -1,3 +1,4 @@
+* Fixed bugs in `ydb scheme rmdir`: 1) do not try to delete subdomains, 2) order the deletion of external tables before the deletion of external data sources.
 * YDB CLI help message improvements. Different display for detailed help and brief help.
 * Support coordination nodes in `ydb scheme rmdir --recursive`.
 * Fixed return code of command `ydb workload * run --check-canonical` for the case when benchmark query results differ from canonical ones.

ydb/library/backup/backup.cpp

@@ -935,9 +935,7 @@ void RemoveClusterDirectory(const TDriver& driver, const TString& path) {
 
 void RemoveClusterDirectoryRecursive(const TDriver& driver, const TString& path) {
     LOG_I("Remove temporary directory " << path.Quote() << " in database");
-    NScheme::TSchemeClient schemeClient(driver);
-    NTable::TTableClient tableClient(driver);
-    TStatus status = NConsoleClient::RemoveDirectoryRecursive(schemeClient, tableClient, path, {}, true, false);
+    TStatus status = NConsoleClient::RemoveDirectoryRecursive(driver, path);
     VerifyStatus(status, TStringBuilder() << "Remove temporary directory " << path.Quote() << " failed");
 }
 

ydb/public/lib/ydb_cli/commands/ydb_service_scheme.cpp

@@ -87,12 +87,10 @@ int TCommandRemoveDirectory::Run(TConfig& config) {
     const auto settings = FillSettings(NScheme::TRemoveDirectorySettings());
 
     if (Recursive) {
-        NTable::TTableClient tableClient(driver);
-        NTopic::TTopicClient topicClient(driver);
-        NQuery::TQueryClient queryClient(driver);
-        NCoordination::TClient coordinationClient(driver);
-        const auto prompt = Prompt.GetOrElse(ERecursiveRemovePrompt::Once);
-        NStatusHelpers::ThrowOnErrorOrPrintIssues(RemoveDirectoryRecursive(schemeClient, tableClient, &topicClient, &queryClient, &coordinationClient, Path, prompt, settings));
+        const auto settings = TRemoveDirectoryRecursiveSettings()
+            .Prompt(Prompt.GetOrElse(ERecursiveRemovePrompt::Once))
+            .CreateProgressBar(true);
+        NStatusHelpers::ThrowOnErrorOrPrintIssues(RemoveDirectoryRecursive(driver, Path, settings));
     } else {
         if (Prompt) {
             if (!NConsoleClient::Prompt(*Prompt, Path, NScheme::ESchemeEntryType::Directory)) {

ydb/public/lib/ydb_cli/commands/ydb_workload.cpp

@@ -389,15 +389,15 @@ int TWorkloadCommandBase::Run(TConfig& config) {
 
 void TWorkloadCommandBase::CleanTables(NYdbWorkload::IWorkloadQueryGenerator& workloadGen, TConfig& config) {
     auto pathsToDelete = workloadGen.GetCleanPaths();
-    TRemovePathRecursiveSettings settings;
+    TRemoveDirectoryRecursiveSettings settings;
     settings.NotExistsIsOk(true);
     for (const auto& path : pathsToDelete) {
         Cout << "Remove path " << path << "..."  << Endl;
         auto fullPath = config.Database + "/" + path.c_str();
         if (DryRun) {
             Cout << "Remove " << fullPath << Endl;
         } else {
-            NStatusHelpers::ThrowOnErrorOrPrintIssues(RemovePathRecursive(*SchemeClient, *TableClient, TopicClient.Get(), QueryClient.Get(), nullptr, fullPath, ERecursiveRemovePrompt::Never, settings));
+            NStatusHelpers::ThrowOnErrorOrPrintIssues(RemovePathRecursive(*Driver.Get(), fullPath, settings));
         }
         Cout << "Remove path " << path << "...Ok"  << Endl;
     }

ydb/public/lib/ydb_cli/common/recursive_remove.cpp

@@ -1,5 +1,4 @@
 #include "interactive.h"
-#include "progress_bar.h"
 #include "recursive_remove.h"
 
 #include <ydb/public/lib/ydb_cli/common/recursive_list.h>
@@ -106,16 +105,17 @@ template <typename TClient, typename TSettings>
 using TRemoveFunc = TStatus(*)(TClient&, const TString&, const TSettings&);
 
 template <typename TClient, typename TSettings>
-TStatus Remove(TRemoveFunc<TClient, TSettings> func, TSchemeClient& schemeClient, TClient* client, const ESchemeEntryType type,
-        const TString& path, ERecursiveRemovePrompt prompt, const TRemoveDirectorySettings& settings)
-{
-    if (!client) {
-        return TStatus(EStatus::GENERIC_ERROR, MakeIssues(TStringBuilder()
-            << TypeName<TClient>() << " not specified"));
-    }
-
+TStatus Remove(
+    TRemoveFunc<TClient, TSettings> func,
+    TSchemeClient& schemeClient,
+    TClient& specializedClient,
+    const ESchemeEntryType type,
+    const TString& path,
+    ERecursiveRemovePrompt prompt,
+    const TRemoveDirectorySettings& settings
+) {
     if (Prompt(prompt, path, type, false)) {
-        auto status = func(*client, path, TSettings(settings));
+        auto status = func(specializedClient, path, TSettings(settings));
         if (status.GetStatus() == EStatus::SCHEME_ERROR && schemeClient.DescribePath(path).ExtractValueSync().GetStatus() == EStatus::SCHEME_ERROR) {
             Cerr << "WARNING: Couldn't delete path: \'" << path << "\'. It was probably already deleted in another process" << Endl;
             return TStatus(EStatus::SUCCESS, {});
@@ -127,12 +127,19 @@ TStatus Remove(TRemoveFunc<TClient, TSettings> func, TSchemeClient& schemeClient
 }
 
 TStatus Remove(
-    TSchemeClient& schemeClient, TTableClient* tableClient, TTopicClient* topicClient, NQuery::TQueryClient* queryClient, NCoordination::TClient* coordinationClient,
-    const ESchemeEntryType type, const TString& path, ERecursiveRemovePrompt prompt, const TRemoveDirectorySettings& settings)
-{
+    TSchemeClient& schemeClient,
+    TTableClient& tableClient,
+    TTopicClient& topicClient,
+    NQuery::TQueryClient& queryClient,
+    NCoordination::TClient& coordinationClient,
+    const ESchemeEntryType type,
+    const TString& path,
+    ERecursiveRemovePrompt prompt,
+    const TRemoveDirectorySettings& settings
+) {
     switch (type) {
     case ESchemeEntryType::Directory:
-        return Remove(&RemoveDirectory, schemeClient, &schemeClient, type, path, prompt, settings);
+        return Remove(&RemoveDirectory, schemeClient, schemeClient, type, path, prompt, settings);
 
     case ESchemeEntryType::ColumnStore:
         return Remove(&RemoveColumnStore, schemeClient, tableClient, type, path, prompt, settings);
@@ -151,6 +158,9 @@ TStatus Remove(
         return Remove(&RemoveView, schemeClient, queryClient, type, path, prompt, settings);
     case ESchemeEntryType::CoordinationNode:
         return Remove(&RemoveCoordinationNode, schemeClient, coordinationClient, type, path, prompt, settings);
+    case ESchemeEntryType::SubDomain:
+        // continue silently
+        return TStatus(EStatus::SUCCESS, {});
 
     default:
         return TStatus(EStatus::UNSUPPORTED, MakeIssues(TStringBuilder()
@@ -159,86 +169,119 @@ TStatus Remove(
 }
 
 TStatus RemoveDirectoryRecursive(
-        TSchemeClient& schemeClient,
-        TTableClient* tableClient,
-        TTopicClient* topicClient,
-        NQuery::TQueryClient* queryClient,
-        NCoordination::TClient* coordinationClient,
-        const TString& path,
-        ERecursiveRemovePrompt prompt,
-        const TRemoveDirectorySettings& settings,
-        bool removeSelf,
-        bool createProgressBar)
-{
-    auto recursiveListResult = RecursiveList(schemeClient, path, {}, removeSelf);
+    TSchemeClient& schemeClient,
+    TTableClient& tableClient,
+    TTopicClient& topicClient,
+    NQuery::TQueryClient& queryClient,
+    NCoordination::TClient& coordinationClient,
+    const TString& path,
+    const TRemoveDirectoryRecursiveSettings& settings
+) {
+    const auto listingSettings = TRecursiveListSettings()
+        .Filter([](const TSchemeEntry& entry) {
+            // explicitly skip subdomains
+            return entry.Type != ESchemeEntryType::SubDomain;
+        });
+    auto recursiveListResult = RecursiveList(schemeClient, path, listingSettings, settings.RemoveSelf_);
     if (!recursiveListResult.Status.IsSuccess()) {
         return recursiveListResult.Status;
     }
 
-    if (prompt == ERecursiveRemovePrompt::Once) {
+    if (settings.Prompt_ == ERecursiveRemovePrompt::Once) {
         if (!Prompt(path, ESchemeEntryType::Directory)) {
             return TStatus(EStatus::SUCCESS, {});
         }
     }
 
-    std::unique_ptr<TProgressBar> bar;
-    if (createProgressBar) {
-        bar = std::make_unique<TProgressBar>(recursiveListResult.Entries.size());
-    }
-    // output order is: Root, Recursive(children)...
-    // we need to reverse it to delete recursively
-    for (auto it = recursiveListResult.Entries.rbegin(); it != recursiveListResult.Entries.rend(); ++it) {
-        if (auto result = Remove(schemeClient, tableClient, topicClient, queryClient, coordinationClient, it->Type, TString{it->Name}, prompt, settings); !result.IsSuccess()) {
-            return result;
-        }
-        if (createProgressBar) {
-            bar->AddProgress(1);
-        }
-    }
-
-    return TStatus(EStatus::SUCCESS, {});
+    // RecursiveList outputs elements in pre-order: root, recurse(children)...
+    // We need to reverse it to delete scheme objects before directories.
+    return RemovePathsRecursive(
+        schemeClient,
+        tableClient,
+        topicClient,
+        queryClient,
+        coordinationClient,
+        recursiveListResult.Entries.rbegin(),
+        recursiveListResult.Entries.rend(),
+        settings
+    );
 }
 
 TStatus RemoveDirectoryRecursive(
-        TSchemeClient& schemeClient,
-        TTableClient& tableClient,
-        const TString& path,
-        const TRemoveDirectorySettings& settings,
-        bool removeSelf,
-        bool createProgressBar)
-{
-    return RemoveDirectoryRecursive(schemeClient, &tableClient, nullptr, nullptr, nullptr, path, ERecursiveRemovePrompt::Never, settings, removeSelf, createProgressBar);
+    const TDriver& driver,
+    const TString& path,
+    const TRemoveDirectoryRecursiveSettings& settings
+) {
+    TSchemeClient schemeClient(driver);
+    TTableClient tableClient(driver);
+    TTopicClient topicClient(driver);
+    NQuery::TQueryClient queryClient(driver);
+    NCoordination::TClient coordinationClient(driver);
+    return RemoveDirectoryRecursive(schemeClient, tableClient, topicClient, queryClient, coordinationClient, path, settings);
 }
 
-TStatus RemoveDirectoryRecursive(
-        TSchemeClient& schemeClient,
-        TTableClient& tableClient,
-        TTopicClient* topicClient,
-        NQuery::TQueryClient* queryClient,
-        NCoordination::TClient* coordinationClient,
-        const TString& path,
-        ERecursiveRemovePrompt prompt,
-        const TRemoveDirectorySettings& settings,
-        bool removeSelf,
-        bool createProgressBar)
-{
-    return RemoveDirectoryRecursive(schemeClient, &tableClient, topicClient, queryClient, coordinationClient, path, prompt, settings, removeSelf, createProgressBar);
-}
-
-NYdb::TStatus RemovePathRecursive(NScheme::TSchemeClient& schemeClient, NTable::TTableClient& tableClient, NTopic::TTopicClient* topicClient, NQuery::TQueryClient* queryClient, NCoordination::TClient* coordinationClient, const TString& path, ERecursiveRemovePrompt prompt, const TRemovePathRecursiveSettings& settings /*= {}*/, bool createProgressBar /*= true*/) {
+TStatus RemovePathRecursive(
+    const TDriver& driver,
+    const TString& path,
+    const TRemoveDirectoryRecursiveSettings& settings
+) {
+    TSchemeClient schemeClient(driver);
     auto entity = schemeClient.DescribePath(path).ExtractValueSync();
     if (!entity.IsSuccess()) {
         if (settings.NotExistsIsOk_ && entity.GetStatus() == EStatus::SCHEME_ERROR && entity.GetIssues().ToString().find("Path not found") != TString::npos) {
             return TStatus(EStatus::SUCCESS, {});
         }
         return entity;
     }
-    switch (entity.GetEntry().Type) {
-    case ESchemeEntryType::Directory:
-    case ESchemeEntryType::ColumnStore:
-        return RemoveDirectoryRecursive(schemeClient, tableClient, topicClient, queryClient, coordinationClient, path, prompt, settings, true, createProgressBar);
-    default:
-        return Remove(schemeClient, &tableClient, topicClient, queryClient, coordinationClient, entity.GetEntry().Type, path, prompt, settings);
+
+    TTableClient tableClient(driver);
+    TTopicClient topicClient(driver);
+    NQuery::TQueryClient queryClient(driver);
+    NCoordination::TClient coordinationClient(driver);
+    auto remover = NInternal::CreateDefaultRemover(schemeClient, tableClient, topicClient, queryClient, coordinationClient, settings);
+    return remover(entity.GetEntry());
+}
+
+TStatus RemovePathRecursive(
+    const TDriver& driver,
+    const TSchemeEntry& entry,
+    const TRemoveDirectoryRecursiveSettings& settings
+) {
+    TSchemeClient schemeClient(driver);
+    TTableClient tableClient(driver);
+    TTopicClient topicClient(driver);
+    NQuery::TQueryClient queryClient(driver);
+    NCoordination::TClient coordinationClient(driver);
+    auto remover = NInternal::CreateDefaultRemover(schemeClient, tableClient, topicClient, queryClient, coordinationClient, settings);
+    return remover(entry);
+}
+
+namespace NInternal {
+
+    TRemover CreateDefaultRemover(
+        NScheme::TSchemeClient& schemeClient,
+        NTable::TTableClient& tableClient,
+        NTopic::TTopicClient& topicClient,
+        NQuery::TQueryClient& queryClient,
+        NCoordination::TClient& coordinationClient,
+        const TRemoveDirectoryRecursiveSettings& settings
+    ) {
+        return [&](const TSchemeEntry& entry) {
+            return Remove(schemeClient, tableClient, topicClient, queryClient, coordinationClient, entry.Type, TString(entry.Name), settings.Prompt_, settings);
+        };
     }
+
+    bool MightHaveDependents(ESchemeEntryType type) {
+        switch (type) {
+        // directories might contain external data sources, which might have dependent objects (i.e. external tables)
+        case ESchemeEntryType::Directory:
+        case ESchemeEntryType::ExternalDataSource:
+            return true;
+        default:
+            return false;
+        }
+    }
+
 }
+
 }