date:20100914

Re: [SSSD] Behaviour of getgrnam/getgrgid

2010-09-14 Thread Simo Sorce

On Tue, 14 Sep 2010 15:13:19 +0200
Ralf Haferkamp  wrote:

> On Friday 10 September 2010 13:48:31 Simo Sorce wrote:
> > On Fri, 10 Sep 2010 12:06:25 +0200
> > 
> > Ralf Haferkamp  wrote:
> > > On Thursday 09 September 2010 18:33:26 Simo Sorce wrote:
> > > > So we have the following scenarios:
> > > > 
> > > > 1) If we use rfc2307 classic with memberUid attributes, we can
> > > > just create the fake/expired users and be done with it.
> > > 
> > > Agreed.
> > > 
> > > > 2) If we use rfc2307bis w/o nested groups we can count the
> > > > number of members on the group entry and switch to a full user
> > > > enumeration if the number of members is greater than a defined
> > > > (potentially user defined) threshold. Assuming a threshold value
> > > > of 100, if we have 10 members we just do 10 lookups, while, if
> > > > we have > 100 member we do a full enumeration (it's a single
> > > > ldap search, and if
> > > > modifyTimestamp is used also highly optimized after the first
> > > > search) so we are sure all users are there with the right name.
> > > 
> > > I think that's an optimization that could be worth exploring
> > > later. In a first iteration I'd just go for implementing the
> > > complete group lookup by looking up every single user regardless
> > > of how large the group is. (plus the possiblity to completely
> > > disable member unrolling through the configuration)
> > 
> > Ok, although performances will suck so much I bet we will have to
> > implement the optimization pretty soon.
> After doing some test here, I really wonder if the LDAP lookups are
> going to be the bottleneck. I just tested a slightly improved version
> of my previous patch with a group with 500 members (not really big, I
> know). Doing "getent group" on that group takes about 2sec on my
> testsystem, which is really slow (nss_ldap needs 0.5sec). But looking
> closer at that, the LDAP searches and the processing of the search
> results accounts only for about 160ms of that two seconds. Much more
> time (more than 1sec) is then spent in saving the user objects and
> the group in sysdb. Probably it would be worth looking for
> optimizations in that area first. Other calls (initgroups?) could
> benefit from that as well.

Not that easy unfortunately.

> Note, I already save all users in one transaction instead of starting
> a new transaction for every user. Also note that, opening the sysdb
> with LDB_FLG_NOSYNC brings down the time to about 1.3sec. 

Yes, we have a bit of a bottlneck issue with LDB, because every
transaction requires at least to fsyncs.
Unfortunately we can't run with LDB_FLG_NOSYNC as that would make it
too easy to corrupt the db in case of power loss.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

[SSSD] [PATCH] Request all group attributes during initgroups processing (sssd-1-2)

2010-09-14 Thread Stephen Gallagher

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

We tried to be too clever and only requested the name of the group,
but we require the objectClass to validate the results.

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


This is rebased from the patches in the thread "Fix two serious issues
with initgroups". The other two patches are unnecessary in sssd-1-2 (bug
620 does not exist there)

- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkyPkKQACgkQeiVVYja6o6Oz7QCdGqpJk4DdXhY98yQG+yyUDbeG
/+QAnijrjOrBOHoBJra/ETAg2nhdjxPN
=QCGD
-END PGP SIGNATURE-
From e7d198d43bbd37d021f2dab7d6050654f98388d9 Mon Sep 17 00:00:00 2001
From: Stephen Gallagher 
Date: Mon, 13 Sep 2010 11:45:42 -0400
Subject: [PATCH] Request all group attributes during initgroups processing

We tried to be too clever and only requested the name of the group,
but we require the objectClass to validate the results.

https://fedorahosted.org/sssd/ticket/622
---
 src/providers/ldap/sdap_async_accounts.c |   11 ++-
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/providers/ldap/sdap_async_accounts.c b/src/providers/ldap/sdap_async_accounts.c
index beab326a05fe429a56d4eee78590ed78a58279c8..d83a9dcf3a257ed442fee1aca5784de87925c9d6 100644
--- a/src/providers/ldap/sdap_async_accounts.c
+++ b/src/providers/ldap/sdap_async_accounts.c
@@ -1508,7 +1508,8 @@ struct tevent_req *sdap_initgr_rfc2307_send(TALLOC_CTX *memctx,
 struct tevent_req *req, *subreq;
 struct sdap_initgr_rfc2307_state *state;
 const char *filter;
-const char *attrs[2];
+const char **attrs;
+errno_t ret;
 
 req = tevent_req_create(memctx, &state, struct sdap_initgr_rfc2307_state);
 if (!req) return NULL;
@@ -1525,12 +1526,12 @@ struct tevent_req *sdap_initgr_rfc2307_send(TALLOC_CTX *memctx,
 return NULL;
 }
 
-attrs[0] = talloc_strdup(state, opts->group_map[SDAP_AT_GROUP_NAME].name);
-if (!attrs[0]) {
-talloc_zfree(req);
+ret = build_attrs_from_map(state, opts->group_map,
+   SDAP_OPTS_GROUP, &attrs);
+if (ret != EOK) {
+talloc_free(req);
 return NULL;
 }
-attrs[1] = NULL;
 
 filter = talloc_asprintf(state, "(&(%s=%s)(objectclass=%s))",
  opts->group_map[SDAP_AT_GROUP_MEMBER].name,
-- 
1.7.2.2



0001-Request-all-group-attributes-during-initgroups-proce.patch.sig
Description: PGP signature
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] Behaviour of getgrnam/getgrgid

2010-09-14 Thread Ralf Haferkamp

On Friday 10 September 2010 13:48:31 Simo Sorce wrote:
> On Fri, 10 Sep 2010 12:06:25 +0200
> 
> Ralf Haferkamp  wrote:
> > On Thursday 09 September 2010 18:33:26 Simo Sorce wrote:
> > > So we have the following scenarios:
> > > 
> > > 1) If we use rfc2307 classic with memberUid attributes, we can
> > > just create the fake/expired users and be done with it.
> > 
> > Agreed.
> > 
> > > 2) If we use rfc2307bis w/o nested groups we can count the number
> > > of members on the group entry and switch to a full user
> > > enumeration if the number of members is greater than a defined
> > > (potentially user defined) threshold. Assuming a threshold value
> > > of 100, if we have 10 members we just do 10 lookups, while, if we
> > > have > 100 member we do a full enumeration (it's a single ldap
> > > search, and if
> > > modifyTimestamp is used also highly optimized after the first
> > > search) so we are sure all users are there with the right name.
> > 
> > I think that's an optimization that could be worth exploring later.
> > In a first iteration I'd just go for implementing the complete group
> > lookup by looking up every single user regardless of how large the
> > group is. (plus the possiblity to completely disable member
> > unrolling through the configuration)
> 
> Ok, although performances will suck so much I bet we will have to
> implement the optimization pretty soon.
After doing some test here, I really wonder if the LDAP lookups are going 
to be the bottleneck. I just tested a slightly improved version of my 
previous patch with a group with 500 members (not really big, I know). 
Doing "getent group" on that group takes about 2sec on my testsystem, 
which is really slow (nss_ldap needs 0.5sec). But looking closer at that, 
the LDAP searches and the processing of the search results accounts only 
for about 160ms of that two seconds. Much more time (more than 1sec) is 
then spent in saving the user objects and the group in sysdb. Probably it 
would be worth looking for optimizations in that area first. Other calls 
(initgroups?) could benefit from that as well.

Note, I already save all users in one transaction instead of starting a
new transaction for every user. Also note that, opening the sysdb with 
LDB_FLG_NOSYNC brings down the time to about 1.3sec. 

[..]

regards,
Ralf
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCHES] Fix two serious issues with initgroups

2010-09-14 Thread Stephen Gallagher

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 09/13/2010 04:36 PM, Simo Sorce wrote:
> On Mon, 13 Sep 2010 15:43:33 -0400
> Stephen Gallagher  wrote:
> 
>> -BEGIN PGP SIGNED MESSAGE-
>> Hash: SHA1
>>
>> On 09/13/2010 03:41 PM, Simo Sorce wrote:
>>> On Mon, 13 Sep 2010 15:09:28 -0400
>>> Stephen Gallagher  wrote:
>>>
 Sorry, noticed a tiny oversight on patch 0001. I had only fixed the
 DEBUG message for the add case, and missed the del case. The new
 patch 0001 corrects this.

>>>
>>> Aren't those bugs a separate issue worth their own patch ?
>>> (there is also no mention of this issue in the commit message
>>> AFAICS)
>>>
>>> Simo.
>>>
>>
>> I can split them out. I only included them because I needed to fix
>> them to help identify the real problem.
> 
> They are a potential crash bug, so it would be better if we separated
> them as they are a fix on their own.
> 
> If you split them you get as bonus an automatic ack for all three :)
> 
> Simo.
> 

