diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 4d33b1d5be9a..781fd5aa54f5 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -43,6 +43,7 @@ Removed Config or Runtime ------------------------- *Normally occurs at the end of the* :ref:`deprecation period ` +* dispatcher: removed legacy socket read/write resumption code path and runtime guard `envoy.reloadable_features.activate_fds_next_event_loop`. * ext_authz: removed auto ignore case in HTTP-based `ext_authz` header matching and the runtime guard `envoy.reloadable_features.ext_authz_http_service_enable_case_sensitive_string_matcher`. To ignore case, set the :ref:`ignore_case ` field to true. * http: flip default HTTP/1 and HTTP/2 server codec implementations to new codecs that remove the use of exceptions for control flow. To revert to old codec behavior, set the runtime feature `envoy.reloadable_features.new_codec_behavior` to false. * http: removed `envoy.reloadable_features.http1_flood_protection` and legacy code path for turning flood protection off. diff --git a/include/envoy/network/listener.h b/include/envoy/network/listener.h index 4401df6cc20c..d708d02ce7e0 100644 --- a/include/envoy/network/listener.h +++ b/include/envoy/network/listener.h @@ -378,9 +378,6 @@ class UdpListener : public virtual Listener { /** * Make this listener readable at the beginning of the next event loop. - * - * @note: it may become readable during the current loop if feature - * ``envoy.reloadable_features.activate_fds_next_event_loop`` is disabled. */ virtual void activateRead() PURE; }; diff --git a/source/common/event/BUILD b/source/common/event/BUILD index 5e538bc52cf6..99c30f683fc6 100644 --- a/source/common/event/BUILD +++ b/source/common/event/BUILD @@ -33,7 +33,6 @@ envoy_cc_library( "//source/common/network:dns_lib", "//source/common/network:connection_lib", "//source/common/network:listener_lib", - "//source/common/runtime:runtime_features_lib", ] + select({ "//bazel:apple": ["//source/common/network:apple_dns_lib"], "//conditions:default": [], diff --git a/source/common/event/file_event_impl.cc b/source/common/event/file_event_impl.cc index 028bd18cfdf0..8dafc9abb675 100644 --- a/source/common/event/file_event_impl.cc +++ b/source/common/event/file_event_impl.cc @@ -4,7 +4,6 @@ #include "common/common/assert.h" #include "common/event/dispatcher_impl.h" -#include "common/runtime/runtime_features.h" #include "event2/event.h" @@ -14,16 +13,10 @@ namespace Event { FileEventImpl::FileEventImpl(DispatcherImpl& dispatcher, os_fd_t fd, FileReadyCb cb, FileTriggerType trigger, uint32_t events) : cb_(cb), fd_(fd), trigger_(trigger), - activate_fd_events_next_event_loop_( - // Only read the runtime feature if the runtime loader singleton has already been created. - // Attempts to access runtime features too early in the initialization sequence triggers - // some spurious, scary-looking logs about not being able to read runtime feature config - // from the singleton. These warnings are caused by creation of filesystem watchers as - // part of the process of loading the runtime configuration from disk. - Runtime::LoaderSingleton::getExisting() - ? Runtime::runtimeFeatureEnabled( - "envoy.reloadable_features.activate_fds_next_event_loop") - : true) { + activation_cb_(dispatcher.createSchedulableCallback([this]() { + ASSERT(injected_activation_events_ != 0); + mergeInjectedEventsAndRunCb(0); + })) { // Treat the lack of a valid fd (which in practice should only happen if we run out of FDs) as // an OOM condition and just crash. RELEASE_ASSERT(SOCKET_VALID(fd), ""); @@ -33,12 +26,6 @@ FileEventImpl::FileEventImpl(DispatcherImpl& dispatcher, os_fd_t fd, FileReadyCb #endif assignEvents(events, &dispatcher.base()); event_add(&raw_event_, nullptr); - if (activate_fd_events_next_event_loop_) { - activation_cb_ = dispatcher.createSchedulableCallback([this]() { - ASSERT(injected_activation_events_ != 0); - mergeInjectedEventsAndRunCb(0); - }); - } } void FileEventImpl::activate(uint32_t events) { @@ -47,26 +34,6 @@ void FileEventImpl::activate(uint32_t events) { // Only supported event types are set. ASSERT((events & (FileReadyType::Read | FileReadyType::Write | FileReadyType::Closed)) == events); - if (!activate_fd_events_next_event_loop_) { - // Legacy implementation - int libevent_events = 0; - if (events & FileReadyType::Read) { - libevent_events |= EV_READ; - } - - if (events & FileReadyType::Write) { - libevent_events |= EV_WRITE; - } - - if (events & FileReadyType::Closed) { - libevent_events |= EV_CLOSED; - } - - ASSERT(libevent_events); - event_active(&raw_event_, libevent_events, 0); - return; - } - // Schedule the activation callback so it runs as part of the next loop iteration if it is not // already scheduled. if (injected_activation_events_ == 0) { @@ -109,7 +76,7 @@ void FileEventImpl::assignEvents(uint32_t events, event_base* base) { } void FileEventImpl::setEnabled(uint32_t events) { - if (activate_fd_events_next_event_loop_ && injected_activation_events_ != 0) { + if (injected_activation_events_ != 0) { // Clear pending events on updates to the fd event mask to avoid delivering events that are no // longer relevant. Updating the event mask will reset the fd edge trigger state so the proxy // will be able to determine the fd read/write state without need for the injected activation @@ -125,7 +92,7 @@ void FileEventImpl::setEnabled(uint32_t events) { } void FileEventImpl::mergeInjectedEventsAndRunCb(uint32_t events) { - if (activate_fd_events_next_event_loop_ && injected_activation_events_ != 0) { + if (injected_activation_events_ != 0) { events |= injected_activation_events_; injected_activation_events_ = 0; activation_cb_->cancel(); diff --git a/source/common/event/file_event_impl.h b/source/common/event/file_event_impl.h index cc3e505d788b..d85969a4bb0a 100644 --- a/source/common/event/file_event_impl.h +++ b/source/common/event/file_event_impl.h @@ -36,10 +36,6 @@ class FileEventImpl : public FileEvent, ImplBase { uint32_t injected_activation_events_{}; // Used to schedule delayed event activation. Armed iff pending_activation_events_ != 0. SchedulableCallbackPtr activation_cb_; - // Latched "envoy.reloadable_features.activate_fds_next_event_loop" runtime feature. If true, fd - // events scheduled via activate are evaluated in the next iteration of the event loop after - // polling and activating new fd events. - const bool activate_fd_events_next_event_loop_; }; } // namespace Event } // namespace Envoy diff --git a/source/common/event/libevent_scheduler.h b/source/common/event/libevent_scheduler.h index ab076cbabd88..398aa2634012 100644 --- a/source/common/event/libevent_scheduler.h +++ b/source/common/event/libevent_scheduler.h @@ -43,8 +43,6 @@ namespace Event { // The same mechanism implements both of these operations, so they are invoked as a group. // - Event::SchedulableCallback::scheduleCallbackCurrentIteration(). Each of these callbacks is // scheduled and invoked independently. -// - Event::FileEvent::activate() if "envoy.reloadable_features.activate_fds_next_event_loop" -// runtime feature is disabled. // - Event::Timer::enableTimer(0) if "envoy.reloadable_features.activate_timers_next_event_loop" // runtime feature is disabled. // diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index a961c25eaebb..fd9907cbef75 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -57,7 +57,6 @@ constexpr const char* runtime_features[] = { // Begin alphabetically sorted section. "envoy.deprecated_features.allow_deprecated_extension_names", "envoy.reloadable_features.always_apply_route_header_rules", - "envoy.reloadable_features.activate_fds_next_event_loop", "envoy.reloadable_features.activate_timers_next_event_loop", "envoy.reloadable_features.allow_500_after_100", "envoy.reloadable_features.allow_prefetch", diff --git a/test/common/event/file_event_impl_test.cc b/test/common/event/file_event_impl_test.cc index 7116fd8bc5f7..e75e78598c3c 100644 --- a/test/common/event/file_event_impl_test.cc +++ b/test/common/event/file_event_impl_test.cc @@ -47,14 +47,9 @@ class FileEventImplTest : public testing::Test { Api::OsSysCalls& os_sys_calls_; }; -class FileEventImplActivateTest - : public testing::TestWithParam> { +class FileEventImplActivateTest : public testing::TestWithParam { public: - FileEventImplActivateTest() : os_sys_calls_(Api::OsSysCallsSingleton::get()) { - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.activate_fds_next_event_loop", - activateFdsNextEventLoop() ? "true" : "false"}}); - } + FileEventImplActivateTest() : os_sys_calls_(Api::OsSysCallsSingleton::get()) {} static void onWatcherReady(evwatch*, const evwatch_prepare_cb_info*, void* arg) { // `arg` contains the ReadyWatcher passed in from evwatch_prepare_new. @@ -62,19 +57,15 @@ class FileEventImplActivateTest watcher->ready(); } - int domain() { - return std::get<0>(GetParam()) == Network::Address::IpVersion::v4 ? AF_INET : AF_INET6; - } - bool activateFdsNextEventLoop() { return std::get<1>(GetParam()); } + int domain() { return GetParam() == Network::Address::IpVersion::v4 ? AF_INET : AF_INET6; } protected: Api::OsSysCalls& os_sys_calls_; TestScopedRuntime scoped_runtime_; }; -INSTANTIATE_TEST_SUITE_P( - IpVersions, FileEventImplActivateTest, - testing::Combine(testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), testing::Bool())); +INSTANTIATE_TEST_SUITE_P(IpVersions, FileEventImplActivateTest, + testing::ValuesIn(TestEnvironment::getIpVersionsForTest())); TEST_P(FileEventImplActivateTest, Activate) { os_fd_t fd = os_sys_calls_.socket(domain(), SOCK_STREAM, 0).rc_; @@ -160,29 +151,17 @@ TEST_P(FileEventImplActivateTest, ActivateChaining) { EXPECT_CALL(fd_event, ready()); EXPECT_CALL(read_event, ready()); EXPECT_CALL(write_event, ready()); - if (activateFdsNextEventLoop()) { - // Second loop iteration: handle write and close events scheduled while handling read. - EXPECT_CALL(prepare_watcher, ready()); - EXPECT_CALL(fd_event, ready()); - EXPECT_CALL(write_event, ready()); - EXPECT_CALL(closed_event, ready()); - // Third loop iteration: handle close event scheduled while handling write. - EXPECT_CALL(prepare_watcher, ready()); - EXPECT_CALL(fd_event, ready()); - EXPECT_CALL(closed_event, ready()); - // Fourth loop iteration: poll returned no new real events. - EXPECT_CALL(prepare_watcher, ready()); - } else { - // Same loop iteration activation: handle write and close events scheduled while handling read. - EXPECT_CALL(fd_event, ready()); - EXPECT_CALL(write_event, ready()); - EXPECT_CALL(closed_event, ready()); - // Second same loop iteration activation: handle close event scheduled while handling write. - EXPECT_CALL(fd_event, ready()); - EXPECT_CALL(closed_event, ready()); - // Second loop iteration: poll returned no new real events. - EXPECT_CALL(prepare_watcher, ready()); - } + // Second loop iteration: handle write and close events scheduled while handling read. + EXPECT_CALL(prepare_watcher, ready()); + EXPECT_CALL(fd_event, ready()); + EXPECT_CALL(write_event, ready()); + EXPECT_CALL(closed_event, ready()); + // Third loop iteration: handle close event scheduled while handling write. + EXPECT_CALL(prepare_watcher, ready()); + EXPECT_CALL(fd_event, ready()); + EXPECT_CALL(closed_event, ready()); + // Fourth loop iteration: poll returned no new real events. + EXPECT_CALL(prepare_watcher, ready()); file_event->activate(FileReadyType::Read); dispatcher->run(Event::Dispatcher::RunType::NonBlock);