Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Record service protocol version and send to service endpoint #1517

Merged
merged 5 commits into from
May 19, 2024

Conversation

tillrohrmann
Copy link
Contributor

This commit introduces the selection of a service protocol version when starting
the InvocationTask. The service protocol version is sent to the service endpoint
via the content type and accept headers in the form of vnd.restate.invocation.vX.
Additionally, we record the chosen service protocol version in the journal metadata
for sanity checks. Note, currently we don't support resuming an invocation that
was started on an older protocol version with a newer protocol version.

This fixes #1510.

This PR is based on #1508.

Copy link
Contributor

@slinkydeveloper slinkydeveloper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good, i left a couple of comments. I think you optimized too much the changes for the invoker, while at the moment, at least, we only have one version and we don't really know what will be the changes we'll need in future. It might be that for many next protocol versions we'll simply just add entries, and none of those require any code reorganization in the invoker.

@tillrohrmann tillrohrmann force-pushed the issues/1510 branch 2 times, most recently from ba0b916 to a526bfd Compare May 16, 2024 12:17
@tillrohrmann
Copy link
Contributor Author

Thanks for the review @slinkydeveloper. I've addressed your comments and rebased onto the latest master. PTAL.

Verified

This commit was signed with the committer’s verified signature.
tillrohrmann Till Rohrmann
git-subtree-dir: crates/types/service-protocol
git-subtree-split: 1898426fc98c16d704068594cd54394912845ff7

Verified

This commit was signed with the committer’s verified signature.
tillrohrmann Till Rohrmann
Moving the restate-service-protocol types to restate-types makes
it simpler to share them across different crates.

Verified

This commit was signed with the committer’s verified signature.
tillrohrmann Till Rohrmann
This commit introduces the selection of a service protocol version when starting
the InvocationTask. The service protocol version is sent to the service endpoint
via the content type and accept headers in the form of vnd.restate.invocation.vX.
Additionally, we record the chosen service protocol version in the journal metadata
for sanity checks. Note, currently we don't support resuming an invocation that
was started on an older protocol version with a newer protocol version.

This fixes restatedev#1510.
@tillrohrmann
Copy link
Contributor Author

Rebased on the latest master. Merging once ci (modulo e2e tests) give green light.

@tillrohrmann tillrohrmann merged commit 964c375 into restatedev:main May 19, 2024
5 of 6 checks passed
@tillrohrmann tillrohrmann deleted the issues/1510 branch May 19, 2024 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Choose max supported protocol version in invoker and record it in journal metadata on new invocations
2 participants