- 
                Notifications
    
You must be signed in to change notification settings  - Fork 20
 
feat: add pause/resume consumer api #129
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
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.
@ShogunPanda ptal
| 
           As an API, I would actually prefer to have a new method that provides a Node.js stream per partition, which would be able to independently have its own backpressure mechanism. That would provide a better DX.  | 
    
          
 I think that depends on the usecase, we actually prefer to have pause/resume because if we want to consume messages in some past datetime range, we don't want to continue consuming from partitions that already satisfied the end date, it just adds unnecessary overhead. So, it's not only useful for backpressure control, and we only have to deal with 1 stream. Just to clarify, I don't find separate streams a bad DX, but rather difficult to implement, at least for me.  | 
    
93ef7ee    to
    44a6e28      
    Compare
  
    Signed-off-by: Mirza Brunjadze <[email protected]>
f7208ed    to
    2ce88da      
    Compare
  
    | 
           I also agree with @mcollina. Given we have followed a  But, @xxzefgh I see your use case. I would add the logic to the  
 Then you modify  Note that if a  This also implies that a call to  Also, use just  What do you think? Are you willing to implement this?  | 
    
| 
           @ShogunPanda Sure, I can get it done. Would you be willing to accept   | 
    
| 
           Amazing!  | 
    
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.
(already answered - Requesting for changes just to make sure this isn't merged by mistake)
Summary
Added pause/resume APIs for consumer
Related issue: #128
Decided to not mess with streams, as a result
.pausewill not have an immediate effect and only prevents next fetch requests.API signature was basically copied from Java client, with added
isPausedfor fast lookups since it's also used internally.Java's API also allows to easily pause all assignments withconsumer.pause(consumer.assignment()), so maybe we should consider changingGroupAssignmenttype or reusing it here.Disclaimer: part of the tests and documentation was written with AI assistance