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

Ensure index is used when running on MySQL #1639

Merged
merged 4 commits into from
May 23, 2022

Conversation

jonnystoten
Copy link
Contributor

This PR fixes a performance issue when running notary server with MySQL. When calling GetCurrent to find the most recent metadata file for a GUN and role, we use this snippet:

q := db.Select("updated_at, data").Where(
	&TUFFile{Gun: gun.String(), Role: tufRole.String()}).Order("version desc").Limit(1).First(&row)

First adds an additional order by clause for the id column, so this corresponds to SQL roughly like:

SELECT updated_at, data FROM tuf_files WHERE gun = ? AND role = ? ORDER BY version DESC, id ASC LIMIT 1

We have an index on gun, role, version, and adding the order by on id prevents this index from being used. Usually adding the primary key should be OK but because it's being added in the opposite direction (version is DESC and id is ASC) it doesn't work. We don't need to add this primary key sort because gun, role, version already uniquely identifies the row.

In the latest version of gorm 1.x there is a new method Take which behaves in the same way as First but doesn't add the primary key sort. From some quick testing on Docker's notary instance, removing this from the query reduces the time this query takes on a very large repository from over 30 seconds to a few milliseconds.

The main fix here is using Take instead of First on the GetCurrent method. I've updated all usages of First to Take in that file for consistency - I don't expect them to have much impact on performance. All of the other changes were necessary because of the gorm upgrade.

The latest version of gorm adds a RWMutex to this struct so we can't
pass by value or we will copy the mutex.

Signed-off-by: Jonny Stoten <[email protected]>
The new version of gorm is a bit more panicky so we need to recover and
return a normal error

Signed-off-by: Jonny Stoten <[email protected]>
@jonnystoten jonnystoten changed the title Use index on mysql Ensure index is used when running on MySQL May 12, 2022
@jonnystoten jonnystoten marked this pull request as ready for review May 12, 2022 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants