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).

CommentFileSizeAuthor
login_history-threshold.patch1.24 KBstBorchert
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

star-szr’s picture

Status: Active » Needs work

Indeed 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.

greggles’s picture

Issue summary: View changes

I 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.

star-szr’s picture

@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.

greggles’s picture

I 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.

mysql> explain select count(1), uid from login_history group by uid having count(1) > 3;
+----+-------------+---------------+------------+-------+------------------------------------------+-------------------+---------+------+------+----------+-------------+
| id | select_type | table         | partitions | type  | possible_keys                            | key               | key_len | ref  | rows | filtered | Extra       |
+----+-------------+---------------+------------+-------+------------------------------------------+-------------------+---------+------+------+----------+-------------+
|  1 | SIMPLE      | login_history | NULL       | index | login_history_uid,login_history_uid_host | login_history_uid | 4       | NULL |    7 |   100.00 | Using index |
+----+-------------+---------------+------------+-------+------------------------------------------+-------------------+---------+------+------+----------+-------------+
1 row in set, 1 warning (0.00 sec)

mysql> explain select count(1) from login_history where uid = 1 having count(1) > 3;
+----+-------------+---------------+------------+------+------------------------------------------+-------------------+---------+-------+------+----------+-------------+
| id | select_type | table         | partitions | type | possible_keys                            | key               | key_len | ref   | rows | filtered | Extra       |
+----+-------------+---------------+------------+------+------------------------------------------+-------------------+---------+-------+------+----------+-------------+
|  1 | SIMPLE      | login_history | NULL       | ref  | login_history_uid,login_history_uid_host | login_history_uid | 4       | const |    5 |   100.00 | Using index |
+----+-------------+---------------+------------+------+------------------------------------------+-------------------+---------+-------+------+----------+-------------+
1 row in set, 1 warning (0.00 sec)

mysql>

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.

star-szr’s picture

@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 :)

greggles’s picture

Yes! For deleting records older than X I agree that cron makes more sense. Clarity, sweet sweet clarity.