Large memory requirements in subscriptions_ui_node_form
| Project: | Subscriptions |
| Version: | 6.x-1.0 |
| Component: | Code |
| Category: | support request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Jump to:
A client is using this module, and it was causing them to run out of memory when visiting node pages using UID 1. I tracked this down to the $subscriptions array that is built in the subscriptions_ui_node_form() function. I did a SELECT COUNT(*) FROM {subscriptions} WHERE module = 'node' AND recipient_uid = 1 which returned over 50,000.
With a quick read through of the code, I'm not understanding why you have to retrieve all subscriptions for this user at this point. And doing this clearly doesn't scale well on large websites with a lot of content, and users subscribed to all of that content.
One quick "fix" I found while tracking this down was to put a LIMIT on the query that is used to build the $subscriptions array (~line 150), though I'm not sure the overall implications of this.
#1
What? User 1 has over 50K subscriptions? This is completely nuts! Are these 50K thread subscriptions to 50K single nodes? Have they clicked on "subscribe to this page" 50,000 times? Or enabled Autosubscribe and imported 50K nodes? That's not how Subscriptions is intended to be used.
If they want to subscribe to all content, then they should subscribe to each content type, which is typically about 10, not to each node separately!
#2
The "Auto-subscribe to new content", "Auto-subscribe to updated content", and "Auto-subscribe to comments" check boxes are checked, however this content has been created by thousands of users -- I do not see why UID1 got autosubscribed to all site content. Is there a setting for this somewhere that I'm not seeing? The content was not imported, it is being created over time by the website users.
It sounds like I should manually delete all subscriptions for UID1 (with a db query), then just subscribe him to all content types? That will get things working again, however I still need to track down why he is getting subscribed to all content.
I'm also still perplexed as to why the code has to load all subscriptions for the given user into memory like this...
#3
Yes, do a
DELETE FROM {subscriptions} WHERE module = 'node' AND field = 'nid' AND recipient_uid = 1— that'll get rid of all the thread subscriptions.I'll be very interested to know where they came from!
We cannot filter the {subscriptions} entries in SQL because we don't know what we're looking for. We gather the $node_options from all Subscriptions add-ons, and then we check whether we have any matching items in $subscriptions.
However, out of the node/nid items we only need the one for the current node, so after seeing this it would make sense to split it into two queries. Try this before deleting the junk: replace
<?php$result = db_query("SELECT sid, module, field, value, author_uid, send_interval, send_updates, send_comments FROM {subscriptions} WHERE module = 'node' AND recipient_uid = %d", $account->uid);
while ($s = db_fetch_array($result)) {
$subscriptions[$s['field']][$s['value']][$s['author_uid']] = $s;
}
?>
with
<?php$result = db_query("SELECT sid, module, field, value, author_uid, send_interval, send_updates, send_comments FROM {subscriptions} WHERE module = 'node' AND field = 'nid' AND value = '%s' AND recipient_uid = %d", $node->nid, $account->uid);
while ($s = db_fetch_array($result)) {
$subscriptions[$s['field']][$s['value']][$s['author_uid']] = $s;
}
$result = db_query("SELECT sid, module, field, value, author_uid, send_interval, send_updates, send_comments FROM {subscriptions} WHERE module = 'node' AND field <> 'nid' AND recipient_uid = %d", $account->uid);
while ($s = db_fetch_array($result)) {
$subscriptions[$s['field']][$s['value']][$s['author_uid']] = $s;
}
?>
Does this help? BTW, you only mentioned memory overflow — what about run-time? Was there no time problem retrieving those 50K records?
This causes a little bit of overhead if there are no (or few) thread subscriptions, but it'll scale better.
(I'll be on the road for the rest of the week and may not be able to reply until I get back.)
#4
Yes, that solves this specific problem very nicely. It seems that only ~20 of the {subscriptions} entries had "field <> 'nid'".
Thanks!
#5
I'd still like to know where those 50K entries came from, and you should probably purge them.
#6
Unfortunately I'll be unable to dig into the issue any further. The records were purged. Having talked with the client about it further, it sounds like UID1 was being used as a subscriptions proxy for a time -- thus the module was clearly being used in a nonstandard way. In any case, your improved queries are still a performance improvement IMO so it would be great if you merge them in.
Feel free to close this issue - the support request is happily solved. Thanks!
#7
Ok, thanks. I've committed a streamlined version of #3 to 6.x-1.x-dev.
#8
Automatically closed -- issue fixed for 2 weeks with no activity.