-
Notifications
You must be signed in to change notification settings - Fork 14
Route set/getFunctionResult
through the planner
#317
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6034109
to
1796a43
Compare
69daff7
to
546ba18
Compare
Codecov Report
@@ Coverage Diff @@
## main #317 +/- ##
==========================================
+ Coverage 82.42% 82.46% +0.04%
==========================================
Files 97 97
Lines 7822 7803 -19
==========================================
- Hits 6447 6435 -12
+ Misses 1375 1368 -7
|
546ba18
to
6b97e48
Compare
…easing a shared lock, and acquiring a full one
6b97e48
to
3b454e4
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In this PR we move the setting/getting of function results to the planner.
When setting a function (message) result, the local scheduler just sends a request to the planner with the message result. The planner then stores it.
When getting a function result, we query the planner for the message result. Two things could happen:
In order to avoid blocking the server threads in the planner, we wait on the client side (i.e. worker thread). In situation 2, i.e. if we have asked for a message result and the message is not there, the planner registers the host's interest in the message result, and the worker waits on a result promise. When the result is finally set, the planner will notifiy all hosts that have registered interest in that message result via a
FunctionCall
.In the process of changing this I also remove the asynchronous waiting implemented in the HTTP endpoint. The reason being twofold:
In addition, I also clean-up and simplify the base fixtures in
tests/util/fixtures.h
. I make sure that fixtures that just mimick one component don't unnecessarily inherit from other fixtures. This was creating friction with Faasm's tests, where these base fixtures are also used. This change creates a lot of noise in the diff, but doesn't change any functionality.