dotnet / orleans

Cloud Native application framework for .NET
https://docs.microsoft.com/dotnet/orleans
MIT License
10.07k stars 2.03k forks source link

Passing state in reminder messages #282

Closed yevhen closed 9 years ago

yevhen commented 9 years ago

There is some pitiful asymmetry between timers and reminders.

While I can specify state to be passed back when registering timer, I can't do the same with reminders. Having support for that, might open up some interesting scenarios, like "sending message to future self". Also it might allow to outsource some temporary state to reminder service :)

I believe, that same serialization mechanisms (via [Serializable]) could be reused for state. Then, it can be simply stored as byte[] column in WATS/MSSQL.

What do you think?

gabikliot commented 9 years ago

That's a great point @yevhen! As far as I remember, back in the days when we came up with Reminders, we thought a lot about this point. On the API and logical level reminders indeed look a lot like timers, but there are some subtle runtime/perf/reliability differences. The objection against adding a state object to reminders was that it will be a backdoor for applications to "squeeze in" some application state into reminders store, state that logically should not belong there, but instead belongs into an "app store". We thought such separation is the right tradeoff both from a design perspective and also from system management perspective: we see reminders store as an internal runtime component, and feeling free to change it as we wish, as long of course as Reminders semantics are preserved. This would not be the case if this Store now also stores app state. We would also need to worry about managing app data versions.

That all said, I think we are open to discussion here. Can you please provide a concrete scenario when the state you want to store with reminders is indeed logically belongs to reminders and not part of an application state? Also, an important question: can this state be updated after stored or not? I hope your answer is not.

Also, can you explain this: "sending message to future self"?

yevhen commented 9 years ago

@gabikliot

Also, can you explain this: "sending message to future self"?

Sure. Please, see this article on Infoq by Greg Young. It's started from this post on DDD/CQRS mailing list. Seems, very relevant to actors and temporal aspects. There a lot of use-cases for that. You can find some in aforementioned sources.

Greg published an interesting Kata on a subject (and his implementation). Might give you some additional clues. Perhaps, in use-case described in Kata, timers could be used instead of reminders, but similar problems in other domains might require durable reminders with highly delayed due time (say, I want to be reminded in one month but I don't want to stay resident all this time).

I realize, that I can store reminder state locally and then correlate back by using unique reminder ids. But it's a more cumbersome solution compared to just passing temporal state back and forth using a message. I can't have distributed transaction between actor and reminder service and that what complicates the picture (for some advanced scenarios).

I believe that state versioning issues are obvious. Durable state is durable state. We are adults :)

The objection against adding a state object to reminders was that it will be a backdoor for applications to "squeeze in" some application state into reminders store, state that logically should not belong there, but instead belongs into an "app store".

I don't see this as a backdoor. Rather, I see it as a nice opt-in feature. I may use it or may not.

yevhen commented 9 years ago

@gabikliot

Also, an important question: can this state be updated after stored or not? I hope your answer is not

Messages are immutable by semantics. I can't mutate message state after creation. It's owned by value.

yevhen commented 9 years ago

If I go with distributed state route, storing temporal state locally. What will be the best strategy here? What should I do in case, if I successfully stored temporal state locally (in actor's partition) and then registration of reminder failed and my cluster dies so I wasn't able to compensate (ie rollback)? CAP theorem in Action.

Should I just re-register all active reminders on actor activation? Basically, storing all reminders locally? I would also need to delete this local state every time the reminder is delivered, right?

[lazy] BTW, what will happen if reminder delivery is failed? Will reminder service re-try delivery? If yes, then in what timeframe? Immediately (with short timeout) or skip and re-deliver it on the next reminder period tick (perhaps, in a month), or unregister it completely as being "failed"? What the criteria?

yevhen commented 9 years ago

Maybe I just don't understand the utility of reminder service? Perhaps, it's more like "periodic re-activation" service?

yevhen commented 9 years ago

Regarding state and versioning issues.

The state which is passed along with reminder message, is usually just an another message - event. It's easy to deal with versioning here by using event upconversion, usually done as just a simple function (in event -> out event). This is where higher-order functions really shine :)

Also, framework can shift all responsibility for serialization/versioning to developer by simply allowing to pass only byte[] instead of object. That will make it explicit and will allow to use different serializers. I can go with more tolerant serializer for reminder state (JSON) and use native serializer for the rest of app.

yevhen commented 9 years ago

This discussion is death by question marks. Sorry for that ... :)

yevhen commented 9 years ago

I think I figured out why this use-case is not relevant for current reminder service. It's because the reminders that I want to set have run once semantics, while Orleans' reminder service is more about recurrence. Implementing run once semantics at application level leads to all those CAP-related issues, since client need to explicitly un-register reminder after its execution, which breaks atomicity.

Anyway, I think that in any case, it will be always like that, and Orleans best effort would still be at least once delivery guarantee. Which still requires storing reminder (state) locally in order to provide idempotent processing.

I can close this issue if you also think that this feature request is non relevant (useful)?

sergeybykov commented 9 years ago

Maybe I just don't understand the utility of reminder service? Perhaps, it's more like "periodic re-activation" service?

