Skip to content

ZOOKEEPER-4419: The potential exception in Learner$LeaderConnector.connectToLeader may cause unnecessary re-election or service delay #1782

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.

Sign up for GitHub

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

Open
functioner wants to merge 2 commits into
base: master
Choose a base branch
from
+19 −23
Open

ZOOKEEPER-4419: The potential exception in Learner$LeaderConnector.connectToLeader may cause unnecessary re-election or service delay #1782

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f0fab48
ZOOKEEPER-4419: The potential exception in Learner.connectToLeader ma…
functioner Nov 27, 2021
8d130a5
update
functioner Jun 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 19 additions & 23 deletions zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Learner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ public void run() {
}

private Socket connectToLeader() throws IOException, X509Exception, InterruptedException {
Socket sock = createSocket();
Socket sock = null;

// leader connection timeout defaults to tickTime * initLimit
int connectTimeout = self.tickTime * self.initLimit;
Expand All @@ -410,8 +410,10 @@ private Socket connectToLeader() throws IOException, X509Exception, InterruptedE
int remainingTimeout;
long startNanoTime = nanoTime();

for (int tries = 0; tries < 5 && socket.get() == null; tries++) {
for (int tries = 0; tries < 5; tries++) {

@kezhuw kezhuw Jul 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

connectToLeader could be called concurrently for different addresses, so it is good to break eagerly when there is a success connection.

try {
sock = createSocket();

// recalculate the init limit time because retries sleep for 1000 milliseconds
remainingTimeout = connectTimeout - (int) ((nanoTime() - startNanoTime) / 1_000_000);
if (remainingTimeout <= 0) {
Expand All @@ -423,35 +425,29 @@ private Socket connectToLeader() throws IOException, X509Exception, InterruptedE
if (self.isSslQuorum()) {
((SSLSocket) sock).startHandshake();
}
sock.setTcpNoDelay(nodelay);

@CriggerMarg CriggerMarg Jul 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@functioner I believe this shouldn't be deleted, along with break statement next line

@kezhuw kezhuw Jul 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I feel same here.

break;
} catch (IOException e) {
// close failed socket before retrying
if (sock != null) {
try {
sock.close();
} catch (IOException closeException) {
LOG.debug("Failed to close socket after connection failure", closeException);
}
sock = null;
}
remainingTimeout = connectTimeout - (int) ((nanoTime() - startNanoTime) / 1_000_000);

if (remainingTimeout <= leaderConnectDelayDuringRetryMs) {
LOG.error(
"Unexpected exception, connectToLeader exceeded. tries={}, remaining init limit={}, connecting to {}",
tries,
remainingTimeout,
address,
e);
LOG.error("Unexpected exception, connectToLeader exceeded. tries={}, remaining init limit={}, connecting to {}",
tries, remainingTimeout, address, e);
throw e;
} else if (tries >= 4) {
LOG.error(
"Unexpected exception, retries exceeded. tries={}, remaining init limit={}, connecting to {}",
tries,
remainingTimeout,
address,
e);
LOG.error("Unexpected exception, retries exceeded. tries={}, remaining init limit={}, connecting to {}",
tries, remainingTimeout, address, e);
throw e;
} else {
LOG.warn(
"Unexpected exception, tries={}, remaining init limit={}, connecting to {}",
tries,
remainingTimeout,
address,
e);
sock = createSocket();
LOG.warn("Unexpected exception, tries={}, remaining init limit={}, connecting to {}",
tries, remainingTimeout, address, e);
}
}
Thread.sleep(leaderConnectDelayDuringRetryMs);
Expand Down
Loading