While working on #326197: Empty data for latest week I noticed that the queries for the daily processing are needlessly updating some rows:

  // Asign project and release node IDs.
  $query = db_query("SELECT DISTINCT project_uri, project_version FROM {project_usage_raw} WHERE pid = 0 OR nid = 0");
  while ($row = db_fetch_object($query)) {
    $pid = db_result(db_query("SELECT pp.nid AS pid FROM {project_projects} pp WHERE pp.uri = '%s'", $row->project_uri));
    if ($pid) {
      $nid = db_result(db_query("SELECT prn.nid FROM {project_release_nodes} prn WHERE prn.pid = %d AND prn.version = '%s'", $pid, $row->project_version));
      db_query("UPDATE {project_usage_raw} SET pid = %d, nid = %d WHERE project_uri = '%s' AND project_version = '%s'", $pid, $nid, $row->project_uri, $row->project_version);
    }
  }

That final UPDATE should just be hitting rows where pid or nid are 0 or NULL.

Comments

dww’s picture

Status: Active » Needs review
StatusFileSize
new1.74 KB

I have no idea if this is going to be faster or not.

dww’s picture

Actually, that "IS NULL" stuff is bunk. project_usage.install's definition of {project_usage_raw} includes:

      pid int unsigned NOT NULL default '0',
      nid int unsigned NOT NULL default '0',

so there's no way those are ever going to be NULL.

drewish’s picture

dww, #2 looks pretty good to me.

dww’s picture

Title: Fix inefficiency in processing daily data » Optimizing processing usage data
StatusFileSize
new1.69 KB

Rerolled now that I committed #342543: Improve logging in project-usage-process.php.

Based on the messages I pasted into #342543-3: Improve logging in project-usage-process.php, it looks like there are some bigger problems here. Let's repurpose this issue to address optimizing the usage processing as a whole, if possible.

david strauss’s picture

Priority: Normal » Minor

While better WHERE criteria will may the final UPDATE query even faster, MySQL is effectively doing no-ops on rows where the UPDATE has no effect. It doesn't even count it as an "updated" row. For that reason, I'm dropping the priority of this issue.

dww’s picture

Priority: Minor » Normal
Status: Needs review » Active

Ok, good to know, thanks. However, there are other potentially more nasty inefficiencies revealed by the finer-grained timing output over at #342543-3: Improve logging in project-usage-process.php, which was the point of re-titling this issue...

drewish’s picture

Where do you think this stands?

dww’s picture

Assigned: dww » Unassigned
Priority: Normal » Minor

Until the d.o DBAs come knocking with pitchforks and torches, I'm not going to worry much more about it at this point... ;)

david strauss’s picture

Speaking as a d.o DBA, this particular request is very low priority.

dww’s picture

@David Strauss: I know you're busy at the field sprint, so don't worry about this now. However, in the future if/when you get a chance, can you look at the d.o watchdog and filter for "project_usage messages" and take a look at the time values listed for a given daily (and weekly) processing run?

For example, yesterday's daily values were:

http://drupal.org/admin/logs/event/114490730: Assigned API version term IDs for 1742168 rows (21 sec).

http://drupal.org/admin/logs/event/114490948: Assigned project and release node IDs to 1741231 rows (1 min 16 sec).

http://drupal.org/admin/logs/event/114491106: Moved usage from raw to daily: 829194 rows added to {project_usage_day}, 829755 rows deleted from {project_usage_raw} (40 sec).

http://drupal.org/admin/logs/event/114491110: Completed daily usage data processing (total time: 2 min 17 sec).

-----

The last weekly is like so:

http://drupal.org/admin/logs/event/113901970: Computed weekly project tallies for 2008-12-07 for 3675 projects (28 sec).

http://drupal.org/admin/logs/event/113902072: Computed weekly release tallies for 2008-12-07 for 8968 releases (26 sec).

http://drupal.org/admin/logs/event/113902078: Completed weekly usage data processing (total time: 54 sec).

Note, also, that from previous testing, on of the most expensive operations of removing stale data that has expired isn't currently getting hit, since I had upped the configuration on d.o to keep data for longer. So, we're in the window where we're filling up the new range of data, but in a few months, we'll start pruning data in every daily processing run again...

Anyway, seems like there's probably some indexes we're missing or some of the processing code is otherwise doing things in an inefficient way. It'd be nice to give all this code another look at some point.

david strauss’s picture

I agree that this can be heavily optimized; it's just not in the way suggested in the patch. We ought to do an UPDATE with JOINs and/or subqueries instead of an UPDATE encapsulated in nested loops driven by SELECT queries.

dww’s picture

Yes, I know the proposed patch is not a major optimization. It was just something small I noticed while reviewing the code...

I'd be happy to review a more major optimization patch if anyone's up for it, but probably not until early January when I return from holiday travels.

Cheers,
-Derek