Skip to content

Commit

Permalink
crawler: don't block during hash expansion
Browse files Browse the repository at this point in the history
if using `lru_crawler metadump hash` the client could block while trying
to grab the maintenance lock potentially held by the hash table
expander. Hash expansion can potentially take a long time.

Further, while waiting for this lock, the crawler is holding the lru
crawler lock, which could cause other clients trying to talk to the
crawler code to themselves block.

So... throw a locked error and don't do that.
  • Loading branch information
dormando committed May 24, 2024
1 parent 0954b53 commit 788f592
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 11 deletions.
7 changes: 5 additions & 2 deletions assoc.c
Original file line number Diff line number Diff line change
Expand Up @@ -307,8 +307,11 @@ void *assoc_get_iterator(void) {
return NULL;
}
// this will hang the caller while a hash table expansion is running.
mutex_lock(&maintenance_lock);
return iter;
if (mutex_trylock(&maintenance_lock) == 0) {
return iter;
} else {
return NULL;
}
}

bool assoc_iterate(void *iterp, item **it) {
Expand Down
53 changes: 44 additions & 9 deletions crawler.c
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ struct _crawler_module_t {
void *data; /* opaque data pointer */
crawler_client_t c;
crawler_module_reg_t *mod;
int status; /* flags/code/etc for internal module usage */
};

static int crawler_expired_init(crawler_module_t *cm, void *data);
Expand All @@ -68,31 +69,33 @@ crawler_module_reg_t crawler_expired_mod = {
.doneclass = crawler_expired_doneclass,
.finalize = crawler_expired_finalize,
.needs_lock = true,
.needs_client = false
.needs_client = false,
};

static int crawler_metadump_init(crawler_module_t *cm, void *data);
static void crawler_metadump_eval(crawler_module_t *cm, item *search, uint32_t hv, int i);
static void crawler_metadump_finalize(crawler_module_t *cm);

crawler_module_reg_t crawler_metadump_mod = {
.init = NULL,
.init = crawler_metadump_init,
.eval = crawler_metadump_eval,
.doneclass = NULL,
.finalize = crawler_metadump_finalize,
.needs_lock = false,
.needs_client = true
.needs_client = true,
};

static int crawler_mgdump_init(crawler_module_t *cm, void *data);
static void crawler_mgdump_eval(crawler_module_t *cm, item *search, uint32_t hv, int i);
static void crawler_mgdump_finalize(crawler_module_t *cm);

crawler_module_reg_t crawler_mgdump_mod = {
.init = NULL,
.init = crawler_mgdump_init,
.eval = crawler_mgdump_eval,
.doneclass = NULL,
.finalize = crawler_mgdump_finalize,
.needs_lock = false,
.needs_client = true
.needs_client = true,
};

crawler_module_reg_t *crawler_mod_regs[4] = {
Expand Down Expand Up @@ -259,6 +262,11 @@ static void crawler_expired_eval(crawler_module_t *cm, item *search, uint32_t hv
pthread_mutex_unlock(&d->lock);
}

static int crawler_metadump_init(crawler_module_t *cm, void *data) {
cm->status = 0;
return 0;
}

static void crawler_metadump_eval(crawler_module_t *cm, item *it, uint32_t hv, int i) {
char keybuf[KEY_MAX_URI_ENCODED_LENGTH];
int is_flushed = item_is_flushed(it);
Expand Down Expand Up @@ -296,12 +304,25 @@ static void crawler_metadump_finalize(crawler_module_t *cm) {
if (cm->c.c != NULL) {
// flush any pending data.
if (lru_crawler_write(&cm->c) == 0) {
memcpy(cm->c.buf, "END\r\n", 5);
cm->c.bufused += 5;
// Only nonzero status right now means we were locked
if (cm->status != 0) {
const char *errstr = "ERROR locked try again later\r\n";
size_t errlen = strlen(errstr);
memcpy(cm->c.buf, errstr, errlen);
cm->c.bufused += errlen;
} else {
memcpy(cm->c.buf, "END\r\n", 5);
cm->c.bufused += 5;
}
}
}
}

static int crawler_mgdump_init(crawler_module_t *cm, void *data) {
cm->status = 0;
return 0;
}

static void crawler_mgdump_eval(crawler_module_t *cm, item *it, uint32_t hv, int i) {
int is_flushed = item_is_flushed(it);
/* Ignore expired content. */
Expand Down Expand Up @@ -335,8 +356,16 @@ static void crawler_mgdump_finalize(crawler_module_t *cm) {
if (cm->c.c != NULL) {
// flush any pending data.
if (lru_crawler_write(&cm->c) == 0) {
memcpy(cm->c.buf, "EN\r\n", 4);
cm->c.bufused += 4;
// Only nonzero status right now means we were locked
if (cm->status != 0) {
const char *errstr = "ERROR locked try again later\r\n";
size_t errlen = strlen(errstr);
memcpy(cm->c.buf, errstr, errlen);
cm->c.bufused += errlen;
} else {
memcpy(cm->c.buf, "EN\r\n", 4);
cm->c.bufused += 4;
}
}
}
}
Expand Down Expand Up @@ -420,6 +449,12 @@ static void item_crawl_hash(void) {
item *it = NULL;
int items = 0;

// Could not get the iterator: probably locked due to hash expansion.
if (iter == NULL) {
active_crawler_mod.status = 1;
return;
}

// loop while iterator returns something
// - iterator func handles bucket-walking
// - iterator returns with bucket locked.
Expand Down
1 change: 1 addition & 0 deletions memcached.h
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,7 @@ void conn_worker_readd(conn *c);
extern int daemonize(int nochdir, int noclose);

#define mutex_lock(x) pthread_mutex_lock(x)
#define mutex_trylock(x) pthread_mutex_trylock(x)
#define mutex_unlock(x) pthread_mutex_unlock(x)

#include "stats_prefix.h"
Expand Down

0 comments on commit 788f592

Please sign in to comment.