Skip to content

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
@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.

2 participants