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

Fix ingress #1559

Merged
Merged

Conversation

slinkydeveloper
Copy link
Contributor

@slinkydeveloper slinkydeveloper commented May 28, 2024

This PR includes two big changes to the ingress:

  • Simplified the correlation of responses in the ingress dispatcher. Now the FSM is aware of the ingress request identifiers (that is, the physical request handlers waiting for responses). The correlation is done now purely on request id now, and not on invocation id/idempotency key/service id. This solves many bugs around wrong response correlation
  • Now /send with idempotency id or to workflow methods return 409 CONFLICT in case the request has been already started.

…rner cases where mixing requests/attach/send didn't work previously.
@slinkydeveloper slinkydeveloper changed the title Fix ingres Fix ingress May 28, 2024
…workflow id, to notify whether they've been already submitted or not.
Copy link
Contributor

@igalshilman igalshilman left a comment

Choose a reason for hiding this comment

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

Ok, thanks for the quick fix @slinkydeveloper.
Onces the e2e tests are passing I'm good to merge this.

@slinkydeveloper slinkydeveloper merged commit 98d71ca into restatedev:main May 28, 2024
4 checks passed
@slinkydeveloper slinkydeveloper deleted the issues/ingress-handler-id branch May 28, 2024 11:31
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.

2 participants