Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LI-CHERRY-PICK] Backport use all dns ips #356

Merged
merged 9 commits into from Jun 14, 2022

Conversation

ZIDAZ
Copy link

@ZIDAZ ZIDAZ commented Jun 14, 2022

use_all_dns_ips config has been backported from 3.0-li to 2.4-li and 2.3-li-1, and set as default. We would like the config to be adopted by LKC9 users, thus backport it to 2.0-li branch as well.

The config use_all_dns_ips was introduced in KIP-302 (apache#4987) and set as default in KIP-602 (apache#8644)

Other commits have to be cherrypicked in order to resolve merge conflicts and unit test failures.

More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.

Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.

Committer Checklist (excluded from commit message)

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

xiowu0 and others added 6 commits June 13, 2022 16:45
TICKET =
LI_DESCRIPTION =
revert LI HOTFIX "Consumer should fetch metadata from the same broker until it is disconnected from that broker".
since sticky metadata request is no longer needed.
The original hotfix was introduced as a short term fix util the actual issue was found, and we no longer need this hotfix now.

EXIT_CRITERIA = MANUAL ["after the original fix has been removed"]
…pache#4987)

Implementation of KIP-302: Based on the new client configuration `client.dns.lookup`, a NetworkClient can use InetAddress.getAllByName to find all IPs and iterate over them when they fail to connect. Only uses either IPv4 or IPv6 addresses similar to the default mode.

Co-authored-by: Edoardo Comar <ecomar@uk.ibm.com>
Co-authored-by: Mickael Maison <mickael.maison@gmail.com>

Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>
…ltipleIPsWithUseAll (#116)

TICKET = N/A
LI_DESCRIPTION = The test fails since the domain kafka.apache.org used to return 3 IPs and is now
only returning two IPs. Furthermore, the upstream fix identified below cannot be cleanly cherry
picked.
EXIT_CRITERIA = when the commit 131d475 is picked from upstream:
KAFKA-12193: Re-resolve IPs after a client disconnects apache#9902
[LI-HOTFIX] Ignoring the failed tests (#188)

TICKET = N/A
LI_DESCRIPTION = Several tests are failing since the domain kafka.apache.org that used to resolve to more than 1 IPv4 addresses are not only resolving to 1 IPv4 address.
The upstream code has overhauled the ClusterConnectionStatesTest. We are simply ignoring these tests for now, and will get the new logic from upstream after a major version rebase.
EXIT_CRITERIA = This hotfix can be removed in the next major version rebase
…olvable (apache#6305)

* Fix for KAFKA-7974: Avoid calling disconnect() when not connecting

* Resolve host only when currentAddress() is called

Moves away from automatically resolving the host when the connection entry is constructed, which can leave ClusterConnectionStates in a confused state.
Instead, resolution is done on demand, ensuring that the entry in the connection list is present even if the resolution failed.

* Add Javadoc to ClusterConnectionStates.connecting()
…lookup` (KIP-602) (apache#8644)

This applies to the producer, consumer, admin client, connect worker
and inter broker communication.

`ClientDnsLookup.DEFAULT` has been deprecated and a warning
will be logged if it's explicitly set in a client config.

Reviewers: Mickael Maison <mickael.maison@gmail.com>, Ismael Juma <ismael@juma.me.uk>
@ZIDAZ ZIDAZ requested a review from kehuum June 14, 2022 05:19
ZIDAZ and others added 3 commits June 13, 2022 22:30
…#329)

The PR #228 attempted to resolve provided boostrap servers when
the metadata is exceeding a staleness threshold.  The config is coverred
both on producer and consumer, and default behavior without configured
value is setting timeout to Long.MAX_VALUE.

However, cruise-control is affected by the behavior as it implements a
similar mechanism on its own and directly uses of NetworkClient. The code
would fail if empty bootstrap server is passed to NetworkClient, which
is the case for internal use of CC.

To resolve this, this patch aims to make default value as -1, and omit
the code path referencing bootstrap server when we see -1.

EXIT_CRITERIA = When #228 is ejected
@ZIDAZ
Copy link
Author

ZIDAZ commented Jun 14, 2022

Note: local test with gradle5.6 cleanTest checkstyleMain checkstyleTest unitTest -x :streams:unitTest rat --no-daemon -PxmlFindBugsReport=true -PtestLoggingEvents=started,passed,skipped,failed -Dorg.gradle.java.home=/Library/Java/JavaVirtualMachines/jdk1.8.0_72.jdk/Contents/Home succeeded.
Failed checks above for SSL unit tests are known to be flaky.

@ZIDAZ ZIDAZ merged commit 3057daf into 2.0-li Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants