Skip to content

KAFKA-8473: Adjust Connect system tests for incremental cooperative rebalancing#6872

Merged
rhauch merged 1 commit into
apache:trunkfrom
kkonstantine:kafka-8473
Jun 3, 2019
Merged

KAFKA-8473: Adjust Connect system tests for incremental cooperative rebalancing#6872
rhauch merged 1 commit into
apache:trunkfrom
kkonstantine:kafka-8473

Conversation

@kkonstantine

Copy link
Copy Markdown
Contributor

As predicted by the new incremental cooperative rebalancing protocol, when a worker leaves the group a delay is triggered, controlled by the scheduled.rebalance.max.delay.ms property, during which the tasks detected as lost due to the workers departure are not reassigned. After this delay, workers are joining for another rebalancing round to reschedule the lost tasks.

System tests that perform worker bounces need to account for this delay.
This PR is also enabled connect system tests on distributed workers for both eager and incremental cooperative rebalancing protocols.

Tested by repeated runs of system tests.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

Comment thread tests/kafkatest/tests/connect/templates/connect-distributed.properties Outdated
@kkonstantine

Copy link
Copy Markdown
Contributor Author

@rhauch this PR aims to resolve unpredictability in test runs that depends on the order of worker departures (and whether a leader leaves the group first or last). I'd appreciate a review.

@rhauch rhauch left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, @kkonstantine! Overall this looks good, and it will be nice to exercise the different rebalance protocols in the system tests. However, I do have a few questions.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does this need to be so large by default?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So, we are trying to strike a balance here. We don't want this to be too high, because tests will start failing since tasks will remain unassigned, but we also want to exercise the delay logic because workers do indeed come back. Usually with class loading and stuff it takes less than 30 seconds for a worker to come back and join the group. But to guard against flakiness I'm making this 60sec.

In two places the delay will need to increase but only for tests setting protocol to compatible. In one the wait time increased from 70sec to 150sec and another case from 30sec to 120sec. Also, this applies to worst case scenarios when a real failure happens. In normal times, the assertions will take a bit less than that, but indeed more than 60sec.

Would you prefer another value?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Your description was very helpful. No need to change the value.

@kkonstantine

Copy link
Copy Markdown
Contributor Author

Thanks @rhauch
FYI, unit tests (which are unaffected by this PR) succeeded during the first run:
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/22426/console

I had to rebase to remove the change on client.id, so we get extra builds after the rebase.

@rhauch rhauch left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Thanks, @kkonstantine!

@rhauch
rhauch merged commit 55d07e7 into apache:trunk Jun 3, 2019
rhauch pushed a commit that referenced this pull request Jun 3, 2019
…ebalancing (#6872)

Author: Konstantine Karantasis <konstantine@confluent.io>
Reviewer: Randall Hauch <rhauch@gmail.com>
omkreddy added a commit to confluentinc/kafka that referenced this pull request Jun 4, 2019
* AK_REPO/2.3:
  KAFKA-8155: Add 2.2.0 release to system tests (apache#6597)
  KAFKA-8404: Add HttpHeader to RestClient HTTP Request and Connector REST API (apache#6791)
  KAFKA-8473: Adjust Connect system tests for incremental cooperative rebalancing (apache#6872)
  KAFKA-8475: Temporarily restore SslFactory.sslContext() helper
  KAFKA-8449: Restart tasks on reconfiguration under incremental cooperative rebalancing (apache#6850)
  KAFKA-8426; Fix for keeping the ConfigProvider configs consistent with KIP-297 (apache#6750)
  KAFKA-8425: Fix for correctly handling immutable maps (KIP-421 bug) (apache#6795)
  MINOR: Reordering the props modification with configs construction
pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
…ebalancing (apache#6872)

Author: Konstantine Karantasis <konstantine@confluent.io>
Reviewer: Randall Hauch <rhauch@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants