[
https://issues.apache.org/jira/browse/ZOOKEEPER-2469?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15368490#comment-15368490
]
Edward Ribeiro commented on ZOOKEEPER-2469:
-------------------------------------------
Hi [~sershe], I was not so clear about my previous comments.
Totally agree with you: a refactoring should be part of another ticket. The
code is *very* entangled indeed. :(
{quote}
Does ZK officially not support Java 6?
{quote}
java.util.concurrent.TimeUnit has been in JDK since Java 1.5
We are in the middle of cutting an alpha release (release 3.5.2-alpha candidate
1), among other things, so *it can take some time until a committer takes a
closer look and apply your patch, ok?*
_Btw, it would be great if you could spare some time to test the 3.5 branch.
Any help is welcome._ ;)
Finally, when I say you can "up" the immediateRetry what I was meaning was to
replace this:
{code}
private synchronized void reLogin(boolean immediateRetry)
throws LoginException {
(...)
if (!hasSufficientTimeElapsed(immediateRetry)) {
return;
}
(...)
private boolean hasSufficientTimeElapsed(boolean immediateRetry) {
long now = Time.currentElapsedTime();
// Ignore the min-time if we are retrying a failed login.
if (!immediateRetry && now - getLastLogin() < MIN_TIME_BEFORE_RELOGIN )
{
LOG.warn("Not attempting to re-login since the last re-login was "
+ "attempted less than {} seconds before.",
(MIN_TIME_BEFORE_RELOGIN / 1000));
return false;
}
// register most recent relogin attempt
setLastLogin(now);
return true;
}
{code}
With this
{code}
private synchronized void reLogin(boolean immediateRetry)
throws LoginException {
(...)
if (!immediateRetry && !hasSufficientTimeElapsed()) {
return;
}
(...)
private boolean hasSufficientTimeElapsed() {
long now = Time.currentElapsedTime();
// Ignore the min-time if we are retrying a failed login.
if (now - getLastLogin() < MIN_TIME_BEFORE_RELOGIN ) {
LOG.warn("Not attempting to re-login since the last re-login was "
+ "attempted less than {} seconds before.",
(MIN_TIME_BEFORE_RELOGIN / 1000));
return false;
}
// register most recent relogin attempt
setLastLogin(now);
return true;
}
{code}
The main reason would be that {{immediateRetry}} doesn't need to be passed down
to {{hasSufficientTimeElapsed}}.
But {{hasSufficientTimeElapsed}} can still be called, and it would only set the
last login if the if condition inside the method was false (among other things,
it requires that {{immediateRetry}} is false in your patch, so I guess it
equates to my code change suggestion, right?). But I am okay with it as-is now,
no problem. I have other questions with your patch, but I would need more
context about this particular code piece to make an informed decision, so I
will absent myself by now. :)
Cheers!
> infinite loop in ZK re-login
> ----------------------------
>
> Key: ZOOKEEPER-2469
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2469
> Project: ZooKeeper
> Issue Type: Bug
> Reporter: Sergey Shelukhin
> Assignee: Sergey Shelukhin
> Attachments: ZOOKEEPER-2469.01.patch, ZOOKEEPER-2469.patch
>
>
> {noformat}
> int retry = 1;
> while (retry >= 0) {
> try {
> reLogin();
> break;
> } catch (LoginException le) {
> if (retry > 0) {
> --retry;
> // sleep for 10 seconds.
> try {
> Thread.sleep(10 * 1000);
> } catch (InterruptedException e) {
> LOG.error("Interrupted during login
> retry after LoginException:", le);
> throw le;
> }
> } else {
> LOG.error("Could not refresh TGT for
> principal: " + principal + ".", le);
> }
> }
> }
> {noformat}
> will retry forever. Should return like the one above
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
