From a194f145e4e03bfee99ea6ae37eb62d7d8c73e06 Mon Sep 17 00:00:00 2001 From: Esun Kim Date: Wed, 21 Jun 2023 12:20:40 -0700 Subject: [PATCH] Revert "[svc-cfg] Move ServiceConfigChannelArgFilter to promises" (#33508) Reverts grpc/grpc#33473 as it caused an internal test breakage. --- BUILD | 2 - .../service_config_channel_arg_filter.cc | 96 +++++++++++-------- 2 files changed, 58 insertions(+), 40 deletions(-) diff --git a/BUILD b/BUILD index 3f903c6a086..bc0ae5b2feb 100644 --- a/BUILD +++ b/BUILD @@ -3033,14 +3033,12 @@ grpc_cc_library( "xds_orca_service_upb", "xds_orca_upb", "//src/core:arena", - "//src/core:arena_promise", "//src/core:channel_args", "//src/core:channel_fwd", "//src/core:channel_init", "//src/core:channel_stack_type", "//src/core:closure", "//src/core:construct_destruct", - "//src/core:context", "//src/core:delegating_helper", "//src/core:dual_ref_counted", "//src/core:env", diff --git a/src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc b/src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc index 81ef16ee58f..d0e6e7f8115 100644 --- a/src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc +++ b/src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc @@ -19,8 +19,7 @@ #include -#include -#include +#include #include #include @@ -35,12 +34,10 @@ #include "src/core/lib/channel/channel_fwd.h" #include "src/core/lib/channel/channel_stack.h" #include "src/core/lib/channel/channel_stack_builder.h" -#include "src/core/lib/channel/context.h" -#include "src/core/lib/channel/promise_based_filter.h" #include "src/core/lib/config/core_configuration.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" -#include "src/core/lib/promise/arena_promise.h" -#include "src/core/lib/promise/context.h" +#include "src/core/lib/iomgr/closure.h" +#include "src/core/lib/iomgr/error.h" #include "src/core/lib/resource_quota/arena.h" #include "src/core/lib/service_config/service_config.h" #include "src/core/lib/service_config/service_config_call_data.h" @@ -48,25 +45,20 @@ #include "src/core/lib/service_config/service_config_parser.h" #include "src/core/lib/surface/channel_init.h" #include "src/core/lib/surface/channel_stack_type.h" -#include "src/core/lib/transport/metadata_batch.h" -#include "src/core/lib/transport/transport.h" namespace grpc_core { namespace { -class ServiceConfigChannelArgFilter : public ChannelFilter { +class ServiceConfigChannelArgChannelData { public: - static absl::StatusOr Create( - const ChannelArgs& args, ChannelFilter::Args) { - return ServiceConfigChannelArgFilter(args); - } - - explicit ServiceConfigChannelArgFilter(const ChannelArgs& args) { - auto service_config_str = args.GetOwnedString(GRPC_ARG_SERVICE_CONFIG); + explicit ServiceConfigChannelArgChannelData( + const grpc_channel_element_args* args) { + auto service_config_str = + args->channel_args.GetOwnedString(GRPC_ARG_SERVICE_CONFIG); if (service_config_str.has_value()) { - auto service_config = - ServiceConfigImpl::Create(args, *service_config_str); + auto service_config = ServiceConfigImpl::Create( + args->channel_args, service_config_str->c_str()); if (!service_config.ok()) { gpr_log(GPR_ERROR, "%s", service_config.status().ToString().c_str()); } else { @@ -75,34 +67,62 @@ class ServiceConfigChannelArgFilter : public ChannelFilter { } } - // Construct a promise for one call. - ArenaPromise MakeCallPromise( - CallArgs call_args, NextPromiseFactory next_promise_factory) override; + RefCountedPtr service_config() const { + return service_config_; + } private: RefCountedPtr service_config_; }; -ArenaPromise -ServiceConfigChannelArgFilter::MakeCallPromise( - CallArgs call_args, NextPromiseFactory next_promise_factory) { +grpc_error_handle ServiceConfigChannelArgInitCallElem( + grpc_call_element* elem, const grpc_call_element_args* args) { + auto* chand = + static_cast(elem->channel_data); + RefCountedPtr service_config = chand->service_config(); const ServiceConfigParser::ParsedConfigVector* method_configs = nullptr; - if (service_config_ != nullptr) { - method_configs = service_config_->GetMethodParsedConfigVector( - call_args.client_initial_metadata->get_pointer(HttpPathMetadata()) - ->c_slice()); + if (service_config != nullptr) { + method_configs = service_config->GetMethodParsedConfigVector(args->path); } - auto* arena = GetContext(); - auto* service_config_call_data = arena->New( - arena, GetContext()); - service_config_call_data->SetServiceConfig(service_config_, method_configs); - return next_promise_factory(std::move(call_args)); + auto* service_config_call_data = + args->arena->New(args->arena, args->context); + service_config_call_data->SetServiceConfig(std::move(service_config), + method_configs); + return absl::OkStatus(); +} + +void ServiceConfigChannelArgDestroyCallElem( + grpc_call_element* /*elem*/, const grpc_call_final_info* /*final_info*/, + grpc_closure* /*then_schedule_closure*/) {} + +grpc_error_handle ServiceConfigChannelArgInitChannelElem( + grpc_channel_element* elem, grpc_channel_element_args* args) { + ServiceConfigChannelArgChannelData* chand = + static_cast(elem->channel_data); + new (chand) ServiceConfigChannelArgChannelData(args); + return absl::OkStatus(); +} + +void ServiceConfigChannelArgDestroyChannelElem(grpc_channel_element* elem) { + ServiceConfigChannelArgChannelData* chand = + static_cast(elem->channel_data); + chand->~ServiceConfigChannelArgChannelData(); } -const grpc_channel_filter kServiceConfigChannelArgFilter = - MakePromiseBasedFilter( - "service_config_channel_arg"); +const grpc_channel_filter ServiceConfigChannelArgFilter = { + grpc_call_next_op, + nullptr, + grpc_channel_next_op, + 0, + ServiceConfigChannelArgInitCallElem, + grpc_call_stack_ignore_set_pollset_or_pollset_set, + ServiceConfigChannelArgDestroyCallElem, + sizeof(ServiceConfigChannelArgChannelData), + ServiceConfigChannelArgInitChannelElem, + grpc_channel_stack_no_post_init, + ServiceConfigChannelArgDestroyChannelElem, + grpc_channel_next_get_info, + "service_config_channel_arg"}; } // namespace @@ -116,7 +136,7 @@ void RegisterServiceConfigChannelArgFilter( !channel_args.GetString(GRPC_ARG_SERVICE_CONFIG).has_value()) { return true; } - builder->PrependFilter(&kServiceConfigChannelArgFilter); + builder->PrependFilter(&ServiceConfigChannelArgFilter); return true; }); }