-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Use an abstract lock layer to allow distributed lock between multiple Gitea instances #26486
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am absolutely not a fan of the variable naming in this PR.
@@ -1395,6 +1395,11 @@ However, later updates removed those options, and now the only options are `gith | |||
However, if you want to use actions from other git server, you can use a complete URL in `uses` field, it's supported by Gitea (but not GitHub). | |||
Like `uses: https://gitea.com/actions/checkout@v3` or `uses: http://your-git-server/actions/checkout@v3`. | |||
|
|||
## Sync (`sync`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a small description of what this section does would be useful.
Sync
is such a universal term that it could mean anything, i.e. "PR reloading settings", "syncs with external mirrors", …
Perhaps we even need to think about renaming this section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we just call it lock
here, or maybe cluster
Co-authored-by: delvh <[email protected]>
Co-authored-by: delvh <[email protected]>
Co-authored-by: delvh <[email protected]>
func newRedisLockService(connection string) *redisLockService { | ||
client := nosql.GetManager().GetRedisClient(connection) | ||
|
||
pool := goredis.NewPool(client) // or, pool := redigo.NewPool(...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is the comment necessary
} | ||
defer func() { | ||
if _, err := lock.Unlock(); err != nil { | ||
log.Error("lock.Unlock: %v", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we want to move the log to inside redisLockService, this "lock.Unlock" doesn't really provide information specific to this function, and then we can just defer lock.Unlock()
here
} | ||
defer func() { | ||
_, err := lock.Unlock() | ||
if err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_, err := lock.Unlock(); err != nil {
@@ -1395,6 +1395,11 @@ However, later updates removed those options, and now the only options are `gith | |||
However, if you want to use actions from other git server, you can use a complete URL in `uses` field, it's supported by Gitea (but not GitHub). | |||
Like `uses: https://gitea.com/actions/checkout@v3` or `uses: http://your-git-server/actions/checkout@v3`. | |||
|
|||
## Sync (`sync`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we just call it lock
here, or maybe cluster
## Sync (`sync`) | ||
|
||
- `LOCK_SERVICE_TYPE`: **memory**: Lock service type, could be `memory` or `redis` | ||
- `LOCK_SERVICE_CONN_STR`: **\<empty\>**: Ignored when `LOCK_SERVICE_TYPE` is `memory`. For `redis` use something like `redis://127.0.0.1:6379/0` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example is different from test, which is addrs=127.0.0.1:6379 db=0
… between multiple Gitea instances (#31813) Replace #26486 Fix #19620 --------- Co-authored-by: Jason Song <[email protected]>
Fix #19620
Replace #22176