Browse Source

another fix for feature flags (#8690)

Alexey Efimov 6 months ago
parent
commit
376e23794b

+ 39 - 0
ydb/core/viewer/json_pipe_req.cpp

@@ -113,6 +113,40 @@ TString TViewerPipeClient::GetPath(TEvTxProxySchemeCache::TEvNavigateKeySetResul
     return {};
 }
 
+bool TViewerPipeClient::IsSuccess(const std::unique_ptr<TEvTxProxySchemeCache::TEvNavigateKeySetResult>& ev) {
+    return (ev->Request->ResultSet.size() == 1) && (ev->Request->ResultSet.begin()->Status == NSchemeCache::TSchemeCacheNavigate::EStatus::Ok);
+}
+
+TString TViewerPipeClient::GetError(const std::unique_ptr<TEvTxProxySchemeCache::TEvNavigateKeySetResult>& ev) {
+    if (ev->Request->ResultSet.size() == 0) {
+        return "empty response";
+    }
+    switch (ev->Request->ResultSet.begin()->Status) {
+        case NSchemeCache::TSchemeCacheNavigate::EStatus::Ok:
+            return "Ok";
+        case NSchemeCache::TSchemeCacheNavigate::EStatus::Unknown:
+            return "Unknown";
+        case NSchemeCache::TSchemeCacheNavigate::EStatus::RootUnknown:
+            return "RootUnknown";
+        case NSchemeCache::TSchemeCacheNavigate::EStatus::PathErrorUnknown:
+            return "PathErrorUnknown";
+        case NSchemeCache::TSchemeCacheNavigate::EStatus::PathNotTable:
+            return "PathNotTable";
+        case NSchemeCache::TSchemeCacheNavigate::EStatus::PathNotPath:
+            return "PathNotPath";
+        case NSchemeCache::TSchemeCacheNavigate::EStatus::TableCreationNotComplete:
+            return "TableCreationNotComplete";
+        case NSchemeCache::TSchemeCacheNavigate::EStatus::LookupError:
+            return "LookupError";
+        case NSchemeCache::TSchemeCacheNavigate::EStatus::RedirectLookupError:
+            return "RedirectLookupError";
+        case NSchemeCache::TSchemeCacheNavigate::EStatus::AccessDenied:
+            return "AccessDenied";
+        default:
+            return ::ToString(static_cast<int>(ev->Request->ResultSet.begin()->Status));
+    }
+}
+
 void TViewerPipeClient::RequestHiveDomainStats(NNodeWhiteboard::TTabletId hiveId) {
     TActorId pipeClient = ConnectTabletPipe(hiveId);
     THolder<TEvHive::TEvRequestHiveDomainStats> request = MakeHolder<TEvHive::TEvRequestHiveDomainStats>();
@@ -266,6 +300,11 @@ TViewerPipeClient::TRequestResponse<NConsole::TEvConsole::TEvGetNodeConfigRespon
     return MakeRequestToPipe<NConsole::TEvConsole::TEvGetNodeConfigResponse>(pipeClient, request.Release(), cookie);
 }
 
+TViewerPipeClient::TRequestResponse<NConsole::TEvConsole::TEvGetAllConfigsResponse> TViewerPipeClient::MakeRequestConsoleGetAllConfigs() {
+    TActorId pipeClient = ConnectTabletPipe(GetConsoleId());
+    return MakeRequestToPipe<NConsole::TEvConsole::TEvGetAllConfigsResponse>(pipeClient, new NConsole::TEvConsole::TEvGetAllConfigsRequest());
+}
+
 void TViewerPipeClient::RequestConsoleGetTenantStatus(const TString& path) {
     TActorId pipeClient = ConnectTabletPipe(GetConsoleId());
     THolder<NConsole::TEvConsole::TEvGetTenantStatusRequest> request = MakeHolder<NConsole::TEvConsole::TEvGetTenantStatusRequest>();

+ 12 - 0
ydb/core/viewer/json_pipe_req.h

@@ -73,6 +73,13 @@ protected:
         TRequestResponse& operator =(TRequestResponse&&) = default;
 
         void Set(std::unique_ptr<T>&& response) {
+            constexpr bool hasErrorCheck = requires(const std::unique_ptr<T>& r) {TViewerPipeClient::IsSuccess(r);};
+            if constexpr (hasErrorCheck) {
+                if (!TViewerPipeClient::IsSuccess(response)) {
+                    Error(TViewerPipeClient::GetError(response));
+                    return;
+                }
+            }
             if (!IsDone()) {
                 Span.EndOk();
             }
@@ -200,12 +207,17 @@ protected:
 
     static TPathId GetPathId(TEvTxProxySchemeCache::TEvNavigateKeySetResult::TPtr& ev);
     static TString GetPath(TEvTxProxySchemeCache::TEvNavigateKeySetResult::TPtr& ev);
+
+    static bool IsSuccess(const std::unique_ptr<TEvTxProxySchemeCache::TEvNavigateKeySetResult>& ev);
+    static TString GetError(const std::unique_ptr<TEvTxProxySchemeCache::TEvNavigateKeySetResult>& ev);
+
     TRequestResponse<TEvHive::TEvResponseHiveDomainStats> MakeRequestHiveDomainStats(TTabletId hiveId);
     TRequestResponse<TEvHive::TEvResponseHiveStorageStats> MakeRequestHiveStorageStats(TTabletId hiveId);
     TRequestResponse<TEvHive::TEvResponseHiveNodeStats> MakeRequestHiveNodeStats(TTabletId hiveId, TEvHive::TEvRequestHiveNodeStats* request);
     void RequestConsoleListTenants();
     TRequestResponse<NConsole::TEvConsole::TEvListTenantsResponse> MakeRequestConsoleListTenants();
     TRequestResponse<NConsole::TEvConsole::TEvGetNodeConfigResponse> MakeRequestConsoleNodeConfigByTenant(TString tenant, ui64 cookie = 0);
+    TRequestResponse<NConsole::TEvConsole::TEvGetAllConfigsResponse> MakeRequestConsoleGetAllConfigs();
     void RequestConsoleGetTenantStatus(const TString& path);
     TRequestResponse<NConsole::TEvConsole::TEvGetTenantStatusResponse> MakeRequestConsoleGetTenantStatus(const TString& path);
     void RequestBSControllerConfig();

+ 1 - 9
ydb/core/viewer/storage_groups.h

@@ -1112,15 +1112,7 @@ public:
             return RequestDone();
         }
         auto& navigateResult(itNavigateKeySetResult->second);
-        if (ev->Get()->Request->ResultSet.size() == 1) {
-            if (ev->Get()->Request->ResultSet.begin()->Status == NSchemeCache::TSchemeCacheNavigate::EStatus::Ok) {
-                navigateResult.Set(std::move(ev));
-            } else {
-                navigateResult.Error(TStringBuilder() << "Error " << ev->Get()->Request->ResultSet.begin()->Status);
-            }
-        } else {
-            navigateResult.Error(TStringBuilder() << "Invalid number of results: " << ev->Get()->Request->ResultSet.size());
-        }
+        navigateResult.Set(std::move(ev));
         if (navigateResult.IsOk()) {
             TString path = CanonizePath(navigateResult->Request->ResultSet.begin()->Path);
             TIntrusiveConstPtr<TSchemeCacheNavigate::TDomainDescription> domainDescription = navigateResult->Request->ResultSet.begin()->DomainDescription;

+ 115 - 74
ydb/core/viewer/viewer_feature_flags.h

@@ -1,6 +1,7 @@
 #pragma once
 #include "json_pipe_req.h"
 #include "viewer.h"
+#include <ydb/library/yaml_config/yaml_config.h>
 
 namespace NKikimr::NViewer {
 
@@ -11,51 +12,41 @@ class TJsonFeatureFlags : public TViewerPipeClient {
     using TBase = TViewerPipeClient;
     TJsonSettings JsonSettings;
     ui32 Timeout = 0;
-
     TString FilterDatabase;
     THashSet<TString> FilterFeatures;
-    THashMap<ui64, TString> DatabaseByCookie;
-    ui64 Cookie = 0;
-    TString DomainPath;
-    bool Direct = false;
-
+    bool ChangedOnly = false;
     TRequestResponse<NConsole::TEvConsole::TEvListTenantsResponse> TenantsResponse;
-    THashMap<TString, TRequestResponse<NConsole::TEvConsole::TEvGetNodeConfigResponse>> NodeConfigResponses;
+    TRequestResponse<NConsole::TEvConsole::TEvGetAllConfigsResponse> AllConfigsResponse;
+    std::unordered_map<TString, TRequestResponse<TEvTxProxySchemeCache::TEvNavigateKeySetResult>> PathNameNavigateKeySetResults;
+    std::unordered_map<TPathId, TRequestResponse<TEvTxProxySchemeCache::TEvNavigateKeySetResult>> PathIdNavigateKeySetResults;
 
 public:
     TJsonFeatureFlags(IViewer* viewer, NMon::TEvHttpInfo::TPtr &ev)
         : TViewerPipeClient(viewer, ev)
     {}
 
-    void MakeNodeConfigRequest(const TString& database) {
-        NodeConfigResponses[database] = MakeRequestConsoleNodeConfigByTenant(database, Cookie);
-        DatabaseByCookie[Cookie++] = database;
-    }
-
     void Bootstrap() override {
         const auto& params(Event->Get()->Request.GetParams());
-
         JsonSettings.EnumAsNumbers = !FromStringWithDefault<bool>(params.Get("enums"), true);
         JsonSettings.UI64AsString = !FromStringWithDefault<bool>(params.Get("ui64"), false);
         FilterDatabase = params.Get("database");
         StringSplitter(params.Get("features")).Split(',').SkipEmpty().Collect(&FilterFeatures);
-        Direct = FromStringWithDefault<bool>(params.Get("direct"), Direct);
+        bool direct = FromStringWithDefault<bool>(params.Get("direct"), false);
         Timeout = FromStringWithDefault<ui32>(params.Get("timeout"), 10000);
+        ChangedOnly = FromStringWithDefault<bool>(params.Get("changed"), ChangedOnly);
 
-        TIntrusivePtr<TDomainsInfo> domains = AppData()->DomainsInfo;
-        auto* domain = domains->GetDomain();
-        DomainPath = "/" + domain->Name;
-
-        Direct |= !TBase::Event->Get()->Request.GetHeader("X-Forwarded-From-Node").empty(); // we're already forwarding
-        Direct |= (FilterDatabase == AppData()->TenantName); // we're already on the right node
-        if (FilterDatabase && !Direct) {
+        direct |= !TBase::Event->Get()->Request.GetHeader("X-Forwarded-From-Node").empty(); // we're already forwarding
+        direct |= (FilterDatabase == AppData()->TenantName); // we're already on the right node
+        if (FilterDatabase && !direct) {
             return RedirectToDatabase(FilterDatabase); // to find some dynamic node and redirect query there
-        } else if (!FilterDatabase) {
-            MakeNodeConfigRequest(DomainPath);
-            TenantsResponse = MakeRequestConsoleListTenants();
+        }
+
+        if (FilterDatabase) {
+            PathNameNavigateKeySetResults[FilterDatabase] = MakeRequestSchemeCacheNavigate(FilterDatabase);
         } else {
-            MakeNodeConfigRequest(FilterDatabase);
+            TenantsResponse = MakeRequestConsoleListTenants();
         }
+        AllConfigsResponse = MakeRequestConsoleGetAllConfigs();
 
         Become(&TThis::StateWork, TDuration::MilliSeconds(Timeout), new TEvents::TEvWakeup());
     }
@@ -63,7 +54,8 @@ public:
     STATEFN(StateWork) {
         switch (ev->GetTypeRewrite()) {
             hFunc(NConsole::TEvConsole::TEvListTenantsResponse, Handle);
-            hFunc(NConsole::TEvConsole::TEvGetNodeConfigResponse, Handle);
+            hFunc(NConsole::TEvConsole::TEvGetAllConfigsResponse, Handle);
+            hFunc(TEvTxProxySchemeCache::TEvNavigateKeySetResult, Handle);
             hFunc(TEvTabletPipe::TEvClientConnected, TBase::Handle);
             cFunc(TEvents::TSystem::Wakeup, HandleTimeout);
         }
@@ -71,67 +63,125 @@ public:
 
     void Handle(NConsole::TEvConsole::TEvListTenantsResponse::TPtr& ev) {
         TenantsResponse.Set(std::move(ev));
-        Ydb::Cms::ListDatabasesResult listDatabasesResult;
-        TenantsResponse->Record.GetResponse().operation().result().UnpackTo(&listDatabasesResult);
-        for (const TString& path : listDatabasesResult.paths()) {
-            MakeNodeConfigRequest(path);
+        if (TenantsResponse.IsOk()) {
+            Ydb::Cms::ListDatabasesResult listDatabasesResult;
+            TenantsResponse->Record.GetResponse().operation().result().UnpackTo(&listDatabasesResult);
+            for (const TString& database : listDatabasesResult.paths()) {
+                if (PathNameNavigateKeySetResults.count(database) == 0) {
+                    PathNameNavigateKeySetResults[database] = MakeRequestSchemeCacheNavigate(database);
+                }
+            }
+        }
+        if (PathNameNavigateKeySetResults.empty()) {
+            if (AppData()->DomainsInfo && AppData()->DomainsInfo->Domain) {
+                TString domain = "/" + AppData()->DomainsInfo->Domain->Name;
+                PathNameNavigateKeySetResults[domain] = MakeRequestSchemeCacheNavigate(domain);
+            }
         }
         RequestDone();
     }
 
-    void Handle(NConsole::TEvConsole::TEvGetNodeConfigResponse::TPtr& ev) {
-        TString database = DatabaseByCookie[ev.Get()->Cookie];
-        NodeConfigResponses[database].Set(std::move(ev));
+    void Handle(NConsole::TEvConsole::TEvGetAllConfigsResponse::TPtr& ev) {
+        AllConfigsResponse.Set(std::move(ev));
         RequestDone();
     }
 
-    THashMap<TString, NKikimrViewer::TFeatureFlagsConfig::TFeatureFlag> ParseFeatureFlags(const NKikimrConfig::TFeatureFlags& featureFlags) {
-        THashMap<TString, NKikimrViewer::TFeatureFlagsConfig::TFeatureFlag> features;
+    void Handle(TEvTxProxySchemeCache::TEvNavigateKeySetResult::TPtr& ev) {
+        TString path = GetPath(ev);
+        if (path) {
+            auto it = PathNameNavigateKeySetResults.find(path);
+            if (it != PathNameNavigateKeySetResults.end() && !it->second.IsDone()) {
+                it->second.Set(std::move(ev));
+                if (it->second.IsOk()) {
+                    TSchemeCacheNavigate::TEntry& entry(it->second->Request->ResultSet.front());
+                    if (entry.DomainInfo) {
+                        if (entry.DomainInfo->ResourcesDomainKey && entry.DomainInfo->DomainKey != entry.DomainInfo->ResourcesDomainKey) {
+                            TPathId resourceDomainKey(entry.DomainInfo->ResourcesDomainKey);
+                            if (PathIdNavigateKeySetResults.count(resourceDomainKey) == 0) {
+                                PathIdNavigateKeySetResults[resourceDomainKey] = MakeRequestSchemeCacheNavigate(resourceDomainKey);
+                            }
+                        }
+                    }
+                }
+                RequestDone();
+                return;
+            }
+        }
+        TPathId pathId = GetPathId(ev);
+        if (pathId) {
+            auto it = PathIdNavigateKeySetResults.find(pathId);
+            if (it != PathIdNavigateKeySetResults.end() && !it->second.IsDone()) {
+                it->second.Set(std::move(ev));
+                RequestDone();
+                return;
+            }
+        }
+    }
+
+    void ParseFeatureFlags(const NKikimrConfig::TFeatureFlags& featureFlags, NKikimrViewer::TFeatureFlagsConfig::TDatabase& result) {
         const google::protobuf::Reflection* reflection = featureFlags.GetReflection();
         const google::protobuf::Descriptor* descriptor = featureFlags.GetDescriptor();
-
         for (int i = 0; i < descriptor->field_count(); ++i) {
             const google::protobuf::FieldDescriptor* field = descriptor->field(i);
             if (field->cpp_type() == google::protobuf::FieldDescriptor::CPPTYPE_BOOL) {
-                auto& feat = features[field->name()];
-                feat.SetName(field->name());
-                if (reflection->HasField(featureFlags, field)) {
-                    feat.SetCurrent(reflection->GetBool(featureFlags, field));
-                }
-                if (field->has_default_value()) {
-                    feat.SetDefault(field->default_value_bool());
+                if (FilterFeatures.empty() || FilterFeatures.count(field->name())) {
+                    bool hasField = reflection->HasField(featureFlags, field);
+                    if (ChangedOnly && !hasField) {
+                        continue;
+                    }
+                    auto flag = result.AddFeatureFlags();
+                    flag->SetName(field->name());
+                    if (hasField) {
+                        flag->SetCurrent(reflection->GetBool(featureFlags, field));
+                    }
+                    if (field->has_default_value()) {
+                        flag->SetDefault(field->default_value_bool());
+                    }
                 }
             }
         }
-        return features;
     }
 
-    void ReplyAndPassAway() override {
-        THashMap<TString, THashMap<TString, NKikimrViewer::TFeatureFlagsConfig::TFeatureFlag>> FeatureFlagsByDatabase;
-        for (const auto& [database, response] : NodeConfigResponses) {
-            NKikimrConsole::TGetNodeConfigResponse rec = response->Record;
-            FeatureFlagsByDatabase[database] = ParseFeatureFlags(rec.GetConfig().GetFeatureFlags());
-        }
-
-        auto domainFeaturesIt = FeatureFlagsByDatabase.find(DomainPath);
-        if (domainFeaturesIt == FeatureFlagsByDatabase.end()) {
-            return TBase::ReplyAndPassAway(GetHTTPINTERNALERROR("text/plain", "No domain info from Console"));
+    void ParseConfig(const TString& database,
+                     const TRequestResponse<TEvTxProxySchemeCache::TEvNavigateKeySetResult>& navigate,
+                     NKikimrViewer::TFeatureFlagsConfig& result) {
+        if (AllConfigsResponse.IsOk()) {
+            TString realDatabase = database;
+            auto databaseProto = result.AddDatabases();
+            databaseProto->SetName(database);
+            TSchemeCacheNavigate::TEntry& entry(navigate->Request->ResultSet.front());
+            if (entry.DomainInfo) {
+                if (entry.DomainInfo->ResourcesDomainKey && entry.DomainInfo->DomainKey != entry.DomainInfo->ResourcesDomainKey) {
+                    TPathId resourceDomainKey(entry.DomainInfo->ResourcesDomainKey);
+                    auto it = PathIdNavigateKeySetResults.find(resourceDomainKey);
+                    if (it != PathIdNavigateKeySetResults.end() && it->second.IsOk() && it->second->Request->ResultSet.size() == 1) {
+                        realDatabase = CanonizePath(it->second->Request->ResultSet.begin()->Path);
+                    }
+                }
+            }
+            NKikimrConfig::TAppConfig appConfig;
+            if (AllConfigsResponse->Record.GetResponse().config()) {
+                try {
+                    NYamlConfig::ResolveAndParseYamlConfig(AllConfigsResponse->Record.GetResponse().config(), {}, {{"tenant", realDatabase}}, appConfig);
+                } catch (const std::exception& e) {
+                    BLOG_ERROR("Failed to parse config for tenant " << realDatabase << ": " << e.what());
+                }
+                ParseFeatureFlags(appConfig.GetFeatureFlags(), *databaseProto);
+            } else {
+                ParseFeatureFlags(AppData()->FeatureFlags, *databaseProto);
+            }
         }
+    }
 
+    void ReplyAndPassAway() override {
         // prepare response
         NKikimrViewer::TFeatureFlagsConfig Result;
         Result.SetVersion(Viewer->GetCapabilityVersion("/viewer/feature_flags"));
-        for (const auto& [database, features] : FeatureFlagsByDatabase) {
-            auto databaseProto = Result.AddDatabases();
-            databaseProto->SetName(database);
-            for (const auto& [name, featProto] : features) {
-                if (FilterFeatures.empty() || FilterFeatures.find(name) != FilterFeatures.end()) {
-                    auto flag = databaseProto->AddFeatureFlags();
-                    flag->CopyFrom(featProto);
-                }
+        for (const auto& [database, navigate] : PathNameNavigateKeySetResults) {
+            if (navigate.IsOk()) {
+                ParseConfig(database, navigate, Result);
             }
         }
-
         TStringStream json;
         TProtoToJson::ProtoToJson(json, Result, JsonSettings);
         TBase::ReplyAndPassAway(GetHTTPOKJSON(json.Str()));
@@ -142,12 +192,13 @@ public:
             .Method = "get",
             .Tag = "viewer",
             .Summary = "Feature flags",
-            .Description = "Returns feature flags of each database"
+            .Description = "Returns feature flags of a database"
         });
         yaml.AddParameter({
             .Name = "database",
             .Description = "database name",
             .Type = "string",
+            .Required = true,
         });
         yaml.AddParameter({
             .Name = "features",
@@ -164,16 +215,6 @@ public:
             .Description = "timeout in ms",
             .Type = "integer",
         });
-        yaml.AddParameter({
-            .Name = "enums",
-            .Description = "convert enums to strings",
-            .Type = "boolean",
-        });
-        yaml.AddParameter({
-            .Name = "ui64",
-            .Description = "return ui64 as number",
-            .Type = "boolean",
-        });
         yaml.SetResponseSchema(TProtoToYaml::ProtoToYamlSchema<NKikimrViewer::TFeatureFlagsConfig>());
         return yaml;
     }

+ 1 - 1
ydb/core/viewer/viewer_tenantinfo.h

@@ -379,7 +379,7 @@ public:
         TString path = GetPath(ev);
         auto& result(NavigateKeySetResult[path]);
         result.Set(std::move(ev));
-        if (result.Get()->Request->ResultSet.size() == 1 && result.Get()->Request->ResultSet.begin()->Status == NSchemeCache::TSchemeCacheNavigate::EStatus::Ok) {
+        if (result.IsOk()) {
             auto domainInfo = result.Get()->Request->ResultSet.begin()->DomainInfo;
             TTabletId hiveId = domainInfo->Params.GetHive();
             if (hiveId) {

+ 2 - 2
ydb/core/viewer/viewer_ut.cpp

@@ -1816,8 +1816,8 @@ Y_UNIT_TEST_SUITE(Viewer) {
         TKeepAliveHttpClient httpClient("localhost", monPort);
         TStringStream responseStream;
         TKeepAliveHttpClient::THeaders headers;
-        headers["Content-Type"] = "application/json";
-        const TKeepAliveHttpClient::THttpCode statusCode = httpClient.DoGet("/viewer/feature_flags?timeout=600000&base64=false", &responseStream, headers);
+        headers["Accept"] = "application/json";
+        const TKeepAliveHttpClient::THttpCode statusCode = httpClient.DoGet("/viewer/feature_flags", &responseStream, headers);
         const TString response = responseStream.ReadAll();
         UNIT_ASSERT_EQUAL_C(statusCode, HTTP_OK, statusCode << ": " << response);
         NJson::TJsonReaderConfig jsonCfg;

+ 1 - 0
ydb/core/viewer/ya.make

@@ -568,6 +568,7 @@ PEERDIR(
     ydb/core/viewer/yaml
     ydb/core/viewer/protos
     ydb/library/persqueue/topic_parser
+    ydb/library/yaml_config
     ydb/public/api/protos
     ydb/public/lib/deprecated/kicli
     ydb/public/lib/json_value