Refactor gitea internal event queues to use external library #8133

Closed
opened 2025-11-02 07:54:55 -06:00 by GiteaMirror · 6 comments
Owner

Originally created by @lafriks on GitHub (Nov 18, 2021).

Maybe we could use watermill library to replace (at least parts of) our queue code https://github.com/ThreeDotsLabs/watermill

Originally created by @lafriks on GitHub (Nov 18, 2021). Maybe we could use watermill library to replace (at least parts of) our queue code https://github.com/ThreeDotsLabs/watermill
GiteaMirror added the type/refactoring label 2025-11-02 07:54:55 -06:00
Author
Owner

@lunny commented on GitHub (Nov 19, 2021):

There are something we have to consider:

  1. looks like it doesn't support unique message
  2. the message sequence is not guaranteed
  3. there is no redis support which may break old version
@lunny commented on GitHub (Nov 19, 2021): There are something we have to consider: 1) looks like it doesn't support unique message 2) the message sequence is not guaranteed 3) there is no redis support which may break old version
Author
Owner

@lafriks commented on GitHub (Nov 19, 2021):

  1. Unique messages can be implemented as router middleware
  2. Message sequence is really dependent on queue provider and mostly is not that important imho
  3. This could be implemented and contributed as additional provider to library (ThreeDotsLabs/watermill#110)
@lafriks commented on GitHub (Nov 19, 2021): 1. Unique messages can be implemented as router middleware 2. Message sequence is really dependent on queue provider and mostly is not that important imho 3. This could be implemented and contributed as additional provider to library (ThreeDotsLabs/watermill#110)
Author
Owner

@zeripath commented on GitHub (Nov 21, 2021):

I think we need specific reasons as to what is hoped to be gained and what's missing from queues.

It's one thing to suggest a drop-in work queue replacement. It's quite another to suggest completely refactoring.

From glancing at the repo this library is mostly focused on pubsub message queues. I think there's definitely something in being able to use pubsub for querying our work queues - but that is something different from what our work queues do.

For a start it really is worth noting that pub sub absolutely blocks a Redis connection - so you need one Redis connection per queue. At present we avoid this. This would limit the number of queues we could use.

It may well be that we could be inspired by watermill or even integrate it into our queue infrastructure but it doesn't seem suitable at first glance as a replacement. Instead this would require a either a complete refactor to use pure MQs or the creation of a separate MQ.

Now how could we use a pure MQ as a unique queue? Well we would have to either use a Redis/LevelDB Set on work retrieval or at message creation - or use some kind of DB locking on the records. That's a major refactor - potentially one that's worth it but it would likely mean that the simple API we currently have for the queues would go away. (Essentially our unique queue API as it stands is Push to the user and to the handler writer simply provide a handler.)

The alternative of adding a side channel event MQ with pubsub would allow us to keep our current queue infrastructure but allow the worker queue readers to be informed by messages on the event queue. I've been toying with the idea of a creating a broadcast queue for cluster events etc and a general MQ which could use pubsub to inform work queues but it seems likely that such a queue would be permanently alerted so wouldn't necessarily be particularly performant.

However I think you need to tell us why you think you'd want to do this and what you hope to gain.

We also need to have confidence that this project will still be there in 4, 2 - even 1 - year's time. Too many of Gitea's dependencies are in a poorly maintained state and we should be thinking much more carefully before adding any more.

@zeripath commented on GitHub (Nov 21, 2021): I think we need specific reasons as to what is hoped to be gained and what's missing from queues. It's one thing to suggest a drop-in work queue replacement. It's quite another to suggest completely refactoring. From glancing at the repo this library is mostly focused on pubsub message queues. I think there's definitely something in being able to use pubsub for querying our work queues - but that is something different from what our work queues do. For a start it really is worth noting that pub sub absolutely blocks a Redis connection - so you need one Redis connection per queue. At present we avoid this. This would limit the number of queues we could use. It may well be that we could be inspired by watermill or even integrate it into our queue infrastructure but it doesn't seem suitable at first glance as a replacement. Instead this would require a either a complete refactor to use pure MQs or the creation of a separate MQ. Now how could we use a pure MQ as a unique queue? Well we would have to either use a Redis/LevelDB Set on work retrieval or at message creation - or use some kind of DB locking on the records. That's a major refactor - potentially one that's worth it but it would likely mean that the simple API we currently have for the queues would go away. (Essentially our unique queue API as it stands is `Push` to the user and to the handler writer simply provide a handler.) The alternative of adding a side channel event MQ with pubsub would allow us to keep our current queue infrastructure but allow the worker queue readers to be informed by messages on the event queue. I've been toying with the idea of a creating a broadcast queue for cluster events etc and a general MQ which could use pubsub to inform work queues but it seems likely that such a queue would be permanently alerted so wouldn't necessarily be particularly performant. However I think you need to tell us why you think you'd want to do this and what you hope to gain. We also need to have confidence that this project will still be there in 4, 2 - even 1 - year's time. Too many of Gitea's dependencies are in a poorly maintained state and we should be thinking much more carefully before adding any more.
Author
Owner

@roblaszczak commented on GitHub (Nov 22, 2021):

Hey @lafriks @zeripath, author of Watermill here!

If you are happy to explore deeper if Watermill may fit your use case I'm happy to assist. I'm also happy to look deeper to this RedisStream implementation (as it's not an official one). If the implementation quality would be good enough we could ask the author if it could be included in the list of official implementations.

From glancing at the repo this library is mostly focused on pubsub message queues. I think there's definitely something in being able to use pubsub for querying our work queues - but that is something different from what our work queues do.

Do you have any high-level overview of how you are using Redis? In general core library is pretty flexible on Pub/Sub model and it depends a lot on the specific implementation of Pub/Sub.

@roblaszczak commented on GitHub (Nov 22, 2021): Hey @lafriks @zeripath, author of Watermill here! If you are happy to explore deeper if Watermill may fit your use case I'm happy to assist. I'm also happy to look deeper to [this](https://github.com/wyxloading/watermill-redisstream) RedisStream implementation (as it's not an official one). If the implementation quality would be good enough we could ask the author if it could be included in the list of official implementations. > From glancing at the repo this library is mostly focused on pubsub message queues. I think there's definitely something in being able to use pubsub for querying our work queues - but that is something different from what our work queues do. Do you have any high-level overview of how you are using Redis? In general core library is pretty flexible on Pub/Sub model and it depends a lot on the specific implementation of Pub/Sub.
Author
Owner

@zeripath commented on GitHub (Nov 22, 2021):

@roblaszczak thanks for getting in touch.

We're not currently using pubsub as the queues aren't really message queues - although they could be used that way and the intention of the queue infrastructure was to move us over time to a place where we could use event queues.

We have essentially two types of work queue - non-unique queues which use a Redis keyed List using RPush, LPop and LLen, and unique queues which wrap a Redis Set around the List. (using SAdd, SRem and SIsMember). The Redis lists are simply polled instead of using pubsub as that blocks redis connections and we're avoiding opening too many connections at present. (This is not ideal but appears to be functional.) We have a leveldb implementation which we use in a similar way.


I would propose that instead of using windmill directly immediately that we would simply create a windmill queue provider (at least in the first instance.) That would significantly minimize the size of PR to add this functionality and would make things a lot simpler. We'd need to think carefully about configuration and how it would be displayed to the user.

Allowing the non-unique work queues to use a windmill provider would be simple - create an event each time there is a push to a queue and wrap the message with an envelope of which work queue it was sent to. We could easily create a worker for each message type which could be one of our currently created dynamic worker pools or, if windmill supports its own we could let windmill do that itself.

Now, the unique work queues would be a bit more difficult - but I guess we could have a unique work queue handle their sets somewhere else and either have the unique message handler deal with the set or send a message back to a specific thing listening to remove things from the set.

(Presumably windmill has some lifecycle controls or allows subscribers to register and deregister on their own accord to allow us to correctly and safely manage shutdown of Gitea by shutting down workers independent of the brokers.)

One final consideration is that we may need some way of querying if the work queue is empty and to flush the work queue. The things that get pushed on to the work queue may unfortunately change between versions so it would be useful to flush them before shutting down for upgrade etc. I'm not sure if windmill has that functionality at a glance. If not we'd need to manage counts of items in each work queue separate from the messages similar to the sets above.


I think it would be quite simple to create a suitable redisstream provider and subscriber ourselves if the mentioned project wasn't ready - we'd likely need to do some of the work ourselves anyway. So the lack of a redis subscriber is not a problem. We'd also need to adjust our modules/nosql/manager_redis.go to give us a new client for each subscriber because of the above blocking.

@zeripath commented on GitHub (Nov 22, 2021): @roblaszczak thanks for getting in touch. We're not currently using pubsub as the queues aren't really message queues - although they could be used that way and the intention of the queue infrastructure was to move us over time to a place where we could use event queues. We have essentially two types of work queue - non-unique queues which use a Redis keyed List using RPush, LPop and LLen, and unique queues which wrap a Redis Set around the List. (using SAdd, SRem and SIsMember). The Redis lists are simply polled instead of using pubsub as that blocks redis connections and we're avoiding opening too many connections at present. (This is not ideal but appears to be functional.) We have a leveldb implementation which we use in a similar way. --- I would propose that instead of using windmill directly immediately that we would simply create a windmill queue provider (at least in the first instance.) That would significantly minimize the size of PR to add this functionality and would make things a lot simpler. We'd need to think carefully about configuration and how it would be displayed to the user. Allowing the non-unique work queues to use a windmill provider would be simple - create an event each time there is a push to a queue and wrap the message with an envelope of which work queue it was sent to. We could easily create a worker for each message type which could be one of our currently created dynamic worker pools or, if windmill supports its own we could let windmill do that itself. Now, the unique work queues would be a bit more difficult - but I guess we could have a unique work queue handle their sets somewhere else and either have the unique message handler deal with the set or send a message back to a specific thing listening to remove things from the set. (Presumably windmill has some lifecycle controls or allows subscribers to register and deregister on their own accord to allow us to correctly and safely manage shutdown of Gitea by shutting down workers independent of the brokers.) One final consideration is that we may need some way of querying if the work queue is empty and to flush the work queue. The things that get pushed on to the work queue may unfortunately change between versions so it would be useful to flush them before shutting down for upgrade etc. I'm not sure if windmill has that functionality at a glance. If not we'd need to manage counts of items in each work queue separate from the messages similar to the sets above. --- I think it would be quite simple to create a suitable redisstream provider and subscriber ourselves if the mentioned project wasn't ready - we'd likely need to do some of the work ourselves anyway. So the lack of a redis subscriber is not a problem. We'd also need to adjust our modules/nosql/manager_redis.go to give us a new client for each subscriber because of the above blocking.
Author
Owner

@wxiaoguang commented on GitHub (May 9, 2023):

The queue has been completely rewritten.

The only blocker is the Gitea's specialized "unique queue" (actually, it doesn't make sense)

So I think this issue could be closed, the new code is much easier to maintain.

@wxiaoguang commented on GitHub (May 9, 2023): The queue has been completely rewritten. The only blocker is the Gitea's specialized "unique queue" (actually, it doesn't make sense) So I think this issue could be closed, the new code is much easier to maintain.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/gitea#8133