replace celery State with lightweight uuid to name cache#21
replace celery State with lightweight uuid to name cache#21tulioz wants to merge 1 commit intoevent-watcher-reconnectfrom
Conversation
Current Aviator status
This PR was merged using Aviator. Stack
See the real-time status of this PR on the
Aviator webapp.
Use the Aviator Chrome Extension
to see the status of your PR within GitHub.
|
|
/gemini review |
b114a2a to
9815901
Compare
0557af8 to
372b1eb
Compare
There was a problem hiding this comment.
Code Review
This pull request replaces Celery's internal state tracking with a custom OrderedDict-based cache for task names in the EventWatcher class. Feedback suggests refactoring the hardcoded cache limit into a class-level constant and using the move_to_end method for more idiomatic cache management.
9815901 to
1e30667
Compare
372b1eb to
1bee059
Compare
|
/gemini review |
1e30667 to
f248e8a
Compare
1bee059 to
d181f72
Compare
There was a problem hiding this comment.
Code Review
This pull request replaces the dependency on Celery's internal state management with a custom LRU cache using an OrderedDict to map task UUIDs to names. This change simplifies the EventWatcher initialization and introduces a configurable cache limit to manage memory usage. Review feedback suggests optimizing the cache update logic by using move_to_end to properly refresh the LRU position for all task events and ensuring that the retrieved task name is never null to maintain compatibility with Prometheus labels.
celerymon only needs task name from the State object, this replaces the full celery State (10k Task objects with 30+ fields, Worker objects, WeakSets, taskheap, mutex) with a simple OrderedDict mapping uuid to task name