Skip to content

KAFKA-4041: Update ZooKeeper to 3.4.13#5376

Merged
ijuma merged 2 commits intoapache:trunkfrom
ijuma:zookeeper-3.4.13
Jul 17, 2018
Merged

KAFKA-4041: Update ZooKeeper to 3.4.13#5376
ijuma merged 2 commits intoapache:trunkfrom
ijuma:zookeeper-3.4.13

Conversation

@ijuma
Copy link
Copy Markdown
Member

@ijuma ijuma commented Jul 17, 2018

This includes a fix for ZOOKEEPER-2184 (Zookeeper Client
should re-resolve hosts when connection attempts fail), which
fixes KAFKA-4041.

Updated a couple of tests as unresolvable addresses are now
retried until the connection timeout. Cleaned up tests a little.

Committer Checklist (excluded from commit message)

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

This includes a fix for ZOOKEEPER-2184, which
fixes KAFKA-4041.
Copy link
Copy Markdown
Contributor

@ewencp ewencp left a comment

Choose a reason for hiding this comment

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

LGTM. Any reason not to include KAFKA-4041 in the commit title since that's the primary reason for doing this?

@ijuma ijuma changed the title MINOR: Update ZooKeeper to 3.4.13 KAFKA-4041: Update ZooKeeper to 3.4.13 Jul 17, 2018
@ijuma
Copy link
Copy Markdown
Member Author

ijuma commented Jul 17, 2018

@ewencp Updated the title.

@ewencp
Copy link
Copy Markdown
Contributor

ewencp commented Jul 17, 2018

@ijuma Hmm, couple of test failures on JDK8 run look like they might actually be related rather than flakes -- change in behavior for re-resolving may be affecting them.

@ijuma
Copy link
Copy Markdown
Member Author

ijuma commented Jul 17, 2018

@ewencp Yes indeed. The changes should only affect tests (ZooKeeperClient catches Exception during ZooKeeper construction) so simply updated the tests.

Copy link
Copy Markdown
Contributor

@rajinisivaram rajinisivaram left a comment

Choose a reason for hiding this comment

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

@ijuma Thanks for the PR, LGTM. I think the test failures in the last run are not related to this change.

@ijuma
Copy link
Copy Markdown
Member Author

ijuma commented Jul 17, 2018

Failures look unrelated. Will run the tests again, just in case.

@ijuma
Copy link
Copy Markdown
Member Author

ijuma commented Jul 17, 2018

Retest this please

@ijuma
Copy link
Copy Markdown
Member Author

ijuma commented Jul 17, 2018

There was 1 Streams failure in one of the jobs and the other was also running Streams tests when the node was taken offline.

@ijuma
Copy link
Copy Markdown
Member Author

ijuma commented Jul 17, 2018

retest this please

@ijuma
Copy link
Copy Markdown
Member Author

ijuma commented Jul 17, 2018

One of the jobs passed, the other had a Streams failure due to a ZK timeout. The timeout is just 8 seconds, but we have a separate PR that is changing the code to use the AdminClient instead. Merging to trunk and 2.0.

@ijuma ijuma merged commit f6219c6 into apache:trunk Jul 17, 2018
@ijuma ijuma deleted the zookeeper-3.4.13 branch July 17, 2018 20:53
ijuma added a commit that referenced this pull request Jul 17, 2018
This includes a fix for ZOOKEEPER-2184 (Zookeeper Client
should re-resolve hosts when connection attempts fail), which
fixes KAFKA-4041.

Updated a couple of tests as unresolvable addresses are now
retried until the connection timeout. Cleaned up tests a little.

Reviewers: Ewen Cheslack-Postava <ewen@confluent.io>, Rajini Sivaram <rajinisivaram@googlemail.com>
mkedwards pushed a commit to bitpusherllc/kafka that referenced this pull request Feb 10, 2019
This includes a fix for ZOOKEEPER-2184 (Zookeeper Client
should re-resolve hosts when connection attempts fail), which
fixes KAFKA-4041.

Updated a couple of tests as unresolvable addresses are now
retried until the connection timeout. Cleaned up tests a little.

Reviewers: Ewen Cheslack-Postava <ewen@confluent.io>, Rajini Sivaram <rajinisivaram@googlemail.com>
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