Only commit when a rebalance is not in progress (Fix for Cooperative-sticky partition assignment strategy with manual commits)

[wmorgan6796]

This is an attempt to bring #4089 by @roxelo over the finish line. I believe the main difference from there is fixing the test case and bringing the PR up to date with latest master branch.

This should make cooperative-sticky work with manual commits.

See #4059

[roxelo]

@edenhill Are you still interested in this fix?

[wmorgan6796]

@emasab would this PR be of interest? As the sticky-cooperative partition assignment is essentially broken with manual commits, I was hoping I could get this reviewed. (@edenhill reviewed the linked PR before so I don't foresee many issues)

[roxelo]

@emasab have you had a chance to take a look at this PR?

[emasab]

@wmorgan6796 @roxelo thanks for this contribution! We're doing changes to the cooperative-sticky assignor and we can check this too while doing that.

[wmorgan6796]

@emasab is there a PR/branch that can be followed?

[milindl]

I had added this earlier to the changes we're making to the assignors, #4252, but stripped the changes recently as we think there are some further complexities we need to address.

The change seems to make it impossible to use this pattern inside a user-defined rebalance callback, which is a valid use case:

...
case RD_KAFKA_RESP_ERR__REVOKE_PARTITIONS:
                  rd_kafka_commit(rk, partitions, 0); // sync commit
 ...

(it's a valid usecase in Java as well, to call commitSync from the onPartitionsRevoked method of the ConsumerRebalanceListener).

Additionally, we were also thinking it's best to include two more tests, first, a test added to 0118 which checks the fact that commit should work inside a rebalance cb (if it's called before calling assign()), and a second test which can replicate the situation defined in #4059 and then work towards fixing that.

Thanks for your patience, we're discussing it further internally, to see how we can best help with it.

[wmorgan6796]

@milindl it seems that the fix for the cooperative sticky partition assignment strategy was figured out? If so I'll close this PR.

[milindl]

Hi @wmorgan6796 , no, it still isn't figured out. We can't simply disallow committing while rebalance is happening (it's a legitimate case to commit in a sync manner inside the rebalance cb), but at the same time, the issue with illegal generation persists. Please leave this PR open as we figure out the fix, it contains the entire context, and I'll add commits to the same branch too, thanks.

[cla-assistant]

All committers have signed the CLA.

[jzvelc]

Hi. Any progress on this? We have also hit this issue.