Invocation epoch #2904

slinkydeveloper · 2025-03-13T15:13:53Z

Followup of #2891, part of #2890.

This implements the InvocationEpoch mechanism, as described in #2890.

tillrohrmann

Thanks for introducing the invocation epoch for enabling trimming and restarting of invocations @slinkydeveloper. The general idea makes sense to me. I think in the implementation there might be a problem wrt to handling errors in the invoker. Moreover, what's not fully clear to me is how the invocation epoch will work during a rolling upgrade where one node is running on an older version and does not know about the invocation epoch and the caller is using it. Would this pose a problem if the response is sent back to the caller with 0 as the invocation epoch and then potentially being ignored?

crates/invoker-impl/src/input_command.rs

crates/invoker-impl/src/lib.rs

crates/worker/src/partition/state_machine/entries/attach_invocation_command.rs

crates/worker/src/partition/state_machine/entries/get_invocation_output_command.rs

crates/worker/src/partition/state_machine/invocation_status_ext.rs

crates/worker/src/partition/state_machine/lifecycle/notify_invocation_response.rs

crates/worker/src/partition/state_machine/lifecycle/notify_sleep_completion.rs

slinkydeveloper · 2025-04-23T15:34:20Z

Moreover, what's not fully clear to me is how the invocation epoch will work during a rolling upgrade where one node is running on an older version and does not know about the invocation epoch and the caller is using it. Would this pose a problem if the response is sent back to the caller with 0 as the invocation epoch and then potentially being ignored?

It simply doesn't work. You can't use the new trim and restart feature unless you're 100% sure you're using the new restate version, that's the constraint.

tillrohrmann · 2025-04-23T15:54:30Z

It simply doesn't work. You can't use the new trim and restart feature unless you're 100% sure you're using the new restate version, that's the constraint.

So should this feature by default disabled with the next version unless users explicitly enable it in the configuration which states that they should only use it if all their nodes have upgraded to the next version and they forfeit being able to go back to an earlier version?

slinkydeveloper · 2025-04-23T16:01:23Z

So should this feature by default disabled with the next version unless users explicitly enable it in the configuration which states that they should only use it if all their nodes have upgraded to the next version and they forfeit being able to go back to an earlier version?

I guess we can, but we can also defer this decision to a later point IMO.

tillrohrmann · 2025-04-23T16:03:51Z

I guess we can, but we can also defer this decision to a later point IMO.

If so, then please create a release blocker issue for the next release. I would, however, advise against postponing making a decision for too long because it will otherwise bite us when we have/want to create a quick release.

tillrohrmann

Thanks for updating the PR @slinkydeveloper. I unresolved a few of my previous comments because it was not clear to me whether you rejected them or forgot to address them.

My main question is how things are supposed to work with Awakeables. It looks as if awakeables will always have the invocation epoch 0. However, once I time travel, I will be in an invocation epoch > 0. If this happens, then it seems that awakaeble completions are no longer accepted. Is this correct or did I overlook something?

I also wanted to ask what's the plan with how to enable/disable this feature to prevent users from shooting themselves in the foot if the time travel during a rolling upgrade. Are you gonna address in the PR where you introduce the time travel feature?

crates/invoker-impl/src/lib.rs

crates/worker/src/partition/state_machine/invocation_status_ext.rs

tillrohrmann · 2025-04-25T15:49:39Z

crates/partition-store/proto/dev/restate/storage/v1/domain.proto

@@ -603,6 +614,7 @@ message OutboxMessage {
  message OutboxServiceInvocationResponse {
    InvocationId invocation_id = 1;
    uint32 entry_index = 2;
+    uint32 caller_invocation_epoch = 4;


I would suggest to sort them in a way that everyone can follow. Your logical sorting might be different than mine.

crates/worker/src/partition/state_machine/invocation_status_ext.rs

crates/storage-api/src/invocation_status_table/mod.rs

crates/invoker-impl/src/lib.rs

crates/worker/src/partition/state_machine/lifecycle/notify_invocation_response.rs

crates/ingress-http/src/handler/awakeables.rs

slinkydeveloper · 2025-04-28T09:11:44Z

I also wanted to ask what's the plan with how to enable/disable this feature to prevent users from shooting themselves in the foot if the time travel during a rolling upgrade. Are you gonna address in the PR where you introduce the time travel feature?

I'm not sure yet. For the time being let's move on this PR, i'm gonna check with @igalshilman about this. This intersects with how we wanna present it in the UI too (it would be bad if we show in the UI the button but then it fails because some config option is not enabled).

tillrohrmann

Thanks for bearing with me and addressing my comments @slinkydeveloper. The changes look good to me :-) +1 for merging.

crates/ingress-http/src/handler/awakeables.rs

crates/worker/src/partition/state_machine/lifecycle/notify_invocation_response.rs

crates/worker/src/partition/state_machine/invocation_status_ext.rs

crates/ingress-http/src/handler/awakeables.rs

crates/invoker-impl/src/lib.rs

crates/worker/src/partition/state_machine/invocation_status_ext.rs

tillrohrmann · 2025-04-28T11:15:04Z

crates/partition-store/proto/dev/restate/storage/v1/domain.proto

@@ -603,6 +614,7 @@ message OutboxMessage {
  message OutboxServiceInvocationResponse {
    InvocationId invocation_id = 1;
    uint32 entry_index = 2;
+    uint32 caller_invocation_epoch = 4;


nit: To make it explicit why I believe this ordering is sub-optimal is because now users need to scan the whole list of fields to figure out what's the current max tag number when adding a new field.

* Add current_invocation_epoch and completion_range_epoch_map, we need this state to support the algorithm described here restatedev#2890 (comment) . This two fields will be updated whenever a new trim happens. * Add propagation of invocation_epoch in all the relevant places. This also includes a little reshape of the InvocationResponse. To support serde front-back compat, we use a proxy struct as done for ServiceInvocation. Also includes a lil test for that. * Added the new fields support to partition-store * Add InvocationEpoch awareness tests. * Implement Invocation Epoch awareness and the fencing off algorithm. The core of the change is in InvocationStatusExt::should_accept_completion, everything else is wiring up and propagation of the InvocationEpoch * Make Invoker InvocationEpoch aware: * Both events from PP and from internal tasks now require the InvocationEpoch. All events except Invoke and Abort will be ignored when epoch doesn't match * Handle the cases where Abort/Invoke are out of order (as described here restatedev#2890 (comment)). Added few tests for that.

tillrohrmann

LGTM. +1 for merging :-)

tillrohrmann · 2025-04-29T15:15:30Z

crates/invoker-impl/src/state_machine_manager.rs

+            Some(f(tx, ism))
+        } else {
+            // If no state machine
+            trace!("No state machine found for selected server header");


What does server header relates to?

it's the header that tells us which SDK the user is using.

crates/storage-api/src/invocation_status_table/mod.rs

tillrohrmann · 2025-04-29T15:25:49Z

crates/worker/src/partition/state_machine/invocation_status_ext.rs

+        assert!(!is.should_accept_completion(0, 2));
+        // In all the other cases, I accept it.
+        assert!(is.should_accept_completion(0, 1));
+        assert!(is.should_accept_completion(1, 1));


What I don't fully understand is how completion id 1 will get the invocation epoch 1 given that the trim point is at completion id 2. But no need to change anything.

Indeed this specific case cannot happen.

slinkydeveloper force-pushed the invocation-epoch branch from e02a3dd to c46d1b9 Compare March 14, 2025 10:58

slinkydeveloper force-pushed the invocation-epoch branch 2 times, most recently from 4b6b994 to f6df49b Compare April 8, 2025 08:57

slinkydeveloper requested a review from tillrohrmann April 15, 2025 07:43

slinkydeveloper force-pushed the invocation-epoch branch 2 times, most recently from 7515ab7 to 52dfc8a Compare April 22, 2025 17:21

slinkydeveloper mentioned this pull request Apr 22, 2025

[OLD] Reset invocation #3186

Open

tillrohrmann requested changes Apr 23, 2025

View reviewed changes

slinkydeveloper requested a review from tillrohrmann April 24, 2025 07:54

tillrohrmann requested changes Apr 25, 2025

View reviewed changes

slinkydeveloper force-pushed the invocation-epoch branch from e047d8b to e47f9e1 Compare April 28, 2025 08:48

slinkydeveloper requested a review from tillrohrmann April 28, 2025 09:12

tillrohrmann approved these changes Apr 28, 2025

View reviewed changes

slinkydeveloper force-pushed the invocation-epoch branch 2 times, most recently from 36f7e11 to 2c31c3a Compare April 28, 2025 14:47

slinkydeveloper requested a review from tillrohrmann April 28, 2025 16:23

slinkydeveloper force-pushed the invocation-epoch branch from 2c31c3a to 4e70add Compare April 29, 2025 12:17

slinkydeveloper force-pushed the invocation-epoch branch from 4e70add to 75bb667 Compare April 29, 2025 12:53

tillrohrmann approved these changes Apr 29, 2025

View reviewed changes

Feedback

35318f0

slinkydeveloper merged commit f6cbbb2 into restatedev:main Apr 29, 2025
27 checks passed

slinkydeveloper deleted the invocation-epoch branch April 29, 2025 16:20

github-actions bot locked and limited conversation to collaborators Apr 29, 2025

Invocation epoch #2904

Invocation epoch #2904

Uh oh!

Conversation

slinkydeveloper commented Mar 13, 2025

Uh oh!

tillrohrmann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

slinkydeveloper commented Apr 23, 2025

Uh oh!

tillrohrmann commented Apr 23, 2025

Uh oh!

slinkydeveloper commented Apr 23, 2025

Uh oh!

tillrohrmann commented Apr 23, 2025

Uh oh!

tillrohrmann left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tillrohrmann Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

slinkydeveloper commented Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tillrohrmann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tillrohrmann Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tillrohrmann left a comment

Choose a reason for hiding this comment

Uh oh!

tillrohrmann Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

slinkydeveloper Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tillrohrmann Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

slinkydeveloper Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tillrohrmann left a comment •

edited

Loading

slinkydeveloper commented Apr 28, 2025 •

edited

Loading

tillrohrmann Apr 28, 2025 •

edited

Loading