Skip to content

Commit

Permalink
sql_cacher: don't query the SQL DB under lock when reloading
Browse files Browse the repository at this point in the history
Previously, retrieving any key from the cache during a reload would
blocked until the SQL DB query completed.

Suggested by Ben Newlin in #1760
  • Loading branch information
rvlad-patrascu committed Jul 12, 2019
1 parent ecea9b0 commit 642271c
Showing 1 changed file with 51 additions and 72 deletions.
123 changes: 51 additions & 72 deletions modules/sql_cacher/sql_cacher.c
Expand Up @@ -743,14 +743,39 @@ static db_handlers_t *db_init_test_conn(cache_entry_t *c_entry)
return new_db_hdls;
}

static int inc_cache_rld_vers(db_handlers_t *db_hdls, int *rld_vers)
{
str rld_vers_key;

rld_vers_key.len = db_hdls->c_entry->id.len + 23;
rld_vers_key.s = pkg_malloc(rld_vers_key.len);
if (!rld_vers_key.s) {
LM_ERR("No more pkg memory\n");
return -1;
}
memcpy(rld_vers_key.s, db_hdls->c_entry->id.s, db_hdls->c_entry->id.len);
memcpy(rld_vers_key.s + db_hdls->c_entry->id.len, "_sql_cacher_reload_vers", 23);

if (db_hdls->cdbf.add(db_hdls->cdbcon, &rld_vers_key, 1, 0, rld_vers) < 0) {
LM_DBG("Failed to increment reload version integer from cachedb\n");
pkg_free(rld_vers_key.s);
return -1;
}

pkg_free(rld_vers_key.s);

return 0;
}

static int load_entire_table(cache_entry_t *c_entry, db_handlers_t *db_hdls,
int reload_version)
int inc_rld_vers)
{
db_key_t *query_cols = NULL;
db_res_t *sql_res = NULL;
db_row_t *row;
db_val_t *values;
int i;
int reload_vers = 0;

query_cols = pkg_malloc((c_entry->nr_columns + 1) * sizeof(db_key_t));
if (!query_cols) {
Expand Down Expand Up @@ -801,10 +826,20 @@ static int load_entire_table(cache_entry_t *c_entry, db_handlers_t *db_hdls,
db_hdls->db_funcs.free_result(db_hdls->db_con, sql_res);
return 0;
}

lock_start_write(db_hdls->c_entry->ref_lock);

if (inc_rld_vers && inc_cache_rld_vers(db_hdls, &reload_vers) < 0) {
lock_stop_write(db_hdls->c_entry->ref_lock);
goto error;
}

row = RES_ROWS(sql_res);
values = ROW_VALUES(row);
if (get_column_types(c_entry, values + 1, ROW_N(row) - 1) < 0)
if (get_column_types(c_entry, values + 1, ROW_N(row) - 1) < 0) {
lock_stop_write(db_hdls->c_entry->ref_lock);
goto error;
}

/* load the rows into the cahchedb */
do {
Expand All @@ -813,21 +848,26 @@ static int load_entire_table(cache_entry_t *c_entry, db_handlers_t *db_hdls,
values = ROW_VALUES(row);
if (!VAL_NULL(values))
if (insert_in_cachedb(c_entry, db_hdls, values ,values + 1,
reload_version, ROW_N(row) - 1) < 0)
reload_vers, ROW_N(row) - 1) < 0) {
lock_stop_write(db_hdls->c_entry->ref_lock);
return -1;
}
}

if (DB_CAPABILITY(db_hdls->db_funcs, DB_CAP_FETCH)) {
if (db_hdls->db_funcs.fetch_result(db_hdls->db_con,&sql_res,fetch_nr_rows)<0) {
LM_ERR("Error fetching rows (1) from SQL DB: %.*s\n",
c_entry->db_url.len, c_entry->db_url.s);
c_entry->db_url.len, c_entry->db_url.s);
lock_stop_write(db_hdls->c_entry->ref_lock);
goto error;
}
} else {
break;
}
} while (RES_ROW_N(sql_res) > 0);

lock_stop_write(db_hdls->c_entry->ref_lock);

db_hdls->db_funcs.free_result(db_hdls->db_con, sql_res);
return 0;

Expand Down Expand Up @@ -947,38 +987,14 @@ static int get_rld_vers_from_cache(cache_entry_t *c_entry, db_handlers_t *db_hdl
void reload_timer(unsigned int ticks, void *param)
{
db_handlers_t *db_hdls;
int rld_vers;
str rld_vers_key;

for (db_hdls = db_hdls_list; db_hdls; db_hdls = db_hdls->next) {
if (db_hdls->c_entry->on_demand)
continue;

rld_vers_key.len = db_hdls->c_entry->id.len + 23;
rld_vers_key.s = pkg_malloc(rld_vers_key.len);
if (!rld_vers_key.s) {
LM_ERR("No more pkg memory\n");
return;
}
memcpy(rld_vers_key.s, db_hdls->c_entry->id.s, db_hdls->c_entry->id.len);
memcpy(rld_vers_key.s + db_hdls->c_entry->id.len, "_sql_cacher_reload_vers", 23);

lock_start_write(db_hdls->c_entry->ref_lock);

if (db_hdls->cdbf.add(db_hdls->cdbcon, &rld_vers_key, 1, 0, &rld_vers) < 0) {
LM_ERR("Failed to increment reload version integer from cachedb\n");
pkg_free(rld_vers_key.s);
lock_stop_write(db_hdls->c_entry->ref_lock);
continue;
}

pkg_free(rld_vers_key.s);

if (load_entire_table(db_hdls->c_entry, db_hdls, rld_vers) < 0)
if (load_entire_table(db_hdls->c_entry, db_hdls, 1) < 0)
LM_ERR("Failed to reload table %.*s\n", db_hdls->c_entry->table.len,
db_hdls->c_entry->table.s);

lock_stop_write(db_hdls->c_entry->ref_lock);
}
}

Expand All @@ -989,8 +1005,7 @@ static mi_item_t *mi_reload(const mi_params_t *params, str *key)
db_res_t *sql_res = NULL;
struct queried_key *it;
str entry_id, src_key;
str rld_vers_key;
int rld_vers = 0, rc;
int rld_vers, rc;

if (get_mi_string_param(params, "id", &entry_id.s, &entry_id.len) < 0)
return init_mi_param_error();
Expand Down Expand Up @@ -1053,50 +1068,14 @@ static mi_item_t *mi_reload(const mi_params_t *params, str *key)
"database, key not found\n"));
} else {
/* 'invalidate' all keys by increasing the reload version counter */
rld_vers_key.len = db_hdls->c_entry->id.len + 23;
rld_vers_key.s = pkg_malloc(rld_vers_key.len);
if (!rld_vers_key.s) {
LM_ERR("No more pkg memory\n");
return NULL;
}
memcpy(rld_vers_key.s, db_hdls->c_entry->id.s, db_hdls->c_entry->id.len);
memcpy(rld_vers_key.s + db_hdls->c_entry->id.len, "_sql_cacher_reload_vers", 23);

if (db_hdls->cdbf.add(db_hdls->cdbcon, &rld_vers_key, 1, 0, &rld_vers) < 0) {
LM_DBG("Failed to increment reload version integer from cachedb\n");
return init_mi_error(500, MI_SSTR("ERROR Reloading SQL database"));
}

pkg_free(rld_vers_key.s);
if (inc_cache_rld_vers(db_hdls, &rld_vers) < 0)
return init_mi_error(500, MI_SSTR("ERROR Invalidating cache"));
}
} else {
rld_vers_key.len = db_hdls->c_entry->id.len + 23;
rld_vers_key.s = pkg_malloc(rld_vers_key.len);
if (!rld_vers_key.s) {
LM_ERR("No more pkg memory\n");
return NULL;
}
memcpy(rld_vers_key.s, db_hdls->c_entry->id.s, db_hdls->c_entry->id.len);
memcpy(rld_vers_key.s + db_hdls->c_entry->id.len, "_sql_cacher_reload_vers", 23);

lock_start_write(db_hdls->c_entry->ref_lock);

if (db_hdls->cdbf.add(db_hdls->cdbcon, &rld_vers_key, 1, 0, &rld_vers) < 0) {
LM_DBG("Failed to increment reload version integer from cachedb\n");
pkg_free(rld_vers_key.s);
lock_stop_write(db_hdls->c_entry->ref_lock);
return init_mi_error(500, MI_SSTR("ERROR Reloading SQL database"));
}

pkg_free(rld_vers_key.s);

if (load_entire_table(db_hdls->c_entry, db_hdls, rld_vers) < 0) {
if (load_entire_table(db_hdls->c_entry, db_hdls, 1) < 0) {
LM_DBG("Failed to reload table\n");
lock_stop_write(db_hdls->c_entry->ref_lock);
return init_mi_error(500, MI_SSTR("ERROR Reloading SQL database"));
}

lock_stop_write(db_hdls->c_entry->ref_lock);
}

return init_mi_result_ok();
Expand Down Expand Up @@ -1158,8 +1137,8 @@ static void cache_init_load(int sender, void *param)
}

/* cache the entire table in full caching mode */
if (!db_hdls->c_entry->on_demand && load_entire_table(db_hdls->c_entry,
db_hdls, 0) < 0) {
if (!db_hdls->c_entry->on_demand &&
load_entire_table(db_hdls->c_entry, db_hdls, 0) < 0) {
LM_ERR("Failed to cache the entire table: %s\n", db_hdls->c_entry->table.s);
continue;
} else
Expand Down

0 comments on commit 642271c

Please sign in to comment.