[SSSD] [PATCHES] Fix severe performance problems with initgroups (master)

Jakub Hrozek jhrozek at redhat.com
Tue Aug 3 16:34:34 UTC 2010


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/02/2010 05:58 PM, Stephen Gallagher wrote:
> Attached are the patches to fix the initgroups performance issue for the
> master branch. Patches 0001-0004 applied trivially to the master,
> patches 0005 and 0006 had to be rewritten to take advantage of the
> synchronous sysdb interface, but this has made them distinctly easier to
> follow.
> 

0001 to 0004 - no changes in the actual code (verified with
vimdiff/interdiff) -> ACK

0005 - works OK, but there's a change of behavior vs. the 1.2 version. I
actually prefer the new one, so I'm inclined to ACK it, but I think it
is worth at least pointing out. The difference is that the 1.2 code
would abort the tevent request in case sysdb_add_group_member_recv() or
sysdb_remove_group_member_recv() failed with tevent_req_error(). The new
version just skips the errors:

+            ret = sysdb_add_group_member(tmp_ctx, sysdb, domain,
+                                         add_groups[i], user);
+            if (ret != EOK) {
+                DEBUG(1, ("Could not add user [%s] to group [%s]. "
+                          "Skipping.\n"));
+                /* Continue on, we should try to finish the rest */

Also, there is a minor difference in the interface as the new one
accepts "const char *" the old one did take "char *"..looking at the
explicit cast in patch 0006 that's probably intentional.

0006 - just two nitpicks: no space after if:

 +    if(!state->name) {

and "else" on separate line:

+    }
+    else {


Also, the 1.2 patch made the sdap_initgr_rfc2307_send properly "static",
might be nice to do that here, too.

Otherwise, looks OK.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkxYRRkACgkQHsardTLnvCWZ3ACeM2wUzlEQ0P4O+8doaPbqeUvI
QT8AoKlitlaRNMpdk3hIhTu4/ksRx147
=NK4C
-----END PGP SIGNATURE-----



More information about the sssd-devel mailing list