Skip to content

bump redis to latest v9 and ratelimit to v10 #204

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: v3
Choose a base branch
from

Conversation

lilien1010
Copy link
Collaborator

@lilien1010 lilien1010 commented May 1, 2025

close #203
close #200

Bump github.com/bsm/redislock
Bump github.com/go-redis/redis_rate/v10 v10.0.1
Bump github.com/redis/go-redis/v9 v9.8.0

Also fix when connect to redis 7.x
taskq: 2025/05/01 16:29:01 queue.go:305: redisq: clean_zombie_consumers failed: redis: got 8 elements in XINFO CONSUMERS reply, wanted 6

@lilien1010 lilien1010 force-pushed the fix-redis-xinfo-param branch 4 times, most recently from b003fda to 4de783f Compare May 1, 2025 12:33
@lilien1010 lilien1010 requested review from Copilot and vmihailenco May 1, 2025 12:33
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request updates dependency versions for Redis and Redis rate limiting, aligning the code with the new go-redis v9 and redis_rate v10 APIs. Key changes include:

  • Switching import paths from go-redis/redis/v8 to redis/go-redis/v9.
  • Updating redis_rate from v9 to v10 in multiple files.
  • Adjusting method names and signatures to match the updated APIs (e.g. XTrim to XTrimMaxLen and changes from pointer to value for redis.Z).

Reviewed Changes

Copilot reviewed 13 out of 17 changed files in this pull request and generated no comments.

Show a summary per file
File Description
taskq.go Updated Redis client import and extended the Redis interface with new methods.
redisq/queue.go Updated method names/signatures (e.g., XTrim -> XTrimMaxLen, ZAdd parameter change).
queue.go Bumped redis_rate dependency to v10.
example_test.go Updated redis_rate dependency version.
example/sqsexample/tasks.go Updated Redis client import.
example/redisexample/tasks.go Updated Redis client import.
consumer_test.go Revised imports and grouped constant declarations.
consumer.go Updated redis_rate dependency.
.github/workflows/build.yml Expanded supported Go version matrix to include newer versions.
Files not reviewed (4)
  • example/redisexample/go.mod: Language not supported
  • example/sqsexample/go.mod: Language not supported
  • extra/taskqotel/go.mod: Language not supported
  • go.mod: Language not supported
Comments suppressed due to low confidence (2)

redisq/queue.go:264

  • Ensure that the renamed method 'XTrimMaxLen' accurately replicates the behavior of the previous 'XTrim', so that stream trimming behaves as expected with the updated Redis client API.
_ = q.redis.XTrimMaxLen(ctx, q.stream, 0).Err()

redisq/queue.go:173

  • Verify that changing the ZAdd parameter from a pointer to a value (from &redis.Z to redis.Z) is consistent with the updated go-redis v9 API and does not introduce side effects in error handling or value comparisons.
return pipe.ZAdd(msg.Ctx, q.zset, redis.Z{

@lilien1010
Copy link
Collaborator Author

Seems always the test case TestRedisqAckMessage is a little bit noise @vmihailenco

@lilien1010 lilien1010 force-pushed the fix-redis-xinfo-param branch 2 times, most recently from 12b29f3 to 23b5110 Compare May 1, 2025 14:55
@lilien1010 lilien1010 force-pushed the fix-redis-xinfo-param branch from 23b5110 to 6b6fc37 Compare May 1, 2025 15:02
@lilien1010
Copy link
Collaborator Author

lilien1010 commented May 2, 2025

CC @rymurr

Seems the test TestRedisqAckMessage is a little bit noisey, I can't find the reason, might also help to verify?

@lilien1010 lilien1010 closed this May 3, 2025
@lilien1010 lilien1010 reopened this May 3, 2025
@piavgh
Copy link

piavgh commented Jun 3, 2025

@lilien1010 : It looks like I can not use taskq with a different client like https://github.com/redis/rueidis?

Because, as I checked, the taskq.Redis interface depends directly on go-redis/v8

This is not a good design IMHO, cc @vmihailenco also

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

XINFO not compatible with Redis 7 v4 is not working with github.com/redis/go-redis/[email protected]
2 participants