Maximum number of notification to send per cron job breaks Digest mode

Thank you for the clear report!

I see the problem. In the cron run, we sequentially go through the notifications queue and once the set number of messages per cron job has been reached, we don't look for additional messages that could be appended to already prepared digest messages (without increasing the message count for this cron job).

This calls for a somewhat modified strategy: When retrieving a notification from the queue and the recipient has digest mode enable, then retrieve all other pending notifications for that user at once. This may actually improve robustness and throughput.

It turns out that this is not easy to implement. Specifically, it caused me to rethink our strategy for supporting more than one send_interval, and I've realized that it's somewhat flawed and needs to be fixed first.

However, I can't let this hold up the release, and so I need to postpone this.

This is very important. I'm running the beta5 and this has become a huge issue for me. Effectively, when a site has more than about 700 subscribers to a content type, in digest mode, it is impossible to get a true digest email out to all of them. Increasing the number of notifications to unlimited does allow the digest to send correctly, but then cron times out and it clears the queue even though most of the emails were not sent. If you decrease the number of notifications sent per cron run, then the digest only gets sent with the last story that would have been in it, instead of the true "digest" email.

Bottom Line: Subscriptions Module, as it is now, can not send digest emails to more than about 400 users.

Please... if you could fix this one problem, all my troubles will be surmounted on my news site.

I'm running FreeBSD 6.3, Drupal 6.12, MySQL 5.0.75, PHP 5.2.9, Apache 2.2.11, Subscriptions 6.x-1.0-beta5

At this point, I'd be willing to pay you to get this fixed. Please tell me what it is worth to you.

As a temporary measure, can you not run cron more frequently, like every 5 minutes? Over time, the digests that are ready for sending should spread out, if you have enough new nodes per day.

(If you have only one per day, then they're all ready at the same time after 24 hours.)

Running cron more frequently will not help, as the digest subscribers are all signed up for the same content. I have more than 400 subscribers, so the module never sees the next story in the queue that belongs in the digest untiul the first story is sent.

Running Digests does not work at all when you have more users signed up for digest than you have seconds of cron run-time available.

In other words, no matter what I set the notification limit to in the settings, the module won't "look ahead" more than that number of queue items to see that there are more items that could be combined.

I think the software should read the entire queue, no matter what the limit of notifications is set to, so it can combine the correct queue items into digests.

As it stands, if the queue holds 600 stories, of which 200 users should get 3-story digests, and the limit is set to 100, then the users will each get 3 digests containing just one story each, because the module won't look ahead in the queue more than 100 stories, even though it could combine them all into 200 emails.

I'm at a catch-22 situation:

If I set the limit to less than my number of digest subscribers, then digests will only have one story in them, and the subscribers will get one digest per story, no matter how often I run cron.

If I turn off the limit by putting zero "0" in the settings, then users get the correct digests, with multiple stories in them, but cron times out halfway through the queue run and deletes the rest of the queue, unfinished.

The module would seem to work correctly if it could just read the entire queue and combine stories into digests as needed, then decide how many to actually send based on the sending limit set in settings.

This is why I say that digests will not work if you have more users than cron-run seconds available. If you have 400 users subscribed to digest content of 4 stories every day, then either the cron-run will time out before all the users get their correct digests, of if you set a notification limit, then each user will get 4 single-story digest emails.

Here's what I was thinking: If we can somehow manage that no more than 100 queue items are ready for sending at any given time, then we should get by.

I've already considered your idea of taking the first queue item and merging it with all others for the same subscriber. The problem here is what do we do if different subscriptions have different intervals. Say, one has an interval of a day, and the other of an hour (or even ASAP). The latter would trigger with every cron run and drag along the notifications for the former. IOW, any longer interval would be overridden by the shortest one. This should not happen.

OTOH, any sending of a notification to a user sets that user's last_sent timestamp. When new notifications are queued, this last_sent timestamp is saved in {subscriptions_queue} and the notification does not become eligible for sending until last_sent+send_interval has expired. Thus, if a user has an ASAP subscription that triggers continuously, then all once-a-day notifications are automatically delayed 24 hours. The solution to this problem seems to be to store a separate subscriptions_user.last_sent timestamp for each possible send_interval.

Obviously, the timeout behavior of losing the accumulated digests is unacceptable. We must definitely find a way to avoid that.

So, we need a strategy that will work in your situation, even if your users also create ASAP subscriptions. ASAP subscriptions should be elegible for notification at the next cron run, so that you can increase the number of cron runs if the queue starts to build up. OTOH, once-a-day notifications must be collected and not sent more frequently than once a day, even in the presence of ASAP notifications.

If we had a daily subscriptions_user.last_sent timestamp, then all daily notifications in the queue would get the same subscriptions_user.last_sent timestamp and they would become eligible for sending at the same time. This means we could merge them into one digest and get the daily "sub-channel" to work as intended no matter what else is going on. Now what happens with the hourly sub-channel? If no hourly notification is eligible, then nothing. If there is one, then we should probably merge it, too, and since all hourly queue items have the same (hourly) send_interval, they would all be eligible, so we'd merge all of them. And so on...

This is debatable, but right now I think that the notifications in a digest should be ordered by time, so we'd merge the various sub-channels like a zipper.

What do you think of this plan? Will it work for you? Will it work in general?

I think it would be totally acceptable behavior that if any digest interval were set, it should be the overrulling interval for all digests per person. In other words, if a person goes and sets a daily digest interval for content AA at daily, then goes and subscribes to content BB and sets it to weekly, then the user should realize that he/she is going to get daily digests of all the content.

This is my scenario, actually:

I first made sure that a user cannot turn off digest -- I don't allow singles, only digest emails.

Then, I set up two taxonomies: "Weekly News" and "Breaking News"

All the users are set up by default with a 17 minute interval.

People can sign up for whatever content types, users, articles, etc, that they want, but when a new user is created, they all get subscribed to these two taxonomies by default.

Then, when a breaking news comes out, I put up the article and it mails in less than 17 minutes.

I hold on to the weekly email articles until Monday morning when I want people to get those, in a single digest. Then (as a workaround for another feature issue I wrote about) I go into the server and turn off my 15 minute cron runs until I get the 5 "Weekly News" articles up on the site. Then I go in and turn on cron and let 'em fly. This eliminated the need to have a different interval set up for a weekly digest.

Since all my users have digest mode on, and all my users have a 17 minute interval by default, they all get breaking news and weekly newsletters whenever I want them to, while still retaining instant notifications of any other content they subscribe to.

This was all supposed to work that way, however, until I discovered that the module couldn't handle my 1600 users all being signed up for the weekly. (In my first attempt, they all started getting what was supposed to be the weekly digest, but they got separate articles in 5 separate digests, each, because the module couldn't "look ahead" in the queue to combine them. I had the cron notification limit set to 200.

Anyway, hope this explains my particular situation with this. lol I'm not sure how you want to make it work, but I wouldn't mind at all if there were nothing BUT digest mode. :-)

I'm SO glad you are willing to listen and help: THANK YOU!

There is no such thing as a "digest interval". Every subscription can have any of the intervals offered by the site admin.

"digest" OTOH can be turned on or off, and it affects all subscriptions. However, turning on digest mode must not cause all digest intervals to implode to the smallest value. This would be inacceptable.

BTW, you don't need to stop cron for getting ready your 5 weekly pieces. Just create them unpublished (or turn sending notifications off), and when you have them then publish them, which should be easy to do in a defined 15 minute interval.

And, no, "nothing BUT digest mode" is not an option.

I'll be on the road for two weeks and unable to pursue this until I get back. If anyone wants to take a stab at it, feel free to post a patch... Be sure to base it on HEAD aka -dev!

There is no such thing as a "digest interval"

Actually, all intervals are "digest interval" if you don't allow the users to turn off digest.

"digest" OTOH can be turned on or off, and it affects all subscriptions

Not if you disable the setting controls in CSS. :-) I'm not suggesting you remove these options or anything... I was simply telling you how I use the modue and how I have it set up, so you could understand better where the module falls short of working as advertised.

And, no, "nothing BUT digest mode" is not an option.

Again... I understand that. I'm just telling you how I use it. I made it "not an option" for my users anyway. :-)

Maybe if it helps get this working, there should be a "digest interval." That way, if something is set to digest, then a universal digest interval per user should apply to it. (it could be different for each user, but a user would only be allowed a single interval for all his/her digest subscriptions.)

There is no such thing as a "digest interval"

Actually, all intervals are "digest interval" if you don't allow the users to turn off digest.

Yes, then again, all intervals are just intervals, whether digest is turned on or off.

Maybe if it helps get this working, there should be a "digest interval." That way, if something is set to digest, then a universal digest interval per user should apply to it. (it could be different for each user, but a user would only be allowed a single interval for all his/her digest subscriptions.)

I don't think so. Even in digest mode, a user might choose to receive some notifications at most once a week (so they're bundled together in one digest, just as you do manually now!) and others once a day or even more frequently. Since you want the Weekly News to go out on Monday morning, you'll still have to release them manually, but I'm sure you see the pattern.

I do see what you are getting at.

I'm just trying to put up some options that may help you get it working easier.

I would, of course, be satisfied with just having it work. :-)

If the queue is read only up to the number of allowed emails per queue run, then it would seem a couple of changes may make this work.

Currently, you have a single queue, with all the possible emails that go out all lined up according to when they were added to the queue. When the module reads, say, 200 queue items, and some are digest, there is difficulty reading ahead in the queue to see if there are more digest-able entries in the queue that can be combined with the first 200 entries. Since the module does not modify the queue after the original entries were made, except only to delete the entries when sent, then digests begin to have problems when the total number of digest-able entries in the queue exceeds the number of allowed emails per cron run, or even when the number exceeds the number of possible emails before a cron run fails.

To work with this queue system that contains single emails and digest(able) emails which are contained in the same queue, the module has difficulty because you must search the whole queue for possible digest entries before sending the combined emails.

It seems to me that one of two things may help alleviate the problem, even if they both would take some possibly major modifications to the module:

First Option)
The module would be endowed with the ability to "look ahead" in the queue, add digests together, and resave the combined formatted digests back into the queue for later sending in future cron-runs. This would also mean that a new digest format would need to be created that can accept consecutive additions to each one. So, if a single cron run were to gather 20 digests, for example, and send out only 10 (because of set limits) it would save the remaining 10 back into the queue, at the front of the queue. This would mean that on consecutive cron runs, more digest-candidates may need to be added to the ones that were resaved back into the queue if more digest candidates arrived between queue runs.

Second Option)
A separate "digest" queue could be created, so that only digest queue entries would be expected in the digest queue, and the module could serve up emails from both queues every cron run. The digest queue could be searched completely each run, and all possible digest entries combined in it. If needed this queue could actually be a secondary queue, that was fed only the digest entries from the first "combined" queue, so that digests could be handled differently without affecting the way the single-email subscriptions work. (Single subscriptions don't have any problems currently anyway — just digests.)

Both sound like a lot of work, but the scalability would be increased greatly either way.

PS: I just read your TODO: Get the next queue item for this user and finish off this user's digest in the subscriptions_mail.module. Seems like this would be a good spot to break all the digests out of the initial queue, and send them to a secondary "digest" queue where they can wait to be sent according to the maximum queue run settings.

Just a couple of ideas — take'em or leave 'em. :-)

What you suggest is too complicated. The only thing that's needed is to pick the first eligible notification record and -- if digest mode is enabled -- to merge all others for this user that are also ready.

I believe, to implement "separate queues" per send interval, we only need to keep separate last_sent values for each send_interval, which means a new {subscriptions_user_last_sent} table.

Sounds great. Whatever works. Thanks for working on it. :-)

Any idea when we can expect some sort of fix?

I can't make any predictions, sorry...

I'm making a couple observations and a related question:

Observation: The module does make all digests correctly when there is no limit set, but times-out cron when there are too many it tries to send.

Observation: The module only reads the queue up to the limit set in the settings when reading the queue, so if there are more digests than the maximum set in the settings, no digests are compiled.

Question: Why can't the module scan the whole queue, as it does when there are no email limits set, then combine the digest ones like when no limit is set, and only send the ones up to the limit, erasing the sent ones from the queue on send? Since it is reading the whole queue in the beginning and combining, it could leave the digestible ones in the queue that weren't sent, and erase the ones that were. Even though it would have to rescan many queue items over each time, at least it would get them all digested and sent, wouldn't it? Wouldn't even need any new tables as far as I can tell.

No, presently, it takes one queue entry after the other. If digest mode is off, then the notification is sent immediately. If it's on, then the message is temporarily stored (and removed from the queue!). The temporary store is not processed until the queue gets empty (meaning all ready-to-send entries are gone), because we haven't gathered all entries until then.

At least that's how I think it currently works, but I haven't looked at it closely.

This must be changed fundamentally to take an entry, and if digest is on, collect all entries for that one user (which should go into the same digest), and then send that digest. The problem is, with this algorithm, the intervals won't work correctly. That's why we need a last_sent value for every send_interval.

Hi Salvis

What a coincidence. My client who needed the Premium module you suggested also needs Subscriptions. In short, she needs her members to be able to sign up for a digest of posts associated with a specific topic.

I think I can get her to defer on the Digest part of Subscriptions but was wondering if you had a time frame for addressing the issues above. If she is successful with this site, she will exceed 400. How can I help.

cindy

Nice to see you again so soon! :-)

It really depends on when I get time to work on this, and I just don't know. I hope it'll be within a few weeks.

Having two well-motivated and knowledgeable testers will certainly help!

I am a third person much in need of help with this issue. I have 3000 subscribers who want only the daily digest version, not individual notices. I thought this module would do that before implementing it. I, too, am sending out thousands of individual messages daily, ticking off the recipients, or sending nothing at all because I am dumping the queue manually to avoid the massive sends.

I'm sorry I haven't been able to get to this yet. I definitely intend to solve this problem, but it will require significant testing and I can't hold back 1.0 any longer.

Ok, here's a patch for RC1 that implements what I have in mind. I've done some testing to ensure that it doesn't break anything, but consider this ALPHA quality: I believe it's correct and complete, but I don't have the resources to test it thoroughly.

Use this at your own risk! Do NOT put it on a production server before you've convinced yourself that it works correctly.

The patch will require a database update, but the update is backwards compatible, i.e. you can go back to RC1 even after the update.

Can't wait to hear from you... :-)

Ok, here's a patch for RC1 that implements what I have in mind.

I may be misunderstanding you... Are you saying this patch (tentatively) fixes the digest problem?

Yes, exactly.

This patch will still take queue items one-at-a-time from the top (if they're ready for sending), but when it processes an item for a user who has digest mode enabled, it changes the retrieval strategy and gets all queue items (that are ready for sending) for that user first and sends off that digest, before returning to the primary strategy.

IOW, it goes depth first on digests, rather than gathering all digest items for all users and trying to send them at the end. I'm positive that it will not lose any items anymore.

I hope the overall behavior is as desired, but I don't have the time and motivation to generate the kind of real-world-like traffic (that exceeds the number of notifications per cron job!) over days and weeks and to analyze what happens. That's where I rely on you who expressed vital interest in this functionality...

Also, I may have broken something by introducing a bug here and there, that's why I'm calling it ALPHA... The code in RC1 has been used (= tested) extensively, but the modifications in this patch haven't.

That sounds great! Now I just need to research how to apply a patch... Is there any way you could roll this in and make it a 6.x-1.x-dev or 6.x-2.x-dev?

This isn't worth going to 2.x. I'll put it into 6.x-1.x-dev as soon as 6.x-1.0 is released, but until then 6.x-1.x-dev is blocked and we only have the patch.

Instructions for applying patches are on http://drupal.org/patch/apply

Sorry about the inconvenience...

Patch applied. Thank you. :-)

Does this patched module need any special treatment to install? I don't want to lose my users' settings. Do I just replace the module and update the database as usual, or do I need to create or remove any database tables?

No, no special treatment. Just replace and run update as usual. It's all there in subscriptions.install: It will migrate {subscriptions_user}.last_sent to the newly created {subscriptions_last_sent} table, which is by-send_interval.

The line to remove the obsolete column is still commented out, so you can go back to RC1 at any time. This will cause a tiny glitch because the old {subscriptions_user}.last_sent will be in the past, i.e. some notifications may be queued for early delivery, but it'll repair itself as soon as a the next one is sent.

You can also go forward again, with the same minor caveat.

I ran tests as follows:

1) Set up 6 users, three subscribed to "blog posts content type" and three subscribed to "specific blog".
2) Set the maximum emails per cron run to 2.
3) Created 3 blog posts in the "specific blog" category.
4) Ran cron manually, 4 times.
5) Repeated steps 3 & 4.

On the first 3 blog posts, all the users only got the third post, alone, in a digest email. They were dished out 2 users at a time until all the users had gotten a digest email. All the users only got the thrid blog post, alone, in their digest email. (However, they did only get one email each! (Yay!)

On the second 3 blog posts (the repeated steps 3 & 4) the digests started coming though with the second set of 3 blog posts all in them — correctly! So, it looks like all 3000 users in my production site will be forced to get at least one more defective digest, but then they should all start getting the correct digests from that point on! Good work! This seems to be working, aside from the initial incorrect one.

Each time, I watched the queue in MySQL and saw that emails for each user were correctly removed when they were combined, and each cron run the emails were sent out 2 by 2 untill the queue was empty.

Bottom line: for me, it seems to be smartly doing what it should do now. :-) THANK YOU!

I've discovered an issue that I'm not sure the patch has created, or if it was there all along, but here it is:

If you create several blog posts without "publishing" them yet, they still go into the queue. However, when cron runs, they are just deleted from the queue without being sent (presumably because they were not "published" yet.) Then, if you go and set them to be "published," they do not appear in the queue again, and never get sent to subscribers on the next cron run. Maybe the module should only add items to the queue after they are set into a "published" state, or there should ba another table column to keep a "published state" entry?

I can work around this for the most part, but some subscriptions will be lost when the posts are not moderator-approved to be published in a timely manner.

UPDATE: If someone other than the superuser creates the blog post, the above is still true, except the super-user will get a correct digest of the posts just before they are deleted from the queue for everyone. If the superuser created the blog posts, he/she does not receive a digest and the cron run deletes them from the queue because they were not published.

Thank you for your testing, that's great news!

I don't quite understand why there's still an initial loss. The entries did go into the queue, didn't they? It shouldn't be possible for them to leave the queue without being sent...

Your findings about unpublished nodes are very interesting, too. I'll investigate this.

The initial loss issue is not nearly as important to me as the loss when unpublished nodes are created.

Addendum: I believe the initial loss may have been because I created the nodes in testing without initially setting them to "published." I then published them and then ran cron manually. So, that would be this issue: http://drupal.org/node/560146

@gregarios: #30 is unrelated to the subject of this thread here. Please open a new issue for it (cut&paste the text in #30), so we can keep things manageable.

The patch in #22 is now in the current -dev version (already available). gregarios has tested it successfully as per #29.

To the others who have shown burning interest in this issue: please test and provide feedback!

(As planned, I've tagged 6.x-1.0 and will announce it shortly. The -dev version is already past 1.0.)

Best I can tell... this issue is fixed! Thank you for your work! :-)

Great, thanks. I'd like to see it running a few weeks on your (as well as kddailey's) production site, before I release it. We've had extensive testing for 6.x-1.0 and some of this testing is invalidated by the changes, so we need to build some confidence...

Still working great as of 2009-09-28. :-) Over 17,000 queue items go out to 1600 users in digest mode every monday, like clockwork.

Perfect — quite a change from what we had before!

Thanks for pushing, throwing ideas back and forth, and testing/reporting!

How many cron runs does it take to get through those 17,000 queue items?

How many cron runs does it take to get through those 17,000 queue items?

About 8. I have the cron run time percentage set to 50%, and the maximum emails per run set to 200. (It nearly always does the full 200 now)
My cron runs every 15 minutes.
So, it is combining 4 or 5 stories for each of the 1600 subscribers, and getting them all out correctly.

Funny thing... I really don't know where it is getting that 17,000 number... it does seem like it should be about half that. I'm not complaining though... I'd rather have it give a wrong number of estimated queue items and send them all out than give the right number and have it fail.

Thanks for all your hard work. :-)

Why do you still limit the number of emails?

The number in "There are %count item(s) in the queue." is simply the number of records in the {subscriptions_queue} table. Do a SELECT * FROM subscriptions_queue WHERE uid = some_uid, then you can see what's queued for that one user. Maybe there are duplicate 'create' and 'update' queue items?

Thanks for all your hard work. :-)

My pleasure! Now, how much did you want to pay in #3 above? — Actually, I'd rather have you stick around here and provide your feedback and insight for the benefit of the Subscriptions community than sending a pile of money. :-)

My pleasure! Now, how much did you want to pay in #3 above? — Actually, I'd rather have you stick around here and provide your feedback and insight for the benefit of the Subscriptions community than sending a pile of money. :-)

Never negotiate a price after delivering the goods. No, but seriously... I would have had to hit my bosses up for some kind of cash if it came down to that, but I wasn't looking forward to it. I will stick around and test as much as I can. :-)

Ok, we got a deal! :-)

Client required that I moved the mailings to a mailing list service, so I no longer have the massive list to test any patch on. Sorry - I do appreciate your response. Now I only have about 20 subscriptions and they are working fine.

Ah, too bad we didn't get this done soon enough for you. Thanks for letting me know.

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