- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 195
 
Add Redis Streams option for job delivery #451
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
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     #451      +/-   ##
==========================================
- Coverage   96.27%   95.93%   -0.35%     
==========================================
  Files          11       11              
  Lines        1074     1107      +33     
  Branches      209      199      -10     
==========================================
+ Hits         1034     1062      +28     
- Misses         19       23       +4     
- Partials       21       22       +1     
 Continue to review full report in Codecov by Sentry. 
  | 
    
| 
           @ajac-zero  This may need a unit-test with   | 
    
| 
           Sure thing @gaby . I was wondering how I should go about that... What I started doing was add a stream parameter to the worker tests, and then wrap them so they run twice, once with stream and once without. But I feel this might not be the best way to do things, maybe I should focus on some vital tests? What do you suggest?  | 
    
| 
           @ajac-zero That's probably a great starting point, running current tests with "stream" set to false. Then running the test suite with "stream" set to True. This will require setting the source of the data to use Streams.  | 
    
| 
           @gaby I finally got around to writing the unit tests. I added a new   | 
    
| 
           @samuelcolvin Thanks! Yes, it might be better to wait and build on top of the new version. I'm really looking forward to the new changes. The DAG and type safety sound awesome. Lmk if I can contribute in some way.  | 
    
| 
           Hi folks, thanks for the implementation, really a needful change. Is there any plan to merge this feat ?  | 
    
This pull request adds a basic implementation of Redis Streams, in order to avoid polling for new jobs in the worker and reduce latency, in accordance with objective 4 of issue #437.
To create a worker that listens to a Redis Stream, we can use the cli or specify it in the code directly.
CLI:
arq worker.WorkerSettings --streamCode:
On the client, they must specify that they want to deliver a job to a worker through a Redis Stream.
Here are the results of a very simple benchmark that showcases the potential of using Redis Streams for improved latency.
Polling:

Average time: 0.268s
Streaming:

Average time: 0.012s