Need to queue mailings across multiple cron runs for scalability

I'm also having problems with the same email being sent multiple times to some of the users because cron fails to complete the list of recipients. Although I'm not sure why, it has gotten worse lately: I went from sporadic cron fails and 1 or 2 repeated emails to having all cron jobs failed and only 3 or 4 email successfully sent (and "perpetually" repeated) per cron job. Might there be some kind of cumulative effect causing this issue?

I also came across this Job Queue project that seem to be a great candidate to solve this problem. Am I correct? Did anyone experiment with it?

Thank you.

I also came across this Job Queue project that seem to be a great candidate to solve this problem. Am I correct? Did anyone experiment with it?

Well, I just did: simply replacing the call to "drupal_mail" following the example from Job Queue module works and, I believe, should solve the 2nd problem reported by jwatte.
In my case, it also showed that my problem is not within Notify but it's the smtp module that's breaking the cron job (will try an smtp issue). Still, with Job Queue and a "high frequency" cron all mails get dispatched.
Hope it helps.

A better look at the emails sent showed me that the notify + job_queue option previously referred ends up with wrong usernames and uid in the emails. In order to have it right, some more tweaks are necessary on "notify_mail" function.

This issue is a priority for me, but I'm going to be swamped for the next couple months on some large projects. I'd be thrilled to bring on a co-maintainer to work on this issue.

Not sure anymore if Job Queue is the right solution, but a solution for scalability is definitely needed.

Here is a patch I made that still needs to be tested thoroughly but appears to work. It's currently in use on http://www.redrocker.com/

The approach is to add a new column to the notify table so each user has its own send_last value. As emails are sent to each user, their send_last column is updated. When cron aborts from taking to long to complete, the notify_send_last system variable never gets set, so the next cron run it starts over, but now it compared each user's send_last value and skips any that are newer than the system value (which didn't get updated on the last run). I could have made it just compile updates since the user's send_last run, but decided that if cron is having trouble finishing in one run, it's better to let it get through all of the users first, and then the next run it will catch those.

@yamboy Attempt to apply patch in #6 on latest 6.x-1.x-dev fails.

I have not yet looked at applying it manually.
Just testing out the module and think this may be needed when enabling the module on an existing site.

I have several questions that I am working my way through, but not specific to this patch.

@yamboy Patch in #6 was applied manually to the latest 6.x-1.x-dev release and with simple testing seems to work fine.

The patch removes the check box on the admin/settings/notify for a notify check box on registration. I am still not clear on the way the notify module is designed to work on registering a new account and that is the subject of a separate issue.

Minor issue, on the user/xxx/notify at the bottom the last send date is shown followed by the "Save settings" button. I think the Save settings button should be on the next line by itself.

I have not changed the status to RTBC because I do not have a test setup with high volume to test the real purpose of this patch but it certainly does not seem to cause any problems in normal use.

Thanks.

I think #6 is not an adequate solution; it' still better to use job queue or something in that vein to eliminate the issue of cron timeouts entirely. For Drupal 7, the Queue API and hook_cron_queue_info would be the obvious solution, but I'm not sure for Drupal 6. Patches implementing job_queue module would be seriously considered, however.

@matt2000, while a proper solution is developed, the patch might be a workable temporary "hack" for those who need it.

I found that the patch in #6 also needs a slight correction to restore the fix in http://drupal.org/node/364585#comment-2539694 that was committed.

I have been digging through the issues to further my understanding of the Notify module.

The patches in this issue thread actually seem to include several different things some of which are already included in the current release.

Just on the question of "need to queue mailings across multiple cron runs" there are suggestions of two approaches.

Comments #2, #3, and #5 discuss using job queue.

The other alternative by yamboy adds a column to the table which shows last send time on the admin display which can be useful.

I have stripped the yamboy patch down to only the parts related to this specific feature. Unfortunately I have not been able to make it into a single patch but rather two. They are attached for anyone interested.

I also find myself leaning towards this approach to avoid the added dependency on another module.

We'll probably need a per-user timestamp to implement a queue anyway, so I'm OK with committing something like this before that point.

But I'm still concerned that we need to implement something to prevent a cron timeout in the first place. For example, what happens if a cron times out, and then new content is created before the next cron run? If i'm not mistaken, I think users would never be notified of that intervening content. This could be a big problem for sites that only send notifications once a day, for example.

Also, a few nit picks:

- The update hook should be named notify_update_6001().
- To roll a complete patch, checkout the module using CVS, then use `cvs diff -up . > name.patch` to generate your patch file.
- The commented out watchdog debugging could should be removed, or else added to a conditional block that runs based on an administrative setting.

Maybe this is too old to worry about it, but anyway i have tried an aproach using Job Queue with the Limit the number of jobs per cron runpatch so server mail per hour limit is not reached.

#3 issue is solved by passing new parameters to drupal_mail.

It seems to be working on http://www.seresromanticos.com

I can't do a patch so here it is the hole module with modifications:

• function notify_mail: adjustment for var substitutions
• function _notify_send: line with drupal_mail adjustment for var assignement

applied the patch. working like charming.

please commit to the latest -dev.

@parasolx which patch did you apply?

#11 patch.. so far no problem regarding this.

but need to flush email queue since no email is send even cron was run

i rebuild the yamboy patch #6, since it using the older version.

this patch make by used the latest -dev version

kindly to test this patch with -dev version.

@parasolx The patch in #18 does apply successfully to the dev release using -p1 however the patch is based on the one in comment #6 which includes several features some of which are addressed in other issues and that is what led me to stripping down that patch into those in comment #11.

Would you consider working from the patches in comment #11 with the comments in #12 or can you explain why you are favoring return to the patch in #6 ?

Thanks.

@izmeez i'm testing by using Job queue module for that patch. so far it work and being queue but since this module focus on simple method. so i think dependency on other module would not so great.

re built new patch from comment #11. addon '

Or used this patch if you need to make "Last send" text to be break from submit button.

suggested to apply patch #20 rather than this. this patch currently used at my site only not for real commit.

running patch comment #21 on my real site with 3000+ users without any problem so far.
cron has been set to run every 3 days and it run perfectly until all users get notification email.

hope get more feedback from community, if there is no problem kindly to commit into -dev

patch #21 solve this problem and work as it.

Thanks for the patch in #21. It applies and appears to work as expected.

In #12 the maintainer suggested the watchdog debugging be removed or made into a conditional. I am comfortable in it being present in the patch as a comment for a reminder and tool until such time as a conditional for administrative use is added.

I have changed the status to RTBC as it is up to the maintainer to determine when the status should be changed to fixed if this is committed.

P.S. With over 4600 sites reported as using this module I thought there would be more interest in seeing this patch committed. What solutions are others using or do they not have a problem?

@parasolx I am changing the status to needs work.

The patch in comment #21 applies without difficulty, but there is something different between it and the two patches in comment #11 that causes applying the patch http://drupal.org/files/issues/870812-notify_defaults_and_details_permis... to fail.

I haven't determined what the difference is. Any thoughts?

Thanks.

ok, the diff between the patches in comment #11 and that in #21 is as follows:

@@ -308,12 +308,12 @@ function notify_user_settings_form(&$for
     '#options' => array(t('Disabled'), t('Enabled')),
     '#description' => t('Include new comments in the notification mail.'),
   );
-  $form['uid'] = array('#type' => 'value', '#value' => $account->uid);
   $form['notify_send_last'] = array('#type' => 'markup',
     '#title' => t('Notify last send'),
     '#description' => t('The date of the last notification for this user.'),
-    '#value' => t('Last Send: !date', array('!date' => $notify->send_last > 0 ? date('n/j/y g:i a', $notify->send_last) : "never"))
-    );
+    '#value' => '<div>' . t('Last Send: !date', array('!date' => $notify->send_last > 0 ? date('n/j/y g:i a', $notify->send_last) : "never")) . '</div>',
+  );
+  $form['uid'] = array('#type' => 'value', '#value' => $account->uid);
   $form['submit'] = array('#type' => 'submit', '#value' => t('Save settings'));
 
   return $form;

If this is indeed the preferred change then the patch in the other queue will need to be reworked.

There is no other place in the notify module that "div" tags are included so they probably should not be added in this patch.

I also do not see any particular reason for moving the line:
$form['uid'] = array('#type' => 'value', '#value' => $account->uid);

What do others say?

@izmeez,

i add

Ok so I'm jumping into this issue cold it was closed and then re-opened. Is the patch in comment #21 solid and can it be committed or is there work still being done? Please comment.

@ishmael-sanchez This issue has been in the works for more than two and a half years. It was closed automatically after being marked as fixed by @parasolx after contributing a patch.

It basically revolves around adding a last sent field to queue mailings across multiple cron runs. This is summarized in comments #11 and 12. The patch in #21 essentially builds on this and also adds a div as identified in comments #28 and #29. Not sure what the consensus is on adding a div here.

Ok thanks @izmeez. I will let the others involved in this thread respond over the next couple days and unless there is a huge objection, I will commit the patch in #21.

@izmeez: i think it's better to commit patch at comment #20 rather than #21. patch #21 is done from which issues i was read before state that the phrase "Last send" should be above button which i comes to add

@parasolx Looking at the patches in #20 and #21 they both look very similar and they both include the introduction of div. Since this module does not have any css files and this is the only place in the entire module that a div is introduced, is there really a need to add this div or is it a separate issue that is being rolled in? I am trying to understand why this is being added here. Maybe a screen capture might help to understand.

Agreed, the div needs to be removed before this can be committed.

@izmeez: argh, my fault. i attach the same patch.

@matt2000: put up new patch without div.

kindly to re-test.

just for additional references, i put some screenshot for reason i put DIV in the last send text.

ps: this doesn't mean i want it to be commit just to tell in clear story manner.

@parasolx Thanks for putting up both patches with and without div. Also thanks for sending me an email showing me how adding the div moves the button to the next line. It does look better.

I am happy to accept whichever the module maintainer wishes.

@parasolx The same can be achieved without a DIV tag by changing the one line in the patch to the following:

    '#value' => t('Last Send: !date <br />', array('!date' => $notify->send_last > 0 ? date('n/j/y g:i a', $notify->send_last) : "never")),

Just another two cents from me, personally I find the code to be more readable when the $form['notify_send_last'] comes after $form['uid'] the way it was in the original patch in comment #6. This way the code for the text is followed by that for the submit button.

  $form['uid'] = array('#type' => 'value', '#value' => $account->uid);
+  $form['notify_send_last'] = array('#type' => 'markup',
+    '#title' => t('Notify last send'),
+    '#description' => t('The date of the last notification for this user.'),
+    '#value' => t('Last Send: !date <br />', array('!date' => $notify->send_last > 0 ? date('n/j/y g:i a', $notify->send_last) : "never")),
+    );
  $form['submit'] = array('#type' => 'submit', '#value' => t('Save settings'));

just a minor point that others may see differently.

@izmeez: thanks for that. actually the main reason I change the last send text above
$form['uid'] = array('#type' => 'value', '#value' => $account->uid);
because it still look same as without div and to make arrange code more easier to look.

since this module does not provide any CSS files, adding div will definitely make the last send text to be break into new line from submit button. of course appearance will difference based on what theme was used.

but the idea to add DIV because in modern browser it has attribute of display:block as default without any insertion of inline style (which I want to avoid it cause inline style can reduce certain performance). The idea is to add it so we can have new line break for submit button what ever theme site is using.

This may be a duplicate of #180052: Allow throttling of mails to prevent cron timeout.

The problem with repeated calls to time() has been fixed in 6.x-1.2, cf. #368175: Race condition in notification.

However, I am not sure that all the problems mentioned in this issue has been resolved.

The patch in #36 (2011-09-14) no longer apply to the latest dev snapshot.

I've no longer running Drupal 6 on any production sites, and my Drupal 6 test site is small and not affected by this. I am not going to spend time setting up a simulation of a large site to review this.

If you want this committed, and you're affected by the issue, please reroll the patch and test against the latest 6.x-1.x-dev version and report back here.

@gisle I appreciate the efforts you are making. Unfortunately we are moving sites away from D6 to D7 and the D6 sites have been working for some time with a number of patches so I do not think we will have the time to look into this much either. Much more likely we will look at were things are going with D7 branch and considering it or other alternatives depending on the needs of the sites. The Notify module for D6 has served some of our sites well for a few years now and we appreciate that. If someone else is interested in doing more on this we will see if we can do anything to help review/test it but time is moving on for D6.

I experienced some problems with no mails sent via cron (using either notify 6.x-1.2 or notify 6.x-1.x on drupal 6.33, php 5.4).

I integrated the patch in #36 based on 6.x-1.x, still had some cron issues : "Cron run exceeded the time limit and was aborted" in my logs. Cron was not run.

After debugging step by step, I found there was a problem with drupal_html_to_text($txt) function (it crashes ?) at the end of _notify_content function. I found a work around (not a specialist of php, sure there is a better way to achieve this) using : strip_tags(html_entity_decode($txt,ENT_QUOTES|ENT_HTML401))

It works now like a charm ! Cron ends and mails are received.

Hope this helps, regards

Laurent