diff options
34 files changed, 316 insertions, 40 deletions
diff --git a/.github/mergeable.yml b/.github/mergeable.yml index 660d8cb440..30692095c4 100644 --- a/.github/mergeable.yml +++ b/.github/mergeable.yml @@ -1,6 +1,14 @@ mergeable: pull_requests: label: - must_include: - regex: "release notes: yes|release notes: no" - message: "Add release notes yes/no label. For yes, add lang label" + or: + - and: + - must_include: + regex: 'release notes: yes' + message: 'Please include release note: yes' + - must_include: + regex: '^lang\/' + message: 'Please include a language label' + - must_include: + regex: 'release notes: no' + message: 'Please include release note: no' @@ -42,6 +42,7 @@ EXPORTS grpc_census_call_get_context grpc_channel_get_target grpc_channel_get_info + grpc_channel_reset_connect_backoff grpc_insecure_channel_create grpc_lame_client_channel_create grpc_channel_destroy diff --git a/include/grpc/grpc.h b/include/grpc/grpc.h index 3245aa5580..348c7a316f 100644 --- a/include/grpc/grpc.h +++ b/include/grpc/grpc.h @@ -274,6 +274,11 @@ GRPCAPI char* grpc_channel_get_target(grpc_channel* channel); GRPCAPI void grpc_channel_get_info(grpc_channel* channel, const grpc_channel_info* channel_info); +/** EXPERIMENTAL. Resets the channel's connect backoff. + TODO(roth): When we see whether this proves useful, either promote + to non-experimental or remove it. */ +GRPCAPI void grpc_channel_reset_connect_backoff(grpc_channel* channel); + /** Create a client channel to 'target'. Additional channel level configuration MAY be provided by grpc_channel_args, though the expectation is that most clients will want to simply pass NULL. The user data in 'args' need only diff --git a/include/grpcpp/channel.h b/include/grpcpp/channel.h index 4b45d5382c..fed02bf7bc 100644 --- a/include/grpcpp/channel.h +++ b/include/grpcpp/channel.h @@ -30,6 +30,14 @@ struct grpc_channel; namespace grpc { + +namespace experimental { +/// Resets the channel's connection backoff. +/// TODO(roth): Once we see whether this proves useful, either create a gRFC +/// and change this to be a method of the Channel class, or remove it. +void ChannelResetConnectionBackoff(Channel* channel); +} // namespace experimental + /// Channels represent a connection to an endpoint. Created by \a CreateChannel. class Channel final : public ChannelInterface, public internal::CallHook, @@ -52,6 +60,7 @@ class Channel final : public ChannelInterface, private: template <class InputMessage, class OutputMessage> friend class internal::BlockingUnaryCallImpl; + friend void experimental::ChannelResetConnectionBackoff(Channel* channel); friend std::shared_ptr<Channel> CreateChannelInternal( const grpc::string& host, grpc_channel* c_channel); Channel(const grpc::string& host, grpc_channel* c_channel); diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index fead8feb17..b06f09d8c7 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -622,6 +622,17 @@ static void start_transport_op_locked(void* arg, grpc_error* error_ignored) { } GRPC_ERROR_UNREF(op->disconnect_with_error); } + + if (op->reset_connect_backoff) { + if (chand->resolver != nullptr) { + chand->resolver->ResetBackoffLocked(); + chand->resolver->RequestReresolutionLocked(); + } + if (chand->lb_policy != nullptr) { + chand->lb_policy->ResetBackoffLocked(); + } + } + GRPC_CHANNEL_STACK_UNREF(chand->owning_stack, "start_transport_op"); GRPC_CLOSURE_SCHED(op->on_consumed, GRPC_ERROR_NONE); diff --git a/src/core/ext/filters/client_channel/lb_policy.h b/src/core/ext/filters/client_channel/lb_policy.h index 31c08246ae..3c0a9c1118 100644 --- a/src/core/ext/filters/client_channel/lb_policy.h +++ b/src/core/ext/filters/client_channel/lb_policy.h @@ -144,7 +144,10 @@ class LoadBalancingPolicy /// consider whether this method is still needed. virtual void ExitIdleLocked() GRPC_ABSTRACT; - /// populates child_subchannels and child_channels with the uuids of this + /// Resets connection backoff. + virtual void ResetBackoffLocked() GRPC_ABSTRACT; + + /// Populates child_subchannels and child_channels with the uuids of this /// LB policy's referenced children. This is not invoked from the /// client_channel's combiner. The implementation is responsible for /// providing its own synchronization. diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc index af6f3fe296..6581385ff9 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc @@ -134,6 +134,7 @@ class GrpcLb : public LoadBalancingPolicy { grpc_error** connectivity_error) override; void HandOffPendingPicksLocked(LoadBalancingPolicy* new_policy) override; void ExitIdleLocked() override; + void ResetBackoffLocked() override; void FillChildRefsForChannelz(ChildRefsList* child_subchannels, ChildRefsList* child_channels) override; @@ -1214,6 +1215,15 @@ void GrpcLb::ExitIdleLocked() { } } +void GrpcLb::ResetBackoffLocked() { + if (lb_channel_ != nullptr) { + grpc_channel_reset_connect_backoff(lb_channel_); + } + if (rr_policy_ != nullptr) { + rr_policy_->ResetBackoffLocked(); + } +} + bool GrpcLb::PickLocked(PickState* pick, grpc_error** error) { PendingPick* pp = PendingPickCreate(pick); bool pick_done = false; diff --git a/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc b/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc index 46acbf628b..2b6a9ba8c5 100644 --- a/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc +++ b/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc @@ -57,6 +57,7 @@ class PickFirst : public LoadBalancingPolicy { grpc_error** connectivity_error) override; void HandOffPendingPicksLocked(LoadBalancingPolicy* new_policy) override; void ExitIdleLocked() override; + void ResetBackoffLocked() override; void FillChildRefsForChannelz(ChildRefsList* child_subchannels, ChildRefsList* ignored) override; @@ -259,6 +260,13 @@ void PickFirst::ExitIdleLocked() { } } +void PickFirst::ResetBackoffLocked() { + subchannel_list_->ResetBackoffLocked(); + if (latest_pending_subchannel_list_ != nullptr) { + latest_pending_subchannel_list_->ResetBackoffLocked(); + } +} + bool PickFirst::PickLocked(PickState* pick, grpc_error** error) { // If we have a selected subchannel already, return synchronously. if (selected_ != nullptr) { diff --git a/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc b/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc index 9c3a15c67b..fea84331d8 100644 --- a/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc +++ b/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc @@ -68,6 +68,7 @@ class RoundRobin : public LoadBalancingPolicy { grpc_error** connectivity_error) override; void HandOffPendingPicksLocked(LoadBalancingPolicy* new_policy) override; void ExitIdleLocked() override; + void ResetBackoffLocked() override; void FillChildRefsForChannelz(ChildRefsList* child_subchannels, ChildRefsList* ignored) override; @@ -333,6 +334,13 @@ void RoundRobin::ExitIdleLocked() { } } +void RoundRobin::ResetBackoffLocked() { + subchannel_list_->ResetBackoffLocked(); + if (latest_pending_subchannel_list_ != nullptr) { + latest_pending_subchannel_list_->ResetBackoffLocked(); + } +} + bool RoundRobin::DoPickLocked(PickState* pick) { const size_t next_ready_index = subchannel_list_->GetNextReadySubchannelIndexLocked(); diff --git a/src/core/ext/filters/client_channel/lb_policy/subchannel_list.h b/src/core/ext/filters/client_channel/lb_policy/subchannel_list.h index 018ac3bb86..0fa2f04e73 100644 --- a/src/core/ext/filters/client_channel/lb_policy/subchannel_list.h +++ b/src/core/ext/filters/client_channel/lb_policy/subchannel_list.h @@ -107,6 +107,11 @@ class SubchannelData { // being unreffed. virtual void UnrefSubchannelLocked(const char* reason); + // Resets the connection backoff. + // TODO(roth): This method should go away when we move the backoff + // code out of the subchannel and into the LB policies. + void ResetBackoffLocked(); + // Starts watching the connectivity state of the subchannel. // ProcessConnectivityChangeLocked() will be called when the // connectivity state changes. @@ -206,6 +211,11 @@ class SubchannelList LoadBalancingPolicy* policy() const { return policy_; } TraceFlag* tracer() const { return tracer_; } + // Resets connection backoff of all subchannels. + // TODO(roth): We will probably need to rethink this as part of moving + // the backoff code out of subchannels and into LB policies. + void ResetBackoffLocked(); + // Note: Caller must ensure that this is invoked inside of the combiner. void Orphan() override { ShutdownLocked(); @@ -300,6 +310,14 @@ void SubchannelData<SubchannelListType, SubchannelDataType>:: template <typename SubchannelListType, typename SubchannelDataType> void SubchannelData<SubchannelListType, + SubchannelDataType>::ResetBackoffLocked() { + if (subchannel_ != nullptr) { + grpc_subchannel_reset_backoff(subchannel_); + } +} + +template <typename SubchannelListType, typename SubchannelDataType> +void SubchannelData<SubchannelListType, SubchannelDataType>::StartConnectivityWatchLocked() { if (subchannel_list_->tracer()->enabled()) { gpr_log(GPR_INFO, @@ -544,6 +562,15 @@ void SubchannelList<SubchannelListType, SubchannelDataType>::ShutdownLocked() { } } +template <typename SubchannelListType, typename SubchannelDataType> +void SubchannelList<SubchannelListType, + SubchannelDataType>::ResetBackoffLocked() { + for (size_t i = 0; i < subchannels_.size(); i++) { + SubchannelDataType* sd = &subchannels_[i]; + sd->ResetBackoffLocked(); + } +} + } // namespace grpc_core #endif /* GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_LB_POLICY_SUBCHANNEL_LIST_H */ diff --git a/src/core/ext/filters/client_channel/resolver.h b/src/core/ext/filters/client_channel/resolver.h index c7e37e4468..48f2e89095 100644 --- a/src/core/ext/filters/client_channel/resolver.h +++ b/src/core/ext/filters/client_channel/resolver.h @@ -94,6 +94,14 @@ class Resolver : public InternallyRefCountedWithTracing<Resolver> { /// throw away unselected subchannels. virtual void RequestReresolutionLocked() GRPC_ABSTRACT; + /// Resets the re-resolution backoff, if any. + /// This needs to be implemented only by pull-based implementations; + /// for push-based implementations, it will be a no-op. + /// TODO(roth): Pull the backoff code out of resolver and into + /// client_channel, so that it can be shared across resolver + /// implementations. At that point, this method can go away. + virtual void ResetBackoffLocked() {} + void Orphan() override { // Invoke ShutdownAndUnrefLocked() inside of the combiner. GRPC_CLOSURE_SCHED( diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc index 7050e82121..f2bb5f3c71 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc @@ -66,6 +66,8 @@ class AresDnsResolver : public Resolver { void RequestReresolutionLocked() override; + void ResetBackoffLocked() override; + void ShutdownLocked() override; private: @@ -187,6 +189,13 @@ void AresDnsResolver::RequestReresolutionLocked() { } } +void AresDnsResolver::ResetBackoffLocked() { + if (have_next_resolution_timer_) { + grpc_timer_cancel(&next_resolution_timer_); + } + backoff_.Reset(); +} + void AresDnsResolver::ShutdownLocked() { if (have_next_resolution_timer_) { grpc_timer_cancel(&next_resolution_timer_); diff --git a/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc b/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc index fae4c33a17..282caf215c 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc @@ -58,6 +58,8 @@ class NativeDnsResolver : public Resolver { void RequestReresolutionLocked() override; + void ResetBackoffLocked() override; + void ShutdownLocked() override; private: @@ -158,6 +160,13 @@ void NativeDnsResolver::RequestReresolutionLocked() { } } +void NativeDnsResolver::ResetBackoffLocked() { + if (have_next_resolution_timer_) { + grpc_timer_cancel(&next_resolution_timer_); + } + backoff_.Reset(); +} + void NativeDnsResolver::ShutdownLocked() { if (have_next_resolution_timer_) { grpc_timer_cancel(&next_resolution_timer_); diff --git a/src/core/ext/filters/client_channel/subchannel.cc b/src/core/ext/filters/client_channel/subchannel.cc index 71ef8c518b..48c6030c89 100644 --- a/src/core/ext/filters/client_channel/subchannel.cc +++ b/src/core/ext/filters/client_channel/subchannel.cc @@ -132,6 +132,8 @@ struct grpc_subchannel { bool have_alarm; /** have we started the backoff loop */ bool backoff_begun; + // reset_backoff() was called while alarm was pending + bool deferred_reset_backoff; /** our alarm */ grpc_timer alarm; @@ -438,6 +440,9 @@ static void on_alarm(void* arg, grpc_error* error) { if (c->disconnected) { error = GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING("Disconnected", &error, 1); + } else if (c->deferred_reset_backoff) { + c->deferred_reset_backoff = false; + error = GRPC_ERROR_NONE; } else { GRPC_ERROR_REF(error); } @@ -675,6 +680,19 @@ static void on_subchannel_connected(void* arg, grpc_error* error) { grpc_channel_args_destroy(delete_channel_args); } +void grpc_subchannel_reset_backoff(grpc_subchannel* subchannel) { + gpr_mu_lock(&subchannel->mu); + if (subchannel->have_alarm) { + subchannel->deferred_reset_backoff = true; + grpc_timer_cancel(&subchannel->alarm); + } else { + subchannel->backoff_begun = false; + subchannel->backoff->Reset(); + maybe_start_connecting_locked(subchannel); + } + gpr_mu_unlock(&subchannel->mu); +} + /* * grpc_subchannel_call implementation */ diff --git a/src/core/ext/filters/client_channel/subchannel.h b/src/core/ext/filters/client_channel/subchannel.h index 9e53f7d542..a135035d62 100644 --- a/src/core/ext/filters/client_channel/subchannel.h +++ b/src/core/ext/filters/client_channel/subchannel.h @@ -145,6 +145,13 @@ grpc_subchannel_get_connected_subchannel(grpc_subchannel* c); const grpc_subchannel_key* grpc_subchannel_get_key( const grpc_subchannel* subchannel); +// Resets the connection backoff of the subchannel. +// TODO(roth): Move connection backoff out of subchannels and up into LB +// policy code (probably by adding a SubchannelGroup between +// SubchannelList and SubchannelData), at which point this method can +// go away. +void grpc_subchannel_reset_backoff(grpc_subchannel* subchannel); + /** continue processing a transport op */ void grpc_subchannel_call_process_op(grpc_subchannel_call* subchannel_call, grpc_transport_stream_op_batch* op); diff --git a/src/core/lib/surface/channel.cc b/src/core/lib/surface/channel.cc index 7cbd61adef..82635d3c21 100644 --- a/src/core/lib/surface/channel.cc +++ b/src/core/lib/surface/channel.cc @@ -281,6 +281,17 @@ void grpc_channel_get_info(grpc_channel* channel, elem->filter->get_channel_info(elem, channel_info); } +void grpc_channel_reset_connect_backoff(grpc_channel* channel) { + grpc_core::ExecCtx exec_ctx; + GRPC_API_TRACE("grpc_channel_reset_connect_backoff(channel=%p)", 1, + (channel)); + grpc_transport_op* op = grpc_make_transport_op(nullptr); + op->reset_connect_backoff = true; + grpc_channel_element* elem = + grpc_channel_stack_element(CHANNEL_STACK_FROM_CHANNEL(channel), 0); + elem->filter->start_transport_op(elem, op); +} + static grpc_call* grpc_channel_create_call_internal( grpc_channel* channel, grpc_call* parent_call, uint32_t propagation_mask, grpc_completion_queue* cq, grpc_pollset_set* pollset_set_alternative, diff --git a/src/core/lib/transport/transport.h b/src/core/lib/transport/transport.h index 585b9dfae9..9e784635c6 100644 --- a/src/core/lib/transport/transport.h +++ b/src/core/lib/transport/transport.h @@ -282,6 +282,8 @@ typedef struct grpc_transport_op { /** Called when the ping ack is received */ grpc_closure* on_ack; } send_ping; + // If true, will reset the channel's connection backoff. + bool reset_connect_backoff; /*************************************************************************** * remaining fields are initialized and used at the discretion of the diff --git a/src/cpp/client/channel_cc.cc b/src/cpp/client/channel_cc.cc index 867f31f025..39b891c2e1 100644 --- a/src/cpp/client/channel_cc.cc +++ b/src/cpp/client/channel_cc.cc @@ -84,6 +84,14 @@ grpc::string Channel::GetServiceConfigJSON() const { &channel_info.service_config_json); } +namespace experimental { + +void ChannelResetConnectionBackoff(Channel* channel) { + grpc_channel_reset_connect_backoff(channel->c_channel_); +} + +} // namespace experimental + internal::Call Channel::CreateCall(const internal::RpcMethod& method, ClientContext* context, CompletionQueue* cq) { diff --git a/src/ruby/ext/grpc/rb_grpc_imports.generated.c b/src/ruby/ext/grpc/rb_grpc_imports.generated.c index 2443532bb8..38b68462df 100644 --- a/src/ruby/ext/grpc/rb_grpc_imports.generated.c +++ b/src/ruby/ext/grpc/rb_grpc_imports.generated.c @@ -65,6 +65,7 @@ grpc_census_call_set_context_type grpc_census_call_set_context_import; grpc_census_call_get_context_type grpc_census_call_get_context_import; grpc_channel_get_target_type grpc_channel_get_target_import; grpc_channel_get_info_type grpc_channel_get_info_import; +grpc_channel_reset_connect_backoff_type grpc_channel_reset_connect_backoff_import; grpc_insecure_channel_create_type grpc_insecure_channel_create_import; grpc_lame_client_channel_create_type grpc_lame_client_channel_create_import; grpc_channel_destroy_type grpc_channel_destroy_import; @@ -315,6 +316,7 @@ void grpc_rb_load_imports(HMODULE library) { grpc_census_call_get_context_import = (grpc_census_call_get_context_type) GetProcAddress(library, "grpc_census_call_get_context"); grpc_channel_get_target_import = (grpc_channel_get_target_type) GetProcAddress(library, "grpc_channel_get_target"); grpc_channel_get_info_import = (grpc_channel_get_info_type) GetProcAddress(library, "grpc_channel_get_info"); + grpc_channel_reset_connect_backoff_import = (grpc_channel_reset_connect_backoff_type) GetProcAddress(library, "grpc_channel_reset_connect_backoff"); grpc_insecure_channel_create_import = (grpc_insecure_channel_create_type) GetProcAddress(library, "grpc_insecure_channel_create"); grpc_lame_client_channel_create_import = (grpc_lame_client_channel_create_type) GetProcAddress(library, "grpc_lame_client_channel_create"); grpc_channel_destroy_import = (grpc_channel_destroy_type) GetProcAddress(library, "grpc_channel_destroy"); diff --git a/src/ruby/ext/grpc/rb_grpc_imports.generated.h b/src/ruby/ext/grpc/rb_grpc_imports.generated.h index b08a1f94f7..d6add00d12 100644 --- a/src/ruby/ext/grpc/rb_grpc_imports.generated.h +++ b/src/ruby/ext/grpc/rb_grpc_imports.generated.h @@ -170,6 +170,9 @@ extern grpc_channel_get_target_type grpc_channel_get_target_import; typedef void(*grpc_channel_get_info_type)(grpc_channel* channel, const grpc_channel_info* channel_info); extern grpc_channel_get_info_type grpc_channel_get_info_import; #define grpc_channel_get_info grpc_channel_get_info_import +typedef void(*grpc_channel_reset_connect_backoff_type)(grpc_channel* channel); +extern grpc_channel_reset_connect_backoff_type grpc_channel_reset_connect_backoff_import; +#define grpc_channel_reset_connect_backoff grpc_channel_reset_connect_backoff_import typedef grpc_channel*(*grpc_insecure_channel_create_type)(const char* target, const grpc_channel_args* args, void* reserved); extern grpc_insecure_channel_create_type grpc_insecure_channel_create_import; #define grpc_insecure_channel_create grpc_insecure_channel_create_import diff --git a/test/core/surface/public_headers_must_be_c89.c b/test/core/surface/public_headers_must_be_c89.c index 9f4ad2b4d7..7b3e875cf0 100644 --- a/test/core/surface/public_headers_must_be_c89.c +++ b/test/core/surface/public_headers_must_be_c89.c @@ -104,6 +104,7 @@ int main(int argc, char **argv) { printf("%lx", (unsigned long) grpc_census_call_get_context); printf("%lx", (unsigned long) grpc_channel_get_target); printf("%lx", (unsigned long) grpc_channel_get_info); + printf("%lx", (unsigned long) grpc_channel_reset_connect_backoff); printf("%lx", (unsigned long) grpc_insecure_channel_create); printf("%lx", (unsigned long) grpc_lame_client_channel_create); printf("%lx", (unsigned long) grpc_channel_destroy); diff --git a/test/cpp/end2end/client_lb_end2end_test.cc b/test/cpp/end2end/client_lb_end2end_test.cc index c5a73a2469..7fe0da8aae 100644 --- a/test/cpp/end2end/client_lb_end2end_test.cc +++ b/test/cpp/end2end/client_lb_end2end_test.cc @@ -408,6 +408,36 @@ TEST_F(ClientLbEnd2endTest, PickFirstBackOffMinReconnect) { gpr_atm_rel_store(&g_connection_delay_ms, 0); } +TEST_F(ClientLbEnd2endTest, PickFirstResetConnectionBackoff) { + ChannelArguments args; + constexpr int kInitialBackOffMs = 1000; + args.SetInt(GRPC_ARG_INITIAL_RECONNECT_BACKOFF_MS, kInitialBackOffMs); + const std::vector<int> ports = {grpc_pick_unused_port_or_die()}; + auto channel = BuildChannel("pick_first", args); + auto stub = BuildStub(channel); + SetNextResolution(ports); + // The channel won't become connected (there's no server). + EXPECT_FALSE( + channel->WaitForConnected(grpc_timeout_milliseconds_to_deadline(10))); + // Bring up a server on the chosen port. + StartServers(1, ports); + const gpr_timespec t0 = gpr_now(GPR_CLOCK_MONOTONIC); + // Wait for connect, but not long enough. This proves that we're + // being throttled by initial backoff. + EXPECT_FALSE( + channel->WaitForConnected(grpc_timeout_milliseconds_to_deadline(10))); + // Reset connection backoff. + experimental::ChannelResetConnectionBackoff(channel.get()); + // Wait for connect. Should happen ~immediately. + EXPECT_TRUE( + channel->WaitForConnected(grpc_timeout_milliseconds_to_deadline(10))); + const gpr_timespec t1 = gpr_now(GPR_CLOCK_MONOTONIC); + const grpc_millis waited_ms = gpr_time_to_millis(gpr_time_sub(t1, t0)); + gpr_log(GPR_DEBUG, "Waited %" PRId64 " milliseconds", waited_ms); + // We should have waited less than kInitialBackOffMs. + EXPECT_LT(waited_ms, kInitialBackOffMs); +} + TEST_F(ClientLbEnd2endTest, PickFirstUpdates) { // Start servers and send one RPC per server. const int kNumServers = 3; diff --git a/test/cpp/util/cli_credentials.cc b/test/cpp/util/cli_credentials.cc index d14dc18f16..acf4ef8ef1 100644 --- a/test/cpp/util/cli_credentials.cc +++ b/test/cpp/util/cli_credentials.cc @@ -20,8 +20,12 @@ #include <gflags/gflags.h> -DEFINE_bool(enable_ssl, false, "Whether to use ssl/tls."); -DEFINE_bool(use_auth, false, "Whether to create default google credentials."); +DEFINE_bool( + enable_ssl, false, + "Whether to use ssl/tls. Deprecated. Use --channel_creds_type=ssl."); +DEFINE_bool(use_auth, false, + "Whether to create default google credentials. Deprecated. Use " + "--channel_creds_type=gdc."); DEFINE_string( access_token, "", "The access token that will be sent to the server to authenticate RPCs."); @@ -29,47 +33,109 @@ DEFINE_string( ssl_target, "", "If not empty, treat the server host name as this for ssl/tls certificate " "validation."); +DEFINE_string( + channel_creds_type, "", + "The channel creds type: insecure, ssl, gdc (Google Default Credentials) " + "or alts."); namespace grpc { namespace testing { -std::shared_ptr<grpc::ChannelCredentials> CliCredentials::GetCredentials() +grpc::string CliCredentials::GetDefaultChannelCredsType() const { + // Compatibility logic for --enable_ssl. + if (FLAGS_enable_ssl) { + fprintf(stderr, + "warning: --enable_ssl is deprecated. Use " + "--channel_creds_type=ssl.\n"); + return "ssl"; + } + // Compatibility logic for --use_auth. + if (FLAGS_access_token.empty() && FLAGS_use_auth) { + fprintf(stderr, + "warning: --use_auth is deprecated. Use " + "--channel_creds_type=gdc.\n"); + return "gdc"; + } + return "insecure"; +} + +std::shared_ptr<grpc::ChannelCredentials> +CliCredentials::GetChannelCredentials() const { + if (FLAGS_channel_creds_type.compare("insecure") == 0) { + return grpc::InsecureChannelCredentials(); + } else if (FLAGS_channel_creds_type.compare("ssl") == 0) { + return grpc::SslCredentials(grpc::SslCredentialsOptions()); + } else if (FLAGS_channel_creds_type.compare("gdc") == 0) { + return grpc::GoogleDefaultCredentials(); + } else if (FLAGS_channel_creds_type.compare("alts") == 0) { + return grpc::experimental::AltsCredentials( + grpc::experimental::AltsCredentialsOptions()); + } + fprintf(stderr, + "--channel_creds_type=%s invalid; must be insecure, ssl, gdc or " + "alts.\n", + FLAGS_channel_creds_type.c_str()); + return std::shared_ptr<grpc::ChannelCredentials>(); +} + +std::shared_ptr<grpc::CallCredentials> CliCredentials::GetCallCredentials() const { if (!FLAGS_access_token.empty()) { if (FLAGS_use_auth) { fprintf(stderr, "warning: use_auth is ignored when access_token is provided."); } - - return grpc::CompositeChannelCredentials( - grpc::SslCredentials(grpc::SslCredentialsOptions()), - grpc::AccessTokenCredentials(FLAGS_access_token)); + return grpc::AccessTokenCredentials(FLAGS_access_token); } + return std::shared_ptr<grpc::CallCredentials>(); +} - if (FLAGS_use_auth) { - return grpc::GoogleDefaultCredentials(); +std::shared_ptr<grpc::ChannelCredentials> CliCredentials::GetCredentials() + const { + if (FLAGS_channel_creds_type.empty()) { + FLAGS_channel_creds_type = GetDefaultChannelCredsType(); + } else if (FLAGS_enable_ssl && FLAGS_channel_creds_type.compare("ssl") != 0) { + fprintf(stderr, + "warning: ignoring --enable_ssl because " + "--channel_creds_type already set to %s.\n", + FLAGS_channel_creds_type.c_str()); + } else if (FLAGS_use_auth && FLAGS_channel_creds_type.compare("gdc") != 0) { + fprintf(stderr, + "warning: ignoring --use_auth because " + "--channel_creds_type already set to %s.\n", + FLAGS_channel_creds_type.c_str()); } - - if (FLAGS_enable_ssl) { - return grpc::SslCredentials(grpc::SslCredentialsOptions()); + // Legacy transport upgrade logic for insecure requests. + if (!FLAGS_access_token.empty() && + FLAGS_channel_creds_type.compare("insecure") == 0) { + fprintf(stderr, + "warning: --channel_creds_type=insecure upgraded to ssl because " + "an access token was provided.\n"); + FLAGS_channel_creds_type = "ssl"; } - - return grpc::InsecureChannelCredentials(); + std::shared_ptr<grpc::ChannelCredentials> channel_creds = + GetChannelCredentials(); + // Composite any call-type credentials on top of the base channel. + std::shared_ptr<grpc::CallCredentials> call_creds = GetCallCredentials(); + return (channel_creds == nullptr || call_creds == nullptr) + ? channel_creds + : grpc::CompositeChannelCredentials(channel_creds, call_creds); } const grpc::string CliCredentials::GetCredentialUsage() const { - return " --enable_ssl ; Set whether to use tls\n" + return " --enable_ssl ; Set whether to use ssl (deprecated)\n" " --use_auth ; Set whether to create default google" " credentials\n" " --access_token ; Set the access token in metadata," " overrides --use_auth\n" - " --ssl_target ; Set server host for tls validation\n"; + " --ssl_target ; Set server host for ssl validation\n" + " --channel_creds_type ; Set to insecure, ssl, gdc, or alts\n"; } const grpc::string CliCredentials::GetSslTargetNameOverride() const { - bool use_tls = - FLAGS_enable_ssl || (FLAGS_access_token.empty() && FLAGS_use_auth); - return use_tls ? FLAGS_ssl_target : ""; + bool use_ssl = FLAGS_channel_creds_type.compare("ssl") == 0 || + FLAGS_channel_creds_type.compare("gdc") == 0; + return use_ssl ? FLAGS_ssl_target : ""; } } // namespace testing diff --git a/test/cpp/util/cli_credentials.h b/test/cpp/util/cli_credentials.h index 8d662356de..4636d3ca14 100644 --- a/test/cpp/util/cli_credentials.h +++ b/test/cpp/util/cli_credentials.h @@ -28,9 +28,22 @@ namespace testing { class CliCredentials { public: virtual ~CliCredentials() {} - virtual std::shared_ptr<grpc::ChannelCredentials> GetCredentials() const; + std::shared_ptr<grpc::ChannelCredentials> GetCredentials() const; virtual const grpc::string GetCredentialUsage() const; virtual const grpc::string GetSslTargetNameOverride() const; + + protected: + // Returns the appropriate channel_creds_type value for the set of legacy + // flag arguments. + virtual grpc::string GetDefaultChannelCredsType() const; + // Returns the base transport channel credentials. Child classes can override + // to support additional channel_creds_types unknown to this base class. + virtual std::shared_ptr<grpc::ChannelCredentials> GetChannelCredentials() + const; + // Returns call credentials to composite onto the base transport channel + // credentials. Child classes can override to support additional + // authentication flags unknown to this base class. + virtual std::shared_ptr<grpc::CallCredentials> GetCallCredentials() const; }; } // namespace testing diff --git a/test/cpp/util/grpc_tool_test.cc b/test/cpp/util/grpc_tool_test.cc index 7e7f44551e..3aae090e81 100644 --- a/test/cpp/util/grpc_tool_test.cc +++ b/test/cpp/util/grpc_tool_test.cc @@ -81,7 +81,7 @@ using grpc::testing::EchoResponse; " peer: \"peer\"\n" \ "}\n\n" -DECLARE_bool(enable_ssl); +DECLARE_string(channel_creds_type); DECLARE_string(ssl_target); namespace grpc { @@ -102,7 +102,8 @@ const int kServerDefaultResponseStreamsToSend = 3; class TestCliCredentials final : public grpc::testing::CliCredentials { public: TestCliCredentials(bool secure = false) : secure_(secure) {} - std::shared_ptr<grpc::ChannelCredentials> GetCredentials() const override { + std::shared_ptr<grpc::ChannelCredentials> GetChannelCredentials() + const override { if (!secure_) { return InsecureChannelCredentials(); } @@ -769,12 +770,12 @@ TEST_F(GrpcToolTest, CallCommandWithBadMetadata) { TEST_F(GrpcToolTest, ListCommand_OverrideSslHostName) { const grpc::string server_address = SetUpServer(true); - // Test input "grpc_cli ls localhost:<port> --enable_ssl + // Test input "grpc_cli ls localhost:<port> --channel_creds_type=ssl // --ssl_target=z.test.google.fr" std::stringstream output_stream; const char* argv[] = {"grpc_cli", "ls", server_address.c_str()}; FLAGS_l = false; - FLAGS_enable_ssl = true; + FLAGS_channel_creds_type = "ssl"; FLAGS_ssl_target = "z.test.google.fr"; EXPECT_TRUE( 0 == GrpcToolMainLib( @@ -784,7 +785,7 @@ TEST_F(GrpcToolTest, ListCommand_OverrideSslHostName) { "grpc.testing.EchoTestService\n" "grpc.reflection.v1alpha.ServerReflection\n")); - FLAGS_enable_ssl = false; + FLAGS_channel_creds_type = ""; FLAGS_ssl_target = ""; ShutdownServer(); } diff --git a/tools/internal_ci/helper_scripts/prepare_build_windows.bat b/tools/internal_ci/helper_scripts/prepare_build_windows.bat index 0164e4a561..f987f8a8cb 100644 --- a/tools/internal_ci/helper_scripts/prepare_build_windows.bat +++ b/tools/internal_ci/helper_scripts/prepare_build_windows.bat @@ -14,7 +14,7 @@ @rem make sure msys binaries are preferred over cygwin binaries @rem set path to python 2.7 -set PATH=C:\tools\msys64\usr\bin;C:\Python27;C:\Python37;%PATH% +set PATH=C:\tools\msys64\usr\bin;C:\Python27;%PATH% @rem If this is a PR using RUN_TESTS_FLAGS var, then add flags to filter tests if defined KOKORO_GITHUB_PULL_REQUEST_NUMBER if defined RUN_TESTS_FLAGS ( @@ -34,9 +34,6 @@ netsh interface ip add dnsservers "Local Area Connection 8" 8.8.4.4 index=3 @rem Needed for big_query_utils python -m pip install google-api-python-client -@rem Install Python 3.7 -chocolatey install -y -r python3 --version 3.7 - @rem Disable some unwanted dotnet options set NUGET_XMLDOC_MODE=skip set DOTNET_SKIP_FIRST_TIME_EXPERIENCE=true diff --git a/tools/internal_ci/linux/grpc_asan_on_foundry.sh b/tools/internal_ci/linux/grpc_asan_on_foundry.sh index 5099fa0301..a6367ad962 100644 --- a/tools/internal_ci/linux/grpc_asan_on_foundry.sh +++ b/tools/internal_ci/linux/grpc_asan_on_foundry.sh @@ -14,6 +14,6 @@ # limitations under the License. export UPLOAD_TEST_RESULTS=true -EXTRA_FLAGS="--copt=-gmlt --strip=never --copt=-fsanitize=address --linkopt=-fsanitize=address --test_timeout=3600 --runs_per_test_detects_flakes --runs_per_test=2" +EXTRA_FLAGS="--copt=-gmlt --strip=never --copt=-fsanitize=address --linkopt=-fsanitize=address --test_timeout=3600 --cache_test_results=no" github/grpc/tools/internal_ci/linux/grpc_bazel_on_foundry_base.sh "${EXTRA_FLAGS}" diff --git a/tools/internal_ci/linux/grpc_bazel_on_foundry_dbg.sh b/tools/internal_ci/linux/grpc_bazel_on_foundry_dbg.sh index 192d9d1b2d..51cb66f5b8 100644 --- a/tools/internal_ci/linux/grpc_bazel_on_foundry_dbg.sh +++ b/tools/internal_ci/linux/grpc_bazel_on_foundry_dbg.sh @@ -14,5 +14,5 @@ # limitations under the License. export UPLOAD_TEST_RESULTS=true -EXTRA_FLAGS="-c dbg --test_timeout=300,450,1200,3600 --runs_per_test_detects_flakes --runs_per_test=2" +EXTRA_FLAGS="-c dbg --test_timeout=300,450,1200,3600 --cache_test_results=no" github/grpc/tools/internal_ci/linux/grpc_bazel_on_foundry_base.sh "${EXTRA_FLAGS}" diff --git a/tools/internal_ci/linux/grpc_bazel_on_foundry_opt.sh b/tools/internal_ci/linux/grpc_bazel_on_foundry_opt.sh index 6fb3c77892..cbba9067ad 100644 --- a/tools/internal_ci/linux/grpc_bazel_on_foundry_opt.sh +++ b/tools/internal_ci/linux/grpc_bazel_on_foundry_opt.sh @@ -14,5 +14,5 @@ # limitations under the License. export UPLOAD_TEST_RESULTS=true -EXTRA_FLAGS="-c opt --test_timeout=300,450,1200,3600 --runs_per_test_detects_flakes --runs_per_test=2" +EXTRA_FLAGS="-c opt --test_timeout=300,450,1200,3600 --cache_test_results=no" github/grpc/tools/internal_ci/linux/grpc_bazel_on_foundry_base.sh "${EXTRA_FLAGS}" diff --git a/tools/internal_ci/linux/grpc_tsan_on_foundry.sh b/tools/internal_ci/linux/grpc_tsan_on_foundry.sh index 25531ead2b..2ba7d469ec 100644 --- a/tools/internal_ci/linux/grpc_tsan_on_foundry.sh +++ b/tools/internal_ci/linux/grpc_tsan_on_foundry.sh @@ -14,5 +14,5 @@ # limitations under the License. export UPLOAD_TEST_RESULTS=true -EXTRA_FLAGS="--copt=-gmlt --strip=never --copt=-fsanitize=thread --linkopt=-fsanitize=thread --test_timeout=3600 --action_env=TSAN_OPTIONS=suppressions=test/core/util/tsan_suppressions.txt:halt_on_error=1:second_deadlock_stack=1 --runs_per_test_detects_flakes --runs_per_test=2" +EXTRA_FLAGS="--copt=-gmlt --strip=never --copt=-fsanitize=thread --linkopt=-fsanitize=thread --test_timeout=3600 --action_env=TSAN_OPTIONS=suppressions=test/core/util/tsan_suppressions.txt:halt_on_error=1:second_deadlock_stack=1 --cache_test_results=no" github/grpc/tools/internal_ci/linux/grpc_bazel_on_foundry_base.sh "${EXTRA_FLAGS}" diff --git a/tools/internal_ci/linux/grpc_ubsan_on_foundry.sh b/tools/internal_ci/linux/grpc_ubsan_on_foundry.sh index d9b039f1ec..338b1b6a0d 100644 --- a/tools/internal_ci/linux/grpc_ubsan_on_foundry.sh +++ b/tools/internal_ci/linux/grpc_ubsan_on_foundry.sh @@ -61,8 +61,7 @@ source tools/internal_ci/helper_scripts/prepare_build_linux_rc --extra_execution_platforms=//third_party/toolchains:rbe_ubuntu1604 \ --host_platform=//third_party/toolchains:rbe_ubuntu1604 \ --platforms=//third_party/toolchains:rbe_ubuntu1604 \ - --runs_per_test_detects_flakes \ - --runs_per_test=2 \ + --cache_test_results=no \ -- //test/... || FAILED="true" # Sleep to let ResultStore finish writing results before querying diff --git a/tools/internal_ci/windows/grpc_build_artifacts.bat b/tools/internal_ci/windows/grpc_build_artifacts.bat index 4d528e0128..0e02bdb6ec 100644 --- a/tools/internal_ci/windows/grpc_build_artifacts.bat +++ b/tools/internal_ci/windows/grpc_build_artifacts.bat @@ -18,6 +18,7 @@ rename C:\Python27_32bit Python27_32bits rename C:\Python34_32bit Python34_32bits rename C:\Python35_32bit Python35_32bits rename C:\Python36_32bit Python36_32bits +rename C:\Python37_32bit Python37_32bits @rem enter repo root cd /d %~dp0\..\..\.. diff --git a/tools/interop_matrix/README.md b/tools/interop_matrix/README.md index 40c02a1bc7..db84d9b454 100644 --- a/tools/interop_matrix/README.md +++ b/tools/interop_matrix/README.md @@ -9,7 +9,7 @@ from specific releases/tag, are used to test version compatiblity between gRPC r We have continuous nightly test setup to test gRPC backward compatibility between old clients and latest server. When a gRPC developer creates a new gRPC release, s/he is also responsible to add the just-released gRPC client to the nightly test. The steps are: - Add (or update) an entry in `./client_matrix.py` file to reference the github tag for the release. - Build new client docker image(s). For example, for C and wrapper languages release `v1.9.9`, do - - `tools/interop_matrix/create_matrix_images.py --git_checkout --release=v1.9.9 --language cxx csharp python ruby php` + - `tools/interop_matrix/create_matrix_images.py --git_checkout --release=v1.9.9 --upload_images --language cxx csharp python ruby php` - Verify that the new docker image was built successfully and uploaded to GCR. For example, - `gcloud beta container images list --repository gcr.io/grpc-testing` shows image repos. - `gcloud beta container images list-tags gcr.io/grpc-testing/grpc_interop_java_oracle8` should show an image entry with tag `v1.9.9`. diff --git a/tools/run_tests/artifacts/artifact_targets.py b/tools/run_tests/artifacts/artifact_targets.py index 01323b5326..a37098d9bf 100644 --- a/tools/run_tests/artifacts/artifact_targets.py +++ b/tools/run_tests/artifacts/artifact_targets.py @@ -392,6 +392,7 @@ def targets(): PythonArtifact('windows', 'x86', 'Python34_32bits'), PythonArtifact('windows', 'x86', 'Python35_32bits'), PythonArtifact('windows', 'x86', 'Python36_32bits'), + PythonArtifact('windows', 'x86', 'Python37_32bits'), PythonArtifact('windows', 'x64', 'Python27'), PythonArtifact('windows', 'x64', 'Python34'), PythonArtifact('windows', 'x64', 'Python35'), |