From 989aee5536fc7589289a9ca633b383acf2a31868 Mon Sep 17 00:00:00 2001 From: Ryan Leung Date: Wed, 22 Jul 2026 04:48:01 +0800 Subject: [PATCH 1/7] client: suppress repeated member refreshes during transport outages Signed-off-by: Ryan Leung --- .../member_refresh_controller.go | 315 +++++++++++++ .../member_refresh_controller_test.go | 431 ++++++++++++++++++ client/servicediscovery/service_discovery.go | 241 +++++++++- .../service_discovery_test.go | 177 +++++++ 4 files changed, 1146 insertions(+), 18 deletions(-) create mode 100644 client/servicediscovery/member_refresh_controller.go create mode 100644 client/servicediscovery/member_refresh_controller_test.go diff --git a/client/servicediscovery/member_refresh_controller.go b/client/servicediscovery/member_refresh_controller.go new file mode 100644 index 0000000000..b569da7bcb --- /dev/null +++ b/client/servicediscovery/member_refresh_controller.go @@ -0,0 +1,315 @@ +// Copyright 2026 TiKV Project Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package servicediscovery + +import ( + "context" + "errors" + "sort" + "sync" + "time" + + "google.golang.org/grpc/codes" + "google.golang.org/grpc/connectivity" + "google.golang.org/grpc/status" + + "github.com/pingcap/kvproto/pkg/pdpb" + + clienterrs "github.com/tikv/pd/client/errs" +) + +type memberFailurePhase string + +const ( + memberFailurePhaseDial memberFailurePhase = "dial" + memberFailurePhaseRPC memberFailurePhase = "rpc" + memberFailurePhaseResponse memberFailurePhase = "response" + memberFailurePhaseClusterID memberFailurePhase = "cluster-id" + memberFailurePhaseLeader memberFailurePhase = "leader" +) + +type memberFailureFingerprint struct { + phase memberFailurePhase + grpcCode codes.Code + pdErrorType pdpb.ErrorType +} + +func (f memberFailureFingerprint) String() string { + switch f.phase { + case memberFailurePhaseRPC: + return string(f.phase) + "/" + f.grpcCode.String() + case memberFailurePhaseResponse: + return string(f.phase) + "/" + f.pdErrorType.String() + default: + return string(f.phase) + } +} + +type memberUpdateFailure struct { + fingerprint memberFailureFingerprint + transport bool +} + +func classifyMemberDialFailure(err error) memberUpdateFailure { + return memberUpdateFailure{ + fingerprint: memberFailureFingerprint{phase: memberFailurePhaseDial}, + transport: errors.Is(err, clienterrs.ErrGRPCDial), + } +} + +func classifyMemberRPCFailure(err error) memberUpdateFailure { + code := status.Code(err) + if errors.Is(err, context.DeadlineExceeded) { + code = codes.DeadlineExceeded + } + return memberUpdateFailure{ + fingerprint: memberFailureFingerprint{phase: memberFailurePhaseRPC, grpcCode: code}, + transport: clienterrs.IsNetworkError(code), + } +} + +func classifyMemberResponseFailure(errorType pdpb.ErrorType) memberUpdateFailure { + return memberUpdateFailure{ + fingerprint: memberFailureFingerprint{phase: memberFailurePhaseResponse, pdErrorType: errorType}, + } +} + +func classifyMemberSemanticFailure(phase memberFailurePhase) memberUpdateFailure { + return memberUpdateFailure{ + fingerprint: memberFailureFingerprint{phase: phase}, + } +} + +type memberUpdateResult struct { + attemptedURLs []string + transportFailures int +} + +func (r *memberUpdateResult) recordFailure(url string, failure memberUpdateFailure) { + r.attemptedURLs = append(r.attemptedURLs, url) + if failure.transport { + r.transportFailures++ + } +} + +func (r *memberUpdateResult) allFailedByTransport(urls []string) bool { + if len(urls) == 0 || len(r.attemptedURLs) != len(urls) || r.transportFailures != len(urls) { + return false + } + return equalMemberURLs(r.attemptedURLs, urls) +} + +type memberConnectionState struct { + observed bool + state connectivity.State +} + +type memberRefreshAction uint8 + +const ( + memberRefreshWait memberRefreshAction = iota + memberRefreshRetryBatch +) + +type memberRefreshDecision struct { + action memberRefreshAction +} + +type memberRefreshController struct { + degraded bool + degradedURLs []string +} + +func (c *memberRefreshController) isDegraded() bool { + return c.degraded +} + +func (c *memberRefreshController) enterDegraded( + result memberUpdateResult, + urls []string, + states []memberConnectionState, +) bool { + if len(urls) != len(states) || !result.allFailedByTransport(urls) { + return false + } + for _, state := range states { + if !state.observed || !isInactiveMemberConnectionState(state.state) { + return false + } + } + c.degraded = true + c.degradedURLs = append(c.degradedURLs[:0], urls...) + return true +} + +func (c *memberRefreshController) inspect(urls []string, states []memberConnectionState) memberRefreshDecision { + if !c.degraded { + return memberRefreshDecision{action: memberRefreshRetryBatch} + } + if len(urls) != len(states) || !equalMemberURLs(c.degradedURLs, urls) { + c.leaveDegraded() + return memberRefreshDecision{action: memberRefreshRetryBatch} + } + for _, state := range states { + if !state.observed || !isInactiveMemberConnectionState(state.state) { + c.leaveDegraded() + return memberRefreshDecision{action: memberRefreshRetryBatch} + } + } + return memberRefreshDecision{action: memberRefreshWait} +} + +func (c *memberRefreshController) leaveDegraded() { + c.degraded = false + c.degradedURLs = nil +} + +func equalMemberURLs(left, right []string) bool { + if len(left) != len(right) { + return false + } + for i := range left { + if left[i] != right[i] { + return false + } + } + return true +} + +func isInactiveMemberConnectionState(state connectivity.State) bool { + return state == connectivity.Idle || + state == connectivity.Connecting || + state == connectivity.TransientFailure +} + +type memberFailureEpisode struct { + firstFailure time.Time + fingerprint memberFailureFingerprint + failedAttempts uint64 + suppressedErrors uint64 +} + +type memberFailureRecovery struct { + url string + failureDuration time.Duration + failedAttempts uint64 + suppressedErrors uint64 +} + +type memberFailureSummary struct { + failedURLs []string + errorClasses []string + failureDuration time.Duration + failedAttempts uint64 + suppressedErrors uint64 +} + +type memberFailureTracker struct { + mu sync.Mutex + episodes map[string]*memberFailureEpisode +} + +// record returns true when the caller should emit the detailed failure log. +func (t *memberFailureTracker) record(now time.Time, url string, failure memberUpdateFailure) bool { + t.mu.Lock() + defer t.mu.Unlock() + + if t.episodes == nil { + t.episodes = make(map[string]*memberFailureEpisode) + } + episode, ok := t.episodes[url] + if !ok { + t.episodes[url] = &memberFailureEpisode{ + firstFailure: now, + fingerprint: failure.fingerprint, + failedAttempts: 1, + } + return true + } + + episode.failedAttempts++ + if episode.fingerprint != failure.fingerprint { + episode.fingerprint = failure.fingerprint + return true + } + episode.suppressedErrors++ + return false +} + +func (t *memberFailureTracker) recover(now time.Time, url string) (memberFailureRecovery, bool) { + t.mu.Lock() + defer t.mu.Unlock() + + episode, ok := t.episodes[url] + if !ok { + return memberFailureRecovery{}, false + } + delete(t.episodes, url) + return memberFailureRecovery{ + url: url, + failureDuration: now.Sub(episode.firstFailure), + failedAttempts: episode.failedAttempts, + suppressedErrors: episode.suppressedErrors, + }, true +} + +func (t *memberFailureTracker) cleanup(urls []string) { + t.mu.Lock() + defer t.mu.Unlock() + + if len(t.episodes) == 0 { + return + } + current := make(map[string]struct{}, len(urls)) + for _, url := range urls { + current[url] = struct{}{} + } + for url := range t.episodes { + if _, ok := current[url]; !ok { + delete(t.episodes, url) + } + } +} + +func (t *memberFailureTracker) summary(now time.Time) (memberFailureSummary, bool) { + t.mu.Lock() + defer t.mu.Unlock() + + if len(t.episodes) == 0 { + return memberFailureSummary{}, false + } + urls := make([]string, 0, len(t.episodes)) + for url := range t.episodes { + urls = append(urls, url) + } + sort.Strings(urls) + + summary := memberFailureSummary{ + failedURLs: urls, + errorClasses: make([]string, 0, len(urls)), + } + earliest := now + for _, url := range urls { + episode := t.episodes[url] + if episode.firstFailure.Before(earliest) { + earliest = episode.firstFailure + } + summary.errorClasses = append(summary.errorClasses, episode.fingerprint.String()) + summary.failedAttempts += episode.failedAttempts + summary.suppressedErrors += episode.suppressedErrors + } + summary.failureDuration = now.Sub(earliest) + return summary, true +} diff --git a/client/servicediscovery/member_refresh_controller_test.go b/client/servicediscovery/member_refresh_controller_test.go new file mode 100644 index 0000000000..e752750f9f --- /dev/null +++ b/client/servicediscovery/member_refresh_controller_test.go @@ -0,0 +1,431 @@ +// Copyright 2026 TiKV Project Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package servicediscovery + +import ( + "context" + "errors" + "fmt" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/require" + "go.uber.org/zap" + "go.uber.org/zap/zaptest/observer" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/connectivity" + "google.golang.org/grpc/status" + + "github.com/pingcap/kvproto/pkg/pdpb" + pingcaplog "github.com/pingcap/log" + + clienterrs "github.com/tikv/pd/client/errs" +) + +func TestMemberRefreshControllerEntersDegradedModeStrictly(t *testing.T) { + t.Parallel() + + transportFailure := memberUpdateFailure{ + fingerprint: memberFailureFingerprint{phase: memberFailurePhaseRPC, grpcCode: codes.Unavailable}, + transport: true, + } + semanticFailure := memberUpdateFailure{ + fingerprint: memberFailureFingerprint{phase: memberFailurePhaseResponse, pdErrorType: pdpb.ErrorType_UNKNOWN}, + } + + testCases := []struct { + name string + result memberUpdateResult + states []memberConnectionState + enter bool + }{ + { + name: "all current urls have transport failures and inactive connections", + result: newFailedMemberUpdateResult(transportFailure, transportFailure, transportFailure), + states: observedMemberConnectionStates(connectivity.Idle, connectivity.Connecting, connectivity.TransientFailure), + enter: true, + }, + { + name: "empty url set", + result: memberUpdateResult{}, + states: nil, + }, + { + name: "not every url was attempted", + result: newFailedMemberUpdateResult(transportFailure), + states: observedMemberConnectionStates(connectivity.TransientFailure, connectivity.TransientFailure), + }, + { + name: "semantic failure", + result: newFailedMemberUpdateResult(transportFailure, semanticFailure), + states: observedMemberConnectionStates(connectivity.TransientFailure, connectivity.TransientFailure), + }, + { + name: "missing connection", + result: newFailedMemberUpdateResult(transportFailure), + states: []memberConnectionState{{}}, + }, + { + name: "ready connection", + result: newFailedMemberUpdateResult(transportFailure), + states: observedMemberConnectionStates(connectivity.Ready), + }, + { + name: "shutdown connection", + result: newFailedMemberUpdateResult(transportFailure), + states: observedMemberConnectionStates(connectivity.Shutdown), + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + controller := memberRefreshController{} + require.Equal(t, testCase.enter, controller.enterDegraded( + testCase.result, + memberTestURLs(len(testCase.states)), + testCase.states, + )) + require.Equal(t, testCase.enter, controller.isDegraded()) + }) + } +} + +func TestMemberRefreshControllerInspectsConnectionStates(t *testing.T) { + t.Parallel() + + transportFailure := memberUpdateFailure{transport: true} + testCases := []struct { + name string + states []memberConnectionState + action memberRefreshAction + }{ + { + name: "idle connections wait without a member refresh", + states: observedMemberConnectionStates(connectivity.Idle, connectivity.Connecting, connectivity.Idle), + action: memberRefreshWait, + }, + { + name: "ready connection refreshes immediately", + states: observedMemberConnectionStates(connectivity.TransientFailure, connectivity.Ready), + action: memberRefreshRetryBatch, + }, + { + name: "missing connection restores normal behavior", + states: []memberConnectionState{{observed: true, state: connectivity.TransientFailure}, {}}, + action: memberRefreshRetryBatch, + }, + { + name: "shutdown connection restores normal behavior", + states: observedMemberConnectionStates(connectivity.TransientFailure, connectivity.Shutdown), + action: memberRefreshRetryBatch, + }, + { + name: "url replacement restores normal behavior", + states: observedMemberConnectionStates(connectivity.TransientFailure, connectivity.TransientFailure), + action: memberRefreshRetryBatch, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + controller := memberRefreshController{} + initialURLs := memberTestURLs(len(testCase.states)) + failures := make([]memberUpdateFailure, len(testCase.states)) + for i := range failures { + failures[i] = transportFailure + } + require.True(t, controller.enterDegraded( + newFailedMemberUpdateResult(failures...), + initialURLs, + observedMemberConnectionStates(repeatedConnectivityState(connectivity.TransientFailure, len(testCase.states))...), + )) + currentURLs := initialURLs + if testCase.name == "url replacement restores normal behavior" { + currentURLs = []string{"url-0", "replacement-url"} + } + decision := controller.inspect(currentURLs, testCase.states) + require.Equal(t, testCase.action, decision.action) + require.Equal(t, testCase.action == memberRefreshWait, controller.isDegraded()) + }) + } +} + +func TestMemberRefreshControllerInspectDoesNotAllocate(t *testing.T) { + transportFailure := memberUpdateFailure{transport: true} + result := newFailedMemberUpdateResult(transportFailure, transportFailure, transportFailure) + urls := memberTestURLs(3) + states := observedMemberConnectionStates(connectivity.Idle, connectivity.Connecting, connectivity.TransientFailure) + controller := memberRefreshController{} + require.True(t, controller.enterDegraded(result, urls, states)) + allocations := testing.AllocsPerRun(1000, func() { + memberRefreshDecisionSink = controller.inspect(urls, states) + }) + require.Zero(t, allocations) +} + +var memberRefreshDecisionSink memberRefreshDecision + +func TestMemberFailureTrackerEpisodes(t *testing.T) { + t.Parallel() + + tracker := memberFailureTracker{} + start := time.Unix(100, 0) + refused := memberUpdateFailure{ + fingerprint: memberFailureFingerprint{phase: memberFailurePhaseRPC, grpcCode: codes.Unavailable}, + transport: true, + } + timeout := memberUpdateFailure{ + fingerprint: memberFailureFingerprint{phase: memberFailurePhaseRPC, grpcCode: codes.DeadlineExceeded}, + transport: true, + } + + require.True(t, tracker.record(start, "http://pd-1:2379", refused)) + require.False(t, tracker.record(start.Add(time.Second), "http://pd-1:2379", refused)) + require.True(t, tracker.record(start.Add(2*time.Second), "http://pd-1:2379", timeout)) + require.True(t, tracker.record(start.Add(3*time.Second), "http://pd-2:2379", refused)) + + summary, ok := tracker.summary(start.Add(4 * time.Second)) + require.True(t, ok) + require.Equal(t, []string{"http://pd-1:2379", "http://pd-2:2379"}, summary.failedURLs) + require.Equal(t, []string{"rpc/DeadlineExceeded", "rpc/Unavailable"}, summary.errorClasses) + require.Equal(t, uint64(4), summary.failedAttempts) + require.Equal(t, uint64(1), summary.suppressedErrors) + require.Equal(t, 4*time.Second, summary.failureDuration) + + recovery, ok := tracker.recover(start.Add(5*time.Second), "http://pd-2:2379") + require.True(t, ok) + require.Equal(t, "http://pd-2:2379", recovery.url) + require.Equal(t, uint64(1), recovery.failedAttempts) + require.Zero(t, recovery.suppressedErrors) + + // A success from pd-2 must not recover pd-1. + summary, ok = tracker.summary(start.Add(6 * time.Second)) + require.True(t, ok) + require.Equal(t, []string{"http://pd-1:2379"}, summary.failedURLs) + + tracker.cleanup([]string{"http://pd-3:2379"}) + _, ok = tracker.summary(start.Add(7 * time.Second)) + require.False(t, ok) +} + +func TestClassifyMemberFailure(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + failure memberUpdateFailure + fingerprint string + transport bool + }{ + { + name: "connection refused rpc", + failure: classifyMemberRPCFailure(status.Error(codes.Unavailable, "dial tcp 192.0.2.1:2379: connect: connection refused")), + fingerprint: "rpc/Unavailable", + transport: true, + }, + { + name: "tls rpc", + failure: classifyMemberRPCFailure(status.Error(codes.Unavailable, "transport: authentication handshake failed: tls certificate expired")), + fingerprint: "rpc/Unavailable", + transport: true, + }, + { + name: "dns rpc", + failure: classifyMemberRPCFailure(status.Error(codes.Unavailable, "lookup pd.invalid: no such host")), + fingerprint: "rpc/Unavailable", + transport: true, + }, + { + name: "deadline rpc", + failure: classifyMemberRPCFailure(status.Error(codes.DeadlineExceeded, "context deadline exceeded")), + fingerprint: "rpc/DeadlineExceeded", + transport: true, + }, + { + name: "reset rpc", + failure: classifyMemberRPCFailure(status.Error(codes.Unavailable, "read: connection reset by peer")), + fingerprint: "rpc/Unavailable", + transport: true, + }, + { + name: "non-network grpc status", + failure: classifyMemberRPCFailure(status.Error(codes.PermissionDenied, "permission denied")), + fingerprint: "rpc/PermissionDenied", + transport: false, + }, + { + name: "blocking dial timeout", + failure: classifyMemberDialFailure(clienterrs.ErrGRPCDial.Wrap(context.DeadlineExceeded).GenWithStackByCause()), + fingerprint: "dial", + transport: true, + }, + { + name: "uncertain dial error", + failure: classifyMemberDialFailure(errors.New("invalid client configuration")), + fingerprint: "dial", + transport: false, + }, + { + name: "pd response error", + failure: classifyMemberResponseFailure(pdpb.ErrorType_NOT_BOOTSTRAPPED), + fingerprint: "response/NOT_BOOTSTRAPPED", + transport: false, + }, + { + name: "cluster id mismatch", + failure: classifyMemberSemanticFailure(memberFailurePhaseClusterID), + fingerprint: "cluster-id", + transport: false, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + require.Equal(t, testCase.fingerprint, testCase.failure.fingerprint.String()) + require.Equal(t, testCase.transport, testCase.failure.transport) + }) + } + + // Dynamic targets must not create a new fingerprint for the same failure class. + first := classifyMemberRPCFailure(status.Error(codes.Unavailable, "dial tcp 192.0.2.1:2379: connection refused")) + second := classifyMemberRPCFailure(status.Error(codes.Unavailable, "dial tcp 198.51.100.8:1234: connection refused")) + require.Equal(t, first.fingerprint, second.fingerprint) +} + +func TestMemberFailureSummaryAndRecoveryLogs(t *testing.T) { + core, observedLogs := observer.New(zap.InfoLevel) + restoreLogger := pingcaplog.ReplaceGlobals(zap.New(core), nil) + t.Cleanup(restoreLogger) + + client := &serviceDiscovery{} + start := time.Unix(100, 0) + failure := memberUpdateFailure{ + fingerprint: memberFailureFingerprint{phase: memberFailurePhaseRPC, grpcCode: codes.Unavailable}, + transport: true, + } + require.True(t, client.memberFailures.record(start, "http://pd-1:2379", failure)) + require.False(t, client.memberFailures.record(start.Add(time.Second), "http://pd-1:2379", failure)) + + client.logMemberFailureSummary(start.Add(2 * time.Second)) + summaryLogs := observedLogs.FilterMessage("[pd] member update failures are being suppressed").All() + require.Len(t, summaryLogs, 1) + summaryFields := summaryLogs[0].ContextMap() + require.Contains(t, summaryFields, "failed-urls") + require.Equal(t, uint64(2), summaryFields["failed-attempts"]) + require.Equal(t, uint64(1), summaryFields["suppressed-errors"]) + require.Contains(t, summaryFields, "error-classes") + + client.logMemberFailureRecovery(start.Add(3*time.Second), "http://pd-1:2379") + recoveryLogs := observedLogs.FilterMessage("[pd] member update from this url recovered").All() + require.Len(t, recoveryLogs, 1) + recoveryFields := recoveryLogs[0].ContextMap() + require.Equal(t, "http://pd-1:2379", recoveryFields["url"]) + require.Equal(t, uint64(2), recoveryFields["failed-attempts"]) + require.Equal(t, uint64(1), recoveryFields["suppressed-errors"]) +} + +func TestMemberFailureTrackerConcurrentAccess(_ *testing.T) { + tracker := memberFailureTracker{} + failure := memberUpdateFailure{ + fingerprint: memberFailureFingerprint{phase: memberFailurePhaseRPC, grpcCode: codes.Unavailable}, + transport: true, + } + now := time.Unix(100, 0) + + var wg sync.WaitGroup + for i := range 8 { + wg.Add(1) + go func(index int) { + defer wg.Done() + url := fmt.Sprintf("http://pd-%d:2379", index%3) + for range 100 { + tracker.record(now, url, failure) + tracker.summary(now.Add(time.Second)) + tracker.recover(now.Add(2*time.Second), url) + tracker.cleanup([]string{url}) + } + }(i) + } + wg.Wait() +} + +func TestMemberFailureTrackerHealthyRecoveryDoesNotAllocate(t *testing.T) { + tracker := memberFailureTracker{} + now := time.Unix(100, 0) + allocations := testing.AllocsPerRun(1000, func() { + _, _ = tracker.recover(now, "http://pd-1:2379") + }) + require.Zero(t, allocations) +} + +func BenchmarkMemberRefreshControllerInspect(b *testing.B) { + transportFailure := memberUpdateFailure{transport: true} + result := newFailedMemberUpdateResult(transportFailure, transportFailure, transportFailure) + urls := memberTestURLs(3) + states := observedMemberConnectionStates(connectivity.TransientFailure, connectivity.Connecting, connectivity.Idle) + controller := memberRefreshController{} + controller.enterDegraded(result, urls, states) + b.ReportAllocs() + for b.Loop() { + controller.inspect(urls, states) + } +} + +func BenchmarkMemberFailureTrackerSuppression(b *testing.B) { + tracker := memberFailureTracker{} + failure := memberUpdateFailure{ + fingerprint: memberFailureFingerprint{phase: memberFailurePhaseRPC, grpcCode: codes.Unavailable}, + transport: true, + } + now := time.Unix(100, 0) + tracker.record(now, "http://pd-1:2379", failure) + b.ReportAllocs() + for b.Loop() { + tracker.record(now, "http://pd-1:2379", failure) + } +} + +func newFailedMemberUpdateResult(failures ...memberUpdateFailure) memberUpdateResult { + result := memberUpdateResult{} + for i, failure := range failures { + result.recordFailure(fmt.Sprintf("url-%d", i), failure) + } + return result +} + +func memberTestURLs(count int) []string { + urls := make([]string, 0, count) + for i := range count { + urls = append(urls, fmt.Sprintf("url-%d", i)) + } + return urls +} + +func repeatedConnectivityState(state connectivity.State, count int) []connectivity.State { + states := make([]connectivity.State, count) + for i := range states { + states[i] = state + } + return states +} + +func observedMemberConnectionStates(states ...connectivity.State) []memberConnectionState { + observed := make([]memberConnectionState, 0, len(states)) + for _, state := range states { + observed = append(observed, memberConnectionState{observed: true, state: state}) + } + return observed +} diff --git a/client/servicediscovery/service_discovery.go b/client/servicediscovery/service_discovery.go index 5716c24125..ff75c2fd97 100644 --- a/client/servicediscovery/service_discovery.go +++ b/client/servicediscovery/service_discovery.go @@ -58,6 +58,10 @@ const ( // UpdateMemberTimeout is the timeout to update the member list. // Use a shorter timeout to recover faster from network isolation. UpdateMemberTimeout = time.Second + // memberConnectionStateCheckInterval is used only after a complete member + // update retry batch confirms that every current endpoint has a transport + // failure. The check is local and does not issue an RPC. + memberConnectionStateCheckInterval = 100 * time.Millisecond serviceModeUpdateInterval = 3 * time.Second ) @@ -453,6 +457,8 @@ type serviceDiscovery struct { option *opt.Option flight singleflight.Group + + memberFailures memberFailureTracker } // NewDefaultServiceDiscovery returns a new default service discovery-based client. @@ -556,19 +562,169 @@ func (c *serviceDiscovery) updateMemberLoop() { defer ticker.Stop() bo := retry.InitialBackoffer(UpdateMemberBackOffBaseTime, UpdateMemberMaxBackoffTime, UpdateMemberTimeout) - for { + controller := memberRefreshController{} + var ( + connectionStateTicker *time.Ticker + connectionStateTick <-chan time.Time + runRetryBatchNow bool + ) + stopConnectionStateTicker := func() { + if connectionStateTicker != nil { + connectionStateTicker.Stop() + connectionStateTicker = nil + connectionStateTick = nil + } + } + defer stopConnectionStateTicker() + startConnectionStateTicker := func() { + if connectionStateTicker == nil { + connectionStateTicker = time.NewTicker(memberConnectionStateCheckInterval) + connectionStateTick = connectionStateTicker.C + } + } + drainScheduledCheck := func() { select { - case <-ctx.Done(): - log.Info("[pd] exit member loop due to context canceled") - return - case <-ticker.C: case <-c.checkMembershipCh: + default: } - err := bo.Exec(ctx, c.updateMember) + } + logBatchFailure := func(err error) { if err != nil { log.Warn("[pd] failed to update member", zap.Strings("sorted-urls", c.GetServiceURLs()), errs.ZapError(err)) } } + runRetryBatch := func() (result memberUpdateResult, err error) { + err = bo.Exec(ctx, func() error { + result, err = c.updateMemberWithResult() + return err + }) + return result, err + } + for { + var ( + periodicCheck bool + inspectConnections bool + ) + if !runRetryBatchNow { + var scheduledCheck <-chan struct{} = c.checkMembershipCh + if controller.isDegraded() { + scheduledCheck = nil + } + select { + case <-ctx.Done(): + log.Info("[pd] exit member loop due to context canceled") + return + case <-ticker.C: + periodicCheck = true + case <-scheduledCheck: + case <-connectionStateTick: + inspectConnections = true + } + } + runRetryBatchNow = false + + if periodicCheck { + c.logMemberFailureSummary(time.Now()) + } + + if controller.isDegraded() { + switch { + case inspectConnections: + snapshot := c.snapshotMemberConnections() + decision := controller.inspect(snapshot.urls, snapshot.states) + if decision.action == memberRefreshWait { + connectIdleMemberConnections(snapshot) + continue + } + stopConnectionStateTicker() + drainScheduledCheck() + runRetryBatchNow = true + continue + case periodicCheck: + // The safety sweep covers the event that may have been coalesced + // while scheduled checks were disabled. + drainScheduledCheck() + result, err := c.updateMemberWithResult() + logBatchFailure(err) + if err == nil { + controller.leaveDegraded() + stopConnectionStateTicker() + continue + } + snapshot := c.snapshotMemberConnections() + if controller.enterDegraded(result, snapshot.urls, snapshot.states) { + decision := controller.inspect(snapshot.urls, snapshot.states) + if decision.action == memberRefreshWait { + connectIdleMemberConnections(snapshot) + } + continue + } + controller.leaveDegraded() + stopConnectionStateTicker() + runRetryBatchNow = true + continue + } + } + + result, err := runRetryBatch() + logBatchFailure(err) + if err == nil { + continue + } + snapshot := c.snapshotMemberConnections() + if !controller.enterDegraded(result, snapshot.urls, snapshot.states) { + continue + } + startConnectionStateTicker() + + // Inspect again after entering degraded mode so a connection that + // became ready as the failed batch completed is refreshed immediately. + snapshot = c.snapshotMemberConnections() + decision := controller.inspect(snapshot.urls, snapshot.states) + if decision.action == memberRefreshRetryBatch { + stopConnectionStateTicker() + drainScheduledCheck() + runRetryBatchNow = true + } else { + connectIdleMemberConnections(snapshot) + } + } +} + +type memberConnectionSnapshot struct { + urls []string + states []memberConnectionState + connections []*grpc.ClientConn +} + +func (c *serviceDiscovery) snapshotMemberConnections() memberConnectionSnapshot { + urls := c.GetServiceURLs() + snapshot := memberConnectionSnapshot{ + urls: urls, + states: make([]memberConnectionState, len(urls)), + connections: make([]*grpc.ClientConn, len(urls)), + } + for i, url := range urls { + value, ok := c.clientConns.Load(url) + if !ok { + continue + } + conn, ok := value.(*grpc.ClientConn) + if !ok || conn == nil { + continue + } + snapshot.states[i] = memberConnectionState{observed: true, state: conn.GetState()} + snapshot.connections[i] = conn + } + return snapshot +} + +func connectIdleMemberConnections(snapshot memberConnectionSnapshot) { + for i, state := range snapshot.states { + if state.observed && state.state == connectivity.Idle && snapshot.connections[i] != nil { + snapshot.connections[i].Connect() + } + } } func (c *serviceDiscovery) updateServiceModeLoop() { @@ -889,8 +1045,14 @@ func (c *serviceDiscovery) checkServiceModeChanged() error { } func (c *serviceDiscovery) updateMember() error { + _, err := c.updateMemberWithResult() + return err +} + +func (c *serviceDiscovery) updateMemberWithResult() (memberUpdateResult, error) { + result := memberUpdateResult{} for _, url := range c.GetServiceURLs() { - members, err := c.getMembers(c.ctx, url, UpdateMemberTimeout) + members, failure, err := c.getMembersWithFailure(c.ctx, url, UpdateMemberTimeout) // Check the cluster ID. updatedClusterID := members.GetHeader().GetClusterId() if err == nil && updatedClusterID != c.clusterID { @@ -898,27 +1060,34 @@ func (c *serviceDiscovery) updateMember() error { zap.Uint64("updated-cluster-id", updatedClusterID), zap.Uint64("expected-cluster-id", c.clusterID)) err = errs.ErrClientUpdateMember.FastGenByArgs(fmt.Sprintf("cluster id does not match: %d != %d", updatedClusterID, c.clusterID)) + failure = classifyMemberSemanticFailure(memberFailurePhaseClusterID) } if err == nil && (members.GetLeader() == nil || len(members.GetLeader().GetClientUrls()) == 0) { err = errs.ErrClientGetLeader.FastGenByArgs("leader url doesn't exist") + failure = classifyMemberSemanticFailure(memberFailurePhaseLeader) } // Failed to get members if err != nil { - log.Info("[pd] cannot update member from this url", - zap.String("url", url), - errs.ZapError(err)) + result.recordFailure(url, failure) + if c.memberFailures.record(time.Now(), url, failure) { + log.Info("[pd] cannot update member from this url", + zap.String("url", url), + errs.ZapError(err)) + } select { case <-c.ctx.Done(): - return errors.WithStack(err) + return result, errors.WithStack(err) default: continue } } + c.logMemberFailureRecovery(time.Now(), url) c.updateURLs(members.GetMembers()) + c.memberFailures.cleanup(c.GetServiceURLs()) - return c.updateServiceClient(members.GetMembers(), members.GetLeader()) + return result, c.updateServiceClient(members.GetMembers(), members.GetLeader()) } - return errs.ErrClientGetMember.FastGenByArgs() + return result, errs.ErrClientGetMember.FastGenByArgs() } func (c *serviceDiscovery) getClusterInfo(ctx context.Context, url string, timeout time.Duration) (*pdpb.GetClusterInfoResponse, error) { @@ -958,11 +1127,20 @@ func (c *serviceDiscovery) getClusterInfo(ctx context.Context, url string, timeo } func (c *serviceDiscovery) getMembers(ctx context.Context, url string, timeout time.Duration) (*pdpb.GetMembersResponse, error) { + members, _, err := c.getMembersWithFailure(ctx, url, timeout) + return members, err +} + +func (c *serviceDiscovery) getMembersWithFailure( + ctx context.Context, + url string, + timeout time.Duration, +) (*pdpb.GetMembersResponse, memberUpdateFailure, error) { ctx, cancel := context.WithTimeout(ctx, timeout) defer cancel() cc, err := c.GetOrCreateGRPCConn(url) if err != nil { - return nil, err + return nil, classifyMemberDialFailure(err), err } start := time.Now() defer func() { metrics.InternalCmdDurationGetMembers.Observe(time.Since(start).Seconds()) }() @@ -974,23 +1152,50 @@ func (c *serviceDiscovery) getMembers(ctx context.Context, url string, timeout t case res := <-r: err = res.Err if err != nil { + failure := classifyMemberRPCFailure(err) metrics.InternalCmdFailedDurationGetMembers.Observe(time.Since(start).Seconds()) attachErr := errors.Errorf("error:%s target:%s status:%s", err, cc.Target(), cc.GetState().String()) - return nil, errs.ErrClientGetMember.Wrap(attachErr).GenWithStackByCause() + return nil, failure, errs.ErrClientGetMember.Wrap(attachErr).GenWithStackByCause() } val := res.Val members := val.(*pdpb.GetMembersResponse) if members.GetHeader().GetError() != nil { metrics.InternalCmdFailedDurationGetMembers.Observe(time.Since(start).Seconds()) attachErr := errors.Errorf("error:%s target:%s status:%s", members.GetHeader().GetError().String(), cc.Target(), cc.GetState().String()) - return nil, errs.ErrClientGetMember.Wrap(attachErr).GenWithStackByCause() + return nil, classifyMemberResponseFailure(members.GetHeader().GetError().GetType()), errs.ErrClientGetMember.Wrap(attachErr).GenWithStackByCause() } - return members, nil + return members, memberUpdateFailure{}, nil case <-ctx.Done(): + failure := classifyMemberRPCFailure(ctx.Err()) attachErr := errors.Errorf("error:%s target:%s status:%s", ctx.Err(), cc.Target(), cc.GetState().String()) metrics.InternalCmdFailedDurationGetMembers.Observe(time.Since(start).Seconds()) - return nil, errs.ErrClientGetMember.Wrap(attachErr).GenWithStackByCause() + return nil, failure, errs.ErrClientGetMember.Wrap(attachErr).GenWithStackByCause() + } +} + +func (c *serviceDiscovery) logMemberFailureRecovery(now time.Time, url string) { + recovery, ok := c.memberFailures.recover(now, url) + if !ok { + return + } + log.Info("[pd] member update from this url recovered", + zap.String("url", recovery.url), + zap.Duration("failure-duration", recovery.failureDuration), + zap.Uint64("failed-attempts", recovery.failedAttempts), + zap.Uint64("suppressed-errors", recovery.suppressedErrors)) +} + +func (c *serviceDiscovery) logMemberFailureSummary(now time.Time) { + summary, ok := c.memberFailures.summary(now) + if !ok { + return } + log.Info("[pd] member update failures are being suppressed", + zap.Strings("failed-urls", summary.failedURLs), + zap.Duration("failure-duration", summary.failureDuration), + zap.Uint64("failed-attempts", summary.failedAttempts), + zap.Uint64("suppressed-errors", summary.suppressedErrors), + zap.Strings("error-classes", summary.errorClasses)) } func (c *serviceDiscovery) updateURLs(members []*pdpb.Member) { diff --git a/client/servicediscovery/service_discovery_test.go b/client/servicediscovery/service_discovery_test.go index 295c4592cd..a03a645dd3 100644 --- a/client/servicediscovery/service_discovery_test.go +++ b/client/servicediscovery/service_discovery_test.go @@ -21,6 +21,7 @@ import ( "log" "net" "net/url" + "sync" "sync/atomic" "testing" "time" @@ -29,11 +30,14 @@ import ( "github.com/stretchr/testify/suite" "go.uber.org/goleak" "google.golang.org/grpc" + "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials/insecure" pb "google.golang.org/grpc/examples/helloworld/helloworld" "google.golang.org/grpc/health" healthpb "google.golang.org/grpc/health/grpc_health_v1" "google.golang.org/grpc/metadata" + "google.golang.org/grpc/status" + "google.golang.org/grpc/test/bufconn" "github.com/pingcap/failpoint" "github.com/pingcap/kvproto/pkg/pdpb" @@ -452,3 +456,176 @@ func TestGRPCDialOption(t *testing.T) { re.Error(err) re.Greater(time.Since(start), 500*time.Millisecond) } + +type memberTestPDServer struct { + pdpb.UnimplementedPDServer + getMembers func() (*pdpb.GetMembersResponse, error) +} + +func (s *memberTestPDServer) GetMembers(context.Context, *pdpb.GetMembersRequest) (*pdpb.GetMembersResponse, error) { + return s.getMembers() +} + +func TestUpdateMemberWithResultPreservesErrorContract(t *testing.T) { + listener := bufconn.Listen(1024 * 1024) + server := grpc.NewServer() + testServer := &memberTestPDServer{} + pdpb.RegisterPDServer(server, testServer) + go func() { + require.NoError(t, server.Serve(listener)) + }() + t.Cleanup(server.Stop) + + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + conn, err := grpc.NewClient( + "passthrough:///bufnet", + grpc.WithContextDialer(func(context.Context, string) (net.Conn, error) { return listener.Dial() }), + grpc.WithTransportCredentials(insecure.NewCredentials()), + ) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, conn.Close()) }) + + const memberURL = "http://pd.test:2379" + testCases := []struct { + name string + response func() (*pdpb.GetMembersResponse, error) + fingerprint string + transport bool + }{ + { + name: "rpc unavailable", + response: func() (*pdpb.GetMembersResponse, error) { + return nil, status.Error(codes.Unavailable, "dial tcp 192.0.2.1:2379: connection refused") + }, + fingerprint: "rpc/Unavailable", + transport: true, + }, + { + name: "response header error", + response: func() (*pdpb.GetMembersResponse, error) { + return &pdpb.GetMembersResponse{ + Header: &pdpb.ResponseHeader{ClusterId: 1, Error: &pdpb.Error{Type: pdpb.ErrorType_UNKNOWN, Message: "not ready"}}, + }, nil + }, + fingerprint: "response/UNKNOWN", + }, + { + name: "cluster id mismatch", + response: func() (*pdpb.GetMembersResponse, error) { + return validMemberTestResponse(memberURL, 2), nil + }, + fingerprint: "cluster-id", + }, + { + name: "missing leader", + response: func() (*pdpb.GetMembersResponse, error) { + response := validMemberTestResponse(memberURL, 1) + response.Leader = nil + return response, nil + }, + fingerprint: "leader", + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + testServer.getMembers = testCase.response + client := newMemberTestServiceDiscovery(ctx, cancel, memberURL, conn) + + result, structuredErr := client.updateMemberWithResult() + compatibilityErr := client.updateMember() + + require.Error(t, structuredErr) + require.Equal(t, structuredErr.Error(), compatibilityErr.Error()) + require.Equal(t, []string{memberURL}, result.attemptedURLs) + require.Equal(t, testCase.transport, result.transportFailures == 1) + + summary, ok := client.memberFailures.summary(time.Now()) + require.True(t, ok) + require.Equal(t, []string{testCase.fingerprint}, summary.errorClasses) + }) + } +} + +func validMemberTestResponse(memberURL string, clusterID uint64) *pdpb.GetMembersResponse { + member := &pdpb.Member{MemberId: 1, ClientUrls: []string{memberURL}} + return &pdpb.GetMembersResponse{ + Header: &pdpb.ResponseHeader{ClusterId: clusterID}, + Members: []*pdpb.Member{member}, + Leader: member, + } +} + +func newMemberTestServiceDiscovery( + ctx context.Context, + cancel context.CancelFunc, + memberURL string, + conn *grpc.ClientConn, +) *serviceDiscovery { + client := &serviceDiscovery{ + ctx: ctx, + cancel: cancel, + callbacks: newServiceCallbacks(), + option: opt.NewOption(), + clusterID: 1, + } + client.urls.Store([]string{memberURL}) + client.clientConns.Store(memberURL, conn) + return client +} + +func TestUpdateMemberLoopSuppressesScheduledRefreshDuringTransportFailure(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + var getMembersCalls atomic.Int32 + conn, err := grpc.NewClient( + "passthrough:///unreachable.test:2379", + grpc.WithContextDialer(func(context.Context, string) (net.Conn, error) { + return nil, errors.New("transport unavailable") + }), + grpc.WithTransportCredentials(insecure.NewCredentials()), + grpc.WithUnaryInterceptor(func( + ctx context.Context, + method string, + req, reply any, + cc *grpc.ClientConn, + invoker grpc.UnaryInvoker, + opts ...grpc.CallOption, + ) error { + if method == "/pdpb.PD/GetMembers" { + getMembersCalls.Add(1) + } + return invoker(ctx, method, req, reply, cc, opts...) + }), + ) + require.NoError(t, err) + + var wg sync.WaitGroup + client := newMemberTestServiceDiscovery(ctx, cancel, "http://unreachable.test:2379", conn) + client.wg = &wg + client.checkMembershipCh = make(chan struct{}, 1) + wg.Add(1) + go client.updateMemberLoop() + t.Cleanup(func() { + cancel() + wg.Wait() + require.NoError(t, conn.Close()) + }) + + client.ScheduleCheckMemberChanged() + require.Eventually(t, func() bool { return getMembersCalls.Load() >= 12 }, 2*time.Second, 10*time.Millisecond) + time.Sleep(50 * time.Millisecond) + callsAfterInitialBatch := getMembersCalls.Load() + require.GreaterOrEqual(t, callsAfterInitialBatch, int32(12)) + + for range 100 { + client.ScheduleCheckMemberChanged() + } + require.Never(t, func() bool { + return getMembersCalls.Load() != callsAfterInitialBatch + }, 300*time.Millisecond, 10*time.Millisecond) + + // A synchronous check is an explicit request and must bypass background suppression. + require.Error(t, client.CheckMemberChanged()) + require.Equal(t, callsAfterInitialBatch+1, getMembersCalls.Load()) +} From b3fb8bbfd3132cabf1a36a8521efbf08ee755915 Mon Sep 17 00:00:00 2001 From: Ryan Leung Date: Mon, 27 Jul 2026 17:30:34 +0800 Subject: [PATCH 2/7] client: track only member transport failures Signed-off-by: Ryan Leung --- .../member_refresh_controller.go | 97 +++------- .../member_refresh_controller_test.go | 166 ++++++------------ client/servicediscovery/service_discovery.go | 40 +++-- .../service_discovery_test.go | 86 +++++++-- 4 files changed, 179 insertions(+), 210 deletions(-) diff --git a/client/servicediscovery/member_refresh_controller.go b/client/servicediscovery/member_refresh_controller.go index b569da7bcb..2308777714 100644 --- a/client/servicediscovery/member_refresh_controller.go +++ b/client/servicediscovery/member_refresh_controller.go @@ -25,47 +25,16 @@ import ( "google.golang.org/grpc/connectivity" "google.golang.org/grpc/status" - "github.com/pingcap/kvproto/pkg/pdpb" - clienterrs "github.com/tikv/pd/client/errs" ) -type memberFailurePhase string - -const ( - memberFailurePhaseDial memberFailurePhase = "dial" - memberFailurePhaseRPC memberFailurePhase = "rpc" - memberFailurePhaseResponse memberFailurePhase = "response" - memberFailurePhaseClusterID memberFailurePhase = "cluster-id" - memberFailurePhaseLeader memberFailurePhase = "leader" -) - -type memberFailureFingerprint struct { - phase memberFailurePhase - grpcCode codes.Code - pdErrorType pdpb.ErrorType -} - -func (f memberFailureFingerprint) String() string { - switch f.phase { - case memberFailurePhaseRPC: - return string(f.phase) + "/" + f.grpcCode.String() - case memberFailurePhaseResponse: - return string(f.phase) + "/" + f.pdErrorType.String() - default: - return string(f.phase) - } -} - type memberUpdateFailure struct { - fingerprint memberFailureFingerprint - transport bool + transport bool } func classifyMemberDialFailure(err error) memberUpdateFailure { return memberUpdateFailure{ - fingerprint: memberFailureFingerprint{phase: memberFailurePhaseDial}, - transport: errors.Is(err, clienterrs.ErrGRPCDial), + transport: errors.Is(err, clienterrs.ErrGRPCDial), } } @@ -75,20 +44,7 @@ func classifyMemberRPCFailure(err error) memberUpdateFailure { code = codes.DeadlineExceeded } return memberUpdateFailure{ - fingerprint: memberFailureFingerprint{phase: memberFailurePhaseRPC, grpcCode: code}, - transport: clienterrs.IsNetworkError(code), - } -} - -func classifyMemberResponseFailure(errorType pdpb.ErrorType) memberUpdateFailure { - return memberUpdateFailure{ - fingerprint: memberFailureFingerprint{phase: memberFailurePhaseResponse, pdErrorType: errorType}, - } -} - -func classifyMemberSemanticFailure(phase memberFailurePhase) memberUpdateFailure { - return memberUpdateFailure{ - fingerprint: memberFailureFingerprint{phase: phase}, + transport: clienterrs.IsNetworkError(code), } } @@ -194,70 +150,63 @@ func isInactiveMemberConnectionState(state connectivity.State) bool { state == connectivity.TransientFailure } -type memberFailureEpisode struct { +type memberTransportFailureEpisode struct { firstFailure time.Time - fingerprint memberFailureFingerprint failedAttempts uint64 suppressedErrors uint64 } -type memberFailureRecovery struct { +type memberTransportFailureRecovery struct { url string failureDuration time.Duration failedAttempts uint64 suppressedErrors uint64 } -type memberFailureSummary struct { +type memberTransportFailureSummary struct { failedURLs []string - errorClasses []string failureDuration time.Duration failedAttempts uint64 suppressedErrors uint64 } -type memberFailureTracker struct { +type memberTransportFailureTracker struct { mu sync.Mutex - episodes map[string]*memberFailureEpisode + episodes map[string]*memberTransportFailureEpisode } // record returns true when the caller should emit the detailed failure log. -func (t *memberFailureTracker) record(now time.Time, url string, failure memberUpdateFailure) bool { +func (t *memberTransportFailureTracker) record(now time.Time, url string) bool { t.mu.Lock() defer t.mu.Unlock() if t.episodes == nil { - t.episodes = make(map[string]*memberFailureEpisode) + t.episodes = make(map[string]*memberTransportFailureEpisode) } episode, ok := t.episodes[url] if !ok { - t.episodes[url] = &memberFailureEpisode{ + t.episodes[url] = &memberTransportFailureEpisode{ firstFailure: now, - fingerprint: failure.fingerprint, failedAttempts: 1, } return true } episode.failedAttempts++ - if episode.fingerprint != failure.fingerprint { - episode.fingerprint = failure.fingerprint - return true - } episode.suppressedErrors++ return false } -func (t *memberFailureTracker) recover(now time.Time, url string) (memberFailureRecovery, bool) { +func (t *memberTransportFailureTracker) recover(now time.Time, url string) (memberTransportFailureRecovery, bool) { t.mu.Lock() defer t.mu.Unlock() episode, ok := t.episodes[url] if !ok { - return memberFailureRecovery{}, false + return memberTransportFailureRecovery{}, false } delete(t.episodes, url) - return memberFailureRecovery{ + return memberTransportFailureRecovery{ url: url, failureDuration: now.Sub(episode.firstFailure), failedAttempts: episode.failedAttempts, @@ -265,7 +214,13 @@ func (t *memberFailureTracker) recover(now time.Time, url string) (memberFailure }, true } -func (t *memberFailureTracker) cleanup(urls []string) { +func (t *memberTransportFailureTracker) discard(url string) { + t.mu.Lock() + defer t.mu.Unlock() + delete(t.episodes, url) +} + +func (t *memberTransportFailureTracker) cleanup(urls []string) { t.mu.Lock() defer t.mu.Unlock() @@ -283,12 +238,12 @@ func (t *memberFailureTracker) cleanup(urls []string) { } } -func (t *memberFailureTracker) summary(now time.Time) (memberFailureSummary, bool) { +func (t *memberTransportFailureTracker) summary(now time.Time) (memberTransportFailureSummary, bool) { t.mu.Lock() defer t.mu.Unlock() if len(t.episodes) == 0 { - return memberFailureSummary{}, false + return memberTransportFailureSummary{}, false } urls := make([]string, 0, len(t.episodes)) for url := range t.episodes { @@ -296,9 +251,8 @@ func (t *memberFailureTracker) summary(now time.Time) (memberFailureSummary, boo } sort.Strings(urls) - summary := memberFailureSummary{ - failedURLs: urls, - errorClasses: make([]string, 0, len(urls)), + summary := memberTransportFailureSummary{ + failedURLs: urls, } earliest := now for _, url := range urls { @@ -306,7 +260,6 @@ func (t *memberFailureTracker) summary(now time.Time) (memberFailureSummary, boo if episode.firstFailure.Before(earliest) { earliest = episode.firstFailure } - summary.errorClasses = append(summary.errorClasses, episode.fingerprint.String()) summary.failedAttempts += episode.failedAttempts summary.suppressedErrors += episode.suppressedErrors } diff --git a/client/servicediscovery/member_refresh_controller_test.go b/client/servicediscovery/member_refresh_controller_test.go index e752750f9f..a95782bd62 100644 --- a/client/servicediscovery/member_refresh_controller_test.go +++ b/client/servicediscovery/member_refresh_controller_test.go @@ -29,7 +29,6 @@ import ( "google.golang.org/grpc/connectivity" "google.golang.org/grpc/status" - "github.com/pingcap/kvproto/pkg/pdpb" pingcaplog "github.com/pingcap/log" clienterrs "github.com/tikv/pd/client/errs" @@ -38,13 +37,8 @@ import ( func TestMemberRefreshControllerEntersDegradedModeStrictly(t *testing.T) { t.Parallel() - transportFailure := memberUpdateFailure{ - fingerprint: memberFailureFingerprint{phase: memberFailurePhaseRPC, grpcCode: codes.Unavailable}, - transport: true, - } - semanticFailure := memberUpdateFailure{ - fingerprint: memberFailureFingerprint{phase: memberFailurePhaseResponse, pdErrorType: pdpb.ErrorType_UNKNOWN}, - } + transportFailure := memberUpdateFailure{transport: true} + nonTransportFailure := memberUpdateFailure{} testCases := []struct { name string @@ -69,8 +63,8 @@ func TestMemberRefreshControllerEntersDegradedModeStrictly(t *testing.T) { states: observedMemberConnectionStates(connectivity.TransientFailure, connectivity.TransientFailure), }, { - name: "semantic failure", - result: newFailedMemberUpdateResult(transportFailure, semanticFailure), + name: "non-transport failure", + result: newFailedMemberUpdateResult(transportFailure, nonTransportFailure), states: observedMemberConnectionStates(connectivity.TransientFailure, connectivity.TransientFailure), }, { @@ -178,31 +172,22 @@ func TestMemberRefreshControllerInspectDoesNotAllocate(t *testing.T) { var memberRefreshDecisionSink memberRefreshDecision -func TestMemberFailureTrackerEpisodes(t *testing.T) { +func TestMemberTransportFailureTrackerEpisodes(t *testing.T) { t.Parallel() - tracker := memberFailureTracker{} + tracker := memberTransportFailureTracker{} start := time.Unix(100, 0) - refused := memberUpdateFailure{ - fingerprint: memberFailureFingerprint{phase: memberFailurePhaseRPC, grpcCode: codes.Unavailable}, - transport: true, - } - timeout := memberUpdateFailure{ - fingerprint: memberFailureFingerprint{phase: memberFailurePhaseRPC, grpcCode: codes.DeadlineExceeded}, - transport: true, - } - require.True(t, tracker.record(start, "http://pd-1:2379", refused)) - require.False(t, tracker.record(start.Add(time.Second), "http://pd-1:2379", refused)) - require.True(t, tracker.record(start.Add(2*time.Second), "http://pd-1:2379", timeout)) - require.True(t, tracker.record(start.Add(3*time.Second), "http://pd-2:2379", refused)) + require.True(t, tracker.record(start, "http://pd-1:2379")) + require.False(t, tracker.record(start.Add(time.Second), "http://pd-1:2379")) + require.False(t, tracker.record(start.Add(2*time.Second), "http://pd-1:2379")) + require.True(t, tracker.record(start.Add(3*time.Second), "http://pd-2:2379")) summary, ok := tracker.summary(start.Add(4 * time.Second)) require.True(t, ok) require.Equal(t, []string{"http://pd-1:2379", "http://pd-2:2379"}, summary.failedURLs) - require.Equal(t, []string{"rpc/DeadlineExceeded", "rpc/Unavailable"}, summary.errorClasses) require.Equal(t, uint64(4), summary.failedAttempts) - require.Equal(t, uint64(1), summary.suppressedErrors) + require.Equal(t, uint64(2), summary.suppressedErrors) require.Equal(t, 4*time.Second, summary.failureDuration) recovery, ok := tracker.recover(start.Add(5*time.Second), "http://pd-2:2379") @@ -221,115 +206,84 @@ func TestMemberFailureTrackerEpisodes(t *testing.T) { require.False(t, ok) } -func TestClassifyMemberFailure(t *testing.T) { +func TestClassifyMemberTransportFailure(t *testing.T) { t.Parallel() testCases := []struct { - name string - failure memberUpdateFailure - fingerprint string - transport bool + name string + failure memberUpdateFailure + transport bool }{ { - name: "connection refused rpc", - failure: classifyMemberRPCFailure(status.Error(codes.Unavailable, "dial tcp 192.0.2.1:2379: connect: connection refused")), - fingerprint: "rpc/Unavailable", - transport: true, - }, - { - name: "tls rpc", - failure: classifyMemberRPCFailure(status.Error(codes.Unavailable, "transport: authentication handshake failed: tls certificate expired")), - fingerprint: "rpc/Unavailable", - transport: true, + name: "connection refused rpc", + failure: classifyMemberRPCFailure(status.Error(codes.Unavailable, "dial tcp 192.0.2.1:2379: connect: connection refused")), + transport: true, }, { - name: "dns rpc", - failure: classifyMemberRPCFailure(status.Error(codes.Unavailable, "lookup pd.invalid: no such host")), - fingerprint: "rpc/Unavailable", - transport: true, + name: "tls rpc", + failure: classifyMemberRPCFailure(status.Error(codes.Unavailable, "transport: authentication handshake failed: tls certificate expired")), + transport: true, }, { - name: "deadline rpc", - failure: classifyMemberRPCFailure(status.Error(codes.DeadlineExceeded, "context deadline exceeded")), - fingerprint: "rpc/DeadlineExceeded", - transport: true, + name: "dns rpc", + failure: classifyMemberRPCFailure(status.Error(codes.Unavailable, "lookup pd.invalid: no such host")), + transport: true, }, { - name: "reset rpc", - failure: classifyMemberRPCFailure(status.Error(codes.Unavailable, "read: connection reset by peer")), - fingerprint: "rpc/Unavailable", - transport: true, + name: "deadline rpc", + failure: classifyMemberRPCFailure(status.Error(codes.DeadlineExceeded, "context deadline exceeded")), + transport: true, }, { - name: "non-network grpc status", - failure: classifyMemberRPCFailure(status.Error(codes.PermissionDenied, "permission denied")), - fingerprint: "rpc/PermissionDenied", - transport: false, + name: "reset rpc", + failure: classifyMemberRPCFailure(status.Error(codes.Unavailable, "read: connection reset by peer")), + transport: true, }, { - name: "blocking dial timeout", - failure: classifyMemberDialFailure(clienterrs.ErrGRPCDial.Wrap(context.DeadlineExceeded).GenWithStackByCause()), - fingerprint: "dial", - transport: true, + name: "non-network grpc status", + failure: classifyMemberRPCFailure(status.Error(codes.PermissionDenied, "permission denied")), + transport: false, }, { - name: "uncertain dial error", - failure: classifyMemberDialFailure(errors.New("invalid client configuration")), - fingerprint: "dial", - transport: false, + name: "blocking dial timeout", + failure: classifyMemberDialFailure(clienterrs.ErrGRPCDial.Wrap(context.DeadlineExceeded).GenWithStackByCause()), + transport: true, }, { - name: "pd response error", - failure: classifyMemberResponseFailure(pdpb.ErrorType_NOT_BOOTSTRAPPED), - fingerprint: "response/NOT_BOOTSTRAPPED", - transport: false, - }, - { - name: "cluster id mismatch", - failure: classifyMemberSemanticFailure(memberFailurePhaseClusterID), - fingerprint: "cluster-id", - transport: false, + name: "uncertain dial error", + failure: classifyMemberDialFailure(errors.New("invalid client configuration")), + transport: false, }, } for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { - require.Equal(t, testCase.fingerprint, testCase.failure.fingerprint.String()) require.Equal(t, testCase.transport, testCase.failure.transport) }) } - - // Dynamic targets must not create a new fingerprint for the same failure class. - first := classifyMemberRPCFailure(status.Error(codes.Unavailable, "dial tcp 192.0.2.1:2379: connection refused")) - second := classifyMemberRPCFailure(status.Error(codes.Unavailable, "dial tcp 198.51.100.8:1234: connection refused")) - require.Equal(t, first.fingerprint, second.fingerprint) } -func TestMemberFailureSummaryAndRecoveryLogs(t *testing.T) { +func TestMemberTransportFailureSummaryAndRecoveryLogs(t *testing.T) { core, observedLogs := observer.New(zap.InfoLevel) restoreLogger := pingcaplog.ReplaceGlobals(zap.New(core), nil) t.Cleanup(restoreLogger) client := &serviceDiscovery{} start := time.Unix(100, 0) - failure := memberUpdateFailure{ - fingerprint: memberFailureFingerprint{phase: memberFailurePhaseRPC, grpcCode: codes.Unavailable}, - transport: true, - } - require.True(t, client.memberFailures.record(start, "http://pd-1:2379", failure)) - require.False(t, client.memberFailures.record(start.Add(time.Second), "http://pd-1:2379", failure)) + require.True(t, client.memberTransportFailures.record(start, "http://pd-1:2379")) + require.False(t, client.memberTransportFailures.record(start.Add(time.Second), "http://pd-1:2379")) - client.logMemberFailureSummary(start.Add(2 * time.Second)) - summaryLogs := observedLogs.FilterMessage("[pd] member update failures are being suppressed").All() + client.logMemberTransportFailureSummary(start.Add(2 * time.Second)) + summaryLogs := observedLogs.FilterMessage("[pd] member transport failures are being suppressed").All() require.Len(t, summaryLogs, 1) summaryFields := summaryLogs[0].ContextMap() require.Contains(t, summaryFields, "failed-urls") require.Equal(t, uint64(2), summaryFields["failed-attempts"]) require.Equal(t, uint64(1), summaryFields["suppressed-errors"]) - require.Contains(t, summaryFields, "error-classes") + require.NotContains(t, summaryFields, "error-classes") - client.logMemberFailureRecovery(start.Add(3*time.Second), "http://pd-1:2379") - recoveryLogs := observedLogs.FilterMessage("[pd] member update from this url recovered").All() + client.logMemberTransportFailureRecovery(start.Add(3*time.Second), "http://pd-1:2379") + recoveryLogs := observedLogs.FilterMessage("[pd] member transport failure recovered").All() require.Len(t, recoveryLogs, 1) recoveryFields := recoveryLogs[0].ContextMap() require.Equal(t, "http://pd-1:2379", recoveryFields["url"]) @@ -337,12 +291,8 @@ func TestMemberFailureSummaryAndRecoveryLogs(t *testing.T) { require.Equal(t, uint64(1), recoveryFields["suppressed-errors"]) } -func TestMemberFailureTrackerConcurrentAccess(_ *testing.T) { - tracker := memberFailureTracker{} - failure := memberUpdateFailure{ - fingerprint: memberFailureFingerprint{phase: memberFailurePhaseRPC, grpcCode: codes.Unavailable}, - transport: true, - } +func TestMemberTransportFailureTrackerConcurrentAccess(_ *testing.T) { + tracker := memberTransportFailureTracker{} now := time.Unix(100, 0) var wg sync.WaitGroup @@ -352,7 +302,7 @@ func TestMemberFailureTrackerConcurrentAccess(_ *testing.T) { defer wg.Done() url := fmt.Sprintf("http://pd-%d:2379", index%3) for range 100 { - tracker.record(now, url, failure) + tracker.record(now, url) tracker.summary(now.Add(time.Second)) tracker.recover(now.Add(2*time.Second), url) tracker.cleanup([]string{url}) @@ -362,8 +312,8 @@ func TestMemberFailureTrackerConcurrentAccess(_ *testing.T) { wg.Wait() } -func TestMemberFailureTrackerHealthyRecoveryDoesNotAllocate(t *testing.T) { - tracker := memberFailureTracker{} +func TestMemberTransportFailureTrackerHealthyRecoveryDoesNotAllocate(t *testing.T) { + tracker := memberTransportFailureTracker{} now := time.Unix(100, 0) allocations := testing.AllocsPerRun(1000, func() { _, _ = tracker.recover(now, "http://pd-1:2379") @@ -384,17 +334,13 @@ func BenchmarkMemberRefreshControllerInspect(b *testing.B) { } } -func BenchmarkMemberFailureTrackerSuppression(b *testing.B) { - tracker := memberFailureTracker{} - failure := memberUpdateFailure{ - fingerprint: memberFailureFingerprint{phase: memberFailurePhaseRPC, grpcCode: codes.Unavailable}, - transport: true, - } +func BenchmarkMemberTransportFailureTrackerSuppression(b *testing.B) { + tracker := memberTransportFailureTracker{} now := time.Unix(100, 0) - tracker.record(now, "http://pd-1:2379", failure) + tracker.record(now, "http://pd-1:2379") b.ReportAllocs() for b.Loop() { - tracker.record(now, "http://pd-1:2379", failure) + tracker.record(now, "http://pd-1:2379") } } diff --git a/client/servicediscovery/service_discovery.go b/client/servicediscovery/service_discovery.go index ff75c2fd97..6b0211f5f7 100644 --- a/client/servicediscovery/service_discovery.go +++ b/client/servicediscovery/service_discovery.go @@ -458,7 +458,7 @@ type serviceDiscovery struct { flight singleflight.Group - memberFailures memberFailureTracker + memberTransportFailures memberTransportFailureTracker } // NewDefaultServiceDiscovery returns a new default service discovery-based client. @@ -624,7 +624,7 @@ func (c *serviceDiscovery) updateMemberLoop() { runRetryBatchNow = false if periodicCheck { - c.logMemberFailureSummary(time.Now()) + c.logMemberTransportFailureSummary(time.Now()) } if controller.isDegraded() { @@ -1060,16 +1060,23 @@ func (c *serviceDiscovery) updateMemberWithResult() (memberUpdateResult, error) zap.Uint64("updated-cluster-id", updatedClusterID), zap.Uint64("expected-cluster-id", c.clusterID)) err = errs.ErrClientUpdateMember.FastGenByArgs(fmt.Sprintf("cluster id does not match: %d != %d", updatedClusterID, c.clusterID)) - failure = classifyMemberSemanticFailure(memberFailurePhaseClusterID) } if err == nil && (members.GetLeader() == nil || len(members.GetLeader().GetClientUrls()) == 0) { err = errs.ErrClientGetLeader.FastGenByArgs("leader url doesn't exist") - failure = classifyMemberSemanticFailure(memberFailurePhaseLeader) } // Failed to get members if err != nil { result.recordFailure(url, failure) - if c.memberFailures.record(time.Now(), url, failure) { + failureTime := time.Now() + shouldLog := true + if failure.transport { + shouldLog = c.memberTransportFailures.record(failureTime, url) + } else { + // Only transport failures are suppressed. A different failure + // ends any transport-failure episode for this URL. + c.memberTransportFailures.discard(url) + } + if shouldLog { log.Info("[pd] cannot update member from this url", zap.String("url", url), errs.ZapError(err)) @@ -1081,9 +1088,11 @@ func (c *serviceDiscovery) updateMemberWithResult() (memberUpdateResult, error) continue } } - c.logMemberFailureRecovery(time.Now(), url) + c.logMemberTransportFailureRecovery(time.Now(), url) c.updateURLs(members.GetMembers()) - c.memberFailures.cleanup(c.GetServiceURLs()) + c.memberTransportFailures.cleanup(c.GetServiceURLs()) + // URLs after the successful one were not observed failing in this refresh. + c.memberTransportFailures.cleanup(result.attemptedURLs) return result, c.updateServiceClient(members.GetMembers(), members.GetLeader()) } @@ -1162,7 +1171,7 @@ func (c *serviceDiscovery) getMembersWithFailure( if members.GetHeader().GetError() != nil { metrics.InternalCmdFailedDurationGetMembers.Observe(time.Since(start).Seconds()) attachErr := errors.Errorf("error:%s target:%s status:%s", members.GetHeader().GetError().String(), cc.Target(), cc.GetState().String()) - return nil, classifyMemberResponseFailure(members.GetHeader().GetError().GetType()), errs.ErrClientGetMember.Wrap(attachErr).GenWithStackByCause() + return nil, memberUpdateFailure{}, errs.ErrClientGetMember.Wrap(attachErr).GenWithStackByCause() } return members, memberUpdateFailure{}, nil case <-ctx.Done(): @@ -1173,29 +1182,28 @@ func (c *serviceDiscovery) getMembersWithFailure( } } -func (c *serviceDiscovery) logMemberFailureRecovery(now time.Time, url string) { - recovery, ok := c.memberFailures.recover(now, url) +func (c *serviceDiscovery) logMemberTransportFailureRecovery(now time.Time, url string) { + recovery, ok := c.memberTransportFailures.recover(now, url) if !ok { return } - log.Info("[pd] member update from this url recovered", + log.Info("[pd] member transport failure recovered", zap.String("url", recovery.url), zap.Duration("failure-duration", recovery.failureDuration), zap.Uint64("failed-attempts", recovery.failedAttempts), zap.Uint64("suppressed-errors", recovery.suppressedErrors)) } -func (c *serviceDiscovery) logMemberFailureSummary(now time.Time) { - summary, ok := c.memberFailures.summary(now) +func (c *serviceDiscovery) logMemberTransportFailureSummary(now time.Time) { + summary, ok := c.memberTransportFailures.summary(now) if !ok { return } - log.Info("[pd] member update failures are being suppressed", + log.Info("[pd] member transport failures are being suppressed", zap.Strings("failed-urls", summary.failedURLs), zap.Duration("failure-duration", summary.failureDuration), zap.Uint64("failed-attempts", summary.failedAttempts), - zap.Uint64("suppressed-errors", summary.suppressedErrors), - zap.Strings("error-classes", summary.errorClasses)) + zap.Uint64("suppressed-errors", summary.suppressedErrors)) } func (c *serviceDiscovery) updateURLs(members []*pdpb.Member) { diff --git a/client/servicediscovery/service_discovery_test.go b/client/servicediscovery/service_discovery_test.go index a03a645dd3..c31f0ebf9d 100644 --- a/client/servicediscovery/service_discovery_test.go +++ b/client/servicediscovery/service_discovery_test.go @@ -488,18 +488,16 @@ func TestUpdateMemberWithResultPreservesErrorContract(t *testing.T) { const memberURL = "http://pd.test:2379" testCases := []struct { - name string - response func() (*pdpb.GetMembersResponse, error) - fingerprint string - transport bool + name string + response func() (*pdpb.GetMembersResponse, error) + transport bool }{ { name: "rpc unavailable", response: func() (*pdpb.GetMembersResponse, error) { return nil, status.Error(codes.Unavailable, "dial tcp 192.0.2.1:2379: connection refused") }, - fingerprint: "rpc/Unavailable", - transport: true, + transport: true, }, { name: "response header error", @@ -508,14 +506,12 @@ func TestUpdateMemberWithResultPreservesErrorContract(t *testing.T) { Header: &pdpb.ResponseHeader{ClusterId: 1, Error: &pdpb.Error{Type: pdpb.ErrorType_UNKNOWN, Message: "not ready"}}, }, nil }, - fingerprint: "response/UNKNOWN", }, { name: "cluster id mismatch", response: func() (*pdpb.GetMembersResponse, error) { return validMemberTestResponse(memberURL, 2), nil }, - fingerprint: "cluster-id", }, { name: "missing leader", @@ -524,7 +520,6 @@ func TestUpdateMemberWithResultPreservesErrorContract(t *testing.T) { response.Leader = nil return response, nil }, - fingerprint: "leader", }, } @@ -532,6 +527,7 @@ func TestUpdateMemberWithResultPreservesErrorContract(t *testing.T) { t.Run(testCase.name, func(t *testing.T) { testServer.getMembers = testCase.response client := newMemberTestServiceDiscovery(ctx, cancel, memberURL, conn) + require.True(t, client.memberTransportFailures.record(time.Now(), memberURL)) result, structuredErr := client.updateMemberWithResult() compatibilityErr := client.updateMember() @@ -541,13 +537,79 @@ func TestUpdateMemberWithResultPreservesErrorContract(t *testing.T) { require.Equal(t, []string{memberURL}, result.attemptedURLs) require.Equal(t, testCase.transport, result.transportFailures == 1) - summary, ok := client.memberFailures.summary(time.Now()) - require.True(t, ok) - require.Equal(t, []string{testCase.fingerprint}, summary.errorClasses) + _, ok := client.memberTransportFailures.summary(time.Now()) + if testCase.transport { + require.True(t, ok) + } else { + require.False(t, ok) + } }) } } +func TestUpdateMemberClearsUnobservedFailureEpisodesAfterRecovery(t *testing.T) { + listener := bufconn.Listen(1024 * 1024) + server := grpc.NewServer() + testServer := &memberTestPDServer{} + pdpb.RegisterPDServer(server, testServer) + go func() { + require.NoError(t, server.Serve(listener)) + }() + t.Cleanup(server.Stop) + + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + conn, err := grpc.NewClient( + "passthrough:///bufnet", + grpc.WithContextDialer(func(context.Context, string) (net.Conn, error) { return listener.Dial() }), + grpc.WithTransportCredentials(insecure.NewCredentials()), + ) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, conn.Close()) }) + + memberURLs := []string{ + "http://pd-1.test:2379", + "http://pd-2.test:2379", + "http://pd-3.test:2379", + } + members := make([]*pdpb.Member, 0, len(memberURLs)) + for i, memberURL := range memberURLs { + members = append(members, &pdpb.Member{MemberId: uint64(i + 1), ClientUrls: []string{memberURL}}) + } + var calls atomic.Int32 + testServer.getMembers = func() (*pdpb.GetMembersResponse, error) { + if calls.Add(1) == 1 { + return nil, status.Error(codes.Unavailable, "transport unavailable") + } + return &pdpb.GetMembersResponse{ + Header: &pdpb.ResponseHeader{ClusterId: 1}, + Members: members, + Leader: members[1], + }, nil + } + + client := newMemberTestServiceDiscovery(ctx, cancel, memberURLs[0], conn) + client.urls.Store(memberURLs) + client.leader.Store(newPDServiceClient(memberURLs[0], memberURLs[0], conn, true)) + client.apiCandidateNodes = [apiKindCount]*serviceBalancer{ + newServiceBalancer(emptyErrorFn), + newServiceBalancer(regionAPIErrorFn), + } + for _, memberURL := range memberURLs { + client.clientConns.Store(memberURL, conn) + require.True(t, client.memberTransportFailures.record(time.Now(), memberURL)) + } + + result, err := client.updateMemberWithResult() + require.NoError(t, err) + require.Equal(t, int32(2), calls.Load()) + require.Equal(t, []string{"http://pd-1.test:2379"}, result.attemptedURLs) + + summary, ok := client.memberTransportFailures.summary(time.Now()) + require.True(t, ok) + require.Equal(t, []string{"http://pd-1.test:2379"}, summary.failedURLs) +} + func validMemberTestResponse(memberURL string, clusterID uint64) *pdpb.GetMembersResponse { member := &pdpb.Member{MemberId: 1, ClientUrls: []string{memberURL}} return &pdpb.GetMembersResponse{ From fa0a551294cf535b6d7c270bb18463e956ff335c Mon Sep 17 00:00:00 2001 From: Ryan Leung Date: Mon, 27 Jul 2026 18:01:22 +0800 Subject: [PATCH 3/7] client: simplify member refresh state handling Signed-off-by: Ryan Leung --- .../member_refresh_controller.go | 109 ++++------- .../member_refresh_controller_test.go | 165 +++++++++-------- client/servicediscovery/service_discovery.go | 169 ++++++++---------- .../service_discovery_test.go | 43 ++++- 4 files changed, 224 insertions(+), 262 deletions(-) diff --git a/client/servicediscovery/member_refresh_controller.go b/client/servicediscovery/member_refresh_controller.go index 2308777714..5741a2b87d 100644 --- a/client/servicediscovery/member_refresh_controller.go +++ b/client/servicediscovery/member_refresh_controller.go @@ -17,10 +17,12 @@ package servicediscovery import ( "context" "errors" + "slices" "sort" "sync" "time" + "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/connectivity" "google.golang.org/grpc/status" @@ -28,122 +30,86 @@ import ( clienterrs "github.com/tikv/pd/client/errs" ) -type memberUpdateFailure struct { - transport bool +func isMemberDialTransportFailure(err error) bool { + return errors.Is(err, clienterrs.ErrGRPCDial) } -func classifyMemberDialFailure(err error) memberUpdateFailure { - return memberUpdateFailure{ - transport: errors.Is(err, clienterrs.ErrGRPCDial), - } -} - -func classifyMemberRPCFailure(err error) memberUpdateFailure { +func isMemberRPCTransportFailure(err error) bool { code := status.Code(err) if errors.Is(err, context.DeadlineExceeded) { code = codes.DeadlineExceeded } - return memberUpdateFailure{ - transport: clienterrs.IsNetworkError(code), - } + return clienterrs.IsNetworkError(code) } type memberUpdateResult struct { - attemptedURLs []string + failedURLs []string transportFailures int } -func (r *memberUpdateResult) recordFailure(url string, failure memberUpdateFailure) { - r.attemptedURLs = append(r.attemptedURLs, url) - if failure.transport { +func (r *memberUpdateResult) recordFailure(url string, transportFailure bool) { + r.failedURLs = append(r.failedURLs, url) + if transportFailure { r.transportFailures++ } } func (r *memberUpdateResult) allFailedByTransport(urls []string) bool { - if len(urls) == 0 || len(r.attemptedURLs) != len(urls) || r.transportFailures != len(urls) { + if len(urls) == 0 || len(r.failedURLs) != len(urls) || r.transportFailures != len(urls) { return false } - return equalMemberURLs(r.attemptedURLs, urls) + return slices.Equal(r.failedURLs, urls) } -type memberConnectionState struct { +type memberConnection struct { observed bool state connectivity.State -} - -type memberRefreshAction uint8 - -const ( - memberRefreshWait memberRefreshAction = iota - memberRefreshRetryBatch -) - -type memberRefreshDecision struct { - action memberRefreshAction + conn *grpc.ClientConn } type memberRefreshController struct { - degraded bool degradedURLs []string } func (c *memberRefreshController) isDegraded() bool { - return c.degraded + return len(c.degradedURLs) > 0 } func (c *memberRefreshController) enterDegraded( result memberUpdateResult, urls []string, - states []memberConnectionState, + connections []memberConnection, ) bool { - if len(urls) != len(states) || !result.allFailedByTransport(urls) { + if len(urls) != len(connections) || !result.allFailedByTransport(urls) { return false } - for _, state := range states { - if !state.observed || !isInactiveMemberConnectionState(state.state) { + for _, connection := range connections { + if !connection.observed || !isInactiveMemberConnectionState(connection.state) { return false } } - c.degraded = true c.degradedURLs = append(c.degradedURLs[:0], urls...) return true } -func (c *memberRefreshController) inspect(urls []string, states []memberConnectionState) memberRefreshDecision { - if !c.degraded { - return memberRefreshDecision{action: memberRefreshRetryBatch} - } - if len(urls) != len(states) || !equalMemberURLs(c.degradedURLs, urls) { +func (c *memberRefreshController) shouldWait(urls []string, connections []memberConnection) bool { + if !c.isDegraded() || len(urls) != len(connections) || !slices.Equal(c.degradedURLs, urls) { c.leaveDegraded() - return memberRefreshDecision{action: memberRefreshRetryBatch} + return false } - for _, state := range states { - if !state.observed || !isInactiveMemberConnectionState(state.state) { + for _, connection := range connections { + if !connection.observed || !isInactiveMemberConnectionState(connection.state) { c.leaveDegraded() - return memberRefreshDecision{action: memberRefreshRetryBatch} + return false } } - return memberRefreshDecision{action: memberRefreshWait} + return true } func (c *memberRefreshController) leaveDegraded() { - c.degraded = false c.degradedURLs = nil } -func equalMemberURLs(left, right []string) bool { - if len(left) != len(right) { - return false - } - for i := range left { - if left[i] != right[i] { - return false - } - } - return true -} - func isInactiveMemberConnectionState(state connectivity.State) bool { return state == connectivity.Idle || state == connectivity.Connecting || @@ -151,13 +117,11 @@ func isInactiveMemberConnectionState(state connectivity.State) bool { } type memberTransportFailureEpisode struct { - firstFailure time.Time - failedAttempts uint64 - suppressedErrors uint64 + firstFailure time.Time + failedAttempts uint64 } type memberTransportFailureRecovery struct { - url string failureDuration time.Duration failedAttempts uint64 suppressedErrors uint64 @@ -193,7 +157,6 @@ func (t *memberTransportFailureTracker) record(now time.Time, url string) bool { } episode.failedAttempts++ - episode.suppressedErrors++ return false } @@ -207,10 +170,9 @@ func (t *memberTransportFailureTracker) recover(now time.Time, url string) (memb } delete(t.episodes, url) return memberTransportFailureRecovery{ - url: url, failureDuration: now.Sub(episode.firstFailure), failedAttempts: episode.failedAttempts, - suppressedErrors: episode.suppressedErrors, + suppressedErrors: episode.failedAttempts - 1, }, true } @@ -220,19 +182,12 @@ func (t *memberTransportFailureTracker) discard(url string) { delete(t.episodes, url) } -func (t *memberTransportFailureTracker) cleanup(urls []string) { +func (t *memberTransportFailureTracker) retain(currentURLs, failedURLs []string) { t.mu.Lock() defer t.mu.Unlock() - if len(t.episodes) == 0 { - return - } - current := make(map[string]struct{}, len(urls)) - for _, url := range urls { - current[url] = struct{}{} - } for url := range t.episodes { - if _, ok := current[url]; !ok { + if !slices.Contains(currentURLs, url) || !slices.Contains(failedURLs, url) { delete(t.episodes, url) } } @@ -261,7 +216,7 @@ func (t *memberTransportFailureTracker) summary(now time.Time) (memberTransportF earliest = episode.firstFailure } summary.failedAttempts += episode.failedAttempts - summary.suppressedErrors += episode.suppressedErrors + summary.suppressedErrors += episode.failedAttempts - 1 } summary.failureDuration = now.Sub(earliest) return summary, true diff --git a/client/servicediscovery/member_refresh_controller_test.go b/client/servicediscovery/member_refresh_controller_test.go index a95782bd62..a19f4717f8 100644 --- a/client/servicediscovery/member_refresh_controller_test.go +++ b/client/servicediscovery/member_refresh_controller_test.go @@ -37,50 +37,50 @@ import ( func TestMemberRefreshControllerEntersDegradedModeStrictly(t *testing.T) { t.Parallel() - transportFailure := memberUpdateFailure{transport: true} - nonTransportFailure := memberUpdateFailure{} + transportFailure := true + nonTransportFailure := false testCases := []struct { - name string - result memberUpdateResult - states []memberConnectionState - enter bool + name string + result memberUpdateResult + connections []memberConnection + enter bool }{ { - name: "all current urls have transport failures and inactive connections", - result: newFailedMemberUpdateResult(transportFailure, transportFailure, transportFailure), - states: observedMemberConnectionStates(connectivity.Idle, connectivity.Connecting, connectivity.TransientFailure), - enter: true, + name: "all current urls have transport failures and inactive connections", + result: newFailedMemberUpdateResult(transportFailure, transportFailure, transportFailure), + connections: observedMemberConnections(connectivity.Idle, connectivity.Connecting, connectivity.TransientFailure), + enter: true, }, { - name: "empty url set", - result: memberUpdateResult{}, - states: nil, + name: "empty url set", + result: memberUpdateResult{}, + connections: nil, }, { - name: "not every url was attempted", - result: newFailedMemberUpdateResult(transportFailure), - states: observedMemberConnectionStates(connectivity.TransientFailure, connectivity.TransientFailure), + name: "not every url was attempted", + result: newFailedMemberUpdateResult(transportFailure), + connections: observedMemberConnections(connectivity.TransientFailure, connectivity.TransientFailure), }, { - name: "non-transport failure", - result: newFailedMemberUpdateResult(transportFailure, nonTransportFailure), - states: observedMemberConnectionStates(connectivity.TransientFailure, connectivity.TransientFailure), + name: "non-transport failure", + result: newFailedMemberUpdateResult(transportFailure, nonTransportFailure), + connections: observedMemberConnections(connectivity.TransientFailure, connectivity.TransientFailure), }, { - name: "missing connection", - result: newFailedMemberUpdateResult(transportFailure), - states: []memberConnectionState{{}}, + name: "missing connection", + result: newFailedMemberUpdateResult(transportFailure), + connections: []memberConnection{{}}, }, { - name: "ready connection", - result: newFailedMemberUpdateResult(transportFailure), - states: observedMemberConnectionStates(connectivity.Ready), + name: "ready connection", + result: newFailedMemberUpdateResult(transportFailure), + connections: observedMemberConnections(connectivity.Ready), }, { - name: "shutdown connection", - result: newFailedMemberUpdateResult(transportFailure), - states: observedMemberConnectionStates(connectivity.Shutdown), + name: "shutdown connection", + result: newFailedMemberUpdateResult(transportFailure), + connections: observedMemberConnections(connectivity.Shutdown), }, } @@ -89,88 +89,84 @@ func TestMemberRefreshControllerEntersDegradedModeStrictly(t *testing.T) { controller := memberRefreshController{} require.Equal(t, testCase.enter, controller.enterDegraded( testCase.result, - memberTestURLs(len(testCase.states)), - testCase.states, + memberTestURLs(len(testCase.connections)), + testCase.connections, )) require.Equal(t, testCase.enter, controller.isDegraded()) }) } } -func TestMemberRefreshControllerInspectsConnectionStates(t *testing.T) { +func TestMemberRefreshControllerShouldWait(t *testing.T) { t.Parallel() - transportFailure := memberUpdateFailure{transport: true} + transportFailure := true testCases := []struct { - name string - states []memberConnectionState - action memberRefreshAction + name string + connections []memberConnection + wait bool }{ { - name: "idle connections wait without a member refresh", - states: observedMemberConnectionStates(connectivity.Idle, connectivity.Connecting, connectivity.Idle), - action: memberRefreshWait, + name: "idle connections wait without a member refresh", + connections: observedMemberConnections(connectivity.Idle, connectivity.Connecting, connectivity.Idle), + wait: true, }, { - name: "ready connection refreshes immediately", - states: observedMemberConnectionStates(connectivity.TransientFailure, connectivity.Ready), - action: memberRefreshRetryBatch, + name: "ready connection refreshes immediately", + connections: observedMemberConnections(connectivity.TransientFailure, connectivity.Ready), }, { - name: "missing connection restores normal behavior", - states: []memberConnectionState{{observed: true, state: connectivity.TransientFailure}, {}}, - action: memberRefreshRetryBatch, + name: "missing connection restores normal behavior", + connections: []memberConnection{{observed: true, state: connectivity.TransientFailure}, {}}, }, { - name: "shutdown connection restores normal behavior", - states: observedMemberConnectionStates(connectivity.TransientFailure, connectivity.Shutdown), - action: memberRefreshRetryBatch, + name: "shutdown connection restores normal behavior", + connections: observedMemberConnections(connectivity.TransientFailure, connectivity.Shutdown), }, { - name: "url replacement restores normal behavior", - states: observedMemberConnectionStates(connectivity.TransientFailure, connectivity.TransientFailure), - action: memberRefreshRetryBatch, + name: "url replacement restores normal behavior", + connections: observedMemberConnections(connectivity.TransientFailure, connectivity.TransientFailure), }, } for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { controller := memberRefreshController{} - initialURLs := memberTestURLs(len(testCase.states)) - failures := make([]memberUpdateFailure, len(testCase.states)) + initialURLs := memberTestURLs(len(testCase.connections)) + failures := make([]bool, len(testCase.connections)) for i := range failures { failures[i] = transportFailure } require.True(t, controller.enterDegraded( newFailedMemberUpdateResult(failures...), initialURLs, - observedMemberConnectionStates(repeatedConnectivityState(connectivity.TransientFailure, len(testCase.states))...), + observedMemberConnections(repeatedConnectivityState(connectivity.TransientFailure, len(testCase.connections))...), )) currentURLs := initialURLs if testCase.name == "url replacement restores normal behavior" { currentURLs = []string{"url-0", "replacement-url"} } - decision := controller.inspect(currentURLs, testCase.states) - require.Equal(t, testCase.action, decision.action) - require.Equal(t, testCase.action == memberRefreshWait, controller.isDegraded()) + wait := controller.shouldWait(currentURLs, testCase.connections) + require.Equal(t, testCase.wait, wait) + require.Equal(t, testCase.wait, controller.isDegraded()) }) } } -func TestMemberRefreshControllerInspectDoesNotAllocate(t *testing.T) { - transportFailure := memberUpdateFailure{transport: true} +func TestMemberRefreshControllerShouldWaitDoesNotAllocate(t *testing.T) { + transportFailure := true result := newFailedMemberUpdateResult(transportFailure, transportFailure, transportFailure) urls := memberTestURLs(3) - states := observedMemberConnectionStates(connectivity.Idle, connectivity.Connecting, connectivity.TransientFailure) + connections := observedMemberConnections(connectivity.Idle, connectivity.Connecting, connectivity.TransientFailure) controller := memberRefreshController{} - require.True(t, controller.enterDegraded(result, urls, states)) + require.True(t, controller.enterDegraded(result, urls, connections)) allocations := testing.AllocsPerRun(1000, func() { - memberRefreshDecisionSink = controller.inspect(urls, states) + memberRefreshWaitSink = controller.shouldWait(urls, connections) }) require.Zero(t, allocations) } -var memberRefreshDecisionSink memberRefreshDecision +var memberRefreshWaitSink bool func TestMemberTransportFailureTrackerEpisodes(t *testing.T) { t.Parallel() @@ -192,7 +188,6 @@ func TestMemberTransportFailureTrackerEpisodes(t *testing.T) { recovery, ok := tracker.recover(start.Add(5*time.Second), "http://pd-2:2379") require.True(t, ok) - require.Equal(t, "http://pd-2:2379", recovery.url) require.Equal(t, uint64(1), recovery.failedAttempts) require.Zero(t, recovery.suppressedErrors) @@ -201,64 +196,64 @@ func TestMemberTransportFailureTrackerEpisodes(t *testing.T) { require.True(t, ok) require.Equal(t, []string{"http://pd-1:2379"}, summary.failedURLs) - tracker.cleanup([]string{"http://pd-3:2379"}) + tracker.retain([]string{"http://pd-3:2379"}, []string{"http://pd-1:2379"}) _, ok = tracker.summary(start.Add(7 * time.Second)) require.False(t, ok) } -func TestClassifyMemberTransportFailure(t *testing.T) { +func TestIsMemberTransportFailure(t *testing.T) { t.Parallel() testCases := []struct { name string - failure memberUpdateFailure + got bool transport bool }{ { name: "connection refused rpc", - failure: classifyMemberRPCFailure(status.Error(codes.Unavailable, "dial tcp 192.0.2.1:2379: connect: connection refused")), + got: isMemberRPCTransportFailure(status.Error(codes.Unavailable, "dial tcp 192.0.2.1:2379: connect: connection refused")), transport: true, }, { name: "tls rpc", - failure: classifyMemberRPCFailure(status.Error(codes.Unavailable, "transport: authentication handshake failed: tls certificate expired")), + got: isMemberRPCTransportFailure(status.Error(codes.Unavailable, "transport: authentication handshake failed: tls certificate expired")), transport: true, }, { name: "dns rpc", - failure: classifyMemberRPCFailure(status.Error(codes.Unavailable, "lookup pd.invalid: no such host")), + got: isMemberRPCTransportFailure(status.Error(codes.Unavailable, "lookup pd.invalid: no such host")), transport: true, }, { name: "deadline rpc", - failure: classifyMemberRPCFailure(status.Error(codes.DeadlineExceeded, "context deadline exceeded")), + got: isMemberRPCTransportFailure(status.Error(codes.DeadlineExceeded, "context deadline exceeded")), transport: true, }, { name: "reset rpc", - failure: classifyMemberRPCFailure(status.Error(codes.Unavailable, "read: connection reset by peer")), + got: isMemberRPCTransportFailure(status.Error(codes.Unavailable, "read: connection reset by peer")), transport: true, }, { name: "non-network grpc status", - failure: classifyMemberRPCFailure(status.Error(codes.PermissionDenied, "permission denied")), + got: isMemberRPCTransportFailure(status.Error(codes.PermissionDenied, "permission denied")), transport: false, }, { name: "blocking dial timeout", - failure: classifyMemberDialFailure(clienterrs.ErrGRPCDial.Wrap(context.DeadlineExceeded).GenWithStackByCause()), + got: isMemberDialTransportFailure(clienterrs.ErrGRPCDial.Wrap(context.DeadlineExceeded).GenWithStackByCause()), transport: true, }, { name: "uncertain dial error", - failure: classifyMemberDialFailure(errors.New("invalid client configuration")), + got: isMemberDialTransportFailure(errors.New("invalid client configuration")), transport: false, }, } for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { - require.Equal(t, testCase.transport, testCase.failure.transport) + require.Equal(t, testCase.transport, testCase.got) }) } } @@ -305,7 +300,7 @@ func TestMemberTransportFailureTrackerConcurrentAccess(_ *testing.T) { tracker.record(now, url) tracker.summary(now.Add(time.Second)) tracker.recover(now.Add(2*time.Second), url) - tracker.cleanup([]string{url}) + tracker.retain([]string{url}, []string{url}) } }(i) } @@ -321,16 +316,16 @@ func TestMemberTransportFailureTrackerHealthyRecoveryDoesNotAllocate(t *testing. require.Zero(t, allocations) } -func BenchmarkMemberRefreshControllerInspect(b *testing.B) { - transportFailure := memberUpdateFailure{transport: true} +func BenchmarkMemberRefreshControllerShouldWait(b *testing.B) { + transportFailure := true result := newFailedMemberUpdateResult(transportFailure, transportFailure, transportFailure) urls := memberTestURLs(3) - states := observedMemberConnectionStates(connectivity.TransientFailure, connectivity.Connecting, connectivity.Idle) + connections := observedMemberConnections(connectivity.TransientFailure, connectivity.Connecting, connectivity.Idle) controller := memberRefreshController{} - controller.enterDegraded(result, urls, states) + controller.enterDegraded(result, urls, connections) b.ReportAllocs() for b.Loop() { - controller.inspect(urls, states) + controller.shouldWait(urls, connections) } } @@ -344,7 +339,7 @@ func BenchmarkMemberTransportFailureTrackerSuppression(b *testing.B) { } } -func newFailedMemberUpdateResult(failures ...memberUpdateFailure) memberUpdateResult { +func newFailedMemberUpdateResult(failures ...bool) memberUpdateResult { result := memberUpdateResult{} for i, failure := range failures { result.recordFailure(fmt.Sprintf("url-%d", i), failure) @@ -368,10 +363,10 @@ func repeatedConnectivityState(state connectivity.State, count int) []connectivi return states } -func observedMemberConnectionStates(states ...connectivity.State) []memberConnectionState { - observed := make([]memberConnectionState, 0, len(states)) +func observedMemberConnections(states ...connectivity.State) []memberConnection { + observed := make([]memberConnection, 0, len(states)) for _, state := range states { - observed = append(observed, memberConnectionState{observed: true, state: state}) + observed = append(observed, memberConnection{observed: true, state: state}) } return observed } diff --git a/client/servicediscovery/service_discovery.go b/client/servicediscovery/service_discovery.go index 6b0211f5f7..101b228cf9 100644 --- a/client/servicediscovery/service_discovery.go +++ b/client/servicediscovery/service_discovery.go @@ -563,23 +563,17 @@ func (c *serviceDiscovery) updateMemberLoop() { bo := retry.InitialBackoffer(UpdateMemberBackOffBaseTime, UpdateMemberMaxBackoffTime, UpdateMemberTimeout) controller := memberRefreshController{} - var ( - connectionStateTicker *time.Ticker - connectionStateTick <-chan time.Time - runRetryBatchNow bool - ) + var connectionStateTicker *time.Ticker stopConnectionStateTicker := func() { if connectionStateTicker != nil { connectionStateTicker.Stop() connectionStateTicker = nil - connectionStateTick = nil } } defer stopConnectionStateTicker() startConnectionStateTicker := func() { if connectionStateTicker == nil { connectionStateTicker = time.NewTicker(memberConnectionStateCheckInterval) - connectionStateTick = connectionStateTicker.C } } drainScheduledCheck := func() { @@ -600,47 +594,15 @@ func (c *serviceDiscovery) updateMemberLoop() { }) return result, err } + var snapshot memberConnectionSnapshot for { - var ( - periodicCheck bool - inspectConnections bool - ) - if !runRetryBatchNow { - var scheduledCheck <-chan struct{} = c.checkMembershipCh - if controller.isDegraded() { - scheduledCheck = nil - } + if controller.isDegraded() { select { case <-ctx.Done(): log.Info("[pd] exit member loop due to context canceled") return case <-ticker.C: - periodicCheck = true - case <-scheduledCheck: - case <-connectionStateTick: - inspectConnections = true - } - } - runRetryBatchNow = false - - if periodicCheck { - c.logMemberTransportFailureSummary(time.Now()) - } - - if controller.isDegraded() { - switch { - case inspectConnections: - snapshot := c.snapshotMemberConnections() - decision := controller.inspect(snapshot.urls, snapshot.states) - if decision.action == memberRefreshWait { - connectIdleMemberConnections(snapshot) - continue - } - stopConnectionStateTicker() - drainScheduledCheck() - runRetryBatchNow = true - continue - case periodicCheck: + c.logMemberTransportFailureSummary(time.Now()) // The safety sweep covers the event that may have been coalesced // while scheduled checks were disabled. drainScheduledCheck() @@ -651,58 +613,74 @@ func (c *serviceDiscovery) updateMemberLoop() { stopConnectionStateTicker() continue } - snapshot := c.snapshotMemberConnections() - if controller.enterDegraded(result, snapshot.urls, snapshot.states) { - decision := controller.inspect(snapshot.urls, snapshot.states) - if decision.action == memberRefreshWait { - connectIdleMemberConnections(snapshot) - } + snapshot = c.snapshotMemberConnections(snapshot) + if controller.enterDegraded(result, snapshot.urls, snapshot.connections) { + connectIdleMemberConnections(snapshot) continue } controller.leaveDegraded() stopConnectionStateTicker() - runRetryBatchNow = true - continue + case <-connectionStateTicker.C: + snapshot = c.snapshotMemberConnections(snapshot) + if controller.shouldWait(snapshot.urls, snapshot.connections) { + connectIdleMemberConnections(snapshot) + continue + } + stopConnectionStateTicker() + drainScheduledCheck() + } + } else { + select { + case <-ctx.Done(): + log.Info("[pd] exit member loop due to context canceled") + return + case <-ticker.C: + c.logMemberTransportFailureSummary(time.Now()) + case <-c.checkMembershipCh: } } - result, err := runRetryBatch() - logBatchFailure(err) - if err == nil { - continue - } - snapshot := c.snapshotMemberConnections() - if !controller.enterDegraded(result, snapshot.urls, snapshot.states) { - continue - } - startConnectionStateTicker() - - // Inspect again after entering degraded mode so a connection that - // became ready as the failed batch completed is refreshed immediately. - snapshot = c.snapshotMemberConnections() - decision := controller.inspect(snapshot.urls, snapshot.states) - if decision.action == memberRefreshRetryBatch { + // Run immediately again if a connection changes while degraded mode is + // being entered. Otherwise return to the outer normal/degraded wait. + for { + result, err := runRetryBatch() + logBatchFailure(err) + if err == nil { + break + } + snapshot = c.snapshotMemberConnections(snapshot) + if !controller.enterDegraded(result, snapshot.urls, snapshot.connections) { + break + } + startConnectionStateTicker() + + // Inspect again after entering degraded mode so a connection that + // became ready as the failed batch completed is refreshed immediately. + snapshot = c.snapshotMemberConnections(snapshot) + if controller.shouldWait(snapshot.urls, snapshot.connections) { + connectIdleMemberConnections(snapshot) + break + } stopConnectionStateTicker() drainScheduledCheck() - runRetryBatchNow = true - } else { - connectIdleMemberConnections(snapshot) } } } type memberConnectionSnapshot struct { urls []string - states []memberConnectionState - connections []*grpc.ClientConn + connections []memberConnection } -func (c *serviceDiscovery) snapshotMemberConnections() memberConnectionSnapshot { +func (c *serviceDiscovery) snapshotMemberConnections(snapshot memberConnectionSnapshot) memberConnectionSnapshot { urls := c.GetServiceURLs() - snapshot := memberConnectionSnapshot{ - urls: urls, - states: make([]memberConnectionState, len(urls)), - connections: make([]*grpc.ClientConn, len(urls)), + snapshot.urls = urls + if cap(snapshot.connections) < len(urls) { + snapshot.connections = make([]memberConnection, len(urls)) + } else { + snapshot.connections = snapshot.connections[:cap(snapshot.connections)] + clear(snapshot.connections) + snapshot.connections = snapshot.connections[:len(urls)] } for i, url := range urls { value, ok := c.clientConns.Load(url) @@ -713,16 +691,15 @@ func (c *serviceDiscovery) snapshotMemberConnections() memberConnectionSnapshot if !ok || conn == nil { continue } - snapshot.states[i] = memberConnectionState{observed: true, state: conn.GetState()} - snapshot.connections[i] = conn + snapshot.connections[i] = memberConnection{observed: true, state: conn.GetState(), conn: conn} } return snapshot } func connectIdleMemberConnections(snapshot memberConnectionSnapshot) { - for i, state := range snapshot.states { - if state.observed && state.state == connectivity.Idle && snapshot.connections[i] != nil { - snapshot.connections[i].Connect() + for _, connection := range snapshot.connections { + if connection.observed && connection.state == connectivity.Idle && connection.conn != nil { + connection.conn.Connect() } } } @@ -1052,7 +1029,7 @@ func (c *serviceDiscovery) updateMember() error { func (c *serviceDiscovery) updateMemberWithResult() (memberUpdateResult, error) { result := memberUpdateResult{} for _, url := range c.GetServiceURLs() { - members, failure, err := c.getMembersWithFailure(c.ctx, url, UpdateMemberTimeout) + members, transportFailure, err := c.getMembersWithTransportFailure(c.ctx, url, UpdateMemberTimeout) // Check the cluster ID. updatedClusterID := members.GetHeader().GetClusterId() if err == nil && updatedClusterID != c.clusterID { @@ -1066,10 +1043,10 @@ func (c *serviceDiscovery) updateMemberWithResult() (memberUpdateResult, error) } // Failed to get members if err != nil { - result.recordFailure(url, failure) + result.recordFailure(url, transportFailure) failureTime := time.Now() shouldLog := true - if failure.transport { + if transportFailure { shouldLog = c.memberTransportFailures.record(failureTime, url) } else { // Only transport failures are suppressed. A different failure @@ -1090,9 +1067,7 @@ func (c *serviceDiscovery) updateMemberWithResult() (memberUpdateResult, error) } c.logMemberTransportFailureRecovery(time.Now(), url) c.updateURLs(members.GetMembers()) - c.memberTransportFailures.cleanup(c.GetServiceURLs()) - // URLs after the successful one were not observed failing in this refresh. - c.memberTransportFailures.cleanup(result.attemptedURLs) + c.memberTransportFailures.retain(c.GetServiceURLs(), result.failedURLs) return result, c.updateServiceClient(members.GetMembers(), members.GetLeader()) } @@ -1136,20 +1111,20 @@ func (c *serviceDiscovery) getClusterInfo(ctx context.Context, url string, timeo } func (c *serviceDiscovery) getMembers(ctx context.Context, url string, timeout time.Duration) (*pdpb.GetMembersResponse, error) { - members, _, err := c.getMembersWithFailure(ctx, url, timeout) + members, _, err := c.getMembersWithTransportFailure(ctx, url, timeout) return members, err } -func (c *serviceDiscovery) getMembersWithFailure( +func (c *serviceDiscovery) getMembersWithTransportFailure( ctx context.Context, url string, timeout time.Duration, -) (*pdpb.GetMembersResponse, memberUpdateFailure, error) { +) (*pdpb.GetMembersResponse, bool, error) { ctx, cancel := context.WithTimeout(ctx, timeout) defer cancel() cc, err := c.GetOrCreateGRPCConn(url) if err != nil { - return nil, classifyMemberDialFailure(err), err + return nil, isMemberDialTransportFailure(err), err } start := time.Now() defer func() { metrics.InternalCmdDurationGetMembers.Observe(time.Since(start).Seconds()) }() @@ -1161,24 +1136,22 @@ func (c *serviceDiscovery) getMembersWithFailure( case res := <-r: err = res.Err if err != nil { - failure := classifyMemberRPCFailure(err) metrics.InternalCmdFailedDurationGetMembers.Observe(time.Since(start).Seconds()) attachErr := errors.Errorf("error:%s target:%s status:%s", err, cc.Target(), cc.GetState().String()) - return nil, failure, errs.ErrClientGetMember.Wrap(attachErr).GenWithStackByCause() + return nil, isMemberRPCTransportFailure(err), errs.ErrClientGetMember.Wrap(attachErr).GenWithStackByCause() } val := res.Val members := val.(*pdpb.GetMembersResponse) if members.GetHeader().GetError() != nil { metrics.InternalCmdFailedDurationGetMembers.Observe(time.Since(start).Seconds()) attachErr := errors.Errorf("error:%s target:%s status:%s", members.GetHeader().GetError().String(), cc.Target(), cc.GetState().String()) - return nil, memberUpdateFailure{}, errs.ErrClientGetMember.Wrap(attachErr).GenWithStackByCause() + return nil, false, errs.ErrClientGetMember.Wrap(attachErr).GenWithStackByCause() } - return members, memberUpdateFailure{}, nil + return members, false, nil case <-ctx.Done(): - failure := classifyMemberRPCFailure(ctx.Err()) attachErr := errors.Errorf("error:%s target:%s status:%s", ctx.Err(), cc.Target(), cc.GetState().String()) metrics.InternalCmdFailedDurationGetMembers.Observe(time.Since(start).Seconds()) - return nil, failure, errs.ErrClientGetMember.Wrap(attachErr).GenWithStackByCause() + return nil, isMemberRPCTransportFailure(ctx.Err()), errs.ErrClientGetMember.Wrap(attachErr).GenWithStackByCause() } } @@ -1188,7 +1161,7 @@ func (c *serviceDiscovery) logMemberTransportFailureRecovery(now time.Time, url return } log.Info("[pd] member transport failure recovered", - zap.String("url", recovery.url), + zap.String("url", url), zap.Duration("failure-duration", recovery.failureDuration), zap.Uint64("failed-attempts", recovery.failedAttempts), zap.Uint64("suppressed-errors", recovery.suppressedErrors)) diff --git a/client/servicediscovery/service_discovery_test.go b/client/servicediscovery/service_discovery_test.go index c31f0ebf9d..38997a9670 100644 --- a/client/servicediscovery/service_discovery_test.go +++ b/client/servicediscovery/service_discovery_test.go @@ -534,7 +534,7 @@ func TestUpdateMemberWithResultPreservesErrorContract(t *testing.T) { require.Error(t, structuredErr) require.Equal(t, structuredErr.Error(), compatibilityErr.Error()) - require.Equal(t, []string{memberURL}, result.attemptedURLs) + require.Equal(t, []string{memberURL}, result.failedURLs) require.Equal(t, testCase.transport, result.transportFailures == 1) _, ok := client.memberTransportFailures.summary(time.Now()) @@ -603,7 +603,7 @@ func TestUpdateMemberClearsUnobservedFailureEpisodesAfterRecovery(t *testing.T) result, err := client.updateMemberWithResult() require.NoError(t, err) require.Equal(t, int32(2), calls.Load()) - require.Equal(t, []string{"http://pd-1.test:2379"}, result.attemptedURLs) + require.Equal(t, []string{"http://pd-1.test:2379"}, result.failedURLs) summary, ok := client.memberTransportFailures.summary(time.Now()) require.True(t, ok) @@ -691,3 +691,42 @@ func TestUpdateMemberLoopSuppressesScheduledRefreshDuringTransportFailure(t *tes require.Error(t, client.CheckMemberChanged()) require.Equal(t, callsAfterInitialBatch+1, getMembersCalls.Load()) } + +var memberConnectionSnapshotSink memberConnectionSnapshot + +func TestMemberConnectionSnapshotReuseDoesNotAllocate(t *testing.T) { + conn, err := grpc.NewClient( + "passthrough:///pd.test:2379", + grpc.WithTransportCredentials(insecure.NewCredentials()), + ) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, conn.Close()) }) + + urls := []string{ + "http://pd-1.test:2379", + "http://pd-2.test:2379", + "http://pd-3.test:2379", + } + client := &serviceDiscovery{} + client.urls.Store(urls) + for _, url := range urls { + client.clientConns.Store(url, conn) + } + + snapshot := client.snapshotMemberConnections(memberConnectionSnapshot{}) + require.Len(t, snapshot.connections, len(urls)) + require.True(t, snapshot.connections[1].observed) + require.Same(t, conn, snapshot.connections[1].conn) + + client.clientConns.Delete(urls[1]) + snapshot = client.snapshotMemberConnections(snapshot) + require.False(t, snapshot.connections[1].observed) + require.Nil(t, snapshot.connections[1].conn) + client.clientConns.Store(urls[1], conn) + memberConnectionSnapshotSink = snapshot + + allocations := testing.AllocsPerRun(1000, func() { + memberConnectionSnapshotSink = client.snapshotMemberConnections(memberConnectionSnapshotSink) + }) + require.Zero(t, allocations) +} From 9501fdd40ec34a936082d98ea8360303cc1c737f Mon Sep 17 00:00:00 2001 From: Ryan Leung Date: Mon, 27 Jul 2026 19:11:53 +0800 Subject: [PATCH 4/7] client: make member refresh tests deterministic Signed-off-by: Ryan Leung --- .../member_refresh_controller_test.go | 8 ++++-- .../service_discovery_test.go | 28 +++++++++++-------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/client/servicediscovery/member_refresh_controller_test.go b/client/servicediscovery/member_refresh_controller_test.go index a19f4717f8..fb3283d0de 100644 --- a/client/servicediscovery/member_refresh_controller_test.go +++ b/client/servicediscovery/member_refresh_controller_test.go @@ -104,6 +104,7 @@ func TestMemberRefreshControllerShouldWait(t *testing.T) { testCases := []struct { name string connections []memberConnection + currentURLs []string wait bool }{ { @@ -126,6 +127,7 @@ func TestMemberRefreshControllerShouldWait(t *testing.T) { { name: "url replacement restores normal behavior", connections: observedMemberConnections(connectivity.TransientFailure, connectivity.TransientFailure), + currentURLs: []string{"url-0", "replacement-url"}, }, } @@ -142,9 +144,9 @@ func TestMemberRefreshControllerShouldWait(t *testing.T) { initialURLs, observedMemberConnections(repeatedConnectivityState(connectivity.TransientFailure, len(testCase.connections))...), )) - currentURLs := initialURLs - if testCase.name == "url replacement restores normal behavior" { - currentURLs = []string{"url-0", "replacement-url"} + currentURLs := testCase.currentURLs + if currentURLs == nil { + currentURLs = initialURLs } wait := controller.shouldWait(currentURLs, testCase.connections) require.Equal(t, testCase.wait, wait) diff --git a/client/servicediscovery/service_discovery_test.go b/client/servicediscovery/service_discovery_test.go index 38997a9670..902904ed9d 100644 --- a/client/servicediscovery/service_discovery_test.go +++ b/client/servicediscovery/service_discovery_test.go @@ -466,15 +466,26 @@ func (s *memberTestPDServer) GetMembers(context.Context, *pdpb.GetMembersRequest return s.getMembers() } -func TestUpdateMemberWithResultPreservesErrorContract(t *testing.T) { +func startMemberTestPDServer(t *testing.T, testServer *memberTestPDServer) *bufconn.Listener { + t.Helper() + listener := bufconn.Listen(1024 * 1024) server := grpc.NewServer() - testServer := &memberTestPDServer{} pdpb.RegisterPDServer(server, testServer) + serveErr := make(chan error, 1) go func() { - require.NoError(t, server.Serve(listener)) + serveErr <- server.Serve(listener) }() - t.Cleanup(server.Stop) + t.Cleanup(func() { + server.Stop() + require.NoError(t, <-serveErr) + }) + return listener +} + +func TestUpdateMemberWithResultPreservesErrorContract(t *testing.T) { + testServer := &memberTestPDServer{} + listener := startMemberTestPDServer(t, testServer) ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) @@ -548,14 +559,8 @@ func TestUpdateMemberWithResultPreservesErrorContract(t *testing.T) { } func TestUpdateMemberClearsUnobservedFailureEpisodesAfterRecovery(t *testing.T) { - listener := bufconn.Listen(1024 * 1024) - server := grpc.NewServer() testServer := &memberTestPDServer{} - pdpb.RegisterPDServer(server, testServer) - go func() { - require.NoError(t, server.Serve(listener)) - }() - t.Cleanup(server.Stop) + listener := startMemberTestPDServer(t, testServer) ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) @@ -676,7 +681,6 @@ func TestUpdateMemberLoopSuppressesScheduledRefreshDuringTransportFailure(t *tes client.ScheduleCheckMemberChanged() require.Eventually(t, func() bool { return getMembersCalls.Load() >= 12 }, 2*time.Second, 10*time.Millisecond) - time.Sleep(50 * time.Millisecond) callsAfterInitialBatch := getMembersCalls.Load() require.GreaterOrEqual(t, callsAfterInitialBatch, int32(12)) From 42a0525965eb4ec93ac8dc29bade712bd6715dee Mon Sep 17 00:00:00 2001 From: Ryan Leung Date: Tue, 28 Jul 2026 16:34:27 +0800 Subject: [PATCH 5/7] client: cover member refresh recovery paths Signed-off-by: Ryan Leung --- client/servicediscovery/service_discovery.go | 12 +- .../service_discovery_test.go | 117 ++++++++++++++++-- 2 files changed, 114 insertions(+), 15 deletions(-) diff --git a/client/servicediscovery/service_discovery.go b/client/servicediscovery/service_discovery.go index 101b228cf9..1ed60bc6db 100644 --- a/client/servicediscovery/service_discovery.go +++ b/client/servicediscovery/service_discovery.go @@ -556,10 +556,14 @@ func (c *serviceDiscovery) initRetry(f func() error) error { func (c *serviceDiscovery) updateMemberLoop() { defer c.wg.Done() - ctx, cancel := context.WithCancel(c.ctx) - defer cancel() ticker := time.NewTicker(MemberUpdateInterval) defer ticker.Stop() + c.runMemberRefreshLoop(ticker.C) +} + +func (c *serviceDiscovery) runMemberRefreshLoop(memberUpdateCh <-chan time.Time) { + ctx, cancel := context.WithCancel(c.ctx) + defer cancel() bo := retry.InitialBackoffer(UpdateMemberBackOffBaseTime, UpdateMemberMaxBackoffTime, UpdateMemberTimeout) controller := memberRefreshController{} @@ -601,7 +605,7 @@ func (c *serviceDiscovery) updateMemberLoop() { case <-ctx.Done(): log.Info("[pd] exit member loop due to context canceled") return - case <-ticker.C: + case <-memberUpdateCh: c.logMemberTransportFailureSummary(time.Now()) // The safety sweep covers the event that may have been coalesced // while scheduled checks were disabled. @@ -634,7 +638,7 @@ func (c *serviceDiscovery) updateMemberLoop() { case <-ctx.Done(): log.Info("[pd] exit member loop due to context canceled") return - case <-ticker.C: + case <-memberUpdateCh: c.logMemberTransportFailureSummary(time.Now()) case <-c.checkMembershipCh: } diff --git a/client/servicediscovery/service_discovery_test.go b/client/servicediscovery/service_discovery_test.go index 902904ed9d..df079850a3 100644 --- a/client/servicediscovery/service_discovery_test.go +++ b/client/servicediscovery/service_discovery_test.go @@ -21,7 +21,6 @@ import ( "log" "net" "net/url" - "sync" "sync/atomic" "testing" "time" @@ -30,6 +29,7 @@ import ( "github.com/stretchr/testify/suite" "go.uber.org/goleak" "google.golang.org/grpc" + grpcbackoff "google.golang.org/grpc/backoff" "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials/insecure" pb "google.golang.org/grpc/examples/helloworld/helloworld" @@ -642,13 +642,35 @@ func newMemberTestServiceDiscovery( return client } -func TestUpdateMemberLoopSuppressesScheduledRefreshDuringTransportFailure(t *testing.T) { +func TestUpdateMemberLoopDegradedModeSafetySweepAndConnectionRecovery(t *testing.T) { + const memberURL = "http://recovering-pd.test:2379" + testServer := &memberTestPDServer{ + getMembers: func() (*pdpb.GetMembersResponse, error) { + return validMemberTestResponse(memberURL, 1), nil + }, + } + listener := startMemberTestPDServer(t, testServer) + ctx, cancel := context.WithCancel(context.Background()) var getMembersCalls atomic.Int32 + var connectionAvailable atomic.Bool + var syntheticMemberResponse atomic.Bool conn, err := grpc.NewClient( - "passthrough:///unreachable.test:2379", + "passthrough:///recovering-pd.test:2379", grpc.WithContextDialer(func(context.Context, string) (net.Conn, error) { - return nil, errors.New("transport unavailable") + if !connectionAvailable.Load() { + return nil, errors.New("transport unavailable") + } + return listener.Dial() + }), + grpc.WithConnectParams(grpc.ConnectParams{ + Backoff: grpcbackoff.Config{ + BaseDelay: 10 * time.Millisecond, + Multiplier: 1, + Jitter: 0, + MaxDelay: 10 * time.Millisecond, + }, + MinConnectTimeout: 10 * time.Millisecond, }), grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithUnaryInterceptor(func( @@ -661,21 +683,33 @@ func TestUpdateMemberLoopSuppressesScheduledRefreshDuringTransportFailure(t *tes ) error { if method == "/pdpb.PD/GetMembers" { getMembersCalls.Add(1) + if syntheticMemberResponse.Load() { + response := validMemberTestResponse(memberURL, 1) + *reply.(*pdpb.GetMembersResponse) = *response + return nil + } } return invoker(ctx, method, req, reply, cc, opts...) }), ) require.NoError(t, err) - var wg sync.WaitGroup - client := newMemberTestServiceDiscovery(ctx, cancel, "http://unreachable.test:2379", conn) - client.wg = &wg + client := newMemberTestServiceDiscovery(ctx, cancel, memberURL, conn) + client.leader.Store(newPDServiceClient(memberURL, memberURL, conn, true)) + client.apiCandidateNodes = [apiKindCount]*serviceBalancer{ + newServiceBalancer(emptyErrorFn), + newServiceBalancer(regionAPIErrorFn), + } client.checkMembershipCh = make(chan struct{}, 1) - wg.Add(1) - go client.updateMemberLoop() + memberUpdateCh := make(chan time.Time, 1) + loopDone := make(chan struct{}) + go func() { + client.runMemberRefreshLoop(memberUpdateCh) + close(loopDone) + }() t.Cleanup(func() { cancel() - wg.Wait() + <-loopDone require.NoError(t, conn.Close()) }) @@ -693,7 +727,68 @@ func TestUpdateMemberLoopSuppressesScheduledRefreshDuringTransportFailure(t *tes // A synchronous check is an explicit request and must bypass background suppression. require.Error(t, client.CheckMemberChanged()) - require.Equal(t, callsAfterInitialBatch+1, getMembersCalls.Load()) + callsAfterSynchronousCheck := getMembersCalls.Load() + require.Equal(t, callsAfterInitialBatch+1, callsAfterSynchronousCheck) + + // The periodic safety sweep must still issue one real membership request + // while asynchronous refreshes are suppressed. + memberUpdateCh <- time.Now() + require.Eventually(t, func() bool { + return getMembersCalls.Load() == callsAfterSynchronousCheck+1 + }, time.Second, 10*time.Millisecond) + callsAfterSafetySweep := getMembersCalls.Load() + + for range 100 { + client.ScheduleCheckMemberChanged() + } + require.Never(t, func() bool { + return getMembersCalls.Load() != callsAfterSafetySweep + }, 300*time.Millisecond, 10*time.Millisecond) + + // A successful safety sweep must leave degraded mode even if the local + // connection-state observation has not changed yet. + syntheticMemberResponse.Store(true) + memberUpdateCh <- time.Now() + require.Eventually(t, func() bool { + _, failed := client.memberTransportFailures.summary(time.Now()) + return !failed && getMembersCalls.Load() == callsAfterSafetySweep+1 + }, time.Second, 10*time.Millisecond) + callsAfterSafetyRecovery := getMembersCalls.Load() + + client.ScheduleCheckMemberChanged() + require.Eventually(t, func() bool { + return getMembersCalls.Load() > callsAfterSafetyRecovery + }, time.Second, 10*time.Millisecond) + + // Re-enter degraded mode so the connection-state recovery path is tested + // independently from the successful safety sweep above. + syntheticMemberResponse.Store(false) + callsBeforeSecondFailureBatch := getMembersCalls.Load() + client.ScheduleCheckMemberChanged() + require.Eventually(t, func() bool { + return getMembersCalls.Load() >= callsBeforeSecondFailureBatch+12 + }, 2*time.Second, 10*time.Millisecond) + callsAfterSecondFailureBatch := getMembersCalls.Load() + for range 100 { + client.ScheduleCheckMemberChanged() + } + require.Never(t, func() bool { + return getMembersCalls.Load() != callsAfterSecondFailureBatch + }, 300*time.Millisecond, 10*time.Millisecond) + + // Once the underlying connection becomes usable, connection-state polling + // must resume the normal refresh loop without waiting for the periodic sweep. + connectionAvailable.Store(true) + require.Eventually(t, func() bool { + _, failed := client.memberTransportFailures.summary(time.Now()) + return !failed && getMembersCalls.Load() > callsAfterSecondFailureBatch + }, 2*time.Second, 10*time.Millisecond) + callsAfterRecovery := getMembersCalls.Load() + + client.ScheduleCheckMemberChanged() + require.Eventually(t, func() bool { + return getMembersCalls.Load() > callsAfterRecovery + }, time.Second, 10*time.Millisecond) } var memberConnectionSnapshotSink memberConnectionSnapshot From 3b3e570025ab2e920fcbaf8ff7ccce9afc4be0b3 Mon Sep 17 00:00:00 2001 From: Ryan Leung Date: Wed, 29 Jul 2026 17:04:23 +0800 Subject: [PATCH 6/7] client: clarify member refresh state semantics Signed-off-by: Ryan Leung --- .../member_refresh_controller.go | 54 +++++++++------- .../member_refresh_controller_test.go | 61 +++++++++++++------ client/servicediscovery/service_discovery.go | 28 ++++++--- .../service_discovery_test.go | 4 +- 4 files changed, 94 insertions(+), 53 deletions(-) diff --git a/client/servicediscovery/member_refresh_controller.go b/client/servicediscovery/member_refresh_controller.go index 5741a2b87d..c525b4ae01 100644 --- a/client/servicediscovery/member_refresh_controller.go +++ b/client/servicediscovery/member_refresh_controller.go @@ -30,10 +30,15 @@ import ( clienterrs "github.com/tikv/pd/client/errs" ) +// isMemberDialTransportFailure recognizes the explicit gRPC dial sentinel. func isMemberDialTransportFailure(err error) bool { return errors.Is(err, clienterrs.ErrGRPCDial) } +// isMemberRPCTransportFailure recognizes Unavailable and DeadlineExceeded, +// including a local context deadline. These classifications only make an +// error eligible for transport-outage suppression; entering degraded mode also +// requires every corresponding gRPC connection to be in a degraded-mode state. func isMemberRPCTransportFailure(err error) bool { code := status.Code(err) if errors.Is(err, context.DeadlineExceeded) { @@ -43,19 +48,20 @@ func isMemberRPCTransportFailure(err error) bool { } type memberUpdateResult struct { - failedURLs []string - transportFailures int + // failedURLs is the ordered prefix attempted before the first success. + failedURLs []string + transportFailureCount int } -func (r *memberUpdateResult) recordFailure(url string, transportFailure bool) { +func (r *memberUpdateResult) recordFailure(url string, isTransportFailure bool) { r.failedURLs = append(r.failedURLs, url) - if transportFailure { - r.transportFailures++ + if isTransportFailure { + r.transportFailureCount++ } } -func (r *memberUpdateResult) allFailedByTransport(urls []string) bool { - if len(urls) == 0 || len(r.failedURLs) != len(urls) || r.transportFailures != len(urls) { +func (r *memberUpdateResult) allCurrentURLsFailedByTransport(urls []string) bool { + if len(urls) == 0 || len(r.failedURLs) != len(urls) || r.transportFailureCount != len(urls) { return false } return slices.Equal(r.failedURLs, urls) @@ -75,16 +81,16 @@ func (c *memberRefreshController) isDegraded() bool { return len(c.degradedURLs) > 0 } -func (c *memberRefreshController) enterDegraded( +func (c *memberRefreshController) tryEnterDegraded( result memberUpdateResult, urls []string, connections []memberConnection, ) bool { - if len(urls) != len(connections) || !result.allFailedByTransport(urls) { + if len(urls) != len(connections) || !result.allCurrentURLsFailedByTransport(urls) { return false } for _, connection := range connections { - if !connection.observed || !isInactiveMemberConnectionState(connection.state) { + if !connection.observed || !isDegradedModeConnectionState(connection.state) { return false } } @@ -92,14 +98,12 @@ func (c *memberRefreshController) enterDegraded( return true } -func (c *memberRefreshController) shouldWait(urls []string, connections []memberConnection) bool { +func (c *memberRefreshController) canRemainDegraded(urls []string, connections []memberConnection) bool { if !c.isDegraded() || len(urls) != len(connections) || !slices.Equal(c.degradedURLs, urls) { - c.leaveDegraded() return false } for _, connection := range connections { - if !connection.observed || !isInactiveMemberConnectionState(connection.state) { - c.leaveDegraded() + if !connection.observed || !isDegradedModeConnectionState(connection.state) { return false } } @@ -110,12 +114,19 @@ func (c *memberRefreshController) leaveDegraded() { c.degradedURLs = nil } -func isInactiveMemberConnectionState(state connectivity.State) bool { +// isDegradedModeConnectionState accepts only non-ready states that can recover +// on the existing connection. Missing and shut-down connections require an +// immediate member refresh instead. +func isDegradedModeConnectionState(state connectivity.State) bool { return state == connectivity.Idle || state == connectivity.Connecting || state == connectivity.TransientFailure } +// A transport-failure episode starts with the first classified transport +// failure for a URL. It ends when that URL succeeds, returns a non-transport +// failure, leaves the current member set, or is not reached because an earlier +// URL completed the refresh. Only a direct success emits a recovery log. type memberTransportFailureEpisode struct { firstFailure time.Time failedAttempts uint64 @@ -128,10 +139,10 @@ type memberTransportFailureRecovery struct { } type memberTransportFailureSummary struct { - failedURLs []string - failureDuration time.Duration - failedAttempts uint64 - suppressedErrors uint64 + failedURLs []string + longestFailureDuration time.Duration + failedAttempts uint64 + suppressedErrors uint64 } type memberTransportFailureTracker struct { @@ -182,7 +193,8 @@ func (t *memberTransportFailureTracker) discard(url string) { delete(t.episodes, url) } -func (t *memberTransportFailureTracker) retain(currentURLs, failedURLs []string) { +// retainCurrentFailures drops stale episodes after a successful refresh. +func (t *memberTransportFailureTracker) retainCurrentFailures(currentURLs, failedURLs []string) { t.mu.Lock() defer t.mu.Unlock() @@ -218,6 +230,6 @@ func (t *memberTransportFailureTracker) summary(now time.Time) (memberTransportF summary.failedAttempts += episode.failedAttempts summary.suppressedErrors += episode.failedAttempts - 1 } - summary.failureDuration = now.Sub(earliest) + summary.longestFailureDuration = now.Sub(earliest) return summary, true } diff --git a/client/servicediscovery/member_refresh_controller_test.go b/client/servicediscovery/member_refresh_controller_test.go index fb3283d0de..51e30d45b5 100644 --- a/client/servicediscovery/member_refresh_controller_test.go +++ b/client/servicediscovery/member_refresh_controller_test.go @@ -44,10 +44,11 @@ func TestMemberRefreshControllerEntersDegradedModeStrictly(t *testing.T) { name string result memberUpdateResult connections []memberConnection + currentURLs []string enter bool }{ { - name: "all current urls have transport failures and inactive connections", + name: "all current urls have transport failures and degraded-mode connections", result: newFailedMemberUpdateResult(transportFailure, transportFailure, transportFailure), connections: observedMemberConnections(connectivity.Idle, connectivity.Connecting, connectivity.TransientFailure), enter: true, @@ -62,6 +63,12 @@ func TestMemberRefreshControllerEntersDegradedModeStrictly(t *testing.T) { result: newFailedMemberUpdateResult(transportFailure), connections: observedMemberConnections(connectivity.TransientFailure, connectivity.TransientFailure), }, + { + name: "url set changed while failures were collected", + result: newFailedMemberUpdateResult(transportFailure, transportFailure), + connections: observedMemberConnections(connectivity.TransientFailure, connectivity.TransientFailure), + currentURLs: []string{"url-0", "replacement-url"}, + }, { name: "non-transport failure", result: newFailedMemberUpdateResult(transportFailure, nonTransportFailure), @@ -87,9 +94,13 @@ func TestMemberRefreshControllerEntersDegradedModeStrictly(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { controller := memberRefreshController{} - require.Equal(t, testCase.enter, controller.enterDegraded( + currentURLs := testCase.currentURLs + if currentURLs == nil { + currentURLs = memberTestURLs(len(testCase.connections)) + } + require.Equal(t, testCase.enter, controller.tryEnterDegraded( testCase.result, - memberTestURLs(len(testCase.connections)), + currentURLs, testCase.connections, )) require.Equal(t, testCase.enter, controller.isDegraded()) @@ -97,7 +108,7 @@ func TestMemberRefreshControllerEntersDegradedModeStrictly(t *testing.T) { } } -func TestMemberRefreshControllerShouldWait(t *testing.T) { +func TestMemberRefreshControllerCanRemainDegraded(t *testing.T) { t.Parallel() transportFailure := true @@ -105,12 +116,12 @@ func TestMemberRefreshControllerShouldWait(t *testing.T) { name string connections []memberConnection currentURLs []string - wait bool + remain bool }{ { name: "idle connections wait without a member refresh", connections: observedMemberConnections(connectivity.Idle, connectivity.Connecting, connectivity.Idle), - wait: true, + remain: true, }, { name: "ready connection refreshes immediately", @@ -139,7 +150,7 @@ func TestMemberRefreshControllerShouldWait(t *testing.T) { for i := range failures { failures[i] = transportFailure } - require.True(t, controller.enterDegraded( + require.True(t, controller.tryEnterDegraded( newFailedMemberUpdateResult(failures...), initialURLs, observedMemberConnections(repeatedConnectivityState(connectivity.TransientFailure, len(testCase.connections))...), @@ -148,27 +159,27 @@ func TestMemberRefreshControllerShouldWait(t *testing.T) { if currentURLs == nil { currentURLs = initialURLs } - wait := controller.shouldWait(currentURLs, testCase.connections) - require.Equal(t, testCase.wait, wait) - require.Equal(t, testCase.wait, controller.isDegraded()) + remain := controller.canRemainDegraded(currentURLs, testCase.connections) + require.Equal(t, testCase.remain, remain) + require.True(t, controller.isDegraded()) }) } } -func TestMemberRefreshControllerShouldWaitDoesNotAllocate(t *testing.T) { +func TestMemberRefreshControllerCanRemainDegradedDoesNotAllocate(t *testing.T) { transportFailure := true result := newFailedMemberUpdateResult(transportFailure, transportFailure, transportFailure) urls := memberTestURLs(3) connections := observedMemberConnections(connectivity.Idle, connectivity.Connecting, connectivity.TransientFailure) controller := memberRefreshController{} - require.True(t, controller.enterDegraded(result, urls, connections)) + require.True(t, controller.tryEnterDegraded(result, urls, connections)) allocations := testing.AllocsPerRun(1000, func() { - memberRefreshWaitSink = controller.shouldWait(urls, connections) + memberRefreshRemainDegradedSink = controller.canRemainDegraded(urls, connections) }) require.Zero(t, allocations) } -var memberRefreshWaitSink bool +var memberRefreshRemainDegradedSink bool func TestMemberTransportFailureTrackerEpisodes(t *testing.T) { t.Parallel() @@ -186,7 +197,7 @@ func TestMemberTransportFailureTrackerEpisodes(t *testing.T) { require.Equal(t, []string{"http://pd-1:2379", "http://pd-2:2379"}, summary.failedURLs) require.Equal(t, uint64(4), summary.failedAttempts) require.Equal(t, uint64(2), summary.suppressedErrors) - require.Equal(t, 4*time.Second, summary.failureDuration) + require.Equal(t, 4*time.Second, summary.longestFailureDuration) recovery, ok := tracker.recover(start.Add(5*time.Second), "http://pd-2:2379") require.True(t, ok) @@ -198,7 +209,7 @@ func TestMemberTransportFailureTrackerEpisodes(t *testing.T) { require.True(t, ok) require.Equal(t, []string{"http://pd-1:2379"}, summary.failedURLs) - tracker.retain([]string{"http://pd-3:2379"}, []string{"http://pd-1:2379"}) + tracker.retainCurrentFailures([]string{"http://pd-3:2379"}, []string{"http://pd-1:2379"}) _, ok = tracker.summary(start.Add(7 * time.Second)) require.False(t, ok) } @@ -231,6 +242,15 @@ func TestIsMemberTransportFailure(t *testing.T) { got: isMemberRPCTransportFailure(status.Error(codes.DeadlineExceeded, "context deadline exceeded")), transport: true, }, + { + name: "local deadline", + got: isMemberRPCTransportFailure(context.DeadlineExceeded), + transport: true, + }, + { + name: "local cancellation", + got: isMemberRPCTransportFailure(context.Canceled), + }, { name: "reset rpc", got: isMemberRPCTransportFailure(status.Error(codes.Unavailable, "read: connection reset by peer")), @@ -275,6 +295,7 @@ func TestMemberTransportFailureSummaryAndRecoveryLogs(t *testing.T) { require.Len(t, summaryLogs, 1) summaryFields := summaryLogs[0].ContextMap() require.Contains(t, summaryFields, "failed-urls") + require.Contains(t, summaryFields, "longest-failure-duration") require.Equal(t, uint64(2), summaryFields["failed-attempts"]) require.Equal(t, uint64(1), summaryFields["suppressed-errors"]) require.NotContains(t, summaryFields, "error-classes") @@ -302,7 +323,7 @@ func TestMemberTransportFailureTrackerConcurrentAccess(_ *testing.T) { tracker.record(now, url) tracker.summary(now.Add(time.Second)) tracker.recover(now.Add(2*time.Second), url) - tracker.retain([]string{url}, []string{url}) + tracker.retainCurrentFailures([]string{url}, []string{url}) } }(i) } @@ -318,16 +339,16 @@ func TestMemberTransportFailureTrackerHealthyRecoveryDoesNotAllocate(t *testing. require.Zero(t, allocations) } -func BenchmarkMemberRefreshControllerShouldWait(b *testing.B) { +func BenchmarkMemberRefreshControllerCanRemainDegraded(b *testing.B) { transportFailure := true result := newFailedMemberUpdateResult(transportFailure, transportFailure, transportFailure) urls := memberTestURLs(3) connections := observedMemberConnections(connectivity.TransientFailure, connectivity.Connecting, connectivity.Idle) controller := memberRefreshController{} - controller.enterDegraded(result, urls, connections) + controller.tryEnterDegraded(result, urls, connections) b.ReportAllocs() for b.Loop() { - controller.shouldWait(urls, connections) + controller.canRemainDegraded(urls, connections) } } diff --git a/client/servicediscovery/service_discovery.go b/client/servicediscovery/service_discovery.go index 1ed60bc6db..2f2c061749 100644 --- a/client/servicediscovery/service_discovery.go +++ b/client/servicediscovery/service_discovery.go @@ -568,16 +568,19 @@ func (c *serviceDiscovery) runMemberRefreshLoop(memberUpdateCh <-chan time.Time) bo := retry.InitialBackoffer(UpdateMemberBackOffBaseTime, UpdateMemberMaxBackoffTime, UpdateMemberTimeout) controller := memberRefreshController{} var connectionStateTicker *time.Ticker + var connectionStateCh <-chan time.Time stopConnectionStateTicker := func() { if connectionStateTicker != nil { connectionStateTicker.Stop() connectionStateTicker = nil } + connectionStateCh = nil } defer stopConnectionStateTicker() startConnectionStateTicker := func() { if connectionStateTicker == nil { connectionStateTicker = time.NewTicker(memberConnectionStateCheckInterval) + connectionStateCh = connectionStateTicker.C } } drainScheduledCheck := func() { @@ -618,18 +621,19 @@ func (c *serviceDiscovery) runMemberRefreshLoop(memberUpdateCh <-chan time.Time) continue } snapshot = c.snapshotMemberConnections(snapshot) - if controller.enterDegraded(result, snapshot.urls, snapshot.connections) { + if controller.tryEnterDegraded(result, snapshot.urls, snapshot.connections) { connectIdleMemberConnections(snapshot) continue } controller.leaveDegraded() stopConnectionStateTicker() - case <-connectionStateTicker.C: + case <-connectionStateCh: snapshot = c.snapshotMemberConnections(snapshot) - if controller.shouldWait(snapshot.urls, snapshot.connections) { + if controller.canRemainDegraded(snapshot.urls, snapshot.connections) { connectIdleMemberConnections(snapshot) continue } + controller.leaveDegraded() stopConnectionStateTicker() drainScheduledCheck() } @@ -653,7 +657,7 @@ func (c *serviceDiscovery) runMemberRefreshLoop(memberUpdateCh <-chan time.Time) break } snapshot = c.snapshotMemberConnections(snapshot) - if !controller.enterDegraded(result, snapshot.urls, snapshot.connections) { + if !controller.tryEnterDegraded(result, snapshot.urls, snapshot.connections) { break } startConnectionStateTicker() @@ -661,10 +665,11 @@ func (c *serviceDiscovery) runMemberRefreshLoop(memberUpdateCh <-chan time.Time) // Inspect again after entering degraded mode so a connection that // became ready as the failed batch completed is refreshed immediately. snapshot = c.snapshotMemberConnections(snapshot) - if controller.shouldWait(snapshot.urls, snapshot.connections) { + if controller.canRemainDegraded(snapshot.urls, snapshot.connections) { connectIdleMemberConnections(snapshot) break } + controller.leaveDegraded() stopConnectionStateTicker() drainScheduledCheck() } @@ -1033,7 +1038,7 @@ func (c *serviceDiscovery) updateMember() error { func (c *serviceDiscovery) updateMemberWithResult() (memberUpdateResult, error) { result := memberUpdateResult{} for _, url := range c.GetServiceURLs() { - members, transportFailure, err := c.getMembersWithTransportFailure(c.ctx, url, UpdateMemberTimeout) + members, isTransportFailure, err := c.getMembersWithTransportFailure(c.ctx, url, UpdateMemberTimeout) // Check the cluster ID. updatedClusterID := members.GetHeader().GetClusterId() if err == nil && updatedClusterID != c.clusterID { @@ -1047,10 +1052,10 @@ func (c *serviceDiscovery) updateMemberWithResult() (memberUpdateResult, error) } // Failed to get members if err != nil { - result.recordFailure(url, transportFailure) + result.recordFailure(url, isTransportFailure) failureTime := time.Now() shouldLog := true - if transportFailure { + if isTransportFailure { shouldLog = c.memberTransportFailures.record(failureTime, url) } else { // Only transport failures are suppressed. A different failure @@ -1071,7 +1076,7 @@ func (c *serviceDiscovery) updateMemberWithResult() (memberUpdateResult, error) } c.logMemberTransportFailureRecovery(time.Now(), url) c.updateURLs(members.GetMembers()) - c.memberTransportFailures.retain(c.GetServiceURLs(), result.failedURLs) + c.memberTransportFailures.retainCurrentFailures(c.GetServiceURLs(), result.failedURLs) return result, c.updateServiceClient(members.GetMembers(), members.GetLeader()) } @@ -1119,6 +1124,9 @@ func (c *serviceDiscovery) getMembers(ctx context.Context, url string, timeout t return members, err } +// getMembersWithTransportFailure reports whether a failed request is eligible +// for transport-outage suppression. It preserves getMembers' response and +// error contract. func (c *serviceDiscovery) getMembersWithTransportFailure( ctx context.Context, url string, @@ -1178,7 +1186,7 @@ func (c *serviceDiscovery) logMemberTransportFailureSummary(now time.Time) { } log.Info("[pd] member transport failures are being suppressed", zap.Strings("failed-urls", summary.failedURLs), - zap.Duration("failure-duration", summary.failureDuration), + zap.Duration("longest-failure-duration", summary.longestFailureDuration), zap.Uint64("failed-attempts", summary.failedAttempts), zap.Uint64("suppressed-errors", summary.suppressedErrors)) } diff --git a/client/servicediscovery/service_discovery_test.go b/client/servicediscovery/service_discovery_test.go index df079850a3..0b90edd1f2 100644 --- a/client/servicediscovery/service_discovery_test.go +++ b/client/servicediscovery/service_discovery_test.go @@ -546,7 +546,7 @@ func TestUpdateMemberWithResultPreservesErrorContract(t *testing.T) { require.Error(t, structuredErr) require.Equal(t, structuredErr.Error(), compatibilityErr.Error()) require.Equal(t, []string{memberURL}, result.failedURLs) - require.Equal(t, testCase.transport, result.transportFailures == 1) + require.Equal(t, testCase.transport, result.transportFailureCount == 1) _, ok := client.memberTransportFailures.summary(time.Now()) if testCase.transport { @@ -558,7 +558,7 @@ func TestUpdateMemberWithResultPreservesErrorContract(t *testing.T) { } } -func TestUpdateMemberClearsUnobservedFailureEpisodesAfterRecovery(t *testing.T) { +func TestUpdateMemberRetainsOnlyFailuresObservedBeforeSuccess(t *testing.T) { testServer := &memberTestPDServer{} listener := startMemberTestPDServer(t, testServer) From 222a48c5213d79f4bf8f16fdfabde2423ad92b31 Mon Sep 17 00:00:00 2001 From: Ryan Leung Date: Thu, 30 Jul 2026 14:36:31 +0800 Subject: [PATCH 7/7] client: clarify member availability failure naming Signed-off-by: Ryan Leung --- .../member_refresh_controller.go | 70 +++++----- .../member_refresh_controller_test.go | 130 +++++++++--------- client/servicediscovery/service_discovery.go | 51 ++++--- .../service_discovery_test.go | 24 ++-- 4 files changed, 137 insertions(+), 138 deletions(-) diff --git a/client/servicediscovery/member_refresh_controller.go b/client/servicediscovery/member_refresh_controller.go index c525b4ae01..1490379590 100644 --- a/client/servicediscovery/member_refresh_controller.go +++ b/client/servicediscovery/member_refresh_controller.go @@ -30,16 +30,16 @@ import ( clienterrs "github.com/tikv/pd/client/errs" ) -// isMemberDialTransportFailure recognizes the explicit gRPC dial sentinel. -func isMemberDialTransportFailure(err error) bool { +// isMemberDialAvailabilityFailure recognizes the explicit gRPC dial sentinel. +func isMemberDialAvailabilityFailure(err error) bool { return errors.Is(err, clienterrs.ErrGRPCDial) } -// isMemberRPCTransportFailure recognizes Unavailable and DeadlineExceeded, -// including a local context deadline. These classifications only make an -// error eligible for transport-outage suppression; entering degraded mode also -// requires every corresponding gRPC connection to be in a degraded-mode state. -func isMemberRPCTransportFailure(err error) bool { +// isMemberRPCAvailabilityFailure recognizes Unavailable and DeadlineExceeded, +// including a local context deadline. An availability failure alone does not +// identify a transport outage; entering degraded mode also requires every +// corresponding gRPC connection to be in a degraded-mode state. +func isMemberRPCAvailabilityFailure(err error) bool { code := status.Code(err) if errors.Is(err, context.DeadlineExceeded) { code = codes.DeadlineExceeded @@ -49,19 +49,19 @@ func isMemberRPCTransportFailure(err error) bool { type memberUpdateResult struct { // failedURLs is the ordered prefix attempted before the first success. - failedURLs []string - transportFailureCount int + failedURLs []string + availabilityFailureCount int } -func (r *memberUpdateResult) recordFailure(url string, isTransportFailure bool) { +func (r *memberUpdateResult) recordFailure(url string, isAvailabilityFailure bool) { r.failedURLs = append(r.failedURLs, url) - if isTransportFailure { - r.transportFailureCount++ + if isAvailabilityFailure { + r.availabilityFailureCount++ } } -func (r *memberUpdateResult) allCurrentURLsFailedByTransport(urls []string) bool { - if len(urls) == 0 || len(r.failedURLs) != len(urls) || r.transportFailureCount != len(urls) { +func (r *memberUpdateResult) allCurrentURLsHaveAvailabilityFailures(urls []string) bool { + if len(urls) == 0 || len(r.failedURLs) != len(urls) || r.availabilityFailureCount != len(urls) { return false } return slices.Equal(r.failedURLs, urls) @@ -86,7 +86,7 @@ func (c *memberRefreshController) tryEnterDegraded( urls []string, connections []memberConnection, ) bool { - if len(urls) != len(connections) || !result.allCurrentURLsFailedByTransport(urls) { + if len(urls) != len(connections) || !result.allCurrentURLsHaveAvailabilityFailures(urls) { return false } for _, connection := range connections { @@ -123,44 +123,44 @@ func isDegradedModeConnectionState(state connectivity.State) bool { state == connectivity.TransientFailure } -// A transport-failure episode starts with the first classified transport -// failure for a URL. It ends when that URL succeeds, returns a non-transport -// failure, leaves the current member set, or is not reached because an earlier -// URL completed the refresh. Only a direct success emits a recovery log. -type memberTransportFailureEpisode struct { +// An availability-failure episode starts with the first availability failure +// for a URL. It ends when that URL succeeds, returns a different failure, +// leaves the current member set, or is not reached because an earlier URL +// completed the refresh. Only a direct success emits a recovery log. +type memberAvailabilityFailureEpisode struct { firstFailure time.Time failedAttempts uint64 } -type memberTransportFailureRecovery struct { +type memberAvailabilityFailureRecovery struct { failureDuration time.Duration failedAttempts uint64 suppressedErrors uint64 } -type memberTransportFailureSummary struct { +type memberAvailabilityFailureSummary struct { failedURLs []string longestFailureDuration time.Duration failedAttempts uint64 suppressedErrors uint64 } -type memberTransportFailureTracker struct { +type memberAvailabilityFailureTracker struct { mu sync.Mutex - episodes map[string]*memberTransportFailureEpisode + episodes map[string]*memberAvailabilityFailureEpisode } // record returns true when the caller should emit the detailed failure log. -func (t *memberTransportFailureTracker) record(now time.Time, url string) bool { +func (t *memberAvailabilityFailureTracker) record(now time.Time, url string) bool { t.mu.Lock() defer t.mu.Unlock() if t.episodes == nil { - t.episodes = make(map[string]*memberTransportFailureEpisode) + t.episodes = make(map[string]*memberAvailabilityFailureEpisode) } episode, ok := t.episodes[url] if !ok { - t.episodes[url] = &memberTransportFailureEpisode{ + t.episodes[url] = &memberAvailabilityFailureEpisode{ firstFailure: now, failedAttempts: 1, } @@ -171,30 +171,30 @@ func (t *memberTransportFailureTracker) record(now time.Time, url string) bool { return false } -func (t *memberTransportFailureTracker) recover(now time.Time, url string) (memberTransportFailureRecovery, bool) { +func (t *memberAvailabilityFailureTracker) recover(now time.Time, url string) (memberAvailabilityFailureRecovery, bool) { t.mu.Lock() defer t.mu.Unlock() episode, ok := t.episodes[url] if !ok { - return memberTransportFailureRecovery{}, false + return memberAvailabilityFailureRecovery{}, false } delete(t.episodes, url) - return memberTransportFailureRecovery{ + return memberAvailabilityFailureRecovery{ failureDuration: now.Sub(episode.firstFailure), failedAttempts: episode.failedAttempts, suppressedErrors: episode.failedAttempts - 1, }, true } -func (t *memberTransportFailureTracker) discard(url string) { +func (t *memberAvailabilityFailureTracker) discard(url string) { t.mu.Lock() defer t.mu.Unlock() delete(t.episodes, url) } // retainCurrentFailures drops stale episodes after a successful refresh. -func (t *memberTransportFailureTracker) retainCurrentFailures(currentURLs, failedURLs []string) { +func (t *memberAvailabilityFailureTracker) retainCurrentFailures(currentURLs, failedURLs []string) { t.mu.Lock() defer t.mu.Unlock() @@ -205,12 +205,12 @@ func (t *memberTransportFailureTracker) retainCurrentFailures(currentURLs, faile } } -func (t *memberTransportFailureTracker) summary(now time.Time) (memberTransportFailureSummary, bool) { +func (t *memberAvailabilityFailureTracker) summary(now time.Time) (memberAvailabilityFailureSummary, bool) { t.mu.Lock() defer t.mu.Unlock() if len(t.episodes) == 0 { - return memberTransportFailureSummary{}, false + return memberAvailabilityFailureSummary{}, false } urls := make([]string, 0, len(t.episodes)) for url := range t.episodes { @@ -218,7 +218,7 @@ func (t *memberTransportFailureTracker) summary(now time.Time) (memberTransportF } sort.Strings(urls) - summary := memberTransportFailureSummary{ + summary := memberAvailabilityFailureSummary{ failedURLs: urls, } earliest := now diff --git a/client/servicediscovery/member_refresh_controller_test.go b/client/servicediscovery/member_refresh_controller_test.go index 51e30d45b5..0c32e32a9b 100644 --- a/client/servicediscovery/member_refresh_controller_test.go +++ b/client/servicediscovery/member_refresh_controller_test.go @@ -37,8 +37,8 @@ import ( func TestMemberRefreshControllerEntersDegradedModeStrictly(t *testing.T) { t.Parallel() - transportFailure := true - nonTransportFailure := false + availabilityFailure := true + otherFailure := false testCases := []struct { name string @@ -48,8 +48,8 @@ func TestMemberRefreshControllerEntersDegradedModeStrictly(t *testing.T) { enter bool }{ { - name: "all current urls have transport failures and degraded-mode connections", - result: newFailedMemberUpdateResult(transportFailure, transportFailure, transportFailure), + name: "all current urls have availability failures and degraded-mode connections", + result: newFailedMemberUpdateResult(availabilityFailure, availabilityFailure, availabilityFailure), connections: observedMemberConnections(connectivity.Idle, connectivity.Connecting, connectivity.TransientFailure), enter: true, }, @@ -60,33 +60,33 @@ func TestMemberRefreshControllerEntersDegradedModeStrictly(t *testing.T) { }, { name: "not every url was attempted", - result: newFailedMemberUpdateResult(transportFailure), + result: newFailedMemberUpdateResult(availabilityFailure), connections: observedMemberConnections(connectivity.TransientFailure, connectivity.TransientFailure), }, { name: "url set changed while failures were collected", - result: newFailedMemberUpdateResult(transportFailure, transportFailure), + result: newFailedMemberUpdateResult(availabilityFailure, availabilityFailure), connections: observedMemberConnections(connectivity.TransientFailure, connectivity.TransientFailure), currentURLs: []string{"url-0", "replacement-url"}, }, { - name: "non-transport failure", - result: newFailedMemberUpdateResult(transportFailure, nonTransportFailure), + name: "other failure", + result: newFailedMemberUpdateResult(availabilityFailure, otherFailure), connections: observedMemberConnections(connectivity.TransientFailure, connectivity.TransientFailure), }, { name: "missing connection", - result: newFailedMemberUpdateResult(transportFailure), + result: newFailedMemberUpdateResult(availabilityFailure), connections: []memberConnection{{}}, }, { name: "ready connection", - result: newFailedMemberUpdateResult(transportFailure), + result: newFailedMemberUpdateResult(availabilityFailure), connections: observedMemberConnections(connectivity.Ready), }, { name: "shutdown connection", - result: newFailedMemberUpdateResult(transportFailure), + result: newFailedMemberUpdateResult(availabilityFailure), connections: observedMemberConnections(connectivity.Shutdown), }, } @@ -111,7 +111,7 @@ func TestMemberRefreshControllerEntersDegradedModeStrictly(t *testing.T) { func TestMemberRefreshControllerCanRemainDegraded(t *testing.T) { t.Parallel() - transportFailure := true + availabilityFailure := true testCases := []struct { name string connections []memberConnection @@ -148,7 +148,7 @@ func TestMemberRefreshControllerCanRemainDegraded(t *testing.T) { initialURLs := memberTestURLs(len(testCase.connections)) failures := make([]bool, len(testCase.connections)) for i := range failures { - failures[i] = transportFailure + failures[i] = availabilityFailure } require.True(t, controller.tryEnterDegraded( newFailedMemberUpdateResult(failures...), @@ -167,8 +167,8 @@ func TestMemberRefreshControllerCanRemainDegraded(t *testing.T) { } func TestMemberRefreshControllerCanRemainDegradedDoesNotAllocate(t *testing.T) { - transportFailure := true - result := newFailedMemberUpdateResult(transportFailure, transportFailure, transportFailure) + availabilityFailure := true + result := newFailedMemberUpdateResult(availabilityFailure, availabilityFailure, availabilityFailure) urls := memberTestURLs(3) connections := observedMemberConnections(connectivity.Idle, connectivity.Connecting, connectivity.TransientFailure) controller := memberRefreshController{} @@ -181,10 +181,10 @@ func TestMemberRefreshControllerCanRemainDegradedDoesNotAllocate(t *testing.T) { var memberRefreshRemainDegradedSink bool -func TestMemberTransportFailureTrackerEpisodes(t *testing.T) { +func TestMemberAvailabilityFailureTrackerEpisodes(t *testing.T) { t.Parallel() - tracker := memberTransportFailureTracker{} + tracker := memberAvailabilityFailureTracker{} start := time.Unix(100, 0) require.True(t, tracker.record(start, "http://pd-1:2379")) @@ -214,84 +214,84 @@ func TestMemberTransportFailureTrackerEpisodes(t *testing.T) { require.False(t, ok) } -func TestIsMemberTransportFailure(t *testing.T) { +func TestIsMemberAvailabilityFailure(t *testing.T) { t.Parallel() testCases := []struct { - name string - got bool - transport bool + name string + got bool + availability bool }{ { - name: "connection refused rpc", - got: isMemberRPCTransportFailure(status.Error(codes.Unavailable, "dial tcp 192.0.2.1:2379: connect: connection refused")), - transport: true, + name: "connection refused rpc", + got: isMemberRPCAvailabilityFailure(status.Error(codes.Unavailable, "dial tcp 192.0.2.1:2379: connect: connection refused")), + availability: true, }, { - name: "tls rpc", - got: isMemberRPCTransportFailure(status.Error(codes.Unavailable, "transport: authentication handshake failed: tls certificate expired")), - transport: true, + name: "tls rpc", + got: isMemberRPCAvailabilityFailure(status.Error(codes.Unavailable, "transport: authentication handshake failed: tls certificate expired")), + availability: true, }, { - name: "dns rpc", - got: isMemberRPCTransportFailure(status.Error(codes.Unavailable, "lookup pd.invalid: no such host")), - transport: true, + name: "dns rpc", + got: isMemberRPCAvailabilityFailure(status.Error(codes.Unavailable, "lookup pd.invalid: no such host")), + availability: true, }, { - name: "deadline rpc", - got: isMemberRPCTransportFailure(status.Error(codes.DeadlineExceeded, "context deadline exceeded")), - transport: true, + name: "deadline rpc", + got: isMemberRPCAvailabilityFailure(status.Error(codes.DeadlineExceeded, "context deadline exceeded")), + availability: true, }, { - name: "local deadline", - got: isMemberRPCTransportFailure(context.DeadlineExceeded), - transport: true, + name: "local deadline", + got: isMemberRPCAvailabilityFailure(context.DeadlineExceeded), + availability: true, }, { name: "local cancellation", - got: isMemberRPCTransportFailure(context.Canceled), + got: isMemberRPCAvailabilityFailure(context.Canceled), }, { - name: "reset rpc", - got: isMemberRPCTransportFailure(status.Error(codes.Unavailable, "read: connection reset by peer")), - transport: true, + name: "reset rpc", + got: isMemberRPCAvailabilityFailure(status.Error(codes.Unavailable, "read: connection reset by peer")), + availability: true, }, { - name: "non-network grpc status", - got: isMemberRPCTransportFailure(status.Error(codes.PermissionDenied, "permission denied")), - transport: false, + name: "non-network grpc status", + got: isMemberRPCAvailabilityFailure(status.Error(codes.PermissionDenied, "permission denied")), + availability: false, }, { - name: "blocking dial timeout", - got: isMemberDialTransportFailure(clienterrs.ErrGRPCDial.Wrap(context.DeadlineExceeded).GenWithStackByCause()), - transport: true, + name: "blocking dial timeout", + got: isMemberDialAvailabilityFailure(clienterrs.ErrGRPCDial.Wrap(context.DeadlineExceeded).GenWithStackByCause()), + availability: true, }, { - name: "uncertain dial error", - got: isMemberDialTransportFailure(errors.New("invalid client configuration")), - transport: false, + name: "uncertain dial error", + got: isMemberDialAvailabilityFailure(errors.New("invalid client configuration")), + availability: false, }, } for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { - require.Equal(t, testCase.transport, testCase.got) + require.Equal(t, testCase.availability, testCase.got) }) } } -func TestMemberTransportFailureSummaryAndRecoveryLogs(t *testing.T) { +func TestMemberAvailabilityFailureSummaryAndRecoveryLogs(t *testing.T) { core, observedLogs := observer.New(zap.InfoLevel) restoreLogger := pingcaplog.ReplaceGlobals(zap.New(core), nil) t.Cleanup(restoreLogger) client := &serviceDiscovery{} start := time.Unix(100, 0) - require.True(t, client.memberTransportFailures.record(start, "http://pd-1:2379")) - require.False(t, client.memberTransportFailures.record(start.Add(time.Second), "http://pd-1:2379")) + require.True(t, client.memberAvailabilityFailures.record(start, "http://pd-1:2379")) + require.False(t, client.memberAvailabilityFailures.record(start.Add(time.Second), "http://pd-1:2379")) - client.logMemberTransportFailureSummary(start.Add(2 * time.Second)) - summaryLogs := observedLogs.FilterMessage("[pd] member transport failures are being suppressed").All() + client.logMemberAvailabilityFailureSummary(start.Add(2 * time.Second)) + summaryLogs := observedLogs.FilterMessage("[pd] member availability failures are being suppressed").All() require.Len(t, summaryLogs, 1) summaryFields := summaryLogs[0].ContextMap() require.Contains(t, summaryFields, "failed-urls") @@ -300,8 +300,8 @@ func TestMemberTransportFailureSummaryAndRecoveryLogs(t *testing.T) { require.Equal(t, uint64(1), summaryFields["suppressed-errors"]) require.NotContains(t, summaryFields, "error-classes") - client.logMemberTransportFailureRecovery(start.Add(3*time.Second), "http://pd-1:2379") - recoveryLogs := observedLogs.FilterMessage("[pd] member transport failure recovered").All() + client.logMemberAvailabilityFailureRecovery(start.Add(3*time.Second), "http://pd-1:2379") + recoveryLogs := observedLogs.FilterMessage("[pd] member availability failure recovered").All() require.Len(t, recoveryLogs, 1) recoveryFields := recoveryLogs[0].ContextMap() require.Equal(t, "http://pd-1:2379", recoveryFields["url"]) @@ -309,8 +309,8 @@ func TestMemberTransportFailureSummaryAndRecoveryLogs(t *testing.T) { require.Equal(t, uint64(1), recoveryFields["suppressed-errors"]) } -func TestMemberTransportFailureTrackerConcurrentAccess(_ *testing.T) { - tracker := memberTransportFailureTracker{} +func TestMemberAvailabilityFailureTrackerConcurrentAccess(_ *testing.T) { + tracker := memberAvailabilityFailureTracker{} now := time.Unix(100, 0) var wg sync.WaitGroup @@ -330,8 +330,8 @@ func TestMemberTransportFailureTrackerConcurrentAccess(_ *testing.T) { wg.Wait() } -func TestMemberTransportFailureTrackerHealthyRecoveryDoesNotAllocate(t *testing.T) { - tracker := memberTransportFailureTracker{} +func TestMemberAvailabilityFailureTrackerHealthyRecoveryDoesNotAllocate(t *testing.T) { + tracker := memberAvailabilityFailureTracker{} now := time.Unix(100, 0) allocations := testing.AllocsPerRun(1000, func() { _, _ = tracker.recover(now, "http://pd-1:2379") @@ -340,8 +340,8 @@ func TestMemberTransportFailureTrackerHealthyRecoveryDoesNotAllocate(t *testing. } func BenchmarkMemberRefreshControllerCanRemainDegraded(b *testing.B) { - transportFailure := true - result := newFailedMemberUpdateResult(transportFailure, transportFailure, transportFailure) + availabilityFailure := true + result := newFailedMemberUpdateResult(availabilityFailure, availabilityFailure, availabilityFailure) urls := memberTestURLs(3) connections := observedMemberConnections(connectivity.TransientFailure, connectivity.Connecting, connectivity.Idle) controller := memberRefreshController{} @@ -352,8 +352,8 @@ func BenchmarkMemberRefreshControllerCanRemainDegraded(b *testing.B) { } } -func BenchmarkMemberTransportFailureTrackerSuppression(b *testing.B) { - tracker := memberTransportFailureTracker{} +func BenchmarkMemberAvailabilityFailureTrackerSuppression(b *testing.B) { + tracker := memberAvailabilityFailureTracker{} now := time.Unix(100, 0) tracker.record(now, "http://pd-1:2379") b.ReportAllocs() diff --git a/client/servicediscovery/service_discovery.go b/client/servicediscovery/service_discovery.go index 2f2c061749..7e6f0fa953 100644 --- a/client/servicediscovery/service_discovery.go +++ b/client/servicediscovery/service_discovery.go @@ -458,7 +458,7 @@ type serviceDiscovery struct { flight singleflight.Group - memberTransportFailures memberTransportFailureTracker + memberAvailabilityFailures memberAvailabilityFailureTracker } // NewDefaultServiceDiscovery returns a new default service discovery-based client. @@ -609,7 +609,7 @@ func (c *serviceDiscovery) runMemberRefreshLoop(memberUpdateCh <-chan time.Time) log.Info("[pd] exit member loop due to context canceled") return case <-memberUpdateCh: - c.logMemberTransportFailureSummary(time.Now()) + c.logMemberAvailabilityFailureSummary(time.Now()) // The safety sweep covers the event that may have been coalesced // while scheduled checks were disabled. drainScheduledCheck() @@ -643,7 +643,7 @@ func (c *serviceDiscovery) runMemberRefreshLoop(memberUpdateCh <-chan time.Time) log.Info("[pd] exit member loop due to context canceled") return case <-memberUpdateCh: - c.logMemberTransportFailureSummary(time.Now()) + c.logMemberAvailabilityFailureSummary(time.Now()) case <-c.checkMembershipCh: } } @@ -1038,7 +1038,7 @@ func (c *serviceDiscovery) updateMember() error { func (c *serviceDiscovery) updateMemberWithResult() (memberUpdateResult, error) { result := memberUpdateResult{} for _, url := range c.GetServiceURLs() { - members, isTransportFailure, err := c.getMembersWithTransportFailure(c.ctx, url, UpdateMemberTimeout) + members, isAvailabilityFailure, err := c.getMembersWithAvailabilityFailure(c.ctx, url, UpdateMemberTimeout) // Check the cluster ID. updatedClusterID := members.GetHeader().GetClusterId() if err == nil && updatedClusterID != c.clusterID { @@ -1052,15 +1052,15 @@ func (c *serviceDiscovery) updateMemberWithResult() (memberUpdateResult, error) } // Failed to get members if err != nil { - result.recordFailure(url, isTransportFailure) + result.recordFailure(url, isAvailabilityFailure) failureTime := time.Now() shouldLog := true - if isTransportFailure { - shouldLog = c.memberTransportFailures.record(failureTime, url) + if isAvailabilityFailure { + shouldLog = c.memberAvailabilityFailures.record(failureTime, url) } else { - // Only transport failures are suppressed. A different failure - // ends any transport-failure episode for this URL. - c.memberTransportFailures.discard(url) + // Only availability failures are suppressed. A different failure + // ends any availability-failure episode for this URL. + c.memberAvailabilityFailures.discard(url) } if shouldLog { log.Info("[pd] cannot update member from this url", @@ -1074,9 +1074,9 @@ func (c *serviceDiscovery) updateMemberWithResult() (memberUpdateResult, error) continue } } - c.logMemberTransportFailureRecovery(time.Now(), url) + c.logMemberAvailabilityFailureRecovery(time.Now(), url) c.updateURLs(members.GetMembers()) - c.memberTransportFailures.retainCurrentFailures(c.GetServiceURLs(), result.failedURLs) + c.memberAvailabilityFailures.retainCurrentFailures(c.GetServiceURLs(), result.failedURLs) return result, c.updateServiceClient(members.GetMembers(), members.GetLeader()) } @@ -1120,14 +1120,13 @@ func (c *serviceDiscovery) getClusterInfo(ctx context.Context, url string, timeo } func (c *serviceDiscovery) getMembers(ctx context.Context, url string, timeout time.Duration) (*pdpb.GetMembersResponse, error) { - members, _, err := c.getMembersWithTransportFailure(ctx, url, timeout) + members, _, err := c.getMembersWithAvailabilityFailure(ctx, url, timeout) return members, err } -// getMembersWithTransportFailure reports whether a failed request is eligible -// for transport-outage suppression. It preserves getMembers' response and -// error contract. -func (c *serviceDiscovery) getMembersWithTransportFailure( +// getMembersWithAvailabilityFailure reports whether a failed request is an +// availability failure. It preserves getMembers' response and error contract. +func (c *serviceDiscovery) getMembersWithAvailabilityFailure( ctx context.Context, url string, timeout time.Duration, @@ -1136,7 +1135,7 @@ func (c *serviceDiscovery) getMembersWithTransportFailure( defer cancel() cc, err := c.GetOrCreateGRPCConn(url) if err != nil { - return nil, isMemberDialTransportFailure(err), err + return nil, isMemberDialAvailabilityFailure(err), err } start := time.Now() defer func() { metrics.InternalCmdDurationGetMembers.Observe(time.Since(start).Seconds()) }() @@ -1150,7 +1149,7 @@ func (c *serviceDiscovery) getMembersWithTransportFailure( if err != nil { metrics.InternalCmdFailedDurationGetMembers.Observe(time.Since(start).Seconds()) attachErr := errors.Errorf("error:%s target:%s status:%s", err, cc.Target(), cc.GetState().String()) - return nil, isMemberRPCTransportFailure(err), errs.ErrClientGetMember.Wrap(attachErr).GenWithStackByCause() + return nil, isMemberRPCAvailabilityFailure(err), errs.ErrClientGetMember.Wrap(attachErr).GenWithStackByCause() } val := res.Val members := val.(*pdpb.GetMembersResponse) @@ -1163,28 +1162,28 @@ func (c *serviceDiscovery) getMembersWithTransportFailure( case <-ctx.Done(): attachErr := errors.Errorf("error:%s target:%s status:%s", ctx.Err(), cc.Target(), cc.GetState().String()) metrics.InternalCmdFailedDurationGetMembers.Observe(time.Since(start).Seconds()) - return nil, isMemberRPCTransportFailure(ctx.Err()), errs.ErrClientGetMember.Wrap(attachErr).GenWithStackByCause() + return nil, isMemberRPCAvailabilityFailure(ctx.Err()), errs.ErrClientGetMember.Wrap(attachErr).GenWithStackByCause() } } -func (c *serviceDiscovery) logMemberTransportFailureRecovery(now time.Time, url string) { - recovery, ok := c.memberTransportFailures.recover(now, url) +func (c *serviceDiscovery) logMemberAvailabilityFailureRecovery(now time.Time, url string) { + recovery, ok := c.memberAvailabilityFailures.recover(now, url) if !ok { return } - log.Info("[pd] member transport failure recovered", + log.Info("[pd] member availability failure recovered", zap.String("url", url), zap.Duration("failure-duration", recovery.failureDuration), zap.Uint64("failed-attempts", recovery.failedAttempts), zap.Uint64("suppressed-errors", recovery.suppressedErrors)) } -func (c *serviceDiscovery) logMemberTransportFailureSummary(now time.Time) { - summary, ok := c.memberTransportFailures.summary(now) +func (c *serviceDiscovery) logMemberAvailabilityFailureSummary(now time.Time) { + summary, ok := c.memberAvailabilityFailures.summary(now) if !ok { return } - log.Info("[pd] member transport failures are being suppressed", + log.Info("[pd] member availability failures are being suppressed", zap.Strings("failed-urls", summary.failedURLs), zap.Duration("longest-failure-duration", summary.longestFailureDuration), zap.Uint64("failed-attempts", summary.failedAttempts), diff --git a/client/servicediscovery/service_discovery_test.go b/client/servicediscovery/service_discovery_test.go index 0b90edd1f2..2e9a833dfc 100644 --- a/client/servicediscovery/service_discovery_test.go +++ b/client/servicediscovery/service_discovery_test.go @@ -499,16 +499,16 @@ func TestUpdateMemberWithResultPreservesErrorContract(t *testing.T) { const memberURL = "http://pd.test:2379" testCases := []struct { - name string - response func() (*pdpb.GetMembersResponse, error) - transport bool + name string + response func() (*pdpb.GetMembersResponse, error) + availability bool }{ { name: "rpc unavailable", response: func() (*pdpb.GetMembersResponse, error) { return nil, status.Error(codes.Unavailable, "dial tcp 192.0.2.1:2379: connection refused") }, - transport: true, + availability: true, }, { name: "response header error", @@ -538,7 +538,7 @@ func TestUpdateMemberWithResultPreservesErrorContract(t *testing.T) { t.Run(testCase.name, func(t *testing.T) { testServer.getMembers = testCase.response client := newMemberTestServiceDiscovery(ctx, cancel, memberURL, conn) - require.True(t, client.memberTransportFailures.record(time.Now(), memberURL)) + require.True(t, client.memberAvailabilityFailures.record(time.Now(), memberURL)) result, structuredErr := client.updateMemberWithResult() compatibilityErr := client.updateMember() @@ -546,10 +546,10 @@ func TestUpdateMemberWithResultPreservesErrorContract(t *testing.T) { require.Error(t, structuredErr) require.Equal(t, structuredErr.Error(), compatibilityErr.Error()) require.Equal(t, []string{memberURL}, result.failedURLs) - require.Equal(t, testCase.transport, result.transportFailureCount == 1) + require.Equal(t, testCase.availability, result.availabilityFailureCount == 1) - _, ok := client.memberTransportFailures.summary(time.Now()) - if testCase.transport { + _, ok := client.memberAvailabilityFailures.summary(time.Now()) + if testCase.availability { require.True(t, ok) } else { require.False(t, ok) @@ -602,7 +602,7 @@ func TestUpdateMemberRetainsOnlyFailuresObservedBeforeSuccess(t *testing.T) { } for _, memberURL := range memberURLs { client.clientConns.Store(memberURL, conn) - require.True(t, client.memberTransportFailures.record(time.Now(), memberURL)) + require.True(t, client.memberAvailabilityFailures.record(time.Now(), memberURL)) } result, err := client.updateMemberWithResult() @@ -610,7 +610,7 @@ func TestUpdateMemberRetainsOnlyFailuresObservedBeforeSuccess(t *testing.T) { require.Equal(t, int32(2), calls.Load()) require.Equal(t, []string{"http://pd-1.test:2379"}, result.failedURLs) - summary, ok := client.memberTransportFailures.summary(time.Now()) + summary, ok := client.memberAvailabilityFailures.summary(time.Now()) require.True(t, ok) require.Equal(t, []string{"http://pd-1.test:2379"}, summary.failedURLs) } @@ -750,7 +750,7 @@ func TestUpdateMemberLoopDegradedModeSafetySweepAndConnectionRecovery(t *testing syntheticMemberResponse.Store(true) memberUpdateCh <- time.Now() require.Eventually(t, func() bool { - _, failed := client.memberTransportFailures.summary(time.Now()) + _, failed := client.memberAvailabilityFailures.summary(time.Now()) return !failed && getMembersCalls.Load() == callsAfterSafetySweep+1 }, time.Second, 10*time.Millisecond) callsAfterSafetyRecovery := getMembersCalls.Load() @@ -780,7 +780,7 @@ func TestUpdateMemberLoopDegradedModeSafetySweepAndConnectionRecovery(t *testing // must resume the normal refresh loop without waiting for the periodic sweep. connectionAvailable.Store(true) require.Eventually(t, func() bool { - _, failed := client.memberTransportFailures.summary(time.Now()) + _, failed := client.memberAvailabilityFailures.summary(time.Now()) return !failed && getMembersCalls.Load() > callsAfterSecondFailureBatch }, 2*time.Second, 10*time.Millisecond) callsAfterRecovery := getMembersCalls.Load()