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: handle async call not being able to be called #1663

Merged
merged 1 commit into from
Jun 5, 2024

Conversation

matt2e
Copy link
Collaborator

@matt2e matt2e commented Jun 5, 2024

Fixes case where async call's early failure skipped clean up and retries.

Issue steps:

  • Deploy a module
    • Deployment and related rows get added to db, but no runner has been assigned yet
  • Add an async call that calls that module
    • A controller picks up the async call and attempts it by calling callWithRequest(...)
    • This gets the schema by calling getActiveSchema which gets the schema filtering by minReplicas > 0 and r.assigned = 'assigned' (link)
    • callWithRequest(...) fails because this new module is not in the fetched schema
    • executeAsyncCalls(...) return early due to this error, not updating the state of the async call
      • the lease gets released rather than timing out (due to the defer in executeAsyncCalls(...))
      • we end up with an async call with state = executing and no lease

So this async call gets stuck forever. For pubsub this means the subscription stops forever.

@matt2e matt2e requested a review from alecthomas as a code owner June 5, 2024 06:54
@matt2e matt2e requested review from a team and deniseli and removed request for a team June 5, 2024 06:54
@ftl-robot ftl-robot mentioned this pull request Jun 5, 2024
@matt2e matt2e force-pushed the matt2e/async-call-dead-end branch from ab0a946 to ff51164 Compare June 5, 2024 23:52
@matt2e matt2e merged commit a26e03e into main Jun 5, 2024
33 checks passed
@matt2e matt2e deleted the matt2e/async-call-dead-end branch June 5, 2024 23:58
@deniseli deniseli added the approved Marks an already closed PR as approved label Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Marks an already closed PR as approved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants