[SSSD] [PATCH] Only try to relink ghost users if we're not enumerating

Jakub Hrozek jhrozek at redhat.com
Mon Apr 29 14:39:03 UTC 2013


On Mon, Apr 29, 2013 at 03:46:25PM +0200, Jakub Hrozek wrote:
> On Mon, Apr 29, 2013 at 03:10:33PM +0200, Jakub Hrozek wrote:
> > On Mon, Apr 29, 2013 at 03:03:14PM +0200, Jakub Hrozek wrote:
> > > https://fedorahosted.org/sssd/ticket/1893
> > > 
> > > When SSSD is not enumerating (which is the default), we are trying to
> > > link any "ghost" entries with a newly created user entry. However, when
> > > enumeration is on, this means a spurious search on adding any user.
> > 
> > btw much of the credit goes to Lukas who bisected the code and was able
> > to pinpoint the commit that broke the enumeration performance.
> 
> Self-nack, the patch breaks the sysdb unit tests.

New patches attached for both master and sssd-1-9.
-------------- next part --------------
>From 17757111bdce4fb70893fa8330176b98f7231569 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 29 Apr 2013 14:37:27 +0200
Subject: [PATCH] Only try to relink ghost users if we're not enumerating

https://fedorahosted.org/sssd/ticket/1893

