Feature request: Max duration including execution time?

[wontheone1]

One of the manipulator says

max-duration - truncate the sequence once the sum of all past delays exceeds the given number

But I think there are use cases to limit the duration by the sum of delay times and the actual time that took for execution. (e.g. if we want to retry calling an external service only until the caller of our service times out, etc).

[liwp]

You should be able to use the callback facility for that:

(I can't test this at the moment, so please forgive me for any broken code)

(def *timeout* 3000)

(again/with-retries
  {::again/callback #(when (< *timeout* (::again/slept %) ::again/fail)
   ::again/strategy ...}
  (my-operation …))

[wontheone1]

I guess my wordings weren't clear about what I wanted to do but ::again/slept is only the sum of all the delay/sleep times between retries, right? @liwp From the readme

;; the sum of all delays thus far (in milliseconds) :again.core/slept …

So this ::again/slept value is sum of the delay/sleep time but in my use case I want to retry based on the sum of delay + execution time for whatever my task that was retried. Basically I have to timeout my retry when the callers of my service timeouts. I found out safely library provides what I want by :timeout option. Please correct me if I am misunderstanding about ::again/slept.

[liwp]

Yes, you're right: ::again/slept is the sum of all the delays between execution attempts (the time spent on the attempts is ignored).

My assumptions were that:

1. that failing attempts are fast, eg the server isn't running so the socket open will fail fast
2. the delays will dominate compared to the time spent on the attempts
3. you're dealing with failures so exact timing are already a lost cause

So therefore I didn't feel the need to keep track of the execution time.

But I appreciate that this might not be the case if your attempts are failing eg because of network timeouts.

You could use the ::again/user-context field in the callback argument to track when you made your first attempt etc, but obviously that's a bit more work for you.

(def *timeout* 3000)

(again/with-retries
  {::again/callback #(let [execution-time (- (System/currentTimeMillis)
                                             (::again/user-context %))]
                       (when (> execution-time *timeout*) ::again/fail))
   ::again/user-context (System/currentTimeMillis)
   ::again/strategy …}
  (my-operation …))

I'll think about adding an ::again/execution-time to the callback argument, but the implementation would probably look exactly as above.

For other readers: safely

[wontheone1]

Thanks! To me, an easier to use api might be that just having an option to set the timeout. Anyways I appreciate your explanation and feel free to close issue if you think this feature is unnecessary. :)

[liwp]

Haha, yes a timeout option to with-retries would be a simpler API, I didn't think of that! The downside of it is that while it solves your specific use-case, it doesn't help anyone else. Eg someone might want to log a warning when the execution time has passed some limit, while still continuing with the retries.

Using the callback argument will allow people to do whatever they want with the execution-time information, while making your specific use-case a bit more involved.

I think I should probably add both at the expense of making the API more complicated.

Anyhow, thanks for the feedback - it's always nice to hear about specific use-cases of the library!

[wontheone1]

Kiitos (I recognize your finnish name) for quick responses and considering my use cases :)