Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
There is no reason for the thread_id column to be in pm_index. In there, it is duplicates *a lot* because it does not change for the same mid.
So we should move it to the pm_message column.
Follow-up of #1192250: Slow queries and other performance bugs
Comment | File | Size | Author |
---|---|---|---|
#3 | issue-1193232-move-thread-id.diff | 11.9 KB | te-brian |
Comments
Comment #1
te-brian CreditAttribution: te-brian commentedStarted working on a patch for this. I wrote an update script to add the thread_id field and migrate the data. I haven't removed the thread_id from pm_index (and there may be reason to keep it in both tables).
Here is a quick preview of one query done against my development data-set.
As you can see dropping the join is an immense improvement.. and if we can order by mid instead of timestamp then its a complete win!
To give you an idea of how important improving even just this query is .. here is this query being run on my live data :(
Ouch!! 1,385,350 rows examined.
The reason we may still need thread_id on pm_index is queries like this that are using a very specific index:
Comment #2
te-brian CreditAttribution: te-brian commentedStill working on some stuff... however ... I discovered that the above query is not from privatemsg.module. It is something that is in one of my custom site modules that displays the latest replies to a user's pms. If I want to keep the functionality I have, I may still need to add thread_id to pm_message, but from my full comb-over of the code base, it seems that a majority of the queries that involved thread_id are only on the pm_index table, or join the pm_index table to other tables like the pm_tags_index table.
I think in only a few circumstances the pm_index table is joined to the pm_message table (where only the thread_id is needed, and not other fields).
This still warrants some investigation and will obviously be affected by any future implementation of pm_thread table.
Comment #3
te-brian CreditAttribution: te-brian commentedHere is the patch I am doing my testing with. Like I said .. there isn't much in the module to actually warrant the thread_id move. It may have been a false alarm raised by my custom code.
Comment #4
BerdirI pretty much came to the same conclusion, the queries that profit from this are the message load query and a query in pm_attachments. Other queries would get worse.
However, this might change after we can get #744374: Introduce a pm_thread table and #647212: Load participants in thread list in a single but separate query in, so we probably want to postpone this until those two issues are resolved.
Comment #5
oadaeh CreditAttribution: oadaeh as a volunteer commentedThis issue is being closed because it is against a branch for a version of Drupal that is no longer supported.
If you feel that this issue is still valid, feel free to re-open and update it (and any possible patch) to work with the 7.x-1.x branch (bug fixes only) or the 7.x-2.x branch.
Thank you.