Re: [SSSD] [PATCH] Optimize gorup enumerations
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 11/20/2009 01:30 PM, Stephen Gallagher wrote: > > You're welcome. That's why the tests are there. > > Ack. > Pushed to master. - -- 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.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAksHD8gACgkQeiVVYja6o6NXwACdF7qjIwl1dxbcm4myQSsBQ5UL CNkAoJjG/lQtekEYGvfaukUu0yYIQNSZ =2eLh -END PGP SIGNATURE- ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] Optimize gorup enumerations
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 11/20/2009 12:49 PM, Simo Sorce wrote: > > Thanks for catching this, it unveiled a serious error. > > Attached patch that correctly passes all tests. > > Simo. > You're welcome. That's why the tests are there. Ack. - -- 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.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAksG4EsACgkQeiVVYja6o6OmugCdFCrYj8M7ZECBd9Trle1qzMQ4 7/wAnjPFg9MwTjWQMNmBkmbzieFIz7lg =LS54 -END PGP SIGNATURE- ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] Optimize gorup enumerations
On Fri, 2009-11-20 at 11:16 -0500, Stephen Gallagher wrote:
> Nack
> You broke the sysdb tests
>
> ../../server/tests/sysdb-tests.c:1758:F:SYSDB
> Tests:test_sysdb_get_user_attr:27010: [5]: Could not get attributes
> for
> user testuser27010
[..]
Thanks for catching this, it unveiled a serious error.
Attached patch that correctly passes all tests.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
>From 38664e3db3833f9e0194c9d738ce0facfadf9a7e Mon Sep 17 00:00:00 2001
From: Simo Sorce
Date: Tue, 17 Nov 2009 20:22:36 -0500
Subject: [PATCH] Optimize sysdb_enumgrent
This brings down the time needed to enumerate my group database
from 2.4 seconds to 0.15 seconds.
---
server/db/sysdb.h | 10 +-
server/db/sysdb_search.c | 347 ++--
server/responder/nss/nsssrv_cmd.c | 350 +++--
3 files changed, 203 insertions(+), 504 deletions(-)
diff --git a/server/db/sysdb.h b/server/db/sysdb.h
index f94b43f..d9f224c 100644
--- a/server/db/sysdb.h
+++ b/server/db/sysdb.h
@@ -31,9 +31,12 @@
#define SYSDB_BASE "cn=sysdb"
#define SYSDB_DOM_BASE "cn=%s,cn=sysdb"
-#define SYSDB_TMPL_USER_BASE "cn=users,cn=%s,"SYSDB_BASE
-#define SYSDB_TMPL_GROUP_BASE "cn=groups,cn=%s,"SYSDB_BASE
-#define SYSDB_TMPL_CUSTOM_BASE "cn=custom,cn=%s,"SYSDB_BASE
+#define SYSDB_USERS_CONTAINER "cn=users"
+#define SYSDB_GROUPS_CONTAINER "cn=groups"
+#define SYSDB_CUSTOM_CONTAINER "cn=custom"
+#define SYSDB_TMPL_USER_BASE SYSDB_USERS_CONTAINER",cn=%s,"SYSDB_BASE
+#define SYSDB_TMPL_GROUP_BASE SYSDB_GROUPS_CONTAINER",cn=%s,"SYSDB_BASE
+#define SYSDB_TMPL_CUSTOM_BASE SYSDB_CUSTOM_CONTAINER",cn=%s,"SYSDB_BASE
#define SYSDB_USER_CLASS "user"
#define SYSDB_GROUP_CLASS "group"
@@ -114,6 +117,7 @@
SYSDB_DEFAULT_ATTRS, \
NULL}
#define SYSDB_GRSRC_ATTRS {SYSDB_NAME, SYSDB_GIDNUM, \
+ SYSDB_MEMBER, \
SYSDB_DEFAULT_ATTRS, \
NULL}
#define SYSDB_GRPW_ATTRS {SYSDB_NAME, SYSDB_UIDNUM, \
diff --git a/server/db/sysdb_search.c b/server/db/sysdb_search.c
index 2b5dc36..4b9470b 100644
--- a/server/db/sysdb_search.c
+++ b/server/db/sysdb_search.c
@@ -35,15 +35,13 @@ struct sysdb_search_ctx {
struct sss_domain_info *domain;
-bool enumeration;
const char *expression;
sysdb_callback_t callback;
void *ptr;
gen_callback gen_aux_fn;
-
-struct get_mem_ctx *gmctx;
+bool gen_conv_mpg_users;
struct ldb_result *res;
@@ -96,12 +94,14 @@ static void request_done(struct sysdb_search_ctx *sctx)
sctx->callback(sctx->ptr, EOK, sctx->res);
}
+static int mpg_convert(struct ldb_message *msg);
+
static int get_gen_callback(struct ldb_request *req,
struct ldb_reply *rep)
{
struct sysdb_search_ctx *sctx;
struct ldb_result *res;
-int n;
+int n, ret;
sctx = talloc_get_type(req->context, struct sysdb_search_ctx);
res = sctx->res;
@@ -117,6 +117,15 @@ static int get_gen_callback(struct ldb_request *req,
switch (rep->type) {
case LDB_REPLY_ENTRY:
+
+if (sctx->gen_conv_mpg_users) {
+ret = mpg_convert(rep->message);
+if (ret != EOK) {
+request_ldberror(sctx, LDB_ERR_OPERATIONS_ERROR);
+return LDB_ERR_OPERATIONS_ERROR;
+}
+}
+
res->msgs = talloc_realloc(res, res->msgs,
struct ldb_message *,
res->count + 2);
@@ -298,8 +307,6 @@ int sysdb_enumpwent(TALLOC_CTX *mem_ctx,
return ENOMEM;
}
-sctx->enumeration = true;
-
if (expression)
sctx->expression = expression;
else
@@ -320,225 +327,6 @@ int sysdb_enumpwent(TALLOC_CTX *mem_ctx,
/* groups */
-struct get_mem_ctx {
-struct sysdb_search_ctx *ret_sctx;
-struct ldb_message **grps;
-int num_grps;
-};
-
-static void get_members(struct sysdb_search_ctx *sctx)
-{
-struct get_mem_ctx *gmctx;
-struct ldb_request *req;
-struct ldb_message *msg;
-struct ldb_dn *dn;
-static const char *attrs[] = SYSDB_GRPW_ATTRS;
-int ret;
-
-gmctx = sctx->gmctx;
-
-if (gmctx->grps[0] == NULL) {
-return request_done(sctx);
-}
-
-/* fetch next group to search for members */
-gmctx->num_grps--;
-msg = gmctx->grps[gmctx->num_grps];
-gmctx->grps[gmctx->num_grps] = NULL;
-
-/* queue the group entry on the final result structure */
-sctx->res->msgs = talloc_realloc(sctx->res, sctx->res->msgs,
- struct ldb_message *,
- sctx->res->count + 2);
-if (!sctx->res->msgs) {
-return request_ldberror(sctx, LDB_ERR_OPERATIONS_ERROR);
-}
-sctx->res->msgs[sctx->res->count + 1] = NULL;
-sctx->res->msgs[sctx->res->count] = talloc_steal(sctx->res->msgs, msg);
-sctx->
Re: [SSSD] [PATCH] Optimize gorup enumerations
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 11/20/2009 05:16 AM, Sumit Bose wrote: > On Tue, Nov 17, 2009 at 08:38:00PM -0500, Simo Sorce wrote: >> I've been thinking about optimizing group enumerations for a while as >> they were way too slow for my taste. >> >> I did that by relying on the way we store users in the database and by >> parsing the member attribute of the groups counting on the fact we build >> the user dn as name=,cn=users, > > We already rely on this in sysdb_search_user_by_name_send() so I think > it's ok to do it here, too. > >> >> This patch does indeed help a lot as the speedup with a large database >> is huge, on my machine the reduction is of at least 1 order of magnitude >> (from 2.5 seconds to 0.15 seconds) >> With this patch we do one search only ( therefore O(n) ) instead of a >> series of searches ( O(n^2) ). >> I also removed a lot of code, which is usually also a good thing. >> > > > You don't like the recommendation in > http://freeipa.org/page/Coding_Style#Declaring, don't you? > > > >> >> The downside is that I don't have a user entry to test for uid range, so >> I can't exclude users based on that. >> >> However I think we should move both name filtering and range filtering >> in the backend code and enforce them once at store time instead of >> testing and enforcing them again and again and again each time we query >> the database. >> >> If the range or list of filtered name changes we should catch that by >> simply filtering the database when the settings change and at startup. >> >> Comments are welcome. >> > > I agree, you have already provided a patch for this and we should > continue discussing the details in that thread. > > > ACK > > bye, > Sumit > >> Simo. >> > ___ > sssd-devel mailing list > sssd-devel@lists.fedorahosted.org > https://fedorahosted.org/mailman/listinfo/sssd-devel Nack You broke the sysdb tests ../../server/tests/sysdb-tests.c:1758:F:SYSDB Tests:test_sysdb_get_user_attr:27010: [5]: Could not get attributes for user testuser27010 ../../server/tests/sysdb-tests.c:1758:F:SYSDB Tests:test_sysdb_get_user_attr:27011: [5]: Could not get attributes for user testuser27011 ../../server/tests/sysdb-tests.c:1758:F:SYSDB Tests:test_sysdb_get_user_attr:27012: [5]: Could not get attributes for user testuser27012 ../../server/tests/sysdb-tests.c:1758:F:SYSDB Tests:test_sysdb_get_user_attr:27013: [5]: Could not get attributes for user testuser27013 ../../server/tests/sysdb-tests.c:1758:F:SYSDB Tests:test_sysdb_get_user_attr:27014: [5]: Could not get attributes for user testuser27014 ../../server/tests/sysdb-tests.c:1758:F:SYSDB Tests:test_sysdb_get_user_attr:27015: [5]: Could not get attributes for user testuser27015 ../../server/tests/sysdb-tests.c:1758:F:SYSDB Tests:test_sysdb_get_user_attr:27016: [5]: Could not get attributes for user testuser27016 ../../server/tests/sysdb-tests.c:1758:F:SYSDB Tests:test_sysdb_get_user_attr:27017: [5]: Could not get attributes for user testuser27017 ../../server/tests/sysdb-tests.c:1758:F:SYSDB Tests:test_sysdb_get_user_attr:27018: [5]: Could not get attributes for user testuser27018 ../../server/tests/sysdb-tests.c:1758:F:SYSDB Tests:test_sysdb_get_user_attr:27019: [5]: Could not get attributes for user testuser27019 - -- 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.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAksGwNgACgkQeiVVYja6o6NSvwCgj0aek/OA6FelHOV1X7SFCHBi SzoAmweNhdJk2exMQB1jg05bpvT4NCKE =9IvK -END PGP SIGNATURE- ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] Optimize gorup enumerations
On Tue, Nov 17, 2009 at 08:38:00PM -0500, Simo Sorce wrote: > I've been thinking about optimizing group enumerations for a while as > they were way too slow for my taste. > > I did that by relying on the way we store users in the database and by > parsing the member attribute of the groups counting on the fact we build > the user dn as name=,cn=users, We already rely on this in sysdb_search_user_by_name_send() so I think it's ok to do it here, too. > > This patch does indeed help a lot as the speedup with a large database > is huge, on my machine the reduction is of at least 1 order of magnitude > (from 2.5 seconds to 0.15 seconds) > With this patch we do one search only ( therefore O(n) ) instead of a > series of searches ( O(n^2) ). > I also removed a lot of code, which is usually also a good thing. > You don't like the recommendation in http://freeipa.org/page/Coding_Style#Declaring, don't you? > > The downside is that I don't have a user entry to test for uid range, so > I can't exclude users based on that. > > However I think we should move both name filtering and range filtering > in the backend code and enforce them once at store time instead of > testing and enforcing them again and again and again each time we query > the database. > > If the range or list of filtered name changes we should catch that by > simply filtering the database when the settings change and at startup. > > Comments are welcome. > I agree, you have already provided a patch for this and we should continue discussing the details in that thread. ACK bye, Sumit > Simo. > ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
