Implement /restate/invocation/{id}/attach and /restate/invocation/{id}/output
Fix #1495 and #1494
The important bits of this PR are the new Commands in the wal protocol, and the changes to the ingress dispatcher.
Tested with https://github.com/restatedev/e2e/pull/334 locally and it works
@tillrohrmann this is ready for another implementation. To make the rebase simpler, i had to squash everything together, i'm sorry.
Among the changes I've made:
- Feedback from your previous review
- Use a direct storage reader for the get output feature
- Added /workflow/../attach and /workflow/../output (we need these for the workflow sdk clients)
- Send attach notification for workflow send calls
Why is it not possible to model the workflow calls as idempotent calls where the idempotency key is derived from the workflow name and key? If it were possible, then we might be able to simplify some special cases in the partition processor state machine.
Because:
- the idempotency key really doesn't make sense in the workflow call case, i would find it surprising to see it in that table.
- the workflow calls do "other" special things, e.g. on cleanup they clean state and promises.
I also don't want to over-generalize now, because I think we're not completely sure about the workflow semantics, and keeping it specialized it's true it's not elegant and adds special cases, but makes it easier to tune for now.
Giving the ingress direct access to the PartitionStoreManager is not ideal since we want to make the ingress independent of the worker (sooner than later). Instead, it would be better to use the network to let the ingress communicate with the worker to retrieve the output results.
Agree, but this can be hidden behind that "reader" abstraction. If that's ok for you, i would like to go ahead with the current implementation and iterate on that later.
Because:
- the idempotency key really doesn't make sense in the workflow call case, i would find it surprising to see it in that table.
- the workflow calls do "other" special things, e.g. on cleanup they clean state and promises.
I also don't want to over-generalize now, because I think we're not completely sure about the workflow semantics, and keeping it specialized it's true it's not elegant and adds special cases, but makes it easier to tune for now.
Thanks for the clarification. The different behavior wrt to state makes sense.
Agree, but this can be hidden behind that "reader" abstraction. If that's ok for you, i would like to go ahead with the current implementation and iterate on that later.
Alrighty. Let's tackle this step after the release.