From dce770645aa39ae836fe63bb4b4a78b7abb2332a Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Fri, 29 Apr 2016 17:51:49 +0200 Subject: [PATCH 09/14] SYSDB: Check if group attributes differ before saving a group Adds a new function sysdb_entry_attrs_diff() used in group saving code. This function is used to check if the result of updating a group would result in actually changing the sysdb entry -- often, we would try to dump the same data to the cache during update. If that's the case, the update code now only updates the timestamp cache, avoiding costly writes. --- src/db/sysdb.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++++ src/db/sysdb_ops.c | 20 ++++++--- src/db/sysdb_private.h | 11 +++++ 3 files changed, 139 insertions(+), 7 deletions(-) diff --git a/src/db/sysdb.c b/src/db/sysdb.c index 1224e96add5e89c9912a332af3196c62905828d3..cb35d1c655576756450a0abe1a742cd7dd2ce084 100644 --- a/src/db/sysdb.c +++ b/src/db/sysdb.c @@ -1791,3 +1791,118 @@ bool sysdb_msg_attrs_modts_differs(struct ldb_message *old_entry, return true; } + +static bool sysdb_ldb_msg_difference(struct ldb_message *db_msg, + struct ldb_message *mod_msg) +{ + struct ldb_message_element *mod_msg_el; + struct ldb_message_element *db_msg_el; + int el_differs; + + for (unsigned i = 0; i < mod_msg->num_elements; i++) { + mod_msg_el = &mod_msg->elements[i]; + + switch (mod_msg_el->flags) { + case 0: + /* Unspecified flags are internally converted to SYSDB_MOD_REP in + * sysdb_set_group_attr, do the same here + */ + case SYSDB_MOD_ADD: + case SYSDB_MOD_REP: + db_msg_el = ldb_msg_find_element(db_msg, mod_msg_el->name); + if (db_msg_el == NULL) { + /* The attribute to be added does not exist in the target + * message, this is a modification. Special-case adding + * empty elements which also do not exist in the target + * message. This is how sysdb callers ensure a particular + * element is not present in the database. + */ + if (mod_msg_el->num_values > 0) { + /* We can ignore additions of timestamp attributes */ + return true; + } + break; + } + + el_differs = ldb_msg_element_compare(db_msg_el, mod_msg_el); + if (el_differs) { + /* We are replacing or extending element, there is a difference. If + * some values already exist and ldb_add is not permissive, + * ldb will throw an error, but that's not our job to check.. + */ + if (is_ts_cache_attr(mod_msg_el->name) == false) { + /* We can ignore changes to timestamp attributes */ + return true; + } + } + break; + case SYSDB_MOD_DEL: + db_msg_el = ldb_msg_find_element(db_msg, mod_msg_el->name); + if (db_msg_el != NULL) { + /* We are deleting a valid element, there is a difference */ + return true; + } + break; + } + } + + return false; +} + +bool sysdb_entry_attrs_diff(struct sysdb_ctx *sysdb, + struct ldb_dn *entry_dn, + struct sysdb_attrs *attrs, + int mod_op) +{ + struct ldb_message *new_entry_msg = NULL; + TALLOC_CTX *tmp_ctx; + bool differs = true; + int lret; + errno_t ret; + struct ldb_result *res; + const char *attrnames[attrs->num+1]; + + if (sysdb->ldb_ts == NULL) { + return true; + } + + if (is_ts_ldb_dn(entry_dn) == false) { + return true; + } + + tmp_ctx = talloc_new(NULL); + if (tmp_ctx == NULL) { + goto done; + } + + new_entry_msg = sysdb_attrs2msg(tmp_ctx, entry_dn, + attrs, mod_op); + if (new_entry_msg == NULL) { + goto done; + } + + for (int i = 0; i < attrs->num; i++) { + attrnames[i] = attrs->a[i].name; + } + attrnames[attrs->num] = NULL; + + lret = ldb_search(sysdb->ldb, tmp_ctx, &res, entry_dn, LDB_SCOPE_BASE, + attrnames, NULL); + if (lret != LDB_SUCCESS) { + ret = sysdb_error_to_errno(lret); + DEBUG(SSSDBG_MINOR_FAILURE, "Cannot search sysdb: %d\n", ret); + goto done; + } + + if (res->count == 0) { + return true; + } else if (res->count != 1) { + ret = EIO; + goto done; + } + + differs = sysdb_ldb_msg_difference(res->msgs[0], new_entry_msg); +done: + talloc_free(tmp_ctx); + return differs; +} diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c index 7ecbfc83c1a1e6a05c07a8e3b94e09e7d78fbdb4..755a94489ce091d5cef14c5dae2a21fd5f604901 100644 --- a/src/db/sysdb_ops.c +++ b/src/db/sysdb_ops.c @@ -1160,10 +1160,15 @@ int sysdb_set_entry_attr(struct sysdb_ctx *sysdb, struct sysdb_attrs *attrs, int mod_op) { + bool sysdb_write = true; errno_t ret = EOK; errno_t tret = EOK; - ret = sysdb_set_cache_entry_attr(sysdb->ldb, entry_dn, attrs, mod_op); + sysdb_write = sysdb_entry_attrs_diff(sysdb, entry_dn, attrs, mod_op); + if (sysdb_write == true) { + ret = sysdb_set_cache_entry_attr(sysdb->ldb, entry_dn, attrs, mod_op); + } + if (ret == EOK) { tret = sysdb_set_ts_entry_attr(sysdb, entry_dn, attrs, mod_op); if (tret != EOK) { @@ -2551,6 +2556,7 @@ static errno_t sysdb_store_new_group(struct sss_domain_info *domain, static errno_t sysdb_store_group_attrs(struct sss_domain_info *domain, const char *name, gid_t gid, + struct ldb_message *cached_group, struct sysdb_attrs *attrs, uint64_t cache_timeout, time_t now); @@ -2563,7 +2569,7 @@ int sysdb_store_group(struct sss_domain_info *domain, time_t now) { TALLOC_CTX *tmp_ctx; - static const char *src_attrs[] = { SYSDB_NAME, SYSDB_GIDNUM, NULL }; + static const char *src_attrs[] = { "*", NULL }; struct ldb_message *msg; bool new_group = false; int ret; @@ -2616,13 +2622,12 @@ int sysdb_store_group(struct sss_domain_info *domain, now = time(NULL); } - /* FIXME: use the remote modification timestamp to know if the - * group needs any update */ - if (new_group) { - ret = sysdb_store_new_group(domain, name, gid, attrs, cache_timeout, now); + ret = sysdb_store_new_group(domain, name, gid, attrs, + cache_timeout, now); } else { - ret = sysdb_store_group_attrs(domain, name, gid, attrs, cache_timeout, now); + ret = sysdb_store_group_attrs(domain, name, gid, msg, attrs, + cache_timeout, now); } if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "Cache update failed: %d\n", ret); @@ -2701,6 +2706,7 @@ static errno_t sysdb_store_new_group(struct sss_domain_info *domain, static errno_t sysdb_store_group_attrs(struct sss_domain_info *domain, const char *name, gid_t gid, + struct ldb_message *cached_group, struct sysdb_attrs *attrs, uint64_t cache_timeout, time_t now) diff --git a/src/db/sysdb_private.h b/src/db/sysdb_private.h index 9d03d59f099dc6413759fad5b2319610cc43f2c5..6f6a19a743514a5e0241269041b615261f7898e6 100644 --- a/src/db/sysdb_private.h +++ b/src/db/sysdb_private.h @@ -264,4 +264,15 @@ struct ldb_message *sysdb_attrs2msg(TALLOC_CTX *mem_ctx, struct sysdb_attrs *attrs, int mod_op); +/* Compares the attributes between the existing attributes of entry_dn and + * the new_entry attributes that are about to be set. If the set would + * not yield into any differences (and therefore a write to the cache is + * not necessary), the function returns false (no diff), otherwise + * the function returns true (a difference exists). + */ +bool sysdb_entry_attrs_diff(struct sysdb_ctx *sysdb, + struct ldb_dn *entry_dn, + struct sysdb_attrs *attrs, + int mod_op); + #endif /* __INT_SYS_DB_H__ */ -- 2.4.11