feat server: open monitor port early (attempt 3)

commit_hash:6d35ef9cc56d58f4b9988e07f7f12fdfb4280446

Author: segoon

core/functional_tests/early_monitor_port_open/tests/test_boot.py

@@ -1,11 +1,9 @@
 import asyncio
 
 import aiohttp
-import pytest
 import pytest_userver.utils.sync as sync
 
 
-@pytest.mark.skip(reason='the feature is not yet enabled')
 async def test_monitor_port_is_open_before_all_components_are_ready(
     ensure_daemon_started,
     service_daemon_scope,

core/include/userver/server/server.hpp

@@ -53,6 +53,7 @@ class Server final : public congestion_control::Limitee, public congestion_contr
 
     const http::HttpRequestHandler& GetHttpRequestHandler(bool is_monitor = false) const;
 
+    void StartMonitorPort();
     void Start();
 
     void Stop();

core/src/components/component_context_impl.cpp

@@ -168,6 +168,11 @@ void ComponentContextImpl::OnAllComponentsAreStopping() {
 }
 
 void ComponentContextImpl::ClearComponents() {
+    if (components_.empty()) {
+        // Already cleared
+        return;
+    }
+
     StopPrintAddingComponentsTask();
     const tracing::Span span(kClearComponentsRootName);
     OnAllComponentsAreStopping();
@@ -182,6 +187,7 @@ void ComponentContextImpl::ClearComponents() {
          false}
     );
 
+    components_.clear();
     LOG_INFO() << "Stopped all components";
 }
 
@@ -197,6 +203,7 @@ void ComponentContextImpl::CancelComponentsLoad() {
         return;
     }
     for (auto& component_info : components_) {
+        LOG_DEBUG() << "Call OnLoadingCancelled() for component '" << component_info->GetName() << "'";
         component_info->OnLoadingCancelled();
     }
 }

core/src/server/component.cpp

@@ -44,6 +44,8 @@ Server::Server(
         statistics_storage.RegisterWriter("http.handler.total", [this](utils::statistics::Writer& writer) {
             return server_->WriteTotalHandlerStatistics(writer);
         });
+
+    server_->StartMonitorPort();
 }
 
 Server::~Server() {

core/src/server/handlers/http_handler_base.cpp

@@ -11,6 +11,7 @@
 #include <userver/server/http/http_request.hpp>
 
 #include <userver/components/component.hpp>
+#include <userver/components/scope.hpp>
 #include <userver/components/statistics_storage.hpp>
 #include <userver/dynamic_config/storage/component.hpp>
 #include <userver/engine/deadline.hpp>
@@ -144,17 +145,26 @@ HttpHandlerBase::HttpHandlerBase(
         LOG_WARNING() << "empty allowed methods list in " << config.Name();
     }
 
-    auto& server_component = context.FindComponent<components::Server>();
+    auto& server = context.FindComponent<components::Server>().GetServer();
 
     engine::TaskProcessor& task_processor =
         GetConfig().task_processor
             ? context.GetTaskProcessor(*GetConfig().task_processor)
             : engine::current_task::GetTaskProcessor();
-    try {
-        server_component.AddHandler(*this, task_processor);
-    } catch (const std::exception& ex) {
-        throw std::runtime_error(std::string("can't add handler to server: ") + ex.what());
-    }
+
+    // Postpone handler registration as a request handling requires
+    // HandleRequest() implementation, which is available only after
+    // the descendant constructor.
+    context.RegisterScope(components::MakeScope([this, &server, &task_processor] {
+        try {
+            server.AddHandler(*this, task_processor);
+
+            // Note: no need to call RemoveHandler on scope exit,
+            // PortInfo::Stop() automatically removes all handlers
+        } catch (const std::exception& ex) {
+            throw std::runtime_error(std::string("can't add handler to server: ") + ex.what());
+        }
+    }));
 
     BuildMiddlewarePipeline(config, context);
 
@@ -188,9 +198,7 @@ HttpHandlerBase::HttpHandlerBase(
     }
 
     set_response_server_hostname_ =
-        GetConfig()
-            .set_response_server_hostname.value_or(server_component.GetServer().GetConfig().set_response_server_hostname
-            );
+        GetConfig().set_response_server_hostname.value_or(server.GetConfig().set_response_server_hostname);
 }
 
 HttpHandlerBase::~HttpHandlerBase() { statistics_holder_.Unregister(); }

core/src/server/server.cpp

@@ -112,7 +112,8 @@ class ServerImpl final {
     );
     ~ServerImpl();
 
-    void StartPortInfos();
+    void StartPortInfo();
+    void StartMonitorPortInfo();
     void Stop();
 
     void AddHandler(const handlers::HttpHandlerBase& handler, engine::TaskProcessor& task_processor);
@@ -178,7 +179,7 @@ ServerImpl::ServerImpl(
 
 ServerImpl::~ServerImpl() { Stop(); }
 
-void ServerImpl::StartPortInfos() {
+void ServerImpl::StartPortInfo() {
     UASSERT(main_port_info_.request_handler);
 
     if (has_requests_view_watchers_.load()) {
@@ -188,13 +189,15 @@ void ServerImpl::StartPortInfos() {
         main_port_info_.request_handler->SetNewRequestHook(hook);
     }
 
-    main_port_info_.Start();
     main_port_info_.request_handler->DisableAddHandler();
+    main_port_info_.Start();
+
     if (monitor_port_info_.request_handler) {
         monitor_port_info_.request_handler->DisableAddHandler();
     }
+}
 
-    // TODO: move to ctr for early start
+void ServerImpl::StartMonitorPortInfo() {
     if (monitor_port_info_.request_handler) {
         monitor_port_info_.Start();
     } else {
@@ -383,10 +386,17 @@ const http::HttpRequestHandler& Server::GetHttpRequestHandler(bool is_monitor) c
     return pimpl_->GetHttpRequestHandler(is_monitor);
 }
 
+void Server::StartMonitorPort()
+{
+    LOG_INFO() << "Starting monitor port";
+    pimpl_->StartMonitorPortInfo();
+    LOG_INFO() << "Monitor port is started";
+}
+
 void Server::Start() {
-    LOG_INFO() << "Starting server";
-    pimpl_->StartPortInfos();
-    LOG_INFO() << "Server is started";
+    LOG_INFO() << "Starting server port";
+    pimpl_->StartPortInfo();
+    LOG_INFO() << "Server port is started";
 }
 
 void Server::Stop() { pimpl_->Stop(); }