Periodical support and jitter

[joshmn]

Discussion continued from #21 (comment)

This is an interesting use case I didn't quite fully think through!

I wonder if it would make sense to support something like:

drip :random_long_term, mailer: ClientMailer, periodical: true, at: :set_time 

def set_time
  index = subscriber.mailings.sent.count 
  subscriber.morning_delivery((index * 2).weeks.from_now + rand(0..2).days) + rand(0..180).minutes 
end

I can't quite remember where/how periodical determines time. It's poorly documented (if that) because I haven't quite used it yet myself... hehe. It could also probably be reworked to hook into the callbacks as well if it's not already.

@jon-sully thoughts?

[joshmn]

I actually almost like this idea too:

drip :welcome, repeat: true, at: :set_time 
drip :welcome, repeat: 5, at: :set_time 
drip :welcome, repeat: :repeat?, at: :set_time

[jon-sully]

Under the hood periodical just clones the was-just-sent mailing and essentially calls

previous_mailing.send_at + params[:every] # e.g. 2.weeks

Which, is already pretty close to being jitter-able, but currently :every has to be a static, set value; it doesn't get re-computed on every clone. If it did, (if :every supported a Proc) then jitter could already be built in 🤔

[jon-sully]

Ah, and one other note about your API thought there ^ (a separate issue we should open) is that calling drip multiple times with the same action name only yields a single mailing based on the way drips are registered into the drip collection! So

drip :welcome, repeat: true, at: :set_time 
drip :welcome, repeat: 5, at: :set_time 
drip :welcome, repeat: :repeat?, at: :set_time

Would effectively ignore/skip the first two calls and only that third drip would actually register.

[jon-sully]

I think the ideal periodical API ought to support a dynamic every and an until arg, (optionally). IMO these should all work:

# these wouldn't be used together, they're just examples
drip :follow_up, every: 2.weeks, until: 10.weeks.from_now
drip :follow_up, every: -> { 2.weeks + rand(0..2).days + rand(0..100).minutes }, start: -> { recipient.next_appropriate_messageable_time }
drip :follow_up, every: -> { recipient.drip_frequency }, until: -> { recipient.out_of_drips? }

(Note that start: already supports a Proc)

There's enough difference here from the standard drip call that I wonder if periodical support ought to be broken out to a periodical_drip call rather than a drip call. Could even combine them.

drip :initial_follow_up, class_name: UserMailer, delay: 10.minutes
periodic_drip :new_post_round_up, class_name: UserMailer, every: -> { recipient.summary_email_frequency }

but I'm not sure if that's better or worse 😛

[jon-sully]

Ah, to my last idea, I think that's already the case. I didn't realize it before but I think that while the every: key is supported in the basic drip call, it won't actually work — you must use the periodical call to get the every: key working right (it's in the implementation of periodical that sends up the after_send callback that creates the next mailing). So in actuality, this is already the required API:

class SomeDripper
  drip :initial_follow_up, class_name: UserMailer, at: 10.minutes.from_now
  periodical :new_post_round_up, class_name: UserMailer, every: 10.days
end

That's a big note! 😅

[joshmn]

Ah, and one other note about your API thought there ^ (a separate issue we should open) is that calling drip multiple times with the same action name only yields a single mailing based on the way drips are registered into the drip collection! So

drip :welcome, repeat: true, at: :set_time 
drip :welcome, repeat: 5, at: :set_time 
drip :welcome, repeat: :repeat?, at: :set_time

Would effectively ignore/skip the first two calls and only that third drip would actually register.

Ah, sorry — should clarify: that was just one example for welcome where you could "max repeat".

Do you have any instance where you have a mix of periodic and regular one-off drips in a Dripper?

[jon-sully]

Do you have any instance where you have a mix of periodic and regular one-off drips in a Dripper?

I don't know that my app has that at the moment, but it's definitely something I could see us doing

[jon-sully]

I'm starting down the track of supporting a Proc for the every: key + adding until: support, since I think that would cover 90% of the bases with much less work than some other ideas I've suggested, but I'm seeing some interesting logic in start: to begin with. Given the following code:

# lib/caffeinate/schedule_evaluator.rb

      if periodical?
        start = mailing.instance_exec(&options[:start])
        start += options[:every] if mailing.caffeinate_campaign_subscription.caffeinate_mailings.count.positive?
        date = start.from_now
      elsif options[:on]
        date = OptionEvaluator.new(options[:on], self, mailing).call
      else
        date = OptionEvaluator.new(options[:delay], self, mailing).call
        if date.respond_to?(:from_now)
          date = date.from_now
        end
      end

And the idea that I have a dripper setup like so:

periodical :round_up, every: 5.days, start: -> { 3.hours }

Let's say I subscribe the user to the dripper at 9am on 2/1/23. start = mailing.instance_exec(&options[:start]) will run and return the value of 3 hours. start += options... won't do anything because no mailings have been filed yet. And date = start.from_now will file the message for 12pm on the same day, 2/1/23. This makes sense as this is the first message.

But then we skip ahead to 12:05pm — the message is sent. The after_ hook runs and sets up the cloned/new/second periodical message. This time through the Schedule Evaluator though, start = mailing.instance_exec(&options[:start]) will still come back as 3.hours. start += options... will run and offset the next message by five days, and ultimately the second message will get filed for sending at 2/6/23 at 3:05pm.

I think that's unexpected? 🤔 I'd expect the start value to only offset the first message in the periodical sequence, not be an every-message delay factor, relative to its prior 🤔. Maybe this is better described as "start relative to prior message"? I'm not sure.

That said, technically, that means we can already introduce 'jitter' by using start: that way:

periodical :round_up, every: 14.days, start: -> { rand(0..2).days + rand(0..60).minutes }

In the above, we setup a "jitter amount" of 0-2 days + 0-1 hours. The first message will send after the random jitter amount, then every subsequent message will send (2 weeks + the jitter amount) later — which is the goal! 😆

Interesting.