@yevhen This is more or less how I personally look at the reminder service. I don't think we want to complicate much more beyond that. That's why we called it a "reminder" service - it merely reminds the app that it need to do something without being too involved in what that something is. I think we should keep it that simple. Maybe add another string parameter for state, so that one wouldn't be compelled to try to encode extra data into the reminder name itself. Maybe not even that.

Less is more, right? :-)

yevhen commented 9 years ago

Maybe add another string parameter for state

I think it's more about whether we want support for run-once scheduling semantics. Perhaps, an overload that accepts only due time. And state :)

The temporal aspect would be obvious in the context of this semantics.

gabikliot commented 9 years ago

I think it's more about whether we want support for run-once scheduling semantics.

A distributed exactly-once execution? At this stage I think we don't. As you pointed out, at least at this stage, Orleans's message delivery is best effort. We are working on distributed transactions for grains. With tx doing exactly-once will be easier. But that is in the future.

I can close this issue if you also think that this feature request is non relevant (useful)?

Yes, I think so.

yevhen commented 9 years ago

A distributed exactly-once execution?

Not exactly-once, but at-least-once. From client PoV - it's -1 additional step (unregistering reminder). Reminder service will automatically unregister reminder after successful delivery.

Anyway, closed.

gabikliot commented 9 years ago

But if you received the reminder, did the operation , but the unregister fails? In such a case you will receive it again. You can either deal with the "again" in your app logic, or if we had transactions you would put the on-reminder logic and unregister into one transaction.

Anyway, seems like the original question about "Passing state in reminder messages" is closed now. If we want to discuss a different question of reminder service semantics (ticks delivery and register/unregister calls) and how one can go around providing stronger guarantees around those given the best effort eventual guarantees of the current Orleans reminders implementation, lets open a new issue.

yevhen commented 9 years ago

Sure, reminder service can itself fail and won't unregister reminder, so it will still be re-delivered and I will still need to handle idempotency. I understand and completely ok with at-least-once delivery guarantee.

But it might look more pretty from API point-of-view, if reminder service supports schedule-once semantics out-of-the-box. Compare code below.

No support for schedule-once from the framework:

async Task OnScanFailed()
{
    var fluffyPeriodWeDontCareAboutSinceItsRunOnce  = TimeSpan.FromMinutes(1);
    await RegisterOrUpdateReminder("retry", due: TimeSpan.FromMinutes(60), fluffyPeriod..) 
}

async Task IRemindable.ReceiveReminder(string reminderName, TickStatus status)
{
    // do retry scan, handle idempotency
    await UnregisterReminder(reminderName); // probably need to put it in try-catch
}

Versus out-of-the box support:

async Task OnScanFailed()
{
    await RegisterOrUpdateReminder("retry", due: TimeSpan.FromMinutes(60)) 
}

async Task IRemindable.ReceiveReminder(string reminderName, TickStatus status)
{
    // do retry scan, handle idempotency
}

Different experience.

gabikliot commented 9 years ago

Yes, but we don't have the implementation for one-time reminders that I can document. What I would write: it's a one time reminder, but it may sometimes be delivered more than once? It's a best-effort once, with sometimes being multiple times?

yevhen commented 9 years ago

Hmm, I would probably describe it as non-recurrent reminder. Like in calendar application, you can set reminders to be fired once, at some particular time, or make it a recurrent event (say, remind me once a year before Dec 31, to celebrate my sister with birthday :)

gabikliot commented 9 years ago

Sorry, I think we are not on the same page. I agreed that from API perspective it would be nice to have a one-time reminder. From practical perspective, we don't have it. We don't have an implementation for it. So we should not be including an API which we don't have an implementation for.

yevhen commented 9 years ago

You think it's better to create separate feature request for non-recurrent reminders (apart from passing state in reminder messages) and the discuss it there?

gabikliot commented 9 years ago

Yes, it would be better to have it as a separate issue. This is what I wrote before:

If we want to discuss a different question of reminder service semantics (ticks delivery and register/unregister calls) and how one can go around providing stronger guarantees around those given the best effort eventual guarantees of the current Orleans reminders implementation, lets open a new issue.

But I know what we are going to answer there. That it is a very nice feature to have, but it is very hard to do in the current implementation and might be possible in the future with transactions, as I also wrote before.

yevhen commented 9 years ago

Ye, but I'm not demanding exactly-once guarantee. Neither, do I think that DTC is a good idea. It's just small api change, for me it looks like the only thing that need to be added is an automatic Unregister call, after successful delivery.

Nevermind. Looks like we really on different pages ....

gabikliot commented 9 years ago

No one was suggesting DTC. Transactions do not necessarily mean DTC. :-)

yevhen commented 9 years ago

I meant distributed transactions. Wrote DTC by reflex :)

talarari commented 7 years ago

i know this is an old issue but i just had the same thought. i need to set a reminder that basically says "do some_action on some_item_id in some_time" how do i pass context to the reminder? in this case the item id? i can some_action/some_item_id as the reminder name but that seems a little weird. what's the best practice here?

sergeybykov commented 7 years ago

I see two options here:

  1. Use the reminder name as a key for loading the associated data from storage. In this case, reminder name can even be a GUID.
  2. Put data (if it's small enough) into the reminder name itself: "action/itemID" or something like that.
yevhen commented 7 years ago

We use both 1 and 2 but reminder row could be easily extended with additional State field, say just a byte[]