Original report: https://bugs.launchpad.net/bugs/941673 The performance of accounts-daemon is unacceptable with large password and/or group files. We have appoximately 40000 users in the passwords file and 16000 lines in the group file. Having looked at the source, it appears to me the problem is that for each user is pulls from the password database (via getpwXXX) its then calls getgrouplist to return the users list of groups. Getgroups must make a complete pass through the group file to determine the groups for the user. When initializing this results in reading the groups file 40000 times. I think a better solution is to build a inverted group file map then first time getgroups is called by reading the entire group database and building a data structure indexed by username where a users group list can by found in a single lookup. Of couse, an even better solution would be for getgroups to do that itself, but that would require much larger code changes. I tried installed nscd to see if this would make a difference but no dice.
AccountsService sort of assumes users in /etc/passwd are "local users" and that the 40000 user network login cases are covered by nis/ldap/etc I guess we should do some sort of sanity check and treat /etc/passwd users as remote in the case the check fails. This way we sidestep all the costly work entirely.
maybe we could just bail if there are greater than N found users. The problem then is finding a suitable value of N.
Created attachment 65941 [details] [review] proposed patch limit the no of user listed by a-s to UID_MAX. Also, sets user-limit to true when this condition is meet, allowing apps like gdm/lightdm to enable manual login option.
Talked with Ritesh about this bug today on IRC. He's going to look into using fgetgrent to create the hash table of local groups and use that.
Created attachment 66171 [details] [review] proposed patch
ritz, any chance you could get this applying against master (preferably as a git-format-patch formatted patch) ?
any updates on this?
Created attachment 73763 [details] [review] patch Hi halfline I have rewritten the code to use hashtable instead of list, leading to lot faster startup time on my test system ( <1s for ~40k users). I am currently testing the code ( add/remove users option ). This is for an older version of a-s (0.6.15) , will update the code against head and post. Anything which does need the user list ( gnome-control-center user manager/ indicator-session/..) still breaks . Personally I believe, the best solution would be to allow user to select an upper limit ( and list only cached users from login history ), and broadcast "user-limit-reached" option. The broadcast could be used by applications to display warning or change their behaviour ( such as disable user list in gdm ).
Created attachment 74177 [details] [review] patch initial patch, against latest release. still need to work quirk against entry_generator_wtmp. It calls getpwnam for every entry ( even if it is a duplicate entry ).
any chance you could update this again for git master?
Created attachment 124799 [details] [review] user: check if user is in wheel more efficiently We currently get all the groups a user belongs to in one pass, then check each one to see if it's wheel. It's much more efficient to just get the wheel group and check if any of its members are the user.
Created attachment 124800 [details] [review] daemon: get local users from /etc/shadow not /etc/passwd For some sites, it's common practice to rsync around large /etc/passwd files containing the password entries for remote users. That means accountsservices' "assume /etc/passwd is local users" heuristic falls over. This commit changes it to only treat users in /etc/shadow as local.
Created attachment 124801 [details] [review] daemon: don't call getspnam for local users We're already iterating over the whole shadow file, so just cache the entries instead of calling getspname a few lines later.
Created attachment 124802 [details] [review] daemon: constrain max local users to 50 Systems with tens of thousands of users don't want all those users showing up in the user list. Set a cap at an even 50, which should cover the lion's share of use cases well. Of course, if a user not in the list explicitly logs in (from Not Listed? or whatever) they get added to the list.
Created attachment 124803 [details] [review] daemon: don't source user list from wtmp wtmp can get rather large on some systems from ssh logins. Furthermore it's pretty much completely redundant given the user cache in /var/lib/AccountService This commit changes the wtmp code to only get used for maintaining login frequency and accounting, not for generating new users.
i'm going to push these patches to master. They're loosely based on ritz work in this bug, but they diverge in some significant ways: 1) i set a limit on the number of local users returned (ala comment 2) 2) i redid the wheel check to avoid needing an admin user cache 3) i cache shadow information a little differently and for a shorter period of time 4) i get "local user list" from /etc/shadow now instead of /etc/passwd (since /etc/passwd gets rsynced around)
Attachment 124799 [details] pushed as c85b41d - user: check if user is in wheel more efficiently Attachment 124800 [details] pushed as 98f4287 - daemon: get local users from /etc/shadow not /etc/passwd Attachment 124801 [details] pushed as 14ca424 - daemon: don't call getspnam for local users Attachment 124802 [details] pushed as 20c0315 - daemon: constrain max local users to 50 Attachment 124803 [details] pushed as a2f448b - daemon: don't source user list from wtmp
Unconditioned #include <shadow.h> breaks the build on FreeBSD. FreeBSD doesn't have shadow.h and /etc/shadow. Hashed passwords are stored in /etc/master.passwd, which is a /etc/passwd with additional fields. There is no API specifically made to read /etc/master.passwd because getpwent on FreeBSD automatically switches to use /etc/spwd.db (the binary form of /etc/master.passwd) instead of /etc/pwd.db (the binary form of /etc/passwd) when the user has permission to read the file.
Same on OpenBSD. Since that change, accountsservice is broken here as well.
is it common practice on BSD to rsync /etc/passwd around? Maybe bsd could go back to using /etc/passwd?
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/accountsservice/accountsservice/issues/39.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.