On my test upgrade of drupal.org (now possible with #211182: Updates run in unpredictable order), the current node_update_7006() is expected to take more then 30 hours. This is absolutely not acceptable. We need to improve that.

CommentFileSizeAuthor
#6 node_body_as_field_update-686128-6.patch2.1 KByched
#4 node_body_as_field_update-686128-4.patch1.48 KByched
#2 node_body_as_field_update-686128-2.patch1.44 KByched

Comments

damien tournoud’s picture

damien tournoud commented

This will be easier to fix then anticipated:

      $batch_size = 50;
      $query = db_select('node', 'n');
      $nr = $query->innerJoin('node_revision', 'nr', 'n.vid = nr.vid');
      $revisions = $query
        ->fields('n', array('type', 'status', 'comment', 'promote', 'sticky'))
        ->fields($nr)
        ->condition('nr.vid', $context['last'], '>')
        ->orderBy('nr.vid', 'ASC')
        ->execute();

Hm. That causes a filesort, and what, $batch_size is not even used?

yched’s picture

yched commented
Status: Active » Needs review
StatusFileSize
new node_body_as_field_update-686128-2.patch1.44 KB

$batch_size is used in

if (--$batch_size == 0) {
  break;
}

Hm, we usually use this kind of param as a range in the query...

+ changed the query so that it uses node_revision as the base table.

damien tournoud’s picture

damien tournoud commented

This looks great. Can we increase the batch size too? On my test system, I do 15 iterations per second :) This sounds like a waste of resources. We could bump that to at least 1000.

[Fri Jan 15 23:35:12 2010] [error] [client 82.230.177.196] node_update_7006, referer: http://do.stage.damz.org/update.php?op=start&id=682703
[Fri Jan 15 23:35:12 2010] [error] [client 82.230.177.196] node_update_7006, referer: http://do.stage.damz.org/update.php?op=start&id=682703
[Fri Jan 15 23:35:12 2010] [error] [client 82.230.177.196] node_update_7006, referer: http://do.stage.damz.org/update.php?op=start&id=682703
[Fri Jan 15 23:35:12 2010] [error] [client 82.230.177.196] node_update_7006, referer: http://do.stage.damz.org/update.php?op=start&id=682703
[Fri Jan 15 23:35:12 2010] [error] [client 82.230.177.196] node_update_7006, referer: http://do.stage.damz.org/update.php?op=start&id=682703
[Fri Jan 15 23:35:12 2010] [error] [client 82.230.177.196] node_update_7006, referer: http://do.stage.damz.org/update.php?op=start&id=682703
[Fri Jan 15 23:35:12 2010] [error] [client 82.230.177.196] node_update_7006, referer: http://do.stage.damz.org/update.php?op=start&id=682703
[Fri Jan 15 23:35:12 2010] [error] [client 82.230.177.196] node_update_7006, referer: http://do.stage.damz.org/update.php?op=start&id=682703
[Fri Jan 15 23:35:12 2010] [error] [client 82.230.177.196] node_update_7006, referer: http://do.stage.damz.org/update.php?op=start&id=682703
[Fri Jan 15 23:35:12 2010] [error] [client 82.230.177.196] node_update_7006, referer: http://do.stage.damz.org/update.php?op=start&id=682703
[Fri Jan 15 23:35:12 2010] [error] [client 82.230.177.196] node_update_7006, referer: http://do.stage.damz.org/update.php?op=start&id=682703
[Fri Jan 15 23:35:12 2010] [error] [client 82.230.177.196] node_update_7006, referer: http://do.stage.damz.org/update.php?op=start&id=682703
[Fri Jan 15 23:35:12 2010] [error] [client 82.230.177.196] node_update_7006, referer: http://do.stage.damz.org/update.php?op=start&id=682703
[Fri Jan 15 23:35:12 2010] [error] [client 82.230.177.196] node_update_7006, referer: http://do.stage.damz.org/update.php?op=start&id=682703
[Fri Jan 15 23:35:13 2010] [error] [client 82.230.177.196] node_update_7006, referer: http://do.stage.damz.org/update.php?op=start&id=682703
yched’s picture

yched commented
StatusFileSize
new node_body_as_field_update-686128-4.patch1.48 KB

Well, batch API doesn't do an ajax loop after each hook_update_N() pass, it processes as much as it can for 1 sec and then reports. So increasing $batch_size doesn't necessarily result in less ajax iterations.

50 is possibly low, but I'd rather be conservative and avoid too high values on shared setups.
Let's say 200 ?

damien tournoud’s picture

damien tournoud commented
Status: Needs review » Needs work 
+        ->fields('nr', array('body', 'teaser', 'format'))
         ->fields('n', array('type', 'status', 'comment', 'promote', 'sticky'))
-        ->fields($nr)

This cannot work, we need most of the fields from 'node_revision' (notably missing here: nid and vid).

yched’s picture

yched commented
Status: Needs work » Needs review
StatusFileSize
new node_body_as_field_update-686128-6.patch2.1 KB

Right, added nid and vid. I don't think we need anything else.

moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Reviewed & tested by the community

seems good afaict

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.