Skip to content

Commit 162a154

Browse files
EXPEbdodlaBhargav Dodlandyakov
authored
fix: Fixed issue with context cancelled error leading to connection spikes on Primary instances (#3190)
* fix: Fixed issue with context cancelled error leading to connection spikes on Master * fix: Added tests * fix: Updated tests --------- Co-authored-by: Bhargav Dodla <[email protected]> Co-authored-by: Nedyalko Dyakov <[email protected]>
1 parent 8e4a2ee commit 162a154

File tree

3 files changed

+45
-1
lines changed

3 files changed

+45
-1
lines changed

error.go

+9
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,15 @@ type Error interface {
3838

3939
var _ Error = proto.RedisError("")
4040

41+
func isContextError(err error) bool {
42+
switch err {
43+
case context.Canceled, context.DeadlineExceeded:
44+
return true
45+
default:
46+
return false
47+
}
48+
}
49+
4150
func shouldRetry(err error, retryTimeout bool) bool {
4251
switch err {
4352
case io.EOF, io.ErrUnexpectedEOF:

osscluster.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -1350,7 +1350,9 @@ func (c *ClusterClient) processPipelineNode(
13501350
_ = node.Client.withProcessPipelineHook(ctx, cmds, func(ctx context.Context, cmds []Cmder) error {
13511351
cn, err := node.Client.getConn(ctx)
13521352
if err != nil {
1353-
node.MarkAsFailing()
1353+
if !isContextError(err) {
1354+
node.MarkAsFailing()
1355+
}
13541356
_ = c.mapCmdsByNode(ctx, failedCmds, cmds)
13551357
setCmdsErr(cmds, err)
13561358
return err

osscluster_test.go

+33
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,39 @@ var _ = Describe("ClusterClient", func() {
539539
AfterEach(func() {})
540540

541541
assertPipeline()
542+
543+
It("doesn't fail node with context.Canceled error", func() {
544+
ctx, cancel := context.WithCancel(context.Background())
545+
cancel()
546+
pipe.Set(ctx, "A", "A_value", 0)
547+
_, err := pipe.Exec(ctx)
548+
549+
Expect(err).To(HaveOccurred())
550+
Expect(errors.Is(err, context.Canceled)).To(BeTrue())
551+
552+
clientNodes, _ := client.Nodes(ctx, "A")
553+
554+
for _, node := range clientNodes {
555+
Expect(node.Failing()).To(BeFalse())
556+
}
557+
})
558+
559+
It("doesn't fail node with context.DeadlineExceeded error", func() {
560+
ctx, cancel := context.WithTimeout(context.Background(), 1*time.Nanosecond)
561+
defer cancel()
562+
563+
pipe.Set(ctx, "A", "A_value", 0)
564+
_, err := pipe.Exec(ctx)
565+
566+
Expect(err).To(HaveOccurred())
567+
Expect(errors.Is(err, context.DeadlineExceeded)).To(BeTrue())
568+
569+
clientNodes, _ := client.Nodes(ctx, "A")
570+
571+
for _, node := range clientNodes {
572+
Expect(node.Failing()).To(BeFalse())
573+
}
574+
})
542575
})
543576

544577
Describe("with TxPipeline", func() {

0 commit comments

Comments
 (0)