Split and attached for posterity. Thanks for the review.

- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkyPZZMACgkQeiVVYja6o6PbKACfc31Om0Z5fAz11kyb9gjtwMjC
FrwAoI1YFW2JiGdmpKF45GQKc2wgBvOr
=p6um
-END PGP SIGNATURE-
From 95e21e11bfc73a07fd0b700607868f89074b4248 Mon Sep 17 00:00:00 2001
From: Stephen Gallagher 
Date: Mon, 13 Sep 2010 11:45:42 -0400
Subject: [PATCH 1/3] Request all group attributes during initgroups processing

We tried to be too clever and only requested the name of the group,
but we require the objectClass to validate the results.

https://fedorahosted.org/sssd/ticket/622
---
 src/providers/ldap/ldap_id.c |1 +
 src/providers/ldap/sdap_async_accounts.c |   11 ++-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c
index d52dcec5b081ccca44e3995a3f7672390df33e5d..0c90773a50fc8a2dbb1ed9ddb58c26b2d8291360 100644
--- a/src/providers/ldap/ldap_id.c
+++ b/src/providers/ldap/ldap_id.c
@@ -619,6 +619,7 @@ static void groups_by_user_done(struct tevent_req *subreq)
 return;
 }
 
+state->dp_error = DP_ERR_OK;
 tevent_req_done(req);
 }
 
diff --git a/src/providers/ldap/sdap_async_accounts.c b/src/providers/ldap/sdap_async_accounts.c
index 8999ba015fc8bf7c7884245cd055b10033b331f0..4db4a4ccd53a05670b63568ef504969a0d04a80a 100644
--- a/src/providers/ldap/sdap_async_accounts.c
+++ b/src/providers/ldap/sdap_async_accounts.c
@@ -1042,7 +1042,8 @@ struct tevent_req *sdap_initgr_rfc2307_send(TALLOC_CTX *memctx,
 struct tevent_req *req, *subreq;
 struct sdap_initgr_rfc2307_state *state;
 const char *filter;
-const char *attrs[2];
+const char **attrs;
+errno_t ret;
 
 req = tevent_req_create(memctx, &state, struct sdap_initgr_rfc2307_state);
 if (!req) return NULL;
@@ -1059,12 +1060,12 @@ struct tevent_req *sdap_initgr_rfc2307_send(TALLOC_CTX *memctx,
 return NULL;
 }
 
-attrs[0] = talloc_strdup(state, opts->group_map[SDAP_AT_GROUP_NAME].name);
-if (!attrs[0]) {
-talloc_zfree(req);
+ret = build_attrs_from_map(state, opts->group_map,
+   SDAP_OPTS_GROUP, &attrs);
+if (ret != EOK) {
+talloc_free(req);
 return NULL;
 }
-attrs[1] = NULL;
 
 filter = talloc_asprintf(state, "(&(%s=%s)(objectclass=%s))",
  opts->group_map[SDAP_AT_GROUP_MEMBER].name,
-- 
1.7.2.2

From 3652c365be56f0d54d92fd67638bdac6377a75fd Mon Sep 17 00:00:00 2001
From: Stephen Gallagher 
Date: Tue, 14 Sep 2010 08:05:18 -0400
Subject: [PATCH 2/3] Fix missing variable substitution in DEBUG message

---
 src/db/sysdb_ops.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index 017f8ebce9fefa4a747bf502c8c7f6ba61193ff8..d20375c32224f983b7fce4602c30f4e7aed4c4a4 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -2227,7 +2227,7 @@ errno_t sysdb_update_members(struct sysdb_ctx *sysdb,
  add_groups[i], user);
 if (ret != EOK) {
 DEBUG(1, ("Could not add user [%s] to group [%s]. "
-  "Skipping.\n"));
+  "Skipping.\n", user, add_groups[i]));
 /* Continue on, we should try to finish the rest */
 }
 }
@@ -2240,7 +2240,7 @@ errno_t sysdb_update_members(struct sysdb_ctx *sysdb,
 del_groups[i], user);
 if (ret != EOK) {
 DEBUG(1, ("Could not remove user [%s] from group [%s]. "
-  "Skipping\n"));
+  "Skipping\n", user, del_group

Re: [SSSD] Behaviour of getgrnam/getgrgid

[SSSD] [PATCH] Request all group attributes during initgroups processing (sssd-1-2)

Re: [SSSD] Behaviour of getgrnam/getgrgid

Re: [SSSD] [PATCHES] Fix two serious issues with initgroups

4 matches

Site Navigation

Mail list logo

Footer information