Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Membe::member_from_autologinhash is slow #11569

Open
2 tasks done
lekoala opened this issue Jan 20, 2025 · 3 comments
Open
2 tasks done

Membe::member_from_autologinhash is slow #11569

lekoala opened this issue Jan 20, 2025 · 3 comments

Comments

@lekoala
Copy link
Contributor

lekoala commented Jan 20, 2025

Module version(s) affected

4x,5x

Description

Member::member_from_autologinhash is slow on large member tables because there are no indexes

There was an index at some point but it was removed

//'AutoLoginHash' => Array('type'=>'unique', 'value'=>'AutoLoginHash', 'ignoreNulls'=>true)

there is no need to have an unique index, a btree should be enough. uniqueness code be enforced at code level.

How to reproduce

  • Have a larger member table
  • Generate an autologin hash
  • Click on the email with the reset link => see the slow query hitting the db due to the missing index

Possible Solution

Restore the btree index

Additional Context

Related
#11384

Validations

  • Check that there isn't already an issue that reports the same bug
  • Double check that your reproduction steps work in a fresh installation of silverstripe/installer (with any code examples you've provided)
@GuySartorelli
Copy link
Member

there is no need to have an unique index, a btree should be enough. uniqueness code be enforced at code level.

What would the advantage be of implementing uniqueness checks in PHP code when the database can handle that for us?

Have a larger member table

How large would be large enough to see these effects?

Click on the mail

What is "the mail"?

@jesscostello
Copy link

How large would be large enough to see these effects?

Hey @GuySartorelli we have also started seeing this issue quite frequently recently, our members table now has just over 1,000,000 records. Hope this helps :)

@lekoala
Copy link
Contributor Author

lekoala commented Jan 22, 2025

there is no need to have an unique index, a btree should be enough. uniqueness code be enforced at code level.

What would the advantage be of implementing uniqueness checks in PHP code when the database can handle that for us?

Have a larger member table

How large would be large enough to see these effects?

Click on the mail

What is "the mail"?

There is no advantage to add uniqueness in php, but since it was disabled for a reason, i guess this hasn't changed. Dealing with "null" unique values is very annoying. Currently, there is no check at all as far as I can tell.
=> step 1 : adding an index to avoid issues
=> step 2 : improve and make sure the autologin token is unique

I don't know at which point a query without an index is going to cause issues, but from my experience, I rarely query something without on index on anything > 10k records
On my current project, I have more than 1 million entries on my member table (well, maybe not the best design decision I made in my life :-D i should probably have created one for "real" users and one for those without login credentials), so these things are very visible.

the mail => the email that contains the reset link, sorry that wasn't clear i updated the issue accordingly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants