Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
As you noted on the project page it would be cool to add a threshold so not all previous logins were saved.
Attached is a very simple solution which simply relies omn a variable (no admin UI yet).
Comment | File | Size | Author |
---|---|---|---|
login_history-threshold.patch | 1.24 KB | stBorchert | |
Comments
Comment #1
star-szrIndeed this would be a nice feature to add, thanks for getting this started @stBorchert :) I think this would be better as a cron operation similar to dblog_cron() rather than checking/deleting on every user login.
Comment #2
gregglesI had initially thought of doing this on cron. That approach works well if you reduce the threshold number and someone is already over it and then they never log in again. However that also seems like it could use up a lot of resources and take a long time.
A benefit of the "on login" approach is that it adds a very minimal overhead operation to the login process (something which people already expect to take a fairly long time).
Basically I can see benefits of both, so I'm not sure that moving to a cron approach should be a blocker.
Comment #3
star-szr@greggles I agree that it's worth considering the on login approach some more. I might be missing something, but wouldn't the "use up a lot of resources and take a long time" apply to an on login approach as well? Especially if logins are not frequent.
Comment #4
gregglesI don't think the on-login approach would take a long time nor use a lot of resources, but I'm making a lot of assumptions :)
If someone has 3 logins and the threshold is 3 then every cron is going to be counting their rows and saying "yep, nothing to do here" but the "on login" approach will only do the query and read their rows at the time when there is something to do: because they just logged in which pushed the number of logins above the threshold.
Here are the 2 queries that I think would be necessary for this feature with explain plans. The first query is what would be needed for cron and the second is for a single uid on login.
The big difference between the plans is the type: index means the whole index will be ready while ref means that just a subset of rows in the index will be read. Which certainly makes sense given the nature of what they are doing.
Comment #5
star-szr@greggles I think some clarity is missing here - by last X logins I was assuming last X logins regardless of user. And I think you (and maybe the OP?) had something else in mind :)
Comment #6
gregglesYes! For deleting records older than X I agree that cron makes more sense. Clarity, sweet sweet clarity.