The problem
Follow-up from: #1488630: Who's online block doesn't work with swappable session backends and lazy session creation and #40545: Improve speed by avoiding unnecessary updates in sess_write().
UserRequestSubscriber updates the {user_field_data}.access column for every auth user, every three minutes by default.
This means a write to a high-read table, invalidating the query cache for it.
Additionally we invalidate the persistent entity cache, which means several queries and a cache set on the next request if the current user account is loaded every request.
If you have a site with lots of authenticated users, this can mean a lot of database churn and cache invalidations.
Then we only use the information on user profile rendering to admins, and admin views by default, which is very rarely viewed compared to the number of times account objects get loaded (Ev
Both last access and last login are really activity logging, not properties of the user.
Proposed solution
Add a new service, maintaining its own storage, that handles getting and setting last access.
For backwards compatibility, call that service from UserStorageController - but leave out the field tables and entity cache.
Also needs views integration for the new table in a way that will work.
Remaining tasks
Agree this is a good idea. Possibly profiling to show how bad things are currently.
Comments
Comment #1
moshe weitzman CreditAttribution: moshe weitzman commentedSounds reasonable to me. It would be ideal if we preserved the ability to track last access time on a 'write once every n minutes' basis without going for the full 'track every page view' thats statistics module does.
Comment #2
Damien Tournoud CreditAttribution: Damien Tournoud commentedI think it makes sense to keep the
{user}.access
too, it's an important information for auditing purposes. But that can be implemented by the user module directly instead of being in the session code.Comment #3
catchI don't feel that {users}.access is that important for auditing, there's still {users.login}. If there's a use-case that really wants up-to-the-(5 by default)-minute access recording then is that important enough that core needs to cater for it by default?
Comment #4
sunI'm not sure... I think I had quite some use-cases for
{users}.access
in the past, but not really for Statistics module.However, I'd certainly agree that all of them were advanced, and all of them were custom to begin with. Having the value there provided by core was merely a nice convenience.
I'm also relatively sure that the last site access tracking could possibly be re-implemented in a better way. To begin with, avoiding recurring writes on the user table (which are not represented in cached entity loads as they are direct table writes).
E.g., if keyvalue_expirable had better query abilities (joins and filtering on values), then I would have suggested that, since old values wouldn't have to be stored forever, but that doesn't seem to be a good match.
I wonder whether it might make most sense to move the functionality into an own separate core module? I almost wonder whether it wouldn't make sense to combine {users}.access + .login with #1120928: Move "history" table into separate History module in such a new module that focuses on user activity tracking?
Comment #5
andypostI think this one is a good candidate for #1029708: History table for any entity
Changing
users.access
means changing entity that does not actually happens, this makes cache for user entities useless so better get rid of the field in favor of history table recordComment #6
catchSaw this write and cache invalidation again when profiling another issue, re-titled and updated the issue summary.