auto commit and add event for rebalance #889
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Should resolve SOHU-Co#787 If auto commit is true, commit before we rejoin the group on a rebalance According to the kafka protcol, commits are allowed during this stage, as the generation ID has not been bumped yet. If auto commit is disabled, implementors need a way to decide what to commit, so an onRebalance option was added. Implementors can perform commits during this method and call back when done to finish the rebalance.
This required special behavior due to the separate commit queue
commit(null, true) will flush what was already in the queue.
|
This should be good to go now (Travis has been pretty unreliable lately, test did pass) An example of using the onRebalance event: async _onRebalance(isMember: boolean, cb: Function) {
try {
if (isMember) {
this.logger.info("Rebalancing");
// Clear and wait for any pending event dispatches, giving them a chance to commit
await this._clearAndWaitForQueue();
// Now any pending tasks are in the commit queue, which will be picked up after this method finishes
}
this.emit("kafka:rebalance");
} catch (e) {
this.logger.error("ErrorKafkaRebalance", e);
} finally {
cb();
}
} |
|
@hyperlink is anything holding this up? |
hyperlink
reviewed
Mar 12, 2018
lib/consumerGroupStream.js
Outdated
| const originalOnRebalance = options.onRebalance; | ||
| options.onRebalance = function (isAlreadyMember, callback) { | ||
| let autoCommitCalled = false; | ||
| const autoCommit = function (err) { |
There was a problem hiding this comment.
could warp this in _.once to avoid having a flag.
hyperlink
approved these changes
Mar 15, 2018
|
@hyperlink dear Sir, any plans to release this? |
|
@hugebdu I will get it prepared. |
|
@hugebdu holding off for now. I'm tracking down an error when running the tests locally. |
|
@hyperlink 10x for update. waiting patiently |
|
Published as 2.5.0. |
|
@hyperlink thanks a lot! |
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Should resolve #787
If auto commit is true, commit before we rejoin the group on a rebalance
According to the kafka protcol, commits are allowed during this stage, as the generation ID has not been bumped yet.
If auto commit is disabled, implementors need a way to decide what to commit, so an onRebalance option was added.
Implementors can perform commits during this method and call back when done to finish the rebalance.