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

fix: redis reacquire #4319

Closed
wants to merge 5 commits into from
Closed

Conversation

seth-shi
Copy link
Contributor

see: TestRedisLock_SameAcquire test

@seth-shi
Copy link
Contributor Author

locker id just use as release key

@kevwan kevwan self-requested a review August 19, 2024 15:31
@kevwan kevwan self-assigned this Aug 19, 2024
@kevwan kevwan requested a review from zcong1993 August 19, 2024 15:32
@kevwan
Copy link
Contributor

kevwan commented Aug 19, 2024

Thanks for your contribution! Would you please resolve conflicts?

@kevwan kevwan changed the title Fix/redis reacquire fix: redis reacquire Aug 19, 2024
@seth-shi
Copy link
Contributor Author

done

Copy link

codecov bot commented Aug 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.11%. Comparing base (8690859) to head (531ccf5).
Report is 124 commits behind head on master.

Additional details and impacted files
Files Coverage Δ
core/stores/redis/redislock.go 86.48% <100.00%> (+5.09%) ⬆️

... and 254 files with indirect coverage changes

@zcong1993
Copy link
Member

zcong1993 commented Aug 19, 2024

go-zero's redis lock is a bit like etcd's lease.
When you first acquire a lock with a TTL, you own the lock, then you can do two things:

  1. release the lock
  2. refresh TTL of the lock, recall Acquire method if you own the lock, same like keepalive of etcd's lease

So the lock's owner can call Acquire multi times, and will always return true if you own the lock.

@seth-shi
Copy link
Contributor Author

that' s a good explanation, but I think the lock should be for other language users not to create ambiguity. because distributed locks can't be reowned even by themselves

@seth-shi
Copy link
Contributor Author

a typical example is 'redis set nx ex'
the effect is the same whether it's the same client or a different client call

@kevwan
Copy link
Contributor

kevwan commented Aug 21, 2024

It's better to do a rebase, which will make your changes easy to review.

@seth-shi seth-shi closed this Aug 21, 2024
@seth-shi seth-shi reopened this Aug 21, 2024
@seth-shi seth-shi closed this Aug 21, 2024
@seth-shi seth-shi mentioned this pull request Aug 21, 2024
@seth-shi
Copy link
Contributor Author

new pr: #4325

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


New on: https://github.com/zeromicro/joe-zero/pull/4325

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.

4 participants