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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix: event multi-execution #3696
Conversation
Why is this even causing the issue? shouldn't it be handled by our default error handler for workers? |
@eldadfux The problem is that if an event comes into a function worker, and there are multiple functions that need to be executed for this event... Well, if the first execution fails, then no other will run. It could fail, for instance, when a function does not have deployment yet. Sooo yes, the worker will handle the error and work on the next task in the queue. But in one task, it was supposed to execute 5 functions, but it only executed one and stopped due to an exception. Function worker receives info that "this event occurred", and it then (as a part of one task) executes multiple functions. So one execution request failing means all the others are skipped. Does it make sense? |
Do we have 5 tasks in a single task because or our new event model? Have we considered pushing 5 tasks for 5 events instead of running 5 events in one task? What is the max amount of events we can have in the same task? |
@eldadfux, when an event comes in for the function worker to process, it loops through each function to check execute the event. If one function throws an exception, the for loop fails to finish iterating over all functions. appwrite/app/workers/functions.php Lines 70 to 88 in e658457
And it's possible for the appwrite/app/workers/functions.php Line 208 in e658457
|
@eldadfux Regarding maximum executions under one event, that is limited as far as I am concerned. For instance, document create: There are 3 dynamic parts that can either be specific ID or wildcard. That makes a combination of 8 options if I am not mistaken. For 4 dynamic parts it would be 16, for 5 it would be 32. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's change the target branch to master
What does this PR do?
With this PR, if multiple functions execute with an event, they will all attempt to be executed instead of stopping when one execution fails. Use case explained well in the issue linked below.
I believe this is the correct behavior, as we don't have a function chaining feature in Appwrite.
Test Plan
Related PRs and Issues
#3666
Have you read the Contributing Guidelines on issues?
Yes 馃槆