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

Drop authtoken_version_index #25217

Merged
merged 1 commit into from
Jan 20, 2021
Merged

Drop authtoken_version_index #25217

merged 1 commit into from
Jan 20, 2021

Conversation

rullzer
Copy link
Member

@rullzer rullzer commented Jan 19, 2021

The index was used when deleting old tokens. On top of that the index is
of course not that great since the version is either 1 or 2.

Signed-off-by: Roeland Jago Douma roeland@famdouma.nl

@MorrisJobke
Copy link
Member

Could you describe the problem here in a bit more detail?

@rullzer
Copy link
Member Author

rullzer commented Jan 20, 2021

Could you describe the problem here in a bit more detail?

It was a cold winter night. The wind was howling around the house. It was a night as you know from the movies right before something terrible happens. It was a night like this I was going over a slow query log. Here I noticed that there were several long running queries looking like:

DELETE FROM `oc_authtoken` 
WHERE (`last_activity` < 1609316716)
AND (`type` = 0) 
AND (`remember` = 1) 
AND (`version` = 2);

I should have known better. But again as in the movies I walked right into the darkness of the database. And I ran an EXPLAIN on this query. Which showed me there were two indexies on this table that could be used: authtoken_last_activity_idx or authtoken_version_index. Then when I looked at the index used breath stopped. Everything went cold. I saw it was using the authtoken_version_index. Which only has a cardinality of 3. Thus resulting in analyzing >80k rows for the final query. But I was determined to kill this monster! I summoned the magic of the dropIndex and made the database respect my wishes! The authtoken_version_index is gone! And the queries on the oc_authtoken table run at much higher speeds again!

@MorrisJobke
Copy link
Member

Now there is only the one that is triggered by deleteByName https://github.com/nextcloud/server/blob/master/lib/private/Authentication/Token/DefaultTokenMapper.php#L168-L170

This one is usually called by the delete client API for OAuth2 and thus not that often like the one you mentioned above.

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me squash and then this looks good 👍

The index was used when deleting old tokens. On top of that the index is
of course not that great since the version is either 1 or 2.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@MorrisJobke MorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jan 20, 2021
@rullzer
Copy link
Member Author

rullzer commented Jan 20, 2021

Now there is only the one that is triggered by deleteByName https://github.com/nextcloud/server/blob/master/lib/private/Authentication/Token/DefaultTokenMapper.php#L168-L170

This one is usually called by the delete client API for OAuth2 and thus not that often like the one you mentioned above.

Yeah but for that a new index on name would probably make sense.

@rullzer rullzer merged commit 2c9345a into master Jan 20, 2021
@rullzer rullzer deleted the enh/drop_index branch January 20, 2021 10:13
@rullzer rullzer mentioned this pull request Jan 21, 2021
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish performance 🚀
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants