Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Add cooperative rebalance support#907

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
thynson wants to merge3 commits intoBlizzard:master
base:master
Choose a base branch
Loading
fromthynson:cooperative-rebalance-support

Conversation

@thynson
Copy link

No description provided.

@thynson
Copy link
Author

incremetalAssign andincrementalUnassign are already exposed to Node, and anyone can call them in a rebalance callback whenpartition.assignment.strategy iscooperative-sticky.
Does theKafkaConsumer need to be also modified to provide a defaultrebalance_cb forcooperative-sticky?

@thynsonthynsonforce-pushed thecooperative-rebalance-support branch fromce559e0 to21a2226CompareAugust 12, 2021 06:40
@stale
Copy link

stalebot commentedJan 8, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stalestalebot added the staleStale issues labelJan 8, 2022
@stalestalebot closed thisMar 2, 2022
@johncsnyder
Copy link

johncsnyder commentedMar 5, 2022•
edited
Loading

I would love to see this feature added. 🙏

As far as I can tell, thecooperative-sticky partition assignment strategy can't work without adding the bindings for incremental assign/unassign.

I've tried but always received the errorapplication *assign() call failed: Changes to the current assignment must be made using incremental_assign() or incremental_unassign() when rebalance protocol type is COOPERATIVE.

@thynson are there any plans to re-open this PR?

@thynson
Copy link
Author

thynson commentedMar 5, 2022•
edited
Loading

As far as I can tell, the cooperative-sticky partition assignment strategy can't work without adding the bindings for incremental assign/unassign.

They were implemented in this PR,21a2226#diff-e574f6617d5a1bd9fb35fab6b02e906b120024e0dcae87b010eed716e23820d4R45, but you need to setpartition.assignment.strategy tocooperative-sticky. Or if you're using customrebalance_cb, callincrementalAssign andincrementalUnassign.

@johncsnyder
Copy link

@thynson Yeah, I saw, looks great.

Any chance this will be merged into master? I'd prefer not to maintain my own fork.

@thynson
Copy link
Author

thynson commentedMar 7, 2022•
edited
Loading

Any chance this will be merged into master? I'd prefer not to maintain my own fork.

If@iradul has time to take a look.

johncsnyder reacted with thumbs up emoji

@iraduliradul reopened thisMar 7, 2022
@stalestalebot removed the staleStale issues labelMar 7, 2022
@johncsnyder
Copy link

Sweet, thanks 🙏

@iradul
Copy link
Collaborator

@thynson, thank you for the PR

Copy link
Collaborator

@iraduliradul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Missing items:

  • assignment_lost implementation
  • e2e test(s)

@thynsonthynsonforce-pushed thecooperative-rebalance-support branch from21a2226 tod55d790CompareMarch 21, 2022 11:19
@thynson
Copy link
Author

I'll try to implement the missing parts, but I'm still busy for some days.

@micheleangioni
Copy link

Any hope this will get implemented?@thynson :)
It would be extremely useful for us

ryancoopersmith1, robinfehr, neuralspin, SashaYurchenko, Archonr, serj026, and SeanReece reacted with thumbs up emoji

@neuralspin
Copy link

@micheleangioni
Did you find a way to solve this? We also really need it@thynson )

SashaYurchenko, Archonr, vratov, dudinm, jurazubach, serj026, and votar1408 reacted with thumbs up emoji

@spalax
Copy link

and we need it a lot :)

SashaYurchenko, jurazubach, Archonr, dudinm, vratov, yura9111, serj026, and votar1408 reacted with thumbs up emoji

@serj026
Copy link
Contributor

We are eagerly awaiting this feature 🙏

aleksandrAndrosov

This comment was marked as outdated.

@notgosu
Copy link

we need it also :)

@votar1408
Copy link

It can save the world, great the feature!@micheleangioni

@yaroslav-fedyshyn-playson

Thanks for the PR. It will significantly reduce our struggling and help us!

@thynsonthynsonforce-pushed thecooperative-rebalance-support branch fromd55d790 to85193d8CompareMay 12, 2023 10:19
@thynsonthynsonforce-pushed thecooperative-rebalance-support branch from85193d8 to3c8e0ddCompareMay 12, 2023 10:21
@thynson
Copy link
Author

I did not have any experience of writing an e2e test for rdkafka. So any help would be great, or I have to take some time to finish the e2e test.

@ofekdeitch-oligo
Copy link

Hi@thynson , why can't we use the e2e tests that@serj026 implemented in his PR?
https://github.com/Blizzard/node-rdkafka/pull/1081/files

Maybe we can merge the two PRs and get this feature finally merged

SeanReece reacted with thumbs up emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@iraduliraduliradul left review comments

+1 more reviewer

@aleksandrAndrosovaleksandrAndrosovaleksandrAndrosov left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

12 participants

@thynson@johncsnyder@iradul@micheleangioni@neuralspin@spalax@serj026@notgosu@votar1408@yaroslav-fedyshyn-playson@ofekdeitch-oligo@aleksandrAndrosov
[8]ページ先頭
©2009-2025 Movatter.jp