<p>Jenkins2 <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/7476">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Joshua Colp: Looks good to me, but someone else must approve
Kevin Harwell: Looks good to me, approved
Jenkins2: Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">astdb: Improve prefix searches in astdb<br><br>Using the LIKE operator requires a full table scan of 'astdb', whereas a<br>comparison operation is able to use the primary key index.<br><br>This patch adds a new function to the AstDB API for quick prefix matches<br>and updates res_sorcery_astdb to utilize it. This showed substantial<br>performance improvement in my test environment.<br><br>Related to ASTERISK~26806, but does not completely resolve it.<br><br>Change-Id: I7d37f9ba2aea139dabf2ca72d31fbe34bd9b2fa1<br>---<br>M include/asterisk/astdb.h<br>M main/db.c<br>M res/res_sorcery_astdb.c<br>3 files changed, 107 insertions(+), 30 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/include/asterisk/astdb.h b/include/asterisk/astdb.h<br>index 8a870ae..383864b 100644<br>--- a/include/asterisk/astdb.h<br>+++ b/include/asterisk/astdb.h<br>@@ -83,6 +83,16 @@<br> */<br> struct ast_db_entry *ast_db_gettree(const char *family, const char *keytree);<br> <br>+/*!<br>+ * \brief Get a list of values with the given key prefix<br>+ *<br>+ * \param family The family to search under<br>+ * \param key_prefix The key prefix to search under<br>+ *<br>+ * \retval NULL An error occurred<br>+ */<br>+struct ast_db_entry *ast_db_gettree_by_prefix(const char *family, const char *key_prefix);<br>+<br> /*! \brief Free structure created by ast_db_gettree() */<br> void ast_db_freetree(struct ast_db_entry *entry);<br> <br>diff --git a/main/db.c b/main/db.c<br>index fa125d7..afaf043 100644<br>--- a/main/db.c<br>+++ b/main/db.c<br>@@ -127,6 +127,20 @@<br> DEFINE_SQL_STATEMENT(showkey_stmt, "SELECT key, value FROM astdb WHERE key LIKE '%' || '/' || ? ORDER BY key")<br> DEFINE_SQL_STATEMENT(create_astdb_stmt, "CREATE TABLE IF NOT EXISTS astdb(key VARCHAR(256), value VARCHAR(256), PRIMARY KEY(key))")<br> <br>+/* This query begs an explanation:<br>+ *<br>+ * First, the parameter binding syntax used here is slightly different then the other<br>+ * queries in that we use a numbered parameter so that we can bind once and get the same<br>+ * value substituted multiple times within the executed query.<br>+ *<br>+ * Second, the key comparison is being used to find all keys that are lexicographically<br>+ * greater than the provided key, but less than the provided key with a high (but<br>+ * invalid) Unicode codepoint appended to it. This will give us all keys in the database<br>+ * that have 'key' as a prefix and performs much better than the equivalent "LIKE key ||<br>+ * '%'" operation.<br>+ */<br>+DEFINE_SQL_STATEMENT(gettree_prefix_stmt, "SELECT key, value FROM astdb WHERE key > ?1 AND key <= ?1 || X'ffff'")<br>+<br> static int init_stmt(sqlite3_stmt **stmt, const char *sql, size_t len)<br> {<br> ast_mutex_lock(&dblock);<br>@@ -167,6 +181,7 @@<br> clean_stmt(&deltree_all_stmt, deltree_all_stmt_sql);<br> clean_stmt(&gettree_stmt, gettree_stmt_sql);<br> clean_stmt(&gettree_all_stmt, gettree_all_stmt_sql);<br>+ clean_stmt(&gettree_prefix_stmt, gettree_prefix_stmt_sql);<br> clean_stmt(&showkey_stmt, showkey_stmt_sql);<br> clean_stmt(&put_stmt, put_stmt_sql);<br> clean_stmt(&create_astdb_stmt, create_astdb_stmt_sql);<br>@@ -182,6 +197,7 @@<br> || init_stmt(&deltree_all_stmt, deltree_all_stmt_sql, sizeof(deltree_all_stmt_sql))<br> || init_stmt(&gettree_stmt, gettree_stmt_sql, sizeof(gettree_stmt_sql))<br> || init_stmt(&gettree_all_stmt, gettree_all_stmt_sql, sizeof(gettree_all_stmt_sql))<br>+ || init_stmt(&gettree_prefix_stmt, gettree_prefix_stmt_sql, sizeof(gettree_prefix_stmt_sql))<br> || init_stmt(&showkey_stmt, showkey_stmt_sql, sizeof(showkey_stmt_sql))<br> || init_stmt(&put_stmt, put_stmt_sql, sizeof(put_stmt_sql));<br> }<br>@@ -473,19 +489,64 @@<br> return res;<br> }<br> <br>+static struct ast_db_entry *db_gettree_common(sqlite3_stmt *stmt)<br>+{<br>+ struct ast_db_entry *head = NULL, *prev = NULL, *cur;<br>+<br>+ while (sqlite3_step(stmt) == SQLITE_ROW) {<br>+ const char *key, *value;<br>+ size_t key_len, value_len;<br>+<br>+ key = (const char *) sqlite3_column_text(stmt, 0);<br>+ value = (const char *) sqlite3_column_text(stmt, 1);<br>+<br>+ if (!key || !value) {<br>+ break;<br>+ }<br>+<br>+ key_len = strlen(key);<br>+ value_len = strlen(value);<br>+<br>+ cur = ast_malloc(sizeof(*cur) + key_len + value_len + 2);<br>+ if (!cur) {<br>+ break;<br>+ }<br>+<br>+ cur->next = NULL;<br>+ cur->key = cur->data + value_len + 1;<br>+ memcpy(cur->data, value, value_len + 1);<br>+ memcpy(cur->key, key, key_len + 1);<br>+<br>+ if (prev) {<br>+ prev->next = cur;<br>+ } else {<br>+ head = cur;<br>+ }<br>+ prev = cur;<br>+ }<br>+<br>+ return head;<br>+}<br>+<br> struct ast_db_entry *ast_db_gettree(const char *family, const char *keytree)<br> {<br> char prefix[MAX_DB_FIELD];<br> sqlite3_stmt *stmt = gettree_stmt;<br>- struct ast_db_entry *cur, *last = NULL, *ret = NULL;<br>+ size_t res = 0;<br>+ struct ast_db_entry *ret;<br> <br> if (!ast_strlen_zero(family)) {<br> if (!ast_strlen_zero(keytree)) {<br> /* Family and key tree */<br>- snprintf(prefix, sizeof(prefix), "/%s/%s", family, keytree);<br>+ res = snprintf(prefix, sizeof(prefix), "/%s/%s", family, keytree);<br> } else {<br> /* Family only */<br>- snprintf(prefix, sizeof(prefix), "/%s", family);<br>+ res = snprintf(prefix, sizeof(prefix), "/%s", family);<br>+ }<br>+<br>+ if (res >= sizeof(prefix)) {<br>+ ast_log(LOG_WARNING, "Requested prefix is too long: %s\n", keytree);<br>+ return NULL;<br> }<br> } else {<br> prefix[0] = '\0';<br>@@ -493,41 +554,47 @@<br> }<br> <br> ast_mutex_lock(&dblock);<br>- if (!ast_strlen_zero(prefix) && (sqlite3_bind_text(stmt, 1, prefix, -1, SQLITE_STATIC) != SQLITE_OK)) {<br>- ast_log(LOG_WARNING, "Could bind %s to stmt: %s\n", prefix, sqlite3_errmsg(astdb));<br>+ if (res && (sqlite3_bind_text(stmt, 1, prefix, res, SQLITE_STATIC) != SQLITE_OK)) {<br>+ ast_log(LOG_WARNING, "Could not bind %s to stmt: %s\n", prefix, sqlite3_errmsg(astdb));<br> sqlite3_reset(stmt);<br> ast_mutex_unlock(&dblock);<br> return NULL;<br> }<br> <br>- while (sqlite3_step(stmt) == SQLITE_ROW) {<br>- const char *key_s, *value_s;<br>- if (!(key_s = (const char *) sqlite3_column_text(stmt, 0))) {<br>- break;<br>- }<br>- if (!(value_s = (const char *) sqlite3_column_text(stmt, 1))) {<br>- break;<br>- }<br>- if (!(cur = ast_malloc(sizeof(*cur) + strlen(key_s) + strlen(value_s) + 2))) {<br>- break;<br>- }<br>- cur->next = NULL;<br>- cur->key = cur->data + strlen(value_s) + 1;<br>- strcpy(cur->data, value_s);<br>- strcpy(cur->key, key_s);<br>- if (last) {<br>- last->next = cur;<br>- } else {<br>- ret = cur;<br>- }<br>- last = cur;<br>- }<br>+ ret = db_gettree_common(stmt);<br> sqlite3_reset(stmt);<br> ast_mutex_unlock(&dblock);<br> <br> return ret;<br> }<br> <br>+struct ast_db_entry *ast_db_gettree_by_prefix(const char *family, const char *key_prefix)<br>+{<br>+ char prefix[MAX_DB_FIELD];<br>+ size_t res;<br>+ struct ast_db_entry *ret;<br>+<br>+ res = snprintf(prefix, sizeof(prefix), "/%s/%s", family, key_prefix);<br>+ if (res >= sizeof(prefix)) {<br>+ ast_log(LOG_WARNING, "Requested key prefix is too long: %s\n", key_prefix);<br>+ return NULL;<br>+ }<br>+<br>+ ast_mutex_lock(&dblock);<br>+ if (sqlite3_bind_text(gettree_prefix_stmt, 1, prefix, res, SQLITE_STATIC) != SQLITE_OK) {<br>+ ast_log(LOG_WARNING, "Could not bind %s to stmt: %s\n", prefix, sqlite3_errmsg(astdb));<br>+ sqlite3_reset(gettree_prefix_stmt);<br>+ ast_mutex_unlock(&dblock);<br>+ return NULL;<br>+ }<br>+<br>+ ret = db_gettree_common(gettree_prefix_stmt);<br>+ sqlite3_reset(gettree_prefix_stmt);<br>+ ast_mutex_unlock(&dblock);<br>+<br>+ return ret;<br>+}<br>+<br> void ast_db_freetree(struct ast_db_entry *dbe)<br> {<br> struct ast_db_entry *last;<br>diff --git a/res/res_sorcery_astdb.c b/res/res_sorcery_astdb.c<br>index 8b93b57..87823be 100644<br>--- a/res/res_sorcery_astdb.c<br>+++ b/res/res_sorcery_astdb.c<br>@@ -334,14 +334,14 @@<br> const char *family_prefix = data;<br> size_t family_len = strlen(family_prefix) + strlen(type) + 1; /* +1 for slash delimiter */<br> char family[family_len + 1];<br>- char tree[prefix_len + sizeof("%")];<br>+ char tree[prefix_len + 1];<br> RAII_VAR(struct ast_db_entry *, entries, NULL, ast_db_freetree);<br> struct ast_db_entry *entry;<br> <br>- snprintf(tree, sizeof(tree), "%.*s%%", (int) prefix_len, prefix);<br>+ snprintf(tree, sizeof(tree), "%.*s", (int) prefix_len, prefix);<br> snprintf(family, sizeof(family), "%s/%s", family_prefix, type);<br> <br>- if (!(entries = ast_db_gettree(family, tree))) {<br>+ if (!(entries = ast_db_gettree_by_prefix(family, tree))) {<br> return;<br> }<br> <br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/7476">change 7476</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/7476"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 15 </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I7d37f9ba2aea139dabf2ca72d31fbe34bd9b2fa1 </div>
<div style="display:none"> Gerrit-Change-Number: 7476 </div>
<div style="display:none"> Gerrit-PatchSet: 6 </div>
<div style="display:none"> Gerrit-Owner: Sean Bright <sean.bright@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Corey Farrell <git@cfware.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Sean Bright <sean.bright@gmail.com> </div>