SYSDB: perf improvements in sysdb_add_group_member_overrides(), part 2

src/db/sysdb_views.c

@@ -1547,7 +1547,6 @@ static errno_t get_user_members_recursively(TALLOC_CTX *mem_ctx,
     char *sanitized_name;
     const char *attrs[] =
         {
-            SYSDB_UIDNUM,
             SYSDB_OVERRIDE_DN,
             SYSDB_NAME,
             SYSDB_DEFAULT_OVERRIDE_NAME,
@@ -1576,16 +1575,17 @@ static errno_t get_user_members_recursively(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
-    filter = talloc_asprintf(tmp_ctx, "(&("SYSDB_UC")("SYSDB_MEMBEROF"=%s))",
+    filter = talloc_asprintf(tmp_ctx,
+                             "(&("SYSDB_UC")("SYSDB_MEMBEROF"=%s)("SYSDB_UIDNUM"=*))",
                              sanitized_name);
     if (filter == NULL) {
         DEBUG(SSSDBG_OP_FAILURE, "talloc_asprintf failed.\n");
         ret = ENOMEM;
         goto done;
     }
 
-    ret = sysdb_search_entry(tmp_ctx, dom->sysdb, base_dn, LDB_SCOPE_SUBTREE,
-                             filter, attrs, &count, &msgs);
+    ret = sysdb_cache_search_entry(tmp_ctx, dom->sysdb->ldb, base_dn, LDB_SCOPE_SUBTREE,
+                                   filter, attrs, &count, &msgs);
     if (ret != EOK) {
         goto done;
     }
@@ -1661,7 +1661,7 @@ errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain,
     struct ldb_result *res_members;
     TALLOC_CTX *tmp_ctx;
     struct ldb_result *override_obj;
-    static const char *member_attrs[] = SYSDB_PW_ATTRS;
+    static const char *member_attrs[] = { SYSDB_NAME, NULL };
     struct ldb_dn *override_dn = NULL;
     const char *memberuid;
     const char *val;
@@ -1670,6 +1670,15 @@ errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain,
         return EOK;
     }
 
+    if (ldb_msg_find_element(obj, SYSDB_MEMBERUID) == NULL) {
+        /* empty memberUid list means there are no user objects in
+         * the cache that would have 'memberOf = obj->dn',
+         * so get_user_members_recursively() will return an empty list
+         * anyway (but may consume a lot of CPU in case of a large cache)
+         */
+        return EOK;
+    }
+
     tmp_ctx = talloc_new(NULL);
     if (tmp_ctx == NULL) {
         DEBUG(SSSDBG_OP_FAILURE, "talloc_new failed.\n");
@@ -1688,12 +1697,6 @@ errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain,
     }
 
     for (c = 0; c < res_members->count; c++) {
-        if (ldb_msg_find_attr_as_uint64(res_members->msgs[c],
-                                        SYSDB_UIDNUM, 0) == 0) {
-            /* Skip non-POSIX-user members i.e. groups and non-POSIX users */
-            continue;
-        }
-
         if (expect_override_dn) {
             /* Creates new DN object. */
             override_dn = ldb_msg_find_attr_as_dn(domain->sysdb->ldb, tmp_ctx,
@@ -1757,7 +1760,8 @@ errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain,
             }
 
             if (memberuid == NULL) {
-                DEBUG(SSSDBG_TRACE_ALL, "No override name available.\n");
+                DEBUG_CONDITIONAL(SSSDBG_TRACE_ALL, "No override name available for %s.\n",
+                                  orig_name);
                 memberuid = orig_name;
             } else {
                 /* add domain name if memberuid is a short name */
@@ -1770,7 +1774,7 @@ errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain,
 
         val = talloc_steal(obj, memberuid);
         if (val == NULL) {
-            DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
+            DEBUG(SSSDBG_OP_FAILURE, "talloc_steal() failed.\n");
             ret = ENOMEM;
             goto done;
         }
@@ -1781,9 +1785,9 @@ errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain,
             ret = sysdb_error_to_errno(ret);
             goto done;
         }
-        DEBUG(SSSDBG_TRACE_ALL, "Added [%s] to [%s].\n", memberuid,
-                                OVERRIDE_PREFIX SYSDB_MEMBERUID);
-
+        DEBUG_CONDITIONAL(SSSDBG_TRACE_ALL,
+                          "Added [%s] to ["OVERRIDE_PREFIX SYSDB_MEMBERUID"].\n",
+                          memberuid);
     }
 
     ret = EOK;

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

@@ -32,7 +32,7 @@ cache_req_group_by_filter_prepare_domain_data(struct cache_req *cr,
                                               struct sss_domain_info *domain)
 {
     TALLOC_CTX *tmp_ctx;
-    const char *name;
+    char *name;
     errno_t ret;
 
     if (cr->data->name.name == NULL) {
@@ -52,11 +52,7 @@ cache_req_group_by_filter_prepare_domain_data(struct cache_req *cr,
         goto done;
     }
 
-    name = sss_reverse_replace_space(tmp_ctx, name, cr->rctx->override_space);
-    if (name == NULL) {
-        ret = ENOMEM;
-        goto done;
-    }
+    sss_reverse_replace_space_inplace(name, cr->rctx->override_space);
 
     talloc_zfree(data->name.lookup);
     data->name.lookup = talloc_steal(data, name);

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

@@ -32,7 +32,7 @@ cache_req_group_by_name_prepare_domain_data(struct cache_req *cr,
                                             struct sss_domain_info *domain)
 {
     TALLOC_CTX *tmp_ctx;
-    const char *name;
+    char *name;
     errno_t ret;
 
     if (cr->data->name.name == NULL) {
@@ -52,11 +52,7 @@ cache_req_group_by_name_prepare_domain_data(struct cache_req *cr,
         goto done;
     }
 
-    name = sss_reverse_replace_space(tmp_ctx, name, cr->rctx->override_space);
-    if (name == NULL) {
-        ret = ENOMEM;
-        goto done;
-    }
+    sss_reverse_replace_space_inplace(name, cr->rctx->override_space);
 
     name = sss_create_internal_fqname(tmp_ctx, name, domain->name);
     if (name == NULL) {

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

@@ -32,7 +32,7 @@ cache_req_initgroups_by_name_prepare_domain_data(struct cache_req *cr,
                                                  struct sss_domain_info *domain)
 {
     TALLOC_CTX *tmp_ctx;
-    const char *name;
+    char *name;
     errno_t ret;
 
     if (cr->data->name.name == NULL) {
@@ -52,11 +52,7 @@ cache_req_initgroups_by_name_prepare_domain_data(struct cache_req *cr,
         goto done;
     }
 
-    name = sss_reverse_replace_space(tmp_ctx, name, cr->rctx->override_space);
-    if (name == NULL) {
-        ret = ENOMEM;
-        goto done;
-    }
+    sss_reverse_replace_space_inplace(name, cr->rctx->override_space);
 
     name = sss_create_internal_fqname(tmp_ctx, name, domain->name);
     if (name == NULL) {

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

@@ -76,7 +76,7 @@ cache_req_object_by_name_prepare_domain_data(struct cache_req *cr,
                                              struct sss_domain_info *domain)
 {
     TALLOC_CTX *tmp_ctx;
-    const char *name;
+    char *name;
     errno_t ret;
 
     if (cr->data->name.name == NULL) {
@@ -96,11 +96,7 @@ cache_req_object_by_name_prepare_domain_data(struct cache_req *cr,
         goto done;
     }
 
-    name = sss_reverse_replace_space(tmp_ctx, name, cr->rctx->override_space);
-    if (name == NULL) {
-        ret = ENOMEM;
-        goto done;
-    }
+    sss_reverse_replace_space_inplace(name, cr->rctx->override_space);
 
     name = sss_create_internal_fqname(tmp_ctx, name, domain->name);
     if (name == NULL) {

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

@@ -30,7 +30,7 @@ cache_req_subid_ranges_by_name_prepare_domain_data(struct cache_req *cr,
                                                    struct sss_domain_info *domain)
 {
     TALLOC_CTX *tmp_ctx;
-    const char *name;
+    char *name;
     errno_t ret;
 
     if (cr->data->name.name == NULL) {
@@ -50,11 +50,7 @@ cache_req_subid_ranges_by_name_prepare_domain_data(struct cache_req *cr,
         goto done;
     }
 
-    name = sss_reverse_replace_space(tmp_ctx, name, cr->rctx->override_space);
-    if (name == NULL) {
-        ret = ENOMEM;
-        goto done;
-    }
+    sss_reverse_replace_space_inplace(name, cr->rctx->override_space);
 
     talloc_zfree(data->name.lookup);
     data->name.lookup = talloc_steal(data, name);

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

@@ -32,7 +32,7 @@ cache_req_user_by_filter_prepare_domain_data(struct cache_req *cr,
                                              struct sss_domain_info *domain)
 {
     TALLOC_CTX *tmp_ctx;
-    const char *name;
+    char *name;
     errno_t ret;
 
     if (cr->data->name.name == NULL) {
@@ -52,11 +52,7 @@ cache_req_user_by_filter_prepare_domain_data(struct cache_req *cr,
         goto done;
     }
 
-    name = sss_reverse_replace_space(tmp_ctx, name, cr->rctx->override_space);
-    if (name == NULL) {
-        ret = ENOMEM;
-        goto done;
-    }
+    sss_reverse_replace_space_inplace(name, cr->rctx->override_space);
 
     talloc_zfree(data->name.lookup);
     data->name.lookup = talloc_steal(data, name);

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

@@ -32,7 +32,7 @@ cache_req_user_by_name_prepare_domain_data(struct cache_req *cr,
                                            struct sss_domain_info *domain)
 {
     TALLOC_CTX *tmp_ctx;
-    const char *name;
+    char *name;
     errno_t ret;
 
     if (cr->data->name.name == NULL) {
@@ -52,11 +52,7 @@ cache_req_user_by_name_prepare_domain_data(struct cache_req *cr,
         goto done;
     }
 
-    name = sss_reverse_replace_space(tmp_ctx, name, cr->rctx->override_space);
-    if (name == NULL) {
-        ret = ENOMEM;
-        goto done;
-    }
+    sss_reverse_replace_space_inplace(name, cr->rctx->override_space);
 
     name = sss_create_internal_fqname(tmp_ctx, name, domain->name);
     if (name == NULL) {

src/responder/common/negcache.c

@@ -96,7 +96,7 @@ static int sss_ncache_check_str(struct sss_nc_ctx *ctx, char *str)
     char *ep;
     int ret;
 
-    DEBUG(SSSDBG_TRACE_INTERNAL, "Checking negative cache for [%s]\n", str);
+    DEBUG_CONDITIONAL(SSSDBG_TRACE_INTERNAL, "Checking negative cache for [%s]\n", str);
 
     data.dptr = NULL;
 

src/responder/common/responder.h

@@ -362,12 +362,6 @@ errno_t sss_parse_inp_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
 const char **parse_attr_list_ex(TALLOC_CTX *mem_ctx, const char *conf_str,
                                 const char **defaults);
 
-char *sss_resp_create_fqname(TALLOC_CTX *mem_ctx,
-                             struct resp_ctx *rctx,
-                             struct sss_domain_info *dom,
-                             bool name_is_upn,
-                             const char *orig_name);
-
 errno_t sss_resp_populate_cr_domains(struct resp_ctx *rctx);
 
 const char *

src/responder/common/responder_common.c

@@ -1657,20 +1657,13 @@ int sized_output_name(TALLOC_CTX *mem_ctx,
                       struct sss_domain_info *name_dom,
                       struct sized_string **_name)
 {
-    TALLOC_CTX *tmp_ctx = NULL;
     errno_t ret;
     char *name_str;
     struct sized_string *name;
 
-    tmp_ctx = talloc_new(NULL);
-    if (tmp_ctx == NULL) {
-        return ENOMEM;
-    }
-
-    name = talloc_zero(tmp_ctx, struct sized_string);
+    name = talloc_zero(mem_ctx, struct sized_string);
     if (name == NULL) {
-        ret = ENOMEM;
-        goto done;
+        return ENOMEM;
     }
 
     ret = sss_output_fqname(name, name_dom, orig_name,
@@ -1680,10 +1673,15 @@ int sized_output_name(TALLOC_CTX *mem_ctx,
     }
 
     to_sized_string(name, name_str);
-    *_name = talloc_steal(mem_ctx, name);
     ret = EOK;
+
 done:
-    talloc_zfree(tmp_ctx);
+    if (ret == EOK) {
+        *_name = name;
+    } else {
+        talloc_free(name);
+    }
+
     return ret;
 }
 
@@ -1692,37 +1690,20 @@ int sized_domain_name(TALLOC_CTX *mem_ctx,
                       const char *member_name,
                       struct sized_string **_name)
 {
-    TALLOC_CTX *tmp_ctx = NULL;
-    errno_t ret;
-    char *domname;
+    const char *domain;
     struct sss_domain_info *member_dom;
 
-    tmp_ctx = talloc_new(NULL);
-    if (tmp_ctx == NULL) {
-        return ENOMEM;
-    }
-
-    ret = sss_parse_internal_fqname(tmp_ctx, member_name, NULL, &domname);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_CRIT_FAILURE, "sss_parse_internal_fqname failed\n");
-        goto done;
-    }
-
-    if (domname == NULL) {
-        ret = ERR_WRONG_NAME_FORMAT;
-        goto done;
+    domain = sss_get_domain_internal_fqname(member_name);
+    if (domain == NULL) {
+        return ERR_WRONG_NAME_FORMAT;
     }
 
     member_dom = find_domain_by_name(get_domains_head(rctx->domains),
-                                     domname, true);
+                                     domain, true);
     if (member_dom == NULL) {
-        ret = ERR_DOMAIN_NOT_FOUND;
-        goto done;
+        return ERR_DOMAIN_NOT_FOUND;
     }
 
-    ret = sized_output_name(mem_ctx, rctx, member_name,
-                            member_dom, _name);
-done:
-    talloc_free(tmp_ctx);
-    return ret;
+    return sized_output_name(mem_ctx, rctx, member_name,
+                             member_dom, _name);
 }