Modify

Opened 10 years ago

Closed 7 years ago

#11879 closed defect (fixed)

User-list is slow

Reported by: anonymous Owned by: Steffen Hoffmann
Priority: normal Component: AccountManagerPlugin
Severity: normal Keywords: sql slow users admin optmize
Cc: Ryan J Ollos Trac Release: 1.0

Description

Listing the user page is slow (/admin/accounts/users)

First it gets all the users:

            SELECT DISTINCT sid
            FROM    session_attribute
            WHERE   authenticated=1
                AND name='password'

For each user it finds it runs (2500-ish users in this case):

        SELECT 1
          FROM session
         WHERE authenticated=1 AND sid='USERNAME'

        SELECT 1
          FROM session
         WHERE authenticated=1 AND sid='USERNAME'

        SELECT  value,sid
          FROM  session_attribute
        WHERE sid='USERNAME' AND authenticated=1 AND name='failed_logins_count'

And then after that it runs:

        SELECT  sid,authenticated,name,value
          FROM  session_attribute

        SELECT sid,last_visit
          FROM session
         WHERE authenticated=1

Since it fetches all the tables after there is no need to do the individual queries for each person?

Attachments (0)

Change History (19)

comment:1 Changed 10 years ago by anonymous

Also, why does it check the AccountGuard.user_locked(USERNAME), it does not show that info in the list anyway?

comment:2 Changed 10 years ago by JellyFrog

Could do something like...

SELECT `T1`.`sid`, `T2`.`value` AS `name`, `T3`.`value` AS `email`, `T4`.`last_visit`
FROM `session_attribute` AS `T1`

LEFT JOIN `session_attribute` `T2` ON `T2`.`sid` = `T1`.`sid` AND `T2`.`name` = 'name'
LEFT JOIN `session_attribute` `T3` ON `T3`.`sid` = `T1`.`sid` AND `T3`.`name` = 'email'
LEFT JOIN `session` `T4` ON `T4`.`sid` = `T1`.`sid`

WHERE `T1`.`authenticated` = 1 AND `T1`.`name` = 'password'

GROUP BY `T1`.`sid`

This query takes (without cache) 0.3s with: session - 116876 rows session_attribute - 414464 rows

comment:3 in reply to:  1 Changed 10 years ago by Steffen Hoffmann

Replying to anonymous:

Also, why does it check the AccountGuard.user_locked(USERNAME), it does not show that info in the list anyway?

Do you assert or query? Did you check that? You shall see a lock icon for list entries of actually locked accounts.

comment:4 in reply to:  2 Changed 10 years ago by Steffen Hoffmann

Replying to JellyFrog:

... This query takes (without cache) 0.3s with: session - 116876 rows session_attribute - 414464 rows

I'm not afraid of numbers, quit the contrary. I know of 700+ user applications. 2500+ is still significantly more, and I'll love to satisfy that need too. Would you provide profiling results of the current code for comparison, please?

On your SQL statements, I'd like to learn about the reason for such excessive quoting of table and column names. Is this just personal style?

comment:5 Changed 10 years ago by JellyFrog

Sorry can't profile now but it takes like.. 40-60s maybe, depending on how busy the server is.

The SQL queries quoting was for http://dev.mysql.com/doc/refman/5.6/en/reserved-words.html, just a habit...

comment:6 Changed 10 years ago by Steffen Hoffmann

Thanks for the additional information.

Sounds like a pretty valid enhancement request, and by looking at the numbers I wonder why no-one threw it in earlier. I never saw multi-seconds response times after adding the pager to the user list, but of course this only fixed the Genshi page template rendering delay on long listings.

comment:7 Changed 10 years ago by Steffen Hoffmann

Cc: Ryan J Ollos added; anonymous removed

I went serious about it by profiling a request on a SQLite test Trac db with 24.000+ users (24443 and entries in session db table and 54669 entries in session_attribute):

From 17.780 s total dispatch time I've seen

  • 9.887 s for checking account locks
  • 6.608 s was DEBUG logging overhead to trac.log file
  • 3.501 s for getting configuration value(s) three times per entry related to checking account locks
  • 2.552 s for prepare all links to account detail pages
  • 1.164 s for conditional email addresses obfuscation
  • 0.718 s for Genshi template rendering (pager: 50 list entries/page)
  • 0.692 s for permission checks on conditional email address obfuscation
  • 0.302 s for 2 all-entries SQL queries
  • 0.267 s for reading from file user store on local SSD
  • 0.086 s for an individual SQL attribute query per account

I conclude, that SQL is one of the minor issues here.

Do you agree? Still I'll attempt improvements but rather in the most critical sections (account locking, link preparation) to get to more reasonable response times.

comment:8 Changed 10 years ago by Ryan J Ollos

I typically see page load times for /admin/accounts/users of close to 1 minute on t.h.o

Last edited 10 years ago by Ryan J Ollos (previous) (diff)

comment:9 Changed 9 years ago by Steffen Hoffmann

#12053 might be fulfilled by changes required here as well.

comment:10 Changed 9 years ago by Dirk Stöcker

This is very likely not an SQL query issue here on t.h.o., but caused by the display of long list. t.h.o. has issues when delivering long rendered HTML. You can test this with Spamfilter user plugin. The overview page comes fast, the full list takes long. Both use the same database queries.

I think installing newest plugin version which groups displayed user into pages already solves this issue. BTW it would be fine, when I can configure the default number of displayed users. 20 is really a very small number.

Last edited 9 years ago by Dirk Stöcker (previous) (diff)

comment:11 in reply to:  10 ; Changed 9 years ago by Steffen Hoffmann

Replying to stoecker:

This is very likely not an SQL query issue here on t.h.o., but caused by the display of long list.

...

I think so too. Furthermore profiling results above don't even require such guessing - you can see, that SQL is the least to worry about.

I think installing newest plugin version which groups displayed user into pages already solves this issue.

No, I fear it will improve only a bit. I understood the complaint and will push performance further as my time permits.

BTW it would be fine, when I can configure the default number of displayed users. 20 is really a very small number.

By preserving the value between successive requests I wanted to obsolete another trac.ini setting, but if you insist, this one is rather easy.

comment:12 in reply to:  11 ; Changed 9 years ago by Dirk Stöcker

Replying to hasienda:

BTW it would be fine, when I can configure the default number of displayed users. 20 is really a very small number.

By preserving the value between successive requests I wanted to obsolete another trac.ini setting, but if you insist, this one is rather easy.

Then it does not work in my instance: TracAccountManager 0.5dev-r14078

Setting "Max accounts per page" from 20 to 600, press "Aktualisieren" (there is a German/English mix for latest, which is horrible) and 600 are displayed. Clicking the link in menu again and it is down to 20.

comment:13 in reply to:  12 ; Changed 9 years ago by Steffen Hoffmann

Replying to stoecker:

Replying to hasienda:

BTW it would be fine, when I can configure the default number of displayed users. 20 is really a very small number.

By preserving the value between successive requests I wanted to obsolete another trac.ini setting, but if you insist, this one is rather easy.

Then it does not work in my instance: TracAccountManager 0.5dev-r14078

It works, at least as far as I considered the need so far.

Setting "Max accounts per page" from 20 to 600, press "Aktualisieren" (there is a German/English mix for latest, which is horrible)

Due to my laziness regarding message ID updates, sorry.

and 600 are displayed. Clicking the link in menu again and it is down to 20.

Main menu doesn't breaks the request series, true. So I'll go for a site-wide configuration setting, or do you see justification for making this a personal setting, that would require reading/saving from to session/user preferences?

comment:14 Changed 9 years ago by Dirk Stöcker

Whether it is site wide or personal I don't care. But is should be able to set it. Personal is probably easier to program and in sync with other functionalities in trac (like timeline display, ...).

comment:15 in reply to:  13 Changed 9 years ago by Ryan J Ollos

Replying to hasienda:

and 600 are displayed. Clicking the link in menu again and it is down to 20.

Main menu doesn't breaks the request series, true. So I'll go for a site-wide configuration setting, or do you see justification for making this a personal setting, that would require reading/saving from to session/user preferences?

I think it would make the most sense to have a hard-coded initial default (doesn't need to be specified in trac.ini) and store the user-entered value in session_attributes. The behavior would be consistent with similar features in Trac:

  • Days back on the timeline
  • Max items per page on the Ticket Query page is stored in the Last query
  • Adjust edit area height on the wiki edit page.

comment:16 Changed 9 years ago by Steffen Hoffmann

Thanks for the pointers. I'll code it similar; the more code I can use from these features in Trac core the sooner.

comment:17 Changed 9 years ago by Steffen Hoffmann

In 14278:

AccountManagerPlugin: Save max items setting for user list pager to preferences, refs #11879.

comment:18 in reply to:  10 Changed 9 years ago by Steffen Hoffmann

Replying to stoecker:

BTW it would be fine, when I can configure the default number of displayed users. 20 is really a very small number.

This should work now as expected.

I wish I had moved to this setting to preferences earlier, because it obsoleted quite some code in templates as well as admin web-UI module.

comment:19 Changed 7 years ago by Ryan J Ollos

Resolution: fixed
Status: newclosed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Steffen Hoffmann.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.