-
-
Notifications
You must be signed in to change notification settings - Fork 945
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
Support redis queue expiration #2251
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2251 +/- ##
==========================================
- Coverage 81.54% 81.51% -0.04%
==========================================
Files 77 77
Lines 9517 9539 +22
Branches 1154 1156 +2
==========================================
+ Hits 7761 7776 +15
- Misses 1564 1570 +6
- Partials 192 193 +1 ☔ View full report in Codecov by Sentry. |
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.
You can run tox -e lint
locally to check the lint errors instead of waiting for the CI
@Nusnus would appreciate feedback and guideline on the further actions. Should I cover by unit tests the code additionally? Since it seems covered in integration tests but codecov is not aware of those |
I will need to properly review this change which might take some time (days to week or two). In the meantime, and until I'll add it to Kombu's CI later this year, you can check manually if the entire test suits of Celery are passing with a patched Kombu running this change. Unit, integration and smoke tests. You don't need to add any new test, just make sure nothing is broken due to the changes of this PR. Let me know if you need any assistance running the CI locally or configuring tox for your patched Kombu. TL;DR remove Kombu from all req.txt files in Celery and add to the |
return None | ||
|
||
def prepare_queue_arguments(self, arguments, **kwargs): | ||
return to_rabbitmq_queue_arguments(arguments, **kwargs) |
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.
should we just return a rabbitmq function or think something else?
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.
it's rather canonical ampq view of queue_arguments, also mongodb uses exactly the same setup so I assumed it should be fine for redis as well
Redis Queue Expiration Feature
Description
This PR adds support for automatically expiring Redis queues after a period of inactivity. This is particularly useful for managing ephemeral queues that should be automatically cleaned up when they're no longer being used.
The implementation adds the following capabilities:
x-expires
queue argument for Redis transport, specifying milliseconds after which an inactive queue is deletedexpires
parameter onQueue
objects (in seconds) which is translated tox-expires
internallyImplementation
The implementation adds two key methods to the Redis transport Channel class:
_maybe_update_queues_expire
: Updates TTL on queue keys when there is activity_get_queue_expire
: Extracts the expiration time from queue argumentsThese methods ensure that Redis queue keys have TTL set appropriately based on the queue definition, and that the TTL gets refreshed on both get and put operations.
Testing
Added comprehensive integration tests that verify:
Documentation
Added documentation of the
x-expires
option to the module docstring in the Transport Options section.Usage Example