Skip to content

Commit 903fcf3

Browse files
committed
fix grpc: fix channel/context Qos config race
commit_hash:0df875ca4d23d8c51dbd4554a48ad6e1e95f27e5
1 parent 3be6853 commit 903fcf3

File tree

6 files changed

+28
-29
lines changed

6 files changed

+28
-29
lines changed

grpc/include/userver/ugrpc/client/impl/channel_arguments_builder.hpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,9 @@ class ChannelArgumentsBuilder final {
5959
ChannelArgumentsBuilder& operator=(const ChannelArgumentsBuilder&) = delete;
6060

6161
grpc::ChannelArguments Build(const ClientQos& client_qos) const;
62-
grpc::ChannelArguments Build() const;
6362

6463
private:
6564
const grpc::ChannelArguments& channel_args_;
66-
const std::optional<std::string>& static_service_config_;
6765

6866
ServiceConfigBuilder service_config_builder_;
6967
};

grpc/include/userver/ugrpc/client/impl/client_data.hpp

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include <userver/testsuite/grpc_control.hpp>
1313
#include <userver/utils/fixed_array.hpp>
1414

15+
#include <userver/ugrpc/client/client_qos.hpp>
1516
#include <userver/ugrpc/client/impl/channel_arguments_builder.hpp>
1617
#include <userver/ugrpc/client/impl/client_internals.hpp>
1718
#include <userver/ugrpc/client/impl/stub_any.hpp>
@@ -32,6 +33,8 @@ struct GenericClientTag final {
3233
class ClientData final {
3334
public:
3435
struct StubState {
36+
ClientQos client_qos;
37+
3538
StubPool stubs;
3639
// method_id -> stub_pool
3740
utils::FixedArray<StubPool> dedicated_stubs;
@@ -52,8 +55,10 @@ class ClientData final {
5255
return StubCast<Stub>(stub_);
5356
}
5457

58+
const ClientQos& GetClientQos() const { return state_->client_qos; }
59+
5560
private:
56-
rcu::ReadablePtr<StubState> state_;
61+
const rcu::ReadablePtr<StubState> state_;
5762
StubAny& stub_;
5863
};
5964

@@ -74,14 +79,14 @@ class ClientData final {
7479
if (internals_.qos) {
7580
SubscribeOnConfigUpdate<Service>(*internals_.qos);
7681
} else {
77-
ConstructStubState<Service>(channel_arguments_builder_->Build());
82+
ConstructStubState<Service>();
7883
}
7984
}
8085

8186
template <typename Service>
8287
ClientData(ClientInternals&& internals, GenericClientTag, std::in_place_type_t<Service>)
8388
: internals_(std::move(internals)), stub_state_(std::make_unique<rcu::Variable<StubState>>()) {
84-
ConstructStubState<Service>(BuildChannelArguments(internals_.channel_args, internals_.default_service_config));
89+
ConstructStubState<Service>();
8590
}
8691

8792
~ClientData();
@@ -154,12 +159,16 @@ class ClientData final {
154159
void OnConfigUpdate(const dynamic_config::Snapshot& config) {
155160
UASSERT(internals_.qos);
156161
const auto& client_qos = config[*internals_.qos];
157-
UASSERT(channel_arguments_builder_);
158-
ConstructStubState<Service>(channel_arguments_builder_->Build(client_qos));
162+
ConstructStubState<Service>(client_qos);
159163
}
160164

161165
template <typename Service>
162-
void ConstructStubState(const grpc::ChannelArguments& channel_args) {
166+
void ConstructStubState(const ClientQos& client_qos = {}) {
167+
const auto channel_args =
168+
channel_arguments_builder_.has_value()
169+
? channel_arguments_builder_->Build(client_qos)
170+
: BuildChannelArguments(internals_.channel_args, internals_.default_service_config);
171+
163172
auto stubs = StubPool::Create<typename Service::Stub>(
164173
internals_.channel_count, internals_.channel_factory, channel_args
165174
);
@@ -171,7 +180,7 @@ class ClientData final {
171180
)
172181
: utils::FixedArray<StubPool>{};
173182

174-
stub_state_->Assign({std::move(stubs), std::move(dedicated_stubs)});
183+
stub_state_->Assign({client_qos, std::move(stubs), std::move(dedicated_stubs)});
175184
}
176185

177186
ClientInternals internals_;

grpc/include/userver/ugrpc/client/impl/codegen_declarations.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#include <grpcpp/completion_queue.h>
99

10+
#include <userver/ugrpc/client/client_qos.hpp>
1011
#include <userver/ugrpc/client/impl/client_data.hpp>
1112
#include <userver/ugrpc/client/middlewares/fwd.hpp>
1213
#include <userver/ugrpc/client/qos.hpp>

grpc/include/userver/ugrpc/client/impl/codegen_definitions.hpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,4 @@
88
#include <string_view>
99
#include <utility>
1010

11-
#include <userver/ugrpc/client/client_qos.hpp>
1211
#include <userver/ugrpc/impl/protobuf_collector.hpp>

grpc/src/ugrpc/client/impl/call_params.cpp

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -43,16 +43,16 @@ void SetDeadline(grpc::ClientContext& client_context, const Qos& qos, const test
4343
// 2. manual client_context manipulation by the user
4444
// 3. GRPC_CLIENT_QOS dynamic config
4545
void ApplyQosConfigs(
46-
const ClientData& client_data,
4746
grpc::ClientContext& client_context,
4847
const Qos& user_qos,
49-
std::string_view call_name
48+
const Qos& dynamic_qos,
49+
const testsuite::GrpcControl& testsuite_grpc
5050
) {
5151
if (user_qos.timeout) {
5252
// Consider the explicit Qos parameter the highest-priority source.
5353
// TODO there is no way to override other sources by setting this timeout
5454
// to infinity (we treat it as "not set")
55-
SetDeadline(client_context, user_qos, client_data.GetTestsuiteControl());
55+
SetDeadline(client_context, user_qos, testsuite_grpc);
5656
return;
5757
}
5858

@@ -62,12 +62,7 @@ void ApplyQosConfigs(
6262
return;
6363
}
6464

65-
if (const auto* const config_key = client_data.GetClientQos()) {
66-
const auto config = client_data.GetConfigSnapshot();
67-
if (const auto dynamic_qos = config[*config_key].GetOptional(call_name)) {
68-
SetDeadline(client_context, *dynamic_qos, client_data.GetTestsuiteControl());
69-
}
70-
}
65+
SetDeadline(client_context, dynamic_qos, testsuite_grpc);
7166
}
7267

7368
} // namespace
@@ -85,14 +80,16 @@ CallParams CreateCallParams(
8580
throw RpcCancelledError(call_name, "RPC construction");
8681
}
8782

88-
ApplyQosConfigs(client_data, *client_context, qos, call_name);
83+
auto stub = client_data.NextStubFromMethodId(method_id);
84+
const auto dynamic_qos = stub.GetClientQos().GetOptional(call_name).value_or(Qos{});
85+
ApplyQosConfigs(*client_context, qos, dynamic_qos, client_data.GetTestsuiteControl());
8986

9087
return CallParams{
9188
client_data.GetClientName(), //
9289
client_data.NextQueue(),
9390
client_data.GetConfigSnapshot(),
9491
{ugrpc::impl::MaybeOwnedString::Ref{}, call_name},
95-
client_data.NextStubFromMethodId(method_id),
92+
std::move(stub),
9693
std::move(client_context),
9794
client_data.GetStatistics(method_id),
9895
client_data.GetMiddlewares(),
@@ -115,7 +112,8 @@ CallParams CreateGenericCallParams(
115112
throw RpcCancelledError(call_name, "RPC construction");
116113
}
117114

118-
ApplyQosConfigs(client_data, *client_context, qos, call_name);
115+
UINVARIANT(!client_data.GetClientQos(), "Client QOS configs are unsupported for generic services");
116+
ApplyQosConfigs(*client_context, qos, {}, client_data.GetTestsuiteControl());
119117

120118
return CallParams{
121119
client_data.GetClientName(), //

grpc/src/ugrpc/client/impl/channel_arguments_builder.cpp

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -231,9 +231,7 @@ ChannelArgumentsBuilder::ChannelArgumentsBuilder(
231231
const std::optional<std::string>& static_service_config,
232232
const ugrpc::impl::StaticServiceMetadata& metadata
233233
)
234-
: channel_args_{channel_args},
235-
static_service_config_{static_service_config},
236-
service_config_builder_{metadata, static_service_config} {}
234+
: channel_args_{channel_args}, service_config_builder_{metadata, static_service_config} {}
237235

238236
grpc::ChannelArguments ChannelArgumentsBuilder::Build(const ClientQos& client_qos) const {
239237
const auto service_config = service_config_builder_.Build(client_qos);
@@ -243,10 +241,6 @@ grpc::ChannelArguments ChannelArgumentsBuilder::Build(const ClientQos& client_qo
243241
return BuildChannelArguments(channel_args_, formats::json::ToString(service_config));
244242
}
245243

246-
grpc::ChannelArguments ChannelArgumentsBuilder::Build() const {
247-
return BuildChannelArguments(channel_args_, static_service_config_);
248-
}
249-
250244
grpc::ChannelArguments
251245
BuildChannelArguments(const grpc::ChannelArguments& channel_args, const std::optional<std::string>& service_config) {
252246
if (!service_config.has_value()) {

0 commit comments

Comments
 (0)