[autobackport: sssd-2-9] Improve the performance when using enumeration

src/db/sysdb.h

@@ -876,10 +876,6 @@ int sysdb_getpwuid(TALLOC_CTX *mem_ctx,
                    uid_t uid,
                    struct ldb_result **res);
 
-int sysdb_enumpwent(TALLOC_CTX *mem_ctx,
-                    struct sss_domain_info *domain,
-                    struct ldb_result **res);
-
 int sysdb_enumpwent_filter(TALLOC_CTX *mem_ctx,
                            struct sss_domain_info *domain,
                            const char *attr,

src/db/sysdb_init.c

@@ -37,6 +37,7 @@
 const char *sysdb_ts_cache_attrs[] = {
     SYSDB_OBJECTCLASS,
     SYSDB_OBJECTCATEGORY,
+    SYSDB_NAME,
     SYSDB_LAST_UPDATE,
     SYSDB_CACHE_EXPIRE,
     SYSDB_ORIG_MODSTAMP,
@@ -566,6 +567,12 @@ static errno_t sysdb_domain_cache_upgrade(TALLOC_CTX *mem_ctx,
         }
     }
 
+    if (strcmp(version, SYSDB_VERSION_0_25) == 0) {
+        ret = sysdb_upgrade_25(sysdb, &version);
+        if (ret != EOK) {
+            goto done;
+        }
+    }
     ret = EOK;
 done:
     sysdb->ldb = save_ldb;

src/db/sysdb_ops.c

@@ -1005,6 +1005,7 @@ static errno_t sysdb_create_ts_entry(struct sysdb_ctx *sysdb,
                                      struct sysdb_attrs *attrs)
 {
     struct ldb_message *msg;
+    const struct ldb_val *rdn_value;
     errno_t ret;
     int lret;
     TALLOC_CTX *tmp_ctx;
@@ -1013,16 +1014,26 @@ static errno_t sysdb_create_ts_entry(struct sysdb_ctx *sysdb,
         return EOK;
     }
 
+    if (entry_dn == NULL) {
+        return EINVAL;
+    }
+
     tmp_ctx = talloc_new(NULL);
     if (tmp_ctx == NULL) {
         return ENOMEM;
     }
 
-    if (entry_dn == NULL) {
+    rdn_value = ldb_dn_get_rdn_val(entry_dn);
+    if (rdn_value == NULL) {
         ret = EINVAL;
         goto done;
     }
 
+    ret = sysdb_attrs_add_val_safe(attrs, SYSDB_NAME, rdn_value);
+    if (ret != EOK) {
+        goto done;
+    }
+
     msg = sysdb_attrs2msg(tmp_ctx, entry_dn, attrs, 0);
     if (msg == NULL) {
         ret = ENOMEM;
@@ -1048,7 +1059,8 @@ static errno_t sysdb_create_ts_entry(struct sysdb_ctx *sysdb,
 }
 
 static struct sysdb_attrs *ts_obj_attrs(TALLOC_CTX *mem_ctx,
-                                        enum sysdb_obj_type obj_type)
+                                        enum sysdb_obj_type obj_type,
+                                        const char *obj_name)
 {
     struct sysdb_attrs *attrs;
     const char *oc;
@@ -1076,6 +1088,12 @@ static struct sysdb_attrs *ts_obj_attrs(TALLOC_CTX *mem_ctx,
         return NULL;
     }
 
+    ret = sysdb_attrs_add_string(attrs, SYSDB_NAME, obj_name);
+    if (ret != EOK) {
+        talloc_free(attrs);
+        return NULL;
+    }
+
     return attrs;
 }
 
@@ -1273,7 +1291,7 @@ static errno_t sysdb_create_ts_obj(struct sss_domain_info *domain,
         goto done;
     }
 
-    ts_attrs = ts_obj_attrs(tmp_ctx, obj_type);
+    ts_attrs = ts_obj_attrs(tmp_ctx, obj_type, obj_name);
     if (ts_attrs == NULL) {
         ret = ENOMEM;
         goto done;

src/db/sysdb_private.h

@@ -23,6 +23,7 @@
 #ifndef __INT_SYS_DB_H__
 #define __INT_SYS_DB_H__
 
+#define SYSDB_VERSION_0_26 "0.26"
 #define SYSDB_VERSION_0_25 "0.25"
 #define SYSDB_VERSION_0_24 "0.24"
 #define SYSDB_VERSION_0_23 "0.23"
@@ -49,7 +50,7 @@
 #define SYSDB_VERSION_0_2 "0.2"
 #define SYSDB_VERSION_0_1 "0.1"
 
-#define SYSDB_VERSION SYSDB_VERSION_0_25
+#define SYSDB_VERSION SYSDB_VERSION_0_26
 
 #define SYSDB_BASE_LDIF \
      "dn: @ATTRIBUTES\n" \
@@ -195,6 +196,7 @@ int sysdb_upgrade_21(struct sysdb_ctx *sysdb, const char **ver);
 int sysdb_upgrade_22(struct sysdb_ctx *sysdb, const char **ver);
 int sysdb_upgrade_23(struct sysdb_ctx *sysdb, const char **ver);
 int sysdb_upgrade_24(struct sysdb_ctx *sysdb, const char **ver);
+int sysdb_upgrade_25(struct sysdb_ctx *sysdb, const char **ver);
 
 int sysdb_ts_upgrade_01(struct sysdb_ctx *sysdb, const char **ver);
 

src/db/sysdb_search.c

@@ -878,9 +878,9 @@ int sysdb_enumpwent_filter(TALLOC_CTX *mem_ctx,
 
     /* Do not look for the user's attribute in the timestamp db as it could
      * not be present. Only look for the name. */
-    if (attr == NULL || is_sysdb_name(attr)) {
+    if (attr != NULL && is_sysdb_name(attr)) {
         ts_filter = enum_filter(tmp_ctx, SYSDB_PWENT_FILTER,
-                                NULL, NULL, NULL, addtl_filter);
+                                attr, attr_filter, domain->name, addtl_filter);
         if (ts_filter == NULL) {
             ret = ENOMEM;
             goto done;
@@ -899,20 +899,34 @@ int sysdb_enumpwent_filter(TALLOC_CTX *mem_ctx,
             goto done;
         }
 
-        ret = sysdb_enum_dn_filter(tmp_ctx, &ts_res, attr_filter, domain->name,
-                                   &dn_filter);
-        if (ret != EOK) {
-            goto done;
+        if (ret == EOK && ts_res.count > 0) {
+            ret = sysdb_enum_dn_filter(tmp_ctx, &ts_res, attr_filter, domain->name,
+                                       &dn_filter);
+            if (ret != EOK) {
+                goto done;
+            }
+            DEBUG(SSSDBG_TRACE_LIBS,
+                  "Searching timestamp entries with [%.50s] (limited to 50 characters)\n",
+                  dn_filter);
+            ret = sysdb_search_ts_matches(tmp_ctx, domain->sysdb, attrs, &ts_res,
+                                          dn_filter, &ts_cache_res);
+            if (ret != EOK && ret != ENOENT) {
+                goto done;
+            }
+        } else {
+            /* If there are no results, EOK and res->count == 0 are expected */
+            ts_cache_res = talloc_zero(tmp_ctx, struct ldb_result);
+            if (ts_cache_res == NULL) {
+                DEBUG(SSSDBG_OP_FAILURE, "talloc_zero() failed.\n");
+                ret = ENOMEM;
+                goto done;
+            }
         }
 
-        DEBUG(SSSDBG_TRACE_LIBS, "Searching timestamp entries with [%s]\n",
-              dn_filter);
-
-        ret = sysdb_search_ts_matches(tmp_ctx, domain->sysdb, attrs, &ts_res,
-                                      dn_filter, &ts_cache_res);
-        if (ret != EOK && ret != ENOENT) {
-            goto done;
-        }
+        ret = EOK;
+        DEBUG(SSSDBG_TRACE_LIBS, "Returning timestamp cache based results [%d].\n", ts_cache_res->count);
+        *_res = talloc_steal(mem_ctx, ts_cache_res);
+        goto done;
     }
 
     filter = enum_filter(tmp_ctx, SYSDB_PWENT_FILTER,
@@ -938,28 +952,13 @@ int sysdb_enumpwent_filter(TALLOC_CTX *mem_ctx,
         ret = EOK;
     }
 
-    if (ts_cache_res != NULL) {
-        res = sss_merge_ldb_results(res, ts_cache_res);
-        if (res == NULL) {
-            ret = ENOMEM;
-            goto done;
-        }
-    }
-
     *_res = talloc_steal(mem_ctx, res);
 
 done:
     talloc_zfree(tmp_ctx);
     return ret;
 }
 
-int sysdb_enumpwent(TALLOC_CTX *mem_ctx,
-                    struct sss_domain_info *domain,
-                    struct ldb_result **_res)
-{
-    return sysdb_enumpwent_filter(mem_ctx, domain, NULL, NULL, NULL, _res);
-}
-
 int sysdb_enumpwent_filter_with_views(TALLOC_CTX *mem_ctx,
                                       struct sss_domain_info *domain,
                                       const char *attr,
@@ -981,7 +980,7 @@ int sysdb_enumpwent_filter_with_views(TALLOC_CTX *mem_ctx,
     ret = sysdb_enumpwent_filter(tmp_ctx, domain, attr, attr_filter,
                                  addtl_filter, &res);
     if (ret != EOK) {
-        DEBUG(SSSDBG_OP_FAILURE, "sysdb_enumpwent failed.\n");
+        DEBUG(SSSDBG_OP_FAILURE, "sysdb_enumpwent_filter failed.\n");
         goto done;
     }
 

src/db/sysdb_upgrade.c

@@ -2801,6 +2801,28 @@ int sysdb_upgrade_24(struct sysdb_ctx *sysdb, const char **ver)
     return ret;
 }
 
+int sysdb_upgrade_25(struct sysdb_ctx *sysdb, const char **ver)
+{
+    struct upgrade_ctx *ctx;
+    errno_t ret;
+
+    ret = commence_upgrade(sysdb, sysdb->ldb, SYSDB_VERSION_0_26, &ctx);
+    if (ret != EOK) {
+        return ret;
+    }
+
+    /* We do nothing because the only goal of this version change is to remove the TS cache. */
+
+    ret = update_version(ctx);
+    if (ret != EOK) {
+        goto done;
+    }
+
+done:
+    ret = finish_upgrade(ret, &ctx, ver);
+    return ret;
+}
+
 /*
  * Example template for future upgrades.
  * Copy and change version numbers as appropriate.

src/responder/common/cache_req/plugins/cache_req_user_by_filter.c

@@ -90,7 +90,11 @@ cache_req_user_by_filter_lookup(TALLOC_CTX *mem_ctx,
      * Neither it is possible to use it when asking for a non-"name" attribute
      * as it could not be present in the timestamp cache.
      */
+<<<<<<< HEAD
     if (is_files_provider(domain) || data->name.attr != NULL) {
+=======
+    if (strcmp(attr, SYSDB_NAME) != 0) {
+>>>>>>> 670db53b1 (NSS: Be coherent when using a lastUpdate filter)
         recent_filter = NULL;
     } else {
         recent_filter = talloc_asprintf(mem_ctx, "(%s>=%"SPRItime")", SYSDB_LAST_UPDATE,

src/tests/cmocka/test_sysdb_views.c

@@ -1083,17 +1083,6 @@ static void check_enumpwent(int ret, struct sss_domain_info *dom,
     assert_user_attrs(res->msgs[2], dom, "bob", views);
 }
 
-static void test_sysdb_enumpwent(void **state)
-{
-    int ret;
-    struct sysdb_test_ctx *test_ctx = talloc_get_type_abort(*state,
-                                                        struct sysdb_test_ctx);
-    struct ldb_result *res;
-
-    ret = sysdb_enumpwent(test_ctx, test_ctx->domain, &res);
-    check_enumpwent(ret, test_ctx->domain, res, false);
-}
-
 static void test_sysdb_enumpwent_views(void **state)
 {
     int ret;
@@ -1160,6 +1149,15 @@ static void test_sysdb_enumpwent_filter(void **state)
     assert_int_equal(ret, EOK);
     assert_int_equal(res->count, 1);
     assert_user_attrs(res->msgs[0], test_ctx->domain, "alice", false);
+
+    ret = sysdb_enumpwent_filter(test_ctx, test_ctx->domain, NULL, NULL,
+                                 NULL, &res);
+    assert_int_equal(ret, EOK);
+    assert_int_equal(res->count, N_ELEMENTS(users)-1);
+    order_ldb_res_msgs(res);
+    assert_user_attrs(res->msgs[0], test_ctx->domain, "alice", false);
+    assert_user_attrs(res->msgs[1], test_ctx->domain, "barney", false);
+    assert_user_attrs(res->msgs[2], test_ctx->domain, "bob", false);
 }
 
 static void test_sysdb_enumpwent_filter_views(void **state)
@@ -1497,9 +1495,6 @@ int main(int argc, const char *argv[])
                                         test_sysdb_setup, test_sysdb_teardown),
         cmocka_unit_test_setup_teardown(test_sysdb_domain_update_domain_template,
                                         test_sysdb_setup, test_sysdb_teardown),
-        cmocka_unit_test_setup_teardown(test_sysdb_enumpwent,
-                                        test_enum_users_setup,
-                                        test_enum_users_teardown),
         cmocka_unit_test_setup_teardown(test_sysdb_enumpwent_views,
                                         test_enum_users_setup,
                                         test_enum_users_teardown),

src/tests/sysdb-tests.c

@@ -1360,33 +1360,6 @@ START_TEST (test_sysdb_enumgrent)
 }
 END_TEST
 
-START_TEST (test_sysdb_enumpwent)
-{
-    struct sysdb_test_ctx *test_ctx;
-    struct ldb_result *res;
-    int ret;
-
-    /* Setup */
-    ret = setup_sysdb_tests(&test_ctx);
-    if (ret != EOK) {
-        ck_abort_msg("Could not set up the test");
-        return;
-    }
-
-    ret = sysdb_enumpwent(test_ctx,
-                          test_ctx->domain,
-                          &res);
-    ck_assert_msg(ret == EOK,
-                "sysdb_enumpwent failed (%d: %s)",
-                ret, strerror(ret));
-
-    sss_ck_fail_if_msg(res->count != 10, "Expected 10 users, got %d", res->count);
-
-    talloc_free(test_ctx);
-}
-END_TEST
-
-
 START_TEST (test_sysdb_set_user_attr)
 {
     struct sysdb_test_ctx *test_ctx;
@@ -7837,9 +7810,6 @@ Suite *create_sysdb_suite(void)
     /* Verify the users can be queried by UID */
     tcase_add_loop_test(tc_sysdb, test_sysdb_getpwuid, 27010, 27020);
 
-    /* Enumerate the users */
-    tcase_add_test(tc_sysdb, test_sysdb_enumpwent);
-
     /* Change their attribute */
     tcase_add_loop_test(tc_sysdb, test_sysdb_set_user_attr, 27010, 27020);