Extend duration documentation

[thomas-nicolai-frequenz]

What's needed?

We need to improve the documentation for the duration parameter as shown below:

Proposed solution

message Dispatch {
  // ...

-  // Duration in seconds
+  // Duration of the dispatch in seconds. 
+  //
+  // !!! note
+  //     This field manages how long the dispatch lasts:
+  //     - `null` value indicates an infinite duration, meaning the dispatch continues indefinitely.
+  //     - A value of `0` indicates a one-time execution, which does not repeat or persist.
+  //     - Positive values specify the duration in seconds for which the dispatch should be active.
+  //
+  //     Example of how to set duration
+  //         duration_seconds: { value: 3600 } // Duration of 1 hour
+  //         duration_seconds: null // Infinite duration
+  //         duration_seconds: { value: 0 } // One-time execution
- //  uint32 duration = 7;
+ //  optional uint32 duration_seconds = 7;
}

Use cases

No response

Alternatives and workarounds

No response

Additional context

No response

[llucax]

Maybe it would be also worth clarifying that this is a duration within one run if it is a recurring dispatch. It is not related to the recurrence itself.

[thomas-nicolai-frequenz]

@leandro-lucarella-frequenz how about?

message Dispatch {
  // ...

-  // Duration in seconds
+  // Duration of the dispatch in seconds. 
+  //
+  // The duration specifies how long each occurrence of the dispatch lasts during 
+  // a single run, applies only to the individual instance and not the overall cycle of 
+  // recurring dispatches. This setting is independent of the recurrence pattern, 
+  // which is governed by separate recurrence settings.
+  // 
+  // !!! note
+  //     This field manages how long each occurrence of the dispatch lasts:
+  //     - `null` value indicates an infinite duration, meaning each dispatch occurrence 
+  //.      continues indefinitely.
+  //     - A value of `0` indicates a one-time execution for that particular occurrence, 
+  //       which does not repeat or persist beyond its initial run.
+  //     - Positive values specify the duration in seconds for each dispatch occurrence 
+  //       should be active.
+  //
+  //     Example of how to set duration
+  //         duration_seconds: { value: 3600 } // Duration of 1 hour
+  //         duration_seconds: null // Infinite duration
+  //         duration_seconds: { value: 0 } // One-time execution    
+  //
- //    uint32 duration = 7;
+ //  optional uint32 duration_seconds = 7;
}

The duration parameter is renamed to duration_seconds so its more distinct from the recurrence pattern settings, ensuring that there's less confusion between the duration of individual runs and the overall scheduling pattern. Moreover, at the top it explicitly states that the duration_seconds applies to each individual occurrence of the dispatch.

[leandro-lucarella-frequenz]

I'd remove the "Maybe it would be also worth clarifying that " ;)

[thomas-nicolai-frequenz]

Oh, sorry I've improved the above.

[Marenz]

While this is clear enough if you read the documentation, I wonder if it would still be better to be even more explicit like:

// Example of how to set duration
//     duration: { seconds: 3600 } // Duration of 1 hour
//     duration: { type: INFINITE } // Infinite duration
//     duration: { type: ONE_SHOT } // One-time execution
oneof {
    uint32 seconds = 1;
    enum {
        INFINITE = 0;
        ONE_SHOT = 1;
    } type = 2;
} duration = 1;

[Marenz]

If it's only based on documentation, it can get easily lost when using the generated bindings and methods (in rust/python/etc) on top of that using it.

If it's part of the structure, it is kind-of self-documenting, no matter how it is further abstracted

[llucax]

That looks more clear to me indeed. Maybe call it special instead of type. We should still document what does seconds: 0 means or if it is disallowed.

Also, now I kind of wonder what ONE_SHOT (or the previous duration of 0) even means, shouldn't repeat being null mean it is a one-shot run? Does it make sense to have it at all as a special meaning in here? If not I have the feeling then duration_seconds with null as infinite is simple and intuitive enough to probably not require the extra complexity in the message.

[thomas-nicolai-frequenz]

We should still document what does seconds: 0 means or if it is disallowed.

Well in this case it would not be allowed anymore as otherwise we would have redundancy as it would have the same meaning as ONE_SHOT. Thats bring me to another point, I really dislike ONE_SHOT. I'd suggest to rather call it ONE_TIME or something.

[thomas-nicolai-frequenz]

or the previous duration of 0) even means

To give you an example: "Switch off an inverter" if you don't want it to continuously being monitored and shut down whenever it gets activated again by something else. Thats one use case. This is more related to maintenance or other things where there is no duration.

[thomas-nicolai-frequenz]

The question I'm having is more like if "duration_seconds: null // Infinite duration" is compatible with a reoccurring dispatch rule at all. If each run is infinite they'd start stacking up over time. Probably nothing we'd want to happen.

[Marenz]

If each run is infinite they'd start stacking up over time.

It's kind of the same problem as sending a dispatch to an actor that is already running and I'd assume it would behave the same as well. I think the current behavior is that it simply keeps running as the "I should be running" state doesn't change.

[Marenz]

Another thing I am wondering:

It's still up to the application to actually behave that way and to interpret the desired duration.

If it's something like your example "turn off inverter", I would assume the receiving side doesn't even check the duration because it has no meaning for it, it just runs the command and done.

What I am trying to say: IF applications can ONLY do one-time/one-shot and have nothing that would be ongoing/lasting(requiring a duration), they don't need to even care about the values we specified. The App KNOWS it's a one-time app.

The question is then only, do we have apps that are one-shot but also sometimes need a duration? If not, I don't think we even need ONE_TIME.

And if we have no ONE_TIME we might also don't need to be so explicit here in regards to INFINITE.

we could rename the attribute from duration to stop_after.

That makes it more clear that it won't stop if it's not set, in that case we basically say "it's up to the application to stop when appropriate" this includes inf and one-shot imo.

[llucax]

Yeah, though, I agree that it makes sense for duration null have different meanings in different contexts, it makes sense that a one time action has duration null instead of zero IMHO. The issue is if for some dispatch it makes sense to have a duration infinite or no duration. If there is, it may be better to have it explicit. Maybe something more like INSTANTANEOUS rather than one time/shot, because that seems too related to recurrence more than the duration.

[Marenz]

I would just say

• "duration > 0" => Duration will be as specified
• All other cases: App decides.

[llucax]

In terms of the dispatch lib, what do you do with duration 0? With duration null you'll never consider the dispatch stopped, right? Actors will never get a "notification" that the dispatched stopped. With duration 0 will it be the same? IMHO this is the important thing to specify, because it is the part that it will be common to all apps/actors.

For me it looks like 0 should just be disallowed and null means no duration (whatever that means instantaneous or infinite can be left to the app/actor), in terms of dispatch lib it means no automatic handling of the duration by the lib.

[Marenz]

I am fine with disallowing 0 as well

[llucax]

We found one case where the distinction between instantaneous (0) and infinite (not present/null) matter. We have a way to check if a dispatch is running/active or not. For instantaneous it should basically always report not running/inactive except for the exact start timestamp, as for infinite it should report that it is running/active after the start timestamp.

[Marenz]

https://github.com/frequenz-floss/frequenz-api-dispatch/pull/200