feat: watch <-context.Done

[monkey92t]

This is an experimental PR, See #2276

[monkey92t]

We still need to keep improving it.

[monkey92t]

benchmark:

redis-benchmark:

monkey@MonkeyiMac redis% ~/redis/redis-7.0.8/src/redis-benchmark -h 127.0.0.1 -p 6379 -c 20 -n 1000000
====== PING_INLINE ======                                                   
  1000000 requests completed in 10.91 seconds
  20 parallel clients
  3 bytes payload
  keep alive: 1
  ............
PING_INLINE: rps=91605.0 (overall: 90421.0) avg_msec=0.129 (overall: 0.134) 

go-redis/[email protected] (10 times average):

ContextTimeoutEnabled = false
n = 1000000, c = 20, bad = 0, ts = 12.10041 seconds, rps = 83822.4786095

Current PR (10 times average):

ContextTimeoutEnabled = true 
n = 1000000, c = 20, bad = 0, ts = 12.4465 seconds, rps = 80824.902

Current PR (10 times average):

ContextTimeoutEnabled = false
n = 1000000, c = 20, bad = 0, ts = 11.652 seconds, rps = 86318.0314

[monkey92t]

Tests occasionally fail:

panic: repeat watchCancel

goroutine 8110 [running]:
github.com/redis/go-redis/v9/internal/pool.(*Conn).WatchCancel(0xc0007fa140?, {0xa665d8?, 0xc0009eaf00?})
	/Users/monkey/Desktop/redis/internal/pool/conn.go:97 +0xc5
github.com/redis/go-redis/v9.(*baseClient).withConn(0xc0007fa140, {0xa665d8, 0xc0009eaf00}, 0xc0006ffd60)
	/Users/monkey/Desktop/redis/redis.go:352 +0xf8
github.com/redis/go-redis/v9.(*baseClient)._process(0xc0007fa140, {0xa665d8, 0xc0009eaf00}, {0xa69878, 0xc000723200}, 0x2?)
	/Users/monkey/Desktop/redis/redis.go:388 +0xe5
github.com/redis/go-redis/v9.(*baseClient).process(0xc0007fa140, {0xa665d8, 0xc0009eaf00}, {0xa69878, 0xc000723200})
	/Users/monkey/Desktop/redis/redis.go:370 +0x78
github.com/redis/go-redis/v9.(*hooksMixin).processHook(...)
	/Users/monkey/Desktop/redis/redis.go:173
github.com/redis/go-redis/v9.(*Client).Process(0xc0006ffe88?, {0xa665d8?, 0xc0009eaf00?}, {0xa69878?, 0xc000723200})
	/Users/monkey/Desktop/redis/redis.go:645 +0x43
github.com/redis/go-redis/v9.cmdable.Ping(0xc00072d170, {0xa665d8?, 0xc0009eaf00})
	/Users/monkey/Desktop/redis/commands.go:542 +0xd7
github.com/redis/go-redis/v9.(*ringSharding).Heartbeat(0xc000903880, {0xa665d8, 0xc0009eaf00}, 0xc000538480?)
	/Users/monkey/Desktop/redis/ring.go:400 +0x1df
created by github.com/redis/go-redis/v9.NewRing
	/Users/monkey/Desktop/redis/ring.go:522 +0x39e
FAIL	github.com/redis/go-redis/v9	91.143s

This doesn't seem to be a problem with WatchCancel, maybe a Ring?

[monkey92t]

Tested and it works fine 😁

[armsnyder]

I ran my new tests from #2433 against this branch, and one is failing: PubSub canceled context [It] should unblock ReceiveMessage. That being said, this is still a huge improvement! 😄

go-redis/pubsub_test.go

Lines 572 to 590 in 462f545

	Describe("canceled context", func() {
	It("should unblock ReceiveMessage", func() {
	pubsub := client.Subscribe(ctx, "mychannel")
	defer pubsub.Close()
	ctx2, cancel := context.WithCancel(ctx)
	errCh := make(chan error, 1)
	go func() {
	_, err := pubsub.ReceiveMessage(ctx2)
	errCh <- err
	}()
	var gotErr error
	Consistently(errCh).ShouldNot(Receive(&gotErr), "Received %v", gotErr)
	cancel()
	Eventually(errCh).Should(Receive(&gotErr))
	Expect(gotErr).To(HaveOccurred())
	})
	})

[monkey92t]

I ran my new tests from #2433 against this branch, and one is failing: PubSub canceled context [It] should unblock ReceiveMessage. That being said, this is still a huge improvement! 😄

go-redis/pubsub_test.go

Lines 572 to 590 in 462f545

	Describe("canceled context", func() {
	It("should unblock ReceiveMessage", func() {
	pubsub := client.Subscribe(ctx, "mychannel")
	defer pubsub.Close()
	ctx2, cancel := context.WithCancel(ctx)
	errCh := make(chan error, 1)
	go func() {
	_, err := pubsub.ReceiveMessage(ctx2)
	errCh <- err
	}()
	var gotErr error
	Consistently(errCh).ShouldNot(Receive(&gotErr), "Received %v", gotErr)
	cancel()
	Eventually(errCh).Should(Receive(&gotErr))
	Expect(gotErr).To(HaveOccurred())
	})
	})

I deliberately ignored the processing of pubsub. Compared with the redis command, pubsub is a special working mode.

[vmihailenco]

I have rather poor experience supporting context timeouts in Redis/PostgreSQL client, i.e. it often makes more harm than good: see 1 and 2

Also reviewing such code is a nightmare. Let's keep this on hold.

[zydovech]

Hello , Can I ask if there are any further plans for this? I hope it can support ctx cancel 。