From da89d5e309a6f6b1053f3ca93a763e2a0d55c3f0 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Fri, 27 May 2016 14:16:12 +0200 Subject: [PATCH 14/14] LDAP: Shortcut looking up for group members sooner This patch is a performance enhancement. When looking for entries to refresh, we always looked up all members in the cache, even if we ended up dereferencing the whole group. If we are about to try dereference, it makes sense to shortcut the lookups after the dereference threshold is reached. In that case, the split_members function returns a special error code and the caller just dereferences the whole group. Only if dereference fails, we fall back to looking up all members so that we can look them up one-by-one. Also adds an integration test to make sure the dereference code works. --- src/providers/ldap/sdap_async_nested_groups.c | 63 ++++++++++++++++++++++----- src/tests/intg/ldap_test.py | 40 +++++++++++++++++ src/util/util_errors.c | 1 + src/util/util_errors.h | 1 + 4 files changed, 95 insertions(+), 10 deletions(-) diff --git a/src/providers/ldap/sdap_async_nested_groups.c b/src/providers/ldap/sdap_async_nested_groups.c index 1ebf66067d9412b8f4bb7fd1f7aa0326f43cd9c2..3e3329c0e8fba1915e2e065abb0cb3f21be36e6f 100644 --- a/src/providers/ldap/sdap_async_nested_groups.c +++ b/src/providers/ldap/sdap_async_nested_groups.c @@ -552,6 +552,7 @@ sdap_nested_member_is_group(struct sdap_nested_group_ctx *group_ctx, static errno_t sdap_nested_group_split_members(TALLOC_CTX *mem_ctx, struct sdap_nested_group_ctx *group_ctx, + int threshold, int nesting_level, struct ldb_message_element *members, struct sdap_nested_group_member **_missing, @@ -687,6 +688,14 @@ sdap_nested_group_split_members(TALLOC_CTX *mem_ctx, missing[num_missing].group_filter = talloc_steal(missing, group_filter); num_missing++; + if (threshold > 0 && num_missing > threshold) { + if (_num_missing) { + *_num_missing = num_missing; + } + + ret = ERR_DEREF_THRESHOLD; + goto done; + } if (type != SDAP_NESTED_GROUP_DN_USER) { num_groups++; @@ -996,9 +1005,11 @@ struct sdap_nested_group_process_state { int num_missing_total; int num_missing_groups; struct ldb_message_element *ext_members; + struct ldb_message_element *members; int nesting_level; char *group_dn; bool deref; + bool deref_shortcut; }; static void sdap_nested_group_process_done(struct tevent_req *subreq); @@ -1014,9 +1025,9 @@ sdap_nested_group_process_send(TALLOC_CTX *mem_ctx, struct sdap_attr_map *group_map = NULL; struct tevent_req *req = NULL; struct tevent_req *subreq = NULL; - struct ldb_message_element *members = NULL; const char *orig_dn = NULL; errno_t ret; + int split_threshold; req = tevent_req_create(mem_ctx, &state, struct sdap_nested_group_process_state); @@ -1052,7 +1063,7 @@ sdap_nested_group_process_send(TALLOC_CTX *mem_ctx, group); ret = sysdb_attrs_get_el_ext(group, group_map[SDAP_AT_GROUP_MEMBER].sys_name, - false, &members); + false, &state->members); if (ret == ENOENT && state->ext_members == NULL) { ret = EOK; /* no members, direct or external */ goto immediately; @@ -1062,15 +1073,25 @@ sdap_nested_group_process_send(TALLOC_CTX *mem_ctx, goto immediately; } + split_threshold = state->group_ctx->try_deref ? \ + state->group_ctx->deref_treshold : \ + -1; + /* get members that need to be refreshed */ PROBE(SDAP_NESTED_GROUP_PROCESS_SPLIT_PRE); ret = sdap_nested_group_split_members(state, state->group_ctx, - state->nesting_level, members, + split_threshold, + state->nesting_level, + state->members, &state->missing, &state->num_missing_total, &state->num_missing_groups); PROBE(SDAP_NESTED_GROUP_PROCESS_SPLIT_POST); - if (ret != EOK) { + if (ret == ERR_DEREF_THRESHOLD) { + DEBUG(SSSDBG_TRACE_FUNC, + "More members were missing than the deref threshold\n"); + state->deref_shortcut = true; + } else if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "Unable to split member list " "[%d]: %s\n", ret, sss_strerror(ret)); goto immediately; @@ -1095,10 +1116,11 @@ sdap_nested_group_process_send(TALLOC_CTX *mem_ctx, * proceed and let the direct lookup code just fall through. */ - DEBUG(SSSDBG_TRACE_INTERNAL, "Looking up %d/%d members of group [%s]\n", - state->num_missing_total, - members ? members->num_values : 0, - orig_dn); + DEBUG(SSSDBG_TRACE_INTERNAL, + "Looking up %d/%d members of group [%s]\n", + state->num_missing_total, + state->members ? state->members->num_values : 0, + orig_dn); /* process members */ if (group_ctx->try_deref @@ -1106,8 +1128,8 @@ sdap_nested_group_process_send(TALLOC_CTX *mem_ctx, DEBUG(SSSDBG_TRACE_INTERNAL, "Dereferencing members of group [%s]\n", orig_dn); state->deref = true; - subreq = sdap_nested_group_deref_send(state, ev, group_ctx, members, - orig_dn, + subreq = sdap_nested_group_deref_send(state, ev, group_ctx, + state->members, orig_dn, state->nesting_level); } else { DEBUG(SSSDBG_TRACE_INTERNAL, "Members of group [%s] will be " @@ -1159,6 +1181,27 @@ static void sdap_nested_group_process_done(struct tevent_req *subreq) DEBUG(SSSDBG_TRACE_INTERNAL, "Members of group [%s] will be " "processed individually\n", state->group_dn); + if (state->deref_shortcut == true) { + /* If we previously short-cut dereference, we need to split the + * members again to get full list of missing member types + */ + PROBE(SDAP_NESTED_GROUP_PROCESS_SPLIT_PRE); + ret = sdap_nested_group_split_members(state, state->group_ctx, + -1, + state->nesting_level, + state->members, + &state->missing, + &state->num_missing_total, + &state->num_missing_groups); + PROBE(SDAP_NESTED_GROUP_PROCESS_SPLIT_POST); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, "Unable to split member list " + "[%d]: %s\n", + ret, sss_strerror(ret)); + goto done; + } + } + subreq = sdap_nested_group_single_send(state, state->ev, state->group_ctx, diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py index 86a3d6e0e7fef3dd55c8a718a5010fd561c35a53..84f7f2d8a1b1830add7325fe16f97150a33263a7 100644 --- a/src/tests/intg/ldap_test.py +++ b/src/tests/intg/ldap_test.py @@ -144,6 +144,21 @@ def format_interactive_conf(ldap_conn, schema): entry_cache_timeout = {0} """).format(INTERACTIVE_TIMEOUT) +def format_rfc2307bis_deref_conf(ldap_conn, schema): + """Format an SSSD configuration with all caches refreshing in 4 seconds""" + return \ + format_basic_conf(ldap_conn, schema, enum=False) + \ + unindent(""" + [nss] + memcache_timeout = 0 + enum_cache_timeout = {0} + entry_negative_timeout = 0 + + [domain/LDAP] + entry_cache_timeout = {0} + ldap_deref_threshold = 1 + """).format(INTERACTIVE_TIMEOUT) + def create_conf_file(contents): """Create sssd.conf with specified contents""" @@ -459,6 +474,26 @@ def user_and_groups_rfc2307_bis(request, ldap_conn): return None +@pytest.fixture +def rfc2307bis_deref_group_with_users(request, ldap_conn): + """ + Create an RFC2307bis directory fixture with interactive SSSD conf, + one user and two groups + """ + ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn) + ent_list.add_user("user1", 1001, 2000) + ent_list.add_user("user2", 1001, 2000) + ent_list.add_user("user3", 1001, 2000) + ent_list.add_group_bis("group1", 20000, member_uids=("user1", "user2")) + create_ldap_fixture(request, ldap_conn, ent_list) + create_conf_fixture(request, + format_rfc2307bis_deref_conf( + ldap_conn, + SCHEMA_RFC2307_BIS)) + create_sssd_fixture(request) + return None + + def test_add_remove_user(ldap_conn, blank_rfc2307): """Test user addition and removal are reflected by SSSD""" e = ldap_ent.user(ldap_conn.ds_inst.base_dn, "user", 2001, 2000) @@ -555,6 +590,11 @@ def test_add_remove_membership_rfc2307_bis(ldap_conn, ent.assert_group_by_name("group1", dict(mem=ent.contains_only())) +def test_ldap_group_dereference(ldap_conn, rfc2307bis_deref_group_with_users): + ent.assert_group_by_name("group1", + dict(mem=ent.contains_only( + "user1", "user2"))) + @pytest.fixture def override_homedir(request, ldap_conn): ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn) diff --git a/src/util/util_errors.c b/src/util/util_errors.c index 2d2bafb1eba8f10b5de00c22aaacf7e4ad6e8059..3addcf1d3bc958e0604d4f8960fc419804398ba0 100644 --- a/src/util/util_errors.c +++ b/src/util/util_errors.c @@ -93,6 +93,7 @@ struct err_string error_to_str[] = { { "Sysdb version is too new" }, /* ERR_SYSDB_VERSION_TOO_NEW */ { "Domain has to timestamp cache" }, /* ERR_NO_TS */ { "No timestamp cache record" }, /* ERR_TS_CACHE_MISS */ + { "Dereference threshold reached" }, /* ERR_DEREF_THRESHOLD */ { "ERR_LAST" } /* ERR_LAST */ }; diff --git a/src/util/util_errors.h b/src/util/util_errors.h index e8ea87765f19bd4ebed0abef1f7efc1ec341144c..fdf443c87923d583dff909737de1b19f32d15ade 100644 --- a/src/util/util_errors.h +++ b/src/util/util_errors.h @@ -115,6 +115,7 @@ enum sssd_errors { ERR_SYSDB_VERSION_TOO_NEW, ERR_NO_TS, ERR_TS_CACHE_MISS, + ERR_DEREF_THRESHOLD, ERR_LAST /* ALWAYS LAST */ }; -- 2.4.11