-
Notifications
You must be signed in to change notification settings - Fork 67
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
Building support for local deployment #142
base: master
Are you sure you want to change the base?
Conversation
sebs/experiments/perf_cost.py
Outdated
@@ -46,21 +46,22 @@ def prepare(self, sebs_client: "SeBS", deployment_client: FaaSSystem): | |||
self._benchmark = sebs_client.get_benchmark( | |||
settings["benchmark"], deployment_client, self.config | |||
) | |||
self._function = deployment_client.get_function(self._benchmark) | |||
self._functions = deployment_client.get_function(self._benchmark, 3) |
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.
The allocation of multiple function instances should happen in the deployment.
For example, AWS/GCP/Azure will create instances for us automatically. In Local, you need to allocate them as requested by spawning more Docker containers hosting the function.
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.
Furthermore, the number 3 should not be hardcoded anywhere.
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.
Sorry I had done this for testing purpose but missed to remove it before pushing. I will remove it.
sebs/experiments/perf_cost.py
Outdated
# prepare benchmark input | ||
self._storage = deployment_client.get_storage(replace_existing=self.config.update_storage) | ||
self._benchmark_input = self._benchmark.prepare_input( | ||
storage=self._storage, size=settings["input-size"] | ||
) | ||
for i in range(len(self._functions)): |
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.
As mentioned above - we should have one function instance, and the Local instance and its triggers will allocate many Docker containers.
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.
I imagine this seems more complex than anticipated since cloud platforms expose an HTTP trigger, and the local deployment would have a different HTTP address for each instance. We don't want to have to change every experiment - we want to hide this complexity behind the trigger.
The simplest solution would be to implement a new sebs.Local.ScalableHTTPTrigger
(or something similar) that would allocate/deallocate container instances, and redirect invocations to the proper HTTPTrigger.
sebs/faas/system.py
Outdated
@@ -115,7 +115,7 @@ def package_code( | |||
pass | |||
|
|||
@abstractmethod | |||
def create_function(self, code_package: Benchmark, func_name: str) -> Function: | |||
def create_function(self, code_package: Benchmark, func_name: str, num: int) -> Function: |
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.
We shouldn't change this API
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.
I had added the num
parameter to know how many containers to dispatch.
sebs/local/function.py
Outdated
@@ -99,6 +99,7 @@ def deserialize(cached_config: dict) -> "LocalFunction": | |||
) | |||
except docker.errors.NotFound: | |||
raise RuntimeError(f"Cached container {instance_id} not available anymore!") | |||
# clear cache |
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.
FIXME?
@mcopik This is the PR related to issue #119 . The perf-cost experiment gives the following output now:
It continues to loop like this.