From 2758fa5a86107579be6d2941b3b86ecc549e41d8 Mon Sep 17 00:00:00 2001 From: Jose Nino Date: Wed, 11 Sep 2019 12:30:16 -0700 Subject: [PATCH 1/3] server: add new lifecycle event notifier Signed-off-by: Jose Nino --- include/envoy/server/lifecycle_notifier.h | 5 ++++ source/server/server.cc | 11 +++++---- test/server/server_test.cc | 28 +++++++++++++++++------ 3 files changed, 33 insertions(+), 11 deletions(-) diff --git a/include/envoy/server/lifecycle_notifier.h b/include/envoy/server/lifecycle_notifier.h index dbda0e2e0055..2358fb824b9d 100644 --- a/include/envoy/server/lifecycle_notifier.h +++ b/include/envoy/server/lifecycle_notifier.h @@ -21,6 +21,11 @@ class ServerLifecycleNotifier { */ Startup, + /** + * The server instance init manager has finished initialization. + */ + PostInit, + /** * The server instance is being shutdown and the dispatcher is about to exit. * This provides listeners a last chance to run a callback on the main dispatcher. diff --git a/source/server/server.cc b/source/server/server.cc index 4994d53ccdf8..437d6b3c36c3 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -464,10 +464,10 @@ void InstanceImpl::loadServerFlags(const absl::optional& flags_path RunHelper::RunHelper(Instance& instance, const Options& options, Event::Dispatcher& dispatcher, Upstream::ClusterManager& cm, AccessLog::AccessLogManager& access_log_manager, Init::Manager& init_manager, OverloadManager& overload_manager, - std::function workers_start_cb) - : init_watcher_("RunHelper", [&instance, workers_start_cb]() { + std::function post_init_cb) + : init_watcher_("RunHelper", [&instance, post_init_cb]() { if (!instance.isShutdown()) { - workers_start_cb(); + post_init_cb(); } }) { // Setup signals. @@ -524,7 +524,10 @@ void InstanceImpl::run() { // startup (see RunHelperTest in server_test.cc). const auto run_helper = RunHelper(*this, options_, *dispatcher_, clusterManager(), access_log_manager_, init_manager_, - overloadManager(), [this] { startWorkers(); }); + overloadManager(), [this] { + dispatcher_->post([this] { notifyCallbacksForStage(Stage::PostInit); }); + startWorkers(); + }); // Run the main dispatch loop waiting to exit. ENVOY_LOG(info, "starting main dispatch loop"); diff --git a/test/server/server_test.cc b/test/server/server_test.cc index 4f0cf9ecbc60..2f0cb301d04a 100644 --- a/test/server/server_test.cc +++ b/test/server/server_test.cc @@ -219,21 +219,26 @@ class ServerInstanceImplTestBase { Thread::ThreadPtr startTestServer(const std::string& bootstrap_path, const bool use_intializing_instance) { absl::Notification started; + absl::Notification post_init; auto server_thread = Thread::threadFactoryForTest().createThread([&] { initialize(bootstrap_path, use_intializing_instance); auto startup_handle = server_->registerCallback(ServerLifecycleNotifier::Stage::Startup, [&] { started.Notify(); }); + auto post_init_handle = server_->registerCallback(ServerLifecycleNotifier::Stage::PostInit, + [&] { post_init.Notify(); }); auto shutdown_handle = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit, [&](Event::PostCb) { FAIL(); }); shutdown_handle = nullptr; // unregister callback server_->run(); startup_handle = nullptr; + post_init_handle = nullptr; server_ = nullptr; thread_local_ = nullptr; }); started.WaitForNotification(); + post_init.WaitForNotification(); return server_thread; } @@ -321,8 +326,8 @@ TEST_P(ServerInstanceImplTest, EmptyShutdownLifecycleNotifications) { } TEST_P(ServerInstanceImplTest, LifecycleNotifications) { - bool startup = false, shutdown = false, shutdown_with_completion = false; - absl::Notification started, shutdown_begin, completion_block, completion_done; + bool startup = false, post_init = false, shutdown = false, shutdown_with_completion = false; + absl::Notification started, post_init_fired, shutdown_begin, completion_block, completion_done; // Run the server in a separate thread so we can test different lifecycle stages. auto server_thread = Thread::threadFactoryForTest().createThread([&] { @@ -331,11 +336,15 @@ TEST_P(ServerInstanceImplTest, LifecycleNotifications) { startup = true; started.Notify(); }); - auto handle2 = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit, [&] { + auto handle2 = server_->registerCallback(ServerLifecycleNotifier::Stage::PostInit, [&] { + post_init = true; + post_init_fired.Notify(); + }); + auto handle3 = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit, [&] { shutdown = true; shutdown_begin.Notify(); }); - auto handle3 = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit, + auto handle4 = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit, [&](Event::PostCb completion_cb) { // Block till we're told to complete completion_block.WaitForNotification(); @@ -343,16 +352,17 @@ TEST_P(ServerInstanceImplTest, LifecycleNotifications) { server_->dispatcher().post(completion_cb); completion_done.Notify(); }); - auto handle4 = + auto handle5 = server_->registerCallback(ServerLifecycleNotifier::Stage::Startup, [&] { FAIL(); }); - handle4 = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit, + handle5 = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit, [&](Event::PostCb) { FAIL(); }); - handle4 = nullptr; + handle5 = nullptr; server_->run(); handle1 = nullptr; handle2 = nullptr; handle3 = nullptr; + handle4 = nullptr; server_ = nullptr; thread_local_ = nullptr; }); @@ -361,6 +371,10 @@ TEST_P(ServerInstanceImplTest, LifecycleNotifications) { EXPECT_TRUE(startup); EXPECT_FALSE(shutdown); + post_init_fired.WaitForNotification(); + EXPECT_TRUE(post_init); + EXPECT_FALSE(shutdown); + server_->dispatcher().post([&] { server_->shutdown(); }); shutdown_begin.WaitForNotification(); EXPECT_TRUE(shutdown); From 90995212d4d48a87f6f0680aba2d9b0c64dacd79 Mon Sep 17 00:00:00 2001 From: Jose Nino Date: Wed, 11 Sep 2019 12:34:21 -0700 Subject: [PATCH 2/3] version history Signed-off-by: Jose Nino --- docs/root/intro/version_history.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 34dbfca93522..2d7d4770bc41 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -52,6 +52,7 @@ Version history * router check tool: add comprehensive coverage reporting. * router check tool: add deprecated field check. * router check tool: add flag for only printing results of failed tests. +* server: added a post initialization lifecycle event, in addition to the existing startup and shutdown events. * thrift_proxy: fix crashing bug on invalid transport/protocol framing * tls: added verification of IP address SAN fields in certificates against configured SANs in the * tracing: added support to the Zipkin reporter for sending list of spans as Zipkin JSON v2 and protobuf message over HTTP. From a48b7d0a8ea5992d8958d1a574894fb972a4053c Mon Sep 17 00:00:00 2001 From: Jose Nino Date: Fri, 13 Sep 2019 11:45:11 -0700 Subject: [PATCH 3/3] comments Signed-off-by: Jose Nino --- source/server/server.cc | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/source/server/server.cc b/source/server/server.cc index 437d6b3c36c3..a92f83dd9620 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -522,12 +522,11 @@ RunHelper::RunHelper(Instance& instance, const Options& options, Event::Dispatch void InstanceImpl::run() { // RunHelper exists primarily to facilitate testing of how we respond to early shutdown during // startup (see RunHelperTest in server_test.cc). - const auto run_helper = - RunHelper(*this, options_, *dispatcher_, clusterManager(), access_log_manager_, init_manager_, - overloadManager(), [this] { - dispatcher_->post([this] { notifyCallbacksForStage(Stage::PostInit); }); - startWorkers(); - }); + const auto run_helper = RunHelper(*this, options_, *dispatcher_, clusterManager(), + access_log_manager_, init_manager_, overloadManager(), [this] { + notifyCallbacksForStage(Stage::PostInit); + startWorkers(); + }); // Run the main dispatch loop waiting to exit. ENVOY_LOG(info, "starting main dispatch loop");