Skip to content

Commit

Permalink
CCBC-153 Reset internal state on lcb_connect()
Browse files Browse the repository at this point in the history
This patch allows callers to use lcb_connect() multiple times to
implement reconnecting using the same lcb_t instance.

Problems
========

* After connection error all subsequent calls to lcb_connect() lead to
  segmentation violation.

* Index used to iterate over list of backup node names wasn't reset

* For systems which don't use lcb_wait() to execute the IO loop, the
  initial timer wasn't set, so that they might wait infinitely if
  error callback wasn't triggered.

Solution
========

* Reset backup node index on lcb_connect() call

* Remove connection logic from wait.c and make it generic.

Change-Id: I63a0f6591e10c103d4a3b4de656623646aaa5c62
Reviewed-on: http://review.couchbase.org/23809
Tested-by: Sergey Avseyev <sergey.avseyev@gmail.com>
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
  • Loading branch information
avsej authored and trondn committed Jan 10, 2013
1 parent 2ce553a commit d494819
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 61 deletions.
5 changes: 5 additions & 0 deletions RELEASE_NOTES.markdown
Expand Up @@ -7,6 +7,11 @@ bugfixes. Do not forget to update this doc in every important patch.

* [minor] Added manual pages for the library

* [major] CCBC-153 Reset internal state on lcb_connect(). Allow caller
to use lcb_connect() multiple times to implement reconnecting using
the same lcb_t instance. Also it sets up the initial-connection
timer for users who don't use lcb_wait() and drive IO loop manually.

## 2.0.2 (2013-01-04)

* [major] CCBC-150 commands sent to multiple servers fail to detect
Expand Down
54 changes: 49 additions & 5 deletions src/instance.c
Expand Up @@ -28,6 +28,8 @@

/* private function to safely free backup_nodes*/
static void free_backup_nodes(lcb_t instance);
static lcb_error_t try_to_connect(lcb_t instance);

/**
* Get the version of the library.
*
Expand Down Expand Up @@ -650,8 +652,10 @@ static void lcb_update_serverlist(lcb_t instance)
instance->vbucket_state_listener(instance->servers + ii);
}
}
instance->callbacks.configuration(instance,
LCB_CONFIGURATION_NEW);
instance->callbacks.configuration(instance, LCB_CONFIGURATION_NEW);
instance->io->v.v0.delete_timer(instance->io, instance->timeout.event);
instance->connected = 1;
lcb_maybe_breakout(instance);
}
}

Expand Down Expand Up @@ -824,7 +828,7 @@ int lcb_switch_to_backup_node(lcb_t instance,

do {
/* Keep on trying the nodes until all of them failed ;-) */
if (lcb_connect(instance) == LCB_SUCCESS) {
if (try_to_connect(instance) == LCB_SUCCESS) {
return 0;
}
} while (instance->backup_nodes[instance->backup_idx] == NULL);
Expand Down Expand Up @@ -1125,8 +1129,7 @@ int lcb_getaddrinfo(lcb_t instance, const char *hostname,
/**
* @todo use async connects etc
*/
LIBCOUCHBASE_API
lcb_error_t lcb_connect(lcb_t instance)
static lcb_error_t try_to_connect(lcb_t instance)
{
int error;

Expand Down Expand Up @@ -1169,6 +1172,47 @@ lcb_error_t lcb_connect(lcb_t instance)
return instance->last_error;
}

static void initial_connect_timeout_handler(lcb_socket_t sock,
short which,
void *arg)
{
lcb_t instance = arg;

/* try to switch to backup node and just return on success */
if (lcb_switch_to_backup_node(instance, LCB_CONNECT_ERROR,
"Could not connect to server within allotted time") != -1) {
return;
}

if (instance->sock != INVALID_SOCKET) {
/* Do we need to delete the event? */
instance->io->v.v0.delete_event(instance->io,
instance->sock,
instance->event);
instance->io->v.v0.close(instance->io, instance->sock);
instance->sock = INVALID_SOCKET;
}

instance->io->v.v0.delete_timer(instance->io, instance->timeout.event);
instance->timeout.next = 0;
lcb_maybe_breakout(instance);

(void)sock;
(void)which;
}

LIBCOUCHBASE_API
lcb_error_t lcb_connect(lcb_t instance)
{
instance->backup_idx = 0;
instance->io->v.v0.update_timer(instance->io,
instance->timeout.event,
instance->timeout.usec,
instance,
initial_connect_timeout_handler);
return try_to_connect(instance);
}

static void free_backup_nodes(lcb_t instance)
{
if (instance->should_free_backup_nodes) {
Expand Down
4 changes: 0 additions & 4 deletions src/internal.h
Expand Up @@ -199,10 +199,6 @@ extern "C" {

vbucket_state_listener_t vbucket_state_listener;

/** for initial configuration.
* see breakout_configuration_callback in wait.c*/
lcb_configuration_callback configuration_callback_last;

RESPONSE_HANDLER response_handler[0x100];
REQUEST_HANDLER request_handler[0x100];

Expand Down
52 changes: 0 additions & 52 deletions src/wait.c
Expand Up @@ -16,44 +16,6 @@
*/
#include "internal.h"

static void breakout_configuration_callback(lcb_t instance, lcb_configuration_t config)
{
instance->callbacks.configuration = instance->configuration_callback_last;
instance->io->v.v0.delete_timer(instance->io, instance->timeout.event);
instance->connected = 1;
lcb_maybe_breakout(instance);
instance->callbacks.configuration(instance, config);
}

static void initial_connect_timeout_handler(lcb_socket_t sock,
short which,
void *arg)
{
lcb_t instance = arg;

/* try to switch to backup node and just return on success */
if (lcb_switch_to_backup_node(instance, LCB_CONNECT_ERROR,
"Could not connect to server within allotted time") != -1) {
return;
}

if (instance->sock != INVALID_SOCKET) {
/* Do we need to delete the event? */
instance->io->v.v0.delete_event(instance->io,
instance->sock,
instance->event);
instance->io->v.v0.close(instance->io, instance->sock);
instance->sock = INVALID_SOCKET;
}

instance->io->v.v0.delete_timer(instance->io, instance->timeout.event);
instance->timeout.next = 0;
lcb_maybe_breakout(instance);

(void)sock;
(void)which;
}

/**
* Returns non zero if the event loop is running now
*
Expand Down Expand Up @@ -88,20 +50,6 @@ lcb_error_t lcb_wait(lcb_t instance)
*/
instance->last_error = LCB_SUCCESS;
instance->wait = 1;
if (!instance->connected) {
if (instance->type == LCB_TYPE_BUCKET) {
/* Initial configuration. Set a timer */
instance->configuration_callback_last = instance->callbacks.configuration;
instance->callbacks.configuration = breakout_configuration_callback;
}

/* Initial connection timeout */
instance->io->v.v0.update_timer(instance->io,
instance->timeout.event,
instance->timeout.usec,
instance,
initial_connect_timeout_handler);
}
if (!instance->connected || lcb_has_data_in_buffers(instance)
|| hashset_num_items(instance->timers) > 0) {
lcb_size_t idx;
Expand Down

0 comments on commit d494819

Please sign in to comment.