[
https://issues.apache.org/jira/browse/ZOOKEEPER-2466?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15367395#comment-15367395
]
Flavio Junqueira commented on ZOOKEEPER-2466:
---------------------------------------------
[~shralex] [~hanm]
bq. From my brief digging, my feeling was that the java way of doing it was
better: statichostprovider is the only one that increments pointers and
gives out addresses and the caller doesn't do any of this... But this may
be too much of a change for C.
I think that if we make the change in ZOOKEEPER-1856, we will have the calls to
addrvec_next concentrated in zoo_cycle_next_server. The only exception is the
processing of read-only state in zookeeper_interest:
{noformat}
if (zh->state == ZOO_READONLY_STATE) {
int idle_ping_rw = calculate_interval(&zh->last_ping_rw, &now);
if (idle_ping_rw >= zh->ping_rw_timeout) {
zh->last_ping_rw = now;
idle_ping_rw = 0;
zh->ping_rw_timeout = min(zh->ping_rw_timeout * 2,
MAX_RW_TIMEOUT);
if (ping_rw_server(zh)) {
struct sockaddr_storage addr;
addrvec_peek(&zh->addrs, &addr);
zh->ping_rw_timeout = MIN_RW_TIMEOUT;
LOG_INFO(LOGCALLBACK(zh),
"r/w server found at %s",
format_endpoint_info(&addr));
handle_error(zh, ZRWSERVERFOUND);
} else {
addrvec_next(&zh->addrs, NULL);
}
}
send_to = min(send_to, zh->ping_rw_timeout - idle_ping_rw);
}
{noformat}
Here we call handle_error, but I'm thinking that we don't really want to skip
to the next server, so it is ok that we don't call addrvec_next in
handle_error. If it is, then we can simply call zoo_cycle here. The call to
addrvec_next is advancing the pointer but not changing zh->addr_cur, so we
can't call replace it with zoo_cycle. I need to think a bit more about how to
remove that addrvec_next call.
> Client skips servers when trying to connect
> -------------------------------------------
>
> Key: ZOOKEEPER-2466
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2466
> Project: ZooKeeper
> Issue Type: Bug
> Components: c client
> Reporter: Flavio Junqueira
> Assignee: Flavio Junqueira
> Priority: Critical
> Fix For: 3.5.3, 3.6.0
>
>
> I've been looking at {{Zookeeper_simpleSystem::testFirstServerDown}} and I
> observed the following behavior. The list of servers to connect contains two
> servers, let's call them S1 and S2. The client never connects, but the odd
> bit is the sequence of servers that the client tries to connect to:
> {noformat}
> S1
> S2
> S1
> S1
> S1
> <keeps repeating S1>
> {noformat}
> It intrigued me that S2 is only tried once and never again. Checking the
> code, here is what happens. Initially, {{zh->reconfig}} is 1, so in
> {{zoo_cycle_next_server}} we return an address from
> {{get_next_server_in_reconfig}}, which is taken from {{zh->addrs_new}} in
> this test case. The attempt to connect fails, and {{handle_error}} is invoked
> in the error handling path. {{handle_error}} actually invokes
> {{addrvec_next}} which changes the address pointer to the next server on the
> list.
> After two attempts, it decides that it has tried all servers in
> {{zoo_cycle_next_server}} and sets {{zh->reconfig}} to zero. Once
> {{zh->reconfig == 0}}, we have that each call to {{zoo_cycle_next_server}}
> moves the address pointer to the next server in {{zh->addrs}}. But, given
> that {{handle_error}} also moves the pointer to the next server, we end up
> moving the pointer ahead twice upon every failed attempt to connect, which is
> wrong.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