When SSSD is not enumerating (which is the default), we are trying to
link any "ghost" entries with a newly created user entry. However, when
enumeration is on, this means a spurious search on adding any user.
---
 src/db/sysdb_ops.c      | 12 ++++++++----
 src/tests/sysdb-tests.c | 12 ++++++++----
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index 1f27af8db02ffcce3f62103a92e57528dc4370ab..c758e727f888fc63098915db973dfe0a1285faa1 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -1199,10 +1199,14 @@ int sysdb_add_user(struct sysdb_ctx *sysdb,
     ret = sysdb_set_user_attr(sysdb, domain, name, attrs, SYSDB_MOD_REP);
     if (ret) goto done;
 
-    /* remove all ghost users */
-    ret = sysdb_remove_ghostattr_from_groups(sysdb, domain,
-                                             orig_dn, attrs, name);
-    if (ret) goto done;
+    if (domain->enumerate == false) {
+        /* If we're not enumerating, previous getgr{nam,gid} calls might
+         * have stored ghost users into the cache, so we need to link them
+         * with the newly-created user entry */
+        ret = sysdb_remove_ghostattr_from_groups(sysdb, domain,
+                                                 orig_dn, attrs, name);
+        if (ret) goto done;
+    }
 
     ret = EOK;
 
diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c
index 765fd09aef38089ae597bda289d034ef241e0dd7..b30f2ed4c8e73663396e9628e5523b5fe652759a 100644
--- a/src/tests/sysdb-tests.c
+++ b/src/tests/sysdb-tests.c
@@ -60,7 +60,7 @@ struct sysdb_test_ctx {
     struct sss_domain_info *domain;
 };
 
-static int setup_sysdb_tests(struct sysdb_test_ctx **ctx)
+static int _setup_sysdb_tests(struct sysdb_test_ctx **ctx, bool enumerate)
 {
     struct sysdb_test_ctx *test_ctx;
     char *conf_db;
@@ -127,7 +127,7 @@ static int setup_sysdb_tests(struct sysdb_test_ctx **ctx)
         return ret;
     }
 
-    val[0] = "TRUE";
+    val[0] = enumerate ? "TRUE" : "FALSE";
     ret = confdb_add_param(test_ctx->confdb, true,
                            "config/domain/LOCAL", "enumerate", val);
     if (ret != EOK) {
@@ -158,6 +158,8 @@ static int setup_sysdb_tests(struct sysdb_test_ctx **ctx)
     return EOK;
 }
 
+#define setup_sysdb_tests(ctx) _setup_sysdb_tests((ctx), false)
+
 struct test_data {
     struct tevent_context *ev;
     struct sysdb_test_ctx *ctx;
@@ -3178,8 +3180,10 @@ START_TEST (test_sysdb_memberof_check_convert)
     struct ldb_message_element *members;
     int exp_mem, exp_gh;
 
-    /* Setup */
-    ret = setup_sysdb_tests(&test_ctx);
+    /* Eplicitly disable enumeration during setup as converting the ghost
+     * users into real ones work only when enumeration is disabled
+     */
+    ret = _setup_sysdb_tests(&test_ctx, false);
     if (ret != EOK) {
         fail("Could not set up the test");
         return;
-- 
1.8.1.4

-------------- next part --------------
>From 982f2aae26a6dcad0299220e05088c0b676f3e54 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 29 Apr 2013 14:37:27 +0200
Subject: [PATCH] Only try to relink ghost users if we're not enumerating

https://fedorahosted.org/sssd/ticket/1893

When SSSD is not enumerating (which is the default), we are trying to
link any "ghost" entries with a newly created user entry. However, when
enumeration is on, this means a spurious search on adding any user.
---
 src/db/sysdb_ops.c      |  8 ++++++++
 src/tests/sysdb-tests.c | 12 ++++++++----
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index 5d074874049c31f351bd71c1fd4f5649d9a8f06c..5e6698010a310a1ea94ac6160e2928fc00e4bb91 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -1071,6 +1071,14 @@ int sysdb_add_user(struct sysdb_ctx *sysdb,
         goto done;
     }
 
+    /* If we're not enumerating, previous getgr{nam,gid} calls might
+     * have stored ghost users into the cache, so we need to link them
+     * with the newly-created user entry */
+    if (domain->enumerate == true) {
+        ret = EOK;
+        goto done;
+    }
+
     /* We need to find all groups that contain this object as a ghost user
      * and replace the ghost user by actual member record in direct parents.
      * Note that this object can be referred to either by its name or any
diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c
index db45b7a6190638f6d9c07a23b35b59fec3d768f6..53152e895ab49274ff0aba6d832dd371568de137 100644
--- a/src/tests/sysdb-tests.c
+++ b/src/tests/sysdb-tests.c
@@ -60,7 +60,7 @@ struct sysdb_test_ctx {
     struct sss_domain_info *domain;
 };
 
-static int setup_sysdb_tests(struct sysdb_test_ctx **ctx)
+static int _setup_sysdb_tests(struct sysdb_test_ctx **ctx, bool enumerate)
 {
     struct sysdb_test_ctx *test_ctx;
     char *conf_db;
@@ -127,7 +127,7 @@ static int setup_sysdb_tests(struct sysdb_test_ctx **ctx)
         return ret;
     }
 
-    val[0] = "TRUE";
+    val[0] = enumerate ? "TRUE" : "FALSE";
     ret = confdb_add_param(test_ctx->confdb, true,
                            "config/domain/LOCAL", "enumerate", val);
     if (ret != EOK) {
@@ -158,6 +158,8 @@ static int setup_sysdb_tests(struct sysdb_test_ctx **ctx)
     return EOK;
 }
 
+#define setup_sysdb_tests(ctx) _setup_sysdb_tests((ctx), false)
+
 struct test_data {
     struct tevent_context *ev;
     struct sysdb_test_ctx *ctx;
@@ -2976,8 +2978,10 @@ START_TEST (test_sysdb_memberof_check_convert)
     struct ldb_message_element *members;
     int exp_mem, exp_gh;
 
-    /* Setup */
-    ret = setup_sysdb_tests(&test_ctx);
+    /* Eplicitly disable enumeration during setup as converting the ghost
+     * users into real ones work only when enumeration is disabled
+     */
+    ret = _setup_sysdb_tests(&test_ctx, false);
     if (ret != EOK) {
         fail("Could not set up the test");
         return;
-- 
1.8.1.4



More information about the sssd-devel mailing list