Skip to content
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

Remove the "state" from a PullConsumer #42

Open
mmmries opened this issue Mar 26, 2022 · 2 comments
Open

Remove the "state" from a PullConsumer #42

mmmries opened this issue Mar 26, 2022 · 2 comments

Comments

@mmmries
Copy link
Owner

mmmries commented Mar 26, 2022

I'm working on #36 and thinking about #41 and one thing that feels a bit awkward to me is the fact that our init/1 callback returns a state term which is then passed in as an argument whenever a message is received.

This feels awkward to me because our PullConsumer can start at any point in the message history, and when we start supporting parallel message processing, which version of the state will we pass to the next message? Will we try to merge the returned state of the different messages that were running in parallel?

I'm thinking that because this consumer might be starting at any point in the stream (including cases like a crash and restart), your application would have to be designed to manage long-lived state somewhere outside the PullConsumer anyway? But, maybe I'm just missing some context about how you plan to use this in the use case that @mkaput and @sswrk are working towards?

@mkaput
Copy link
Collaborator

mkaput commented Mar 29, 2022

This feels awkward to me because our PullConsumer can start at any point in the message history, and when we start supporting parallel message processing, which version of the state will we pass to the next message? Will we try to merge the returned state of the different messages that were running in parallel?

See my response here: #41 (comment).

My idea for PullConsumer was to model it after GenServer and generally support all patterns of that behaviour. GenServer has a state, so this is why PullConsumer also has it 😄 Of course this design may not be visible yet because some things are missing: #28 #45 #46

I'm thinking that because this consumer might be starting at any point in the stream (including cases like a crash and restart), your application would have to be designed to manage long-lived state somewhere outside the PullConsumer anyway? But, maybe I'm just missing some context about how you plan to use this in the use case that @mkaput and @sswrk are working towards?

We cannot assume anything about the state term value and also how to serialize or persist it. Or even whether user wants to persist the state. With pure OTP gen_server you must do this manually in your server code. State should be saved in terminate/2 callback and restored in init/1. I would recommend same thing for our PullConsumer.

@brandynbennett
Copy link
Collaborator

I think if the Jetstream.PullConsumer is trying to have a GenServer-like interface then we should keep the state part of the callbacks. It's possible it could be used to store the messages in memory, but it won't necessarily be used that way. It could also store domain-specific information that could be useful in processing messages and we would want to allow developers access to that information in the callbacks. I also think it's best to not assume what will be in the state value and leave the responsibility handling and persisting message to the library users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants