Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
taxonomy_update_7005() is a batched update, running
$query = 'SELECT td.vid AS vocab_id, td.tid, tn.nid, tn.vid, n.type, n2.created, n2.sticky, n2.nid AS is_current FROM {taxonomy_term_data} td INNER JOIN {taxonomy_term_node} tn ON td.tid = tn.tid INNER JOIN {node} n ON tn.nid = n.nid LEFT JOIN {node} n2 ON tn.vid = n2.vid ORDER BY tn.vid, td.weight ASC, tn.tid';
for every batch. This query is slow and I think adds up to a lot of time. Drupal.org uses 1418 batches.
We should do this query once and store the data.
Comment | File | Size | Author |
---|---|---|---|
#11 | 1549390.patch | 6.27 KB | BTMash |
#10 | 1549390.patch | 6.32 KB | BTMash |
#3 | 1549390.patch | 6.37 KB | drumm |
#1 | 1549390.patch | 6.72 KB | drumm |
Comments
Comment #1
drummLet's see if this untested patch works.
Comment #3
drummFix whitespace.
Comment #4
xjm"Order by n"? Typo?
Comment #5
xjmAlright, drumm clarified this in IRC. n is here:
Comment #7
BTMash CreditAttribution: BTMash commented#3: 1549390.patch queued for re-testing.
Comment #9
drummI think the problem is that the taxonomy update exits unexpectedly somehow and leaves the rest of the updates undone.
Comment #10
BTMash CreditAttribution: BTMash commentedI went through this to see what is going on and what I found is that DBTNG is being strict about the query and there being null values for created, is_current, sticky. I moved the query out just so it was easier to debug which led me to the following error:
Attaching revised patch which still fails (thus not changing status).
Comment #11
BTMash CreditAttribution: BTMash commentedThis patch is with created, sticky, is_current being allowed to be null values. The test still fails thought efails are now on the taxonomy checks (so atleast it goes through the upgrade). Maybe testbot will give even more info?
Comment #12
drummGreat, I can try this out next time we try a fresh upgrade of Drupal.org's DB. One small thing I noticed is the "This query must return a consistent ordering across multiple calls." comment can be removed or edited; we no longer run that slow query across multiple calls, and the order is still important.
Comment #13
BTMash CreditAttribution: BTMash commentedWow...this is really strange. I had a massive number of errors popping up on my local installation so this *shouldn't* be working...I'll try and make some time to look into this over the afternoon.
Comment #14
BTMash CreditAttribution: BTMash commentedI took a look to see what was going on locally and its again due to #1504994: Upgrade tests for taxonomy are failing. So this patch is actually working correctly.
Comment #16
drummtag
Comment #17
drummI tested this on http://7.devdrupal.org/. Plenty of other stuff is broken, but posts still have taxonomy terms, like http://7.devdrupal.org/node/1548238. And the update finished approximately 20% faster, down to 7 hr 44 min. Details about the upgrade process as a whole are at #1559652: Get update.php running smoothly on 7.devdrupal.org
Comment #18
chx CreditAttribution: chx commentedThanks for the patch. Aside from the uneasiness coming from changing an existing update function, this does seem to be OK and it's tested automatically and manually so I hope we do not open a can of worms again. Let's go with it.
Comment #19
drummAll the logic should be the same, just rearranging so a slow query only needs to be done once. The riskiest part is the DBTNG conversion of
$query
.Comment #20
David_Rothstein CreditAttribution: David_Rothstein commentedA little scary, but looks good to me too. And the fact that it's been well-tested is a good thing :)
Committed to 7.x - thanks! http://drupalcode.org/project/drupal.git/commit/a80c7e8