Skip to content
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

feat: upgrade go-redis to v9 #3088

Merged
merged 15 commits into from
Jan 13, 2024
Merged

feat: upgrade go-redis to v9 #3088

merged 15 commits into from
Jan 13, 2024

Conversation

kevwan
Copy link
Contributor

@kevwan kevwan commented Apr 2, 2023

  • update go-redis to v9
  • fix the Hook interface changes
  • fix test failure, waiting v9 to fix data race bug split go test into two parts: normal and race, to bypass redis v9 race issue
  • update go.mod in goctl, but depends on go-zero release
    • release go-zero version first, the version that uses v9
    • then change the go.mod and release goctl

@kevwan kevwan requested a review from zcong1993 April 2, 2023 03:31
@kevwan kevwan marked this pull request as draft April 2, 2023 03:31
@kevwan kevwan added this to the v1.5.3 milestone Apr 24, 2023
@zcong1993
Copy link
Member

wait for redis v9 fix race bug redis/go-redis#2557

@codecov
Copy link

codecov bot commented Oct 7, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7822a4c) 94.67% compared to head (b0b1802) 94.69%.

Additional details and impacted files
Files Coverage Δ
core/stores/redis/hook.go 100.00% <100.00%> (+3.50%) ⬆️
core/stores/redis/metrics.go 100.00% <ø> (ø)
core/stores/redis/redis.go 98.30% <100.00%> (ø)
core/stores/redis/redisblockingnode.go 91.42% <ø> (ø)
core/stores/redis/redisclientmanager.go 81.25% <ø> (ø)
core/stores/redis/redisclustermanager.go 85.71% <ø> (ø)
core/stores/redis/redislock.go 81.39% <ø> (ø)

@zcong1993
Copy link
Member

wait for redis v9 fix race bug redis/go-redis#2557

@kevwan This issue has been pending for a long time, so we need to find a way to bypass it. We can split go test command into two parts: normal test and race test:

  1. normal test run without -race for all packages test and collect coverage
  2. race test run with packages which not depend on redis v9
    a. ignore whole packages that rely heavily on redis v9, eg: stores/redis, stores/sqlc
    b. add //go:build !race tags for other package test files which depend on redis v9, we run race test with -tags=race so the files will be ignored

@zcong1993 zcong1993 self-assigned this Oct 7, 2023
@zcong1993 zcong1993 marked this pull request as ready for review October 7, 2023 17:58
@zcong1993
Copy link
Member

wait for redis v9 fix race bug redis/go-redis#2557

@kevwan This issue has been pending for a long time, so we need to find a way to bypass it. We can split go test command into two parts: normal test and race test:

  1. normal test run without -race for all packages test and collect coverage
  2. race test run with packages which not depend on redis v9
    a. ignore whole packages that rely heavily on redis v9, eg: stores/redis, stores/sqlc
    b. add //go:build !race tags for other package test files which depend on redis v9, we run race test with -tags=race so the files will be ignored

go-redis fixed race issue in v9.3.1 redis/go-redis#2814, we only need upgrade version now.

@kevwan kevwan merged commit 368caa7 into zeromicro:master Jan 13, 2024
6 checks passed
@kevwan kevwan deleted the feat/redis-v9 branch January 13, 2024 14:41
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.

None yet

2 participants