Skip to content

Conversation

hazzadous
Copy link
Contributor

@hazzadous hazzadous commented Apr 24, 2023

For some reason some partitions aren't being consumed. So I'll simply revert this for now. We should, errrm, make it configurable next time, promise.

Reverts #15197

@hazzadous hazzadous changed the title Revert "chore(recordings): use cooperative-sticky rebalance strategy" revert(recordings): use cooperative-sticky rebalance strategy Apr 24, 2023
@hazzadous hazzadous enabled auto-merge (squash) April 24, 2023 14:53
@hazzadous hazzadous merged commit a40f011 into master Apr 24, 2023
@hazzadous hazzadous deleted the revert-15197-chore/use-cooperative-rebalance branch April 24, 2023 15:06
@pauldambra
Copy link
Member

I'm about 70% sure that even only setting rebalance_cb: true means we need to implement assignment behaviour in the callback we provide.

(I think we even have to seek the appropriate offset on the partition ourselves - but haven't figured out a good way to test/debug this)

@hazzadous
Copy link
Contributor Author

I'm about 70% sure that even only setting rebalance_cb: true means we need to implement assignment behaviour in the callback we provide.

Having a look here, it looks like it will use either eager or cooperative in the case that rebalance_cb is a boolean, and only if it's a function will it be expecting you to perform the rebalance yourself, does that sound right?

@hazzadous
Copy link
Contributor Author

And then here in the node based rebalance code it handles the emit of the rebalance event. And if you don't specify anything you'll not have any events emitted and librdkafka will handle the rebalance internally(?)

@pauldambra
Copy link
Member

Ah, and in the vanilla one it handles the boolean case https://github.com/Blizzard/node-rdkafka/blob/master/lib/kafka-consumer.js#L54

OK, 70% was an over-estimate 🤣

hazzadous pushed a commit that referenced this pull request Apr 26, 2023
Revert "revert(recordings): use cooperative-sticky rebalance strategy (#15211)"

This reverts commit a40f011.
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

Successfully merging this pull request may close these issues.

3 participants