Skip to content

Some more FORCE INDEX - #870

Merged
squeaky-pl merged 5 commits into
masterfrom
one-more-force-index
Sep 3, 2024
Merged

Some more FORCE INDEX#870
squeaky-pl merged 5 commits into
masterfrom
one-more-force-index

Conversation

@squeaky-pl

@squeaky-pl squeaky-pl commented Aug 29, 2024

Copy link
Copy Markdown
Contributor

This rewrites one query which is off the beaten path and not that important from the performance point of view, I am doing this rather for consistency because this query could be faster. It also makes all those queries easier to find in the source code. This part of code is only executed if there's a new message in your folder which is "rare" compared to everything else that happens.

Also the code before actually fetches the row (to know the id) and we are only interested to know if it exists. That's what EXISTS clause in SQL exists for. This way we can read everything from the index and not look at the table at all because all the columns involved are already in the index.

Before

SELECT * FROM imapuid WHERE imapuid.account_id = %s AND imapuid.folder_id = %s AND imapuid.msg_uid = %s

After

SELECT EXISTS (SELECT 1 
FROM imapuid FORCE INDEX(ix_imapuid_account_id_folder_id_msg_uid_desc) 
WHERE imapuid.account_id = %s AND imapuid.folder_id = %s AND imapuid.msg_uid = %s) AS anon_1

It also adds the hints in some other parts just to switch away from the old index to the new index, my objective is to be sure that I can safely remove the old index.

Memo, how to check that index is no longer used in MySQL:

select COUNT_STAR from performance_schema.table_io_waits_summary_by_index_usage where object_schema = 'inbox' AND object_name = 'imapuid' AND index_name
= 'folder_id'\G

log.warning(
"Expected to create imapuid, but existing row found",
remote_msg_uid=raw_message.uid,
existing_imapuid=existing_imapuid.id,

@squeaky-pl squeaky-pl Aug 29, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's not really interesting to know the id column here, all we care about is the triple (account_id, folder_id, imap_uid) is unique, which can be known from the unique index (account_id, folder_id, imap_uid DESC) which is covering for this query. Faster.

@squeaky-pl squeaky-pl changed the title One more FORCE INDEX Some more FORCE INDEX Aug 29, 2024
Comment on lines +160 to +168
session.query(ImapUid)
.filter(
ImapUid.account_id == account_id,
ImapUid.folder_id == folder_id,
ImapUid.msg_uid.in_(new_flags),
)
.with_hint(
ImapUid, "FORCE INDEX (ix_imapuid_account_id_folder_id_msg_uid_desc)"
)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not a covering index since we retrieve other columns, but does not make anything slower, just switches the index.

Comment on lines +213 to +216
.with_hint(
ImapUid,
"FORCE INDEX (ix_imapuid_account_id_folder_id_msg_uid_desc)",
)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not a covering index since we retrieve other columns, but does not make anything slower, just switches the index.

@squeaky-pl
squeaky-pl force-pushed the one-more-force-index branch from f757e8f to 90d3495 Compare August 29, 2024 17:16
Comment on lines +520 to +526
for uid, in db_session.query(ImapUid.msg_uid)
.filter(
ImapUid.account_id == self.account_id,
ImapUid.folder_id == self.folder_id,
)
.with_hint(
ImapUid, "FORCE INDEX(ix_imapuid_account_id_folder_id_msg_uid_desc)"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Covering index since we filter by account_id, folder_id and retrieve msg_uid.

load_only("msg_uid"), joinedload("message").load_only("g_msgid")
)
.filter_by(account_id=self.account_id, folder_id=self.folder_id)
.filter(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Covering index since we filter by account_id, folder_id and retrieve msg_uid.

@mrhiggi-close mrhiggi-close left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, forcing these to use the new index makes sense.

@squeaky-pl
squeaky-pl merged commit 1d3a000 into master Sep 3, 2024
@squeaky-pl
squeaky-pl deleted the one-more-force-index branch September 3, 2024 15:15
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