This happens regardless of whether you're counting campaigns by ad impressions or not. The issue is in the function _simpleads_campaigns_complete. On my site, I've got half a million ad impressions, and the table was being counted every time an ad loaded, which was causing the ad to take 6+ seconds to show up on the screen, not to mention pegging the CPU. For people who are running ads by date and not impression, this count is unnecessary, so I moved the counts into an if{} block, which fixed the issue for me specifically. That being said, this will continue to be an issue for sites counting campaigns by impressions and clicks, so for a real fix, I'd recommend creating a summary table that stores impression counts and updating it from cron.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

activelink’s picture

This issue becomes a database-killer when the simpleads_impressions database table becomes large, as it is counting the number of impressions every time a Simplead advert is shown. It's not obvious when the module is first deployed, as this table will be small, but it will cause a site's performance to disimprove over time until it becomes a serious problem on busy sites.

Here's a fix, based on BartK's suggestion. It leaves all functionality intact but skips database queries if they're not needed, but I strongly suggest not using impression counts for metering Simpleads until the code for this has been taken out into cron.

In file modules/simpleads_campaigns/campaigns.helper.inc

In function _simpleads_campaigns_complete replace:

    $all_clicks = _simpleads_campaigns_count_ad_clicks($campaign_start_time, $ad->nid);
    $all_impressions = _simpleads_campaigns_count_ad_impressions($campaign_start_time, $ad->nid);

With:

    if ($by_clicks) {                                 
      $all_clicks = _simpleads_campaigns_count_ad_clicks($campaign_start_time, $ad->nid);
    }                                                 
    if ($by_impressions) {                            
      $all_impressions = _simpleads_campaigns_count_ad_impressions($campaign_start_time, $ad->nid);
    }
minnur’s picture

Assigned: Unassigned » minnur

I am working on this issue. I am experiencing the same problem on some of my sites.

patrickfgoddard’s picture

Having a similar issue not with displaying ads but in create a custom report via views. Our table, like the first post, has over half a million records, and trying to do any sort of meaningful querying is not possible, get memory out of exhaustion errors. Very curious to follow development of a summary table.

drupalerocant’s picture

Hello, I have the same problem, as my simplead_impressions table has already around 2 million records.
I get out of memory errors quite often and I wonder if simpleads module is the cause, but I really dont know.
By the way I already disabled the campaigns module just in case it was causing the problem as my client fro the moment is not using the clicks campaigns.
Is there a way I can be sure this is the cause?

thanks

minnur’s picture

FileSize
1.15 KB

Hi There, I think I have addressed this issue in the development release. Please give it a try, but I would not recommend to install it on production. Try it on stage or local development environment..

If you are already have 1-2 or more million records, I would recommend to run a separate script (attachement).

1. Upgrade the module
2. Copy fixads.php to your websites web root directory.
3. Execute fixads.php from browser (it will take about 5-10 minutes to complete the request).

minnur’s picture

One more note: A new aggregated stats table is not integrated with Views yet.

drupalerocant’s picture

@activelink: do you know if this happens as well when you have the campaigns module disabled?

drupalerocant’s picture

thanks minnur, I will give it a try in a local environment and say something. I wont be able to try it right now, but will come here as soon as I do it as it is a problematic issue for me.

minnur’s picture

This worked for me. I have had similar problem with more than 10 million records in the stats tables.

activelink’s picture

I tried the simpleads-7.x-1.x-dev update. I was updating from 7.x-1.8. It breaks SimpleAds on my installation. Drush reported that it ran "7006 - Create SimpleAds aggregates stats table." with no errors indicated. But then SimpleAds fail to display, and I'm seeing missing columns in my Drupal logs (some parts obfuscated by me):

PDOException: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'sc.date' in 'where clause': SELECT SUM(s.clicks_unique) FROM {simpleads_stats} s WHERE s.nid = :nid AND UNIX_TIMESTAMP(STR_TO_DATE(sc.date, '%Y%m%d')) >= :timest; Array ( [:nid] => 11188 [:timest] => 1356885180 ) in _simpleads_campaigns_count_ad_clicks() (line 174 of /usr/home/someuser/sites/www.some.domain/sites/all/modules/simpleads/modules/simpleads_campaigns/campaigns.helper.inc).

In case this was something to do with Drush, I ran the update again on a fresh site copy the old-fashioned way (upload the new module and run update.php) with the same results - no errors were indicated during the upgrade.

I decided to run fixads.php after running the update, to see if it generated any errors (or fixed any). This gave me a missing table error:

PDOException: SQLSTATE[42S02]: Base table or view not found: 1146 Table 'mysite.simpleads_daily' doesn't exist: INSERT INTO {simpleads_daily} (date, nid, clicks, clicks_unique, impressions, impressions_unique) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5); Array ( [:db_insert_placeholder_0] => 20121229 [:db_insert_placeholder_1] => 11183 [:db_insert_placeholder_2] => 0 [:db_insert_placeholder_3] => 0 [:db_insert_placeholder_4] => 1 [:db_insert_placeholder_5] => 1 ) in main() (line 37 of /usr/home/someuser/sites/www.some.domain/fixads.php).

@minnur: Is it possible that in your tests that you had already manually created columns/tables in the database?

@drupalerocant: I don't know - it all seems to be tied into the campaigns functionality, which I need for my site.

djhspence’s picture

I have the same error as #10 when trying to run fixads.php.

When I updated to simpleads-7.x-1.x-dev all clicks/impressions on the ad stats page showed 0 and they don't increment. I ran fixads.php hoping that would fix it but got the error above re: simpleads_daily not existing.

minnur’s picture

FileSize
1.26 KB

Updated version of fixads.php file. The table name was renamed from simpleads_daily to simpleads_stats

activelink’s picture

The problem with the update in #10 affects campaigns only - there are errors in 2 helper functions in campaigns.helper.inc, here are the corrected functions:

/**
 * Helper function.
 * Get number of unique impressions.
 *
 * @param int $timestamp
 * @param int $nid
 */
function _simpleads_campaigns_count_ad_impressions($timestamp, $nid) {
  $count = db_query("SELECT SUM(s.impressions_unique) FROM {simpleads_stats} s
    WHERE s.nid = :nid AND UNIX_TIMESTAMP(STR_TO_DATE(s.date, '%Y%m%d')) >= :timest", array(':nid' => $nid, ':timest' => $timestamp))
  ->fetchField();
  return $count;
}

/**
 * Helper function.
 * Get number of unique clicks.
 *
 * @param int $timestamp
 * @param int $nid
 */
function _simpleads_campaigns_count_ad_clicks($timestamp, $nid) {
  $count = db_query("SELECT SUM(s.clicks_unique) FROM {simpleads_stats} s
    WHERE s.nid = :nid AND UNIX_TIMESTAMP(STR_TO_DATE(s.date, '%Y%m%d')) >= :timest", array(':nid' => $nid, ':timest' => $timestamp))
  ->fetchField();
  return $count;
}

The new version of fixads.php appears to work.

What happens to the huge simpleads_impressions table? Does it continue to grow?

minnur’s picture

simpleads_impressions table won't grow. Once a day a cron will aggregate the data and collect daily stats in the simpleads_stats table.

minnur’s picture

I fixed the campaign module. Drupal.org doesn't creates a new dev release immediately, so please check back later today or tomorrow. Or checkout from git.

activelink’s picture

Thanks minnur - I'll test the new dev update in my sandbox tomorrow and see how it works out. I'll post a follow-up here.

activelink’s picture

Before I tested the updated dev module I took Drupal core to 7.20, as most people probably will be doing this soon.

The update ran perfectly, fixads.php from #12 ran. Campaigns are working. It looks to be good!

====

Update: I now have this running on my main production site and it appears to perform well under load.

drupalerocant’s picture

Thanks minnur. Just tested it in my live instalation and everything went fine!
I have drupal 7.16 at the moment, I am not using campaigns module (at the moment it is not installed).
I have 17 active ads at the moment with 56 adGroups (not yet all in use). My simpleads_impressions table still has about 2.5 milliion rows though.

I updated to the dev version and everything went fine. The ads look not taking so much time in loading.
I ran cron twice but nothing changed in this table.
I will keep you informed how it is going.

edit---------------------------------------------------------------------------------
Well, I missed the fixeads script. I cannot run it right now as it is a busy time. I will do it tonight and report later.

patrickfgoddard’s picture

Like others, I grabbed simpleads-7.x-1.x-dev, ran fixads.php from #12, and all went perfectly. Thank you for doing this! You mentioned that the aggregate table is not integrated with views yet. Do you think that might be in the near future? Just curious.

drupalerocant’s picture

Hello, I just tried the fixads.php script in my local instalation and it returned a fatal error:
Fatal error: Call to undefined function memory_put() in zzzzzzzzz/includes/bootstrap.inc on line 2170
Is it safe for me to run it online? Do you know what can be the problem?

minnur’s picture

@drupalerocant, I would recommend to create backup first.

1. I would recommend to refine SELECT queries in the fixads.php file so you aggregate data for N dates on each execution otherwise you may get timeout message or ran out of memory.

2. memory_put() error - I don't think this is related to the fixads.php file.

3. I ran it on my production sites using option 1 (see above).

4. Possible problems:
- If you have really large database (in my case it was > 32M records) I had to aggregate 100K records at a time. Otherwise it loads the server and it may go down.

drupalerocant’s picture

Hello minnur,
thank you very much for the advice. The problem is that my php/sql is not good and I am not able to really understand the code to implement the aggregation of N data on each execution. Can you help me easily? or is it a complex change on the script? if at least you can point me to where to move I can study it and give it a try.

djhspence’s picture

Version: 7.x-1.8 » 7.x-1.x-dev

In the latest dev version I noticed that the ad stats page no longer shows "Last Hour" and "Today" columns -- will those be coming back?

Also, the "Yesterday" column always shows zero. However, the "Last Week" column increases by about the right amount, so the stats are being tracked but not displayed for "Yesterday" (confirmed this by looking at the simpleads_impressions table, and the daily totals seem to be accurately reflected in simpleads_stats).

Also, where it says "X days left" for the ad run, it shows the total number of days for the campaign, not the number of days remaining.

patrickfgoddard’s picture

As a side note, but related to this thread, I have a need to generate an aggregated impression count for a certain time range (as opposed to looking at stats one ad stats page at a time.)

In theory, this should be done via views as counts and impressions are exposed to views, but as this issue thread points out, if the impressions table is big (I'm only at ~700k records), common to run into php/apache memory issues.

As an alternative, I wrote a custom mysql query for the impressions, that you can run in your preferred mysql client:

select nid, count(*) as num_impressions from simpleads_impressions where FROM_UNIXTIME(timestamp) >= '2013-01-16' AND FROM_UNIXTIME(timestamp) <= '2013-01-31' group by nid

(Then you can export results to csv or whatever)

Consider this a stopgap for your reporting needs until the aggregated stats table that @minnur is building is ready for release.

Posting in case that helps anyone.

webopius’s picture

This fix also worked for me. I had over 5 million rows in the Simpleads_Impressions table so what I did was modify fixads.php to process rows on a monthly basis rather than the whole lot.

At the end of running fixads.php, I deleted all rows in the impressions table so that the next time the new cron ran, it wouldn't try to process all 5m+ rows.

Ref #23, to get yesterday's stats to show up, edit simpleads.stat.inc, find the _simpleads_get_statistics() method and wrap the subdate call with a date() call like this: date(SUBDATE(NOW(), INTERVAL 1 DAY))

Do this for both clicks and impressions stats to fix the problem.

cimo75’s picture

Hi I ve also 500k rows in this table.
I ve uploaded the --dev version and the site throws an error as in #10.
I try to run the script but the sites just keeps on throwing an error so the script is not launched..
Any idea?
tx
Simone

TomasBru’s picture

Can anyone upload updated script/patch, because after upgrading, simpleads_impressions table still grows and I have to empty it manually time by time, otherwise site just crashes due to CPU overload. 7.x-1.x-dev.
Thank you.

minnur’s picture

Issue summary: View changes
Status: Active » Needs work
esbite’s picture

I see you are working on Simpleads 2.0 now, but here's a patch that makes fixads.php unnecessary for the initial aggregation of impressions.

Just download 7.x-1.x-dev and apply this. Every cron run it will happily chug along with a limited amount of aggregation. Currently it is set at 200 000 impressions at a time. It seemed to work well for me. I had 12 million impressions to start with. When all impressions are done it aggregates the clicks.

sahaj’s picture

SimleAds 7.2 still have CPU usage issues (https://www.drupal.org/node/2306053). Is the fixads.php fix also applying there?

Ada Hernandez’s picture

Assigned: minnur » Unassigned
Status: Needs work » Needs review
FileSize
3.48 KB

Yesterday, Today and Last Hour are not found in the data summary tables. Attached patch fixes this.

heddn’s picture

Status: Needs review » Needs work

We need to add the translation strings into the patch.

 +t('Last Hour');
 +t('Today');
  t('Yesterday');
  t('Last Week');
  t('Last Month');
Ada Hernandez’s picture

Status: Needs work » Needs review
FileSize
4.32 KB

Yesterday, Last Hour added for the translation strings

Ada Hernandez’s picture

Today and Last Hour added, attached interdiff.

heddn’s picture

I can RTBC this for impressions. Can someone else test the clicks?

rogcorr’s picture

Hello everyone.

I'm having an issue with my drupal website. After updating some modules (the latest simpleads included), I noticed that the website became really slow.

So, I logged into my webserver, and realized that the CPU usage was like 43%, with peaks of 71%. Most of the usage came from mysql - more specifically the drupal db user.

Turns out there was 4 to 5 queries trying to sort results from simpleads_impressions table. Currently, that table has close to 17 million records, with a total of 1.4GB of data.

Can I simply delete (or truncate) all the records from that table ?

Any help is appreciated. Thanks in advance.

edysmp’s picture

simplified the code and fixed logic for yesterday, which was not working.

joshuautley’s picture

I have no data being displayed for "Yesterday". Instead it only displays "0/0".

Anyone else experience this?

heddn’s picture

minnur’s picture

Status: Needs review » Closed (won't fix)