-
Notifications
You must be signed in to change notification settings - Fork 20
draft of per-partition streaming #130
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
|
on the performance thing, i suppose it's possible we could do pause tracking using the internal state from #129, and move the |
|
I'm not exactly convinced about the DX of WDYT? |
|
def agree the current API is a little wonky, couple thoughts:
an additional issue i was just thinking about this morning was the |
|
About the DX I agree with @mcollina: let's destroy everything and create from scratch. Something like: Note that batching shouldn't be an issue as in-memory messages are still processed when the stream is gracefully closed. About the |
working on a rough pass at #128
current approach is intended to require minimal refactoring while achieving the developer-facing API i think probably will work best. some tests are not working yet, i will work on getting existing tests to pass and then add some for this new behavior if things seem reasonable.
as i dug into how to make multi-topic consumption and rebalancing work, i adjusted the DX from my initial code example in #128. the idea is to implement a new method,
consumeByPartition, that upon initial consume returns a map of topic-partition streams where the key is of the formtopic:partition. after a rebalance/rejoin, IF the assignment has changed such that the downstream app code needs to account for added/removed partitions, then a requiredonAssignmentChange()callback is called with a new map containing the full set of topic-partition streams:when the streams for individual topic-partitions detect that the consumer no longer has an assignment for their topic-partition after a join/rebalance, they automatically close themselves, so in most cases developers do not have to write bespoke logic for cleaning up removed partition streams. if their consuming app code handles the stream ending correctly, their code will do the right thing by default. the case that needs hand-written logic after a rebalance is creating new stream consumers in app code when new partitions are assigned. giving developers a full map of the new topic-partition assignments allows them to serve any advanced/unusual needs they may have around comparing old/new streams.
i originally considered adding a mode/config option to the existing
consume()method, and also considered adding a config at theConsumerclass level. but in both cases mixing the type signatures and logic between the two modes was complex, and i didn't feel that complexity was buying a whole lot in terms of DX. i also thing splitting it into a separate method has the nice side benefit of keeping the normalconsume()api very easy to read in getting started examples so the library stays approachable, while allowing more advanced users to reach for this method if they actually need it.the primary thing that i'd like to see improved about performance is that calls to listOffsets, listCommittedOffsets, and fetch all still operate at the individual stream level, which means that the number of requests to those operations now increases with the number of partitions instead of one-per-leader. I think to resolve that we'd have to move some of the logic for listing offsets/commits and doing fetching up above the level of individual MessageStreams and coordinate them more. Given that the main goal here is to decouple fetching backpressure between topic-partitions, we likely don't want to fully consolidate fetching together into one coordinated thing, but i can imagine there are better ways to do it than the naive way it works right now. listing offsets/commits only needs to happen on initial join and rebalancing so i think that one is the more important candidate for optimization since ideally we only make one call per rejoin.