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

Tweaked vSwarm deployer #604

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Tweaked vSwarm deployer #604

wants to merge 15 commits into from

Conversation

aryans1204
Copy link
Contributor

@aryans1204 aryans1204 commented Feb 10, 2025

Summary

A small summary of the requirements (in one/two sentences).

Implementation Notes ⚒️

  • Briefly outline the overall technical solution. If necessary, identify talking points where the reviewer's attention should be drawn to.
    Goal of this PR is to add functionality in the loader to be able to execute vSwarm functions from a mapper_output.json file.

The added functionality pertains to:

  1. Added parsing capabilities under pkg/trace/mapper_trace_parser.go to parse mapper_output.json files and extract vSwarm functions.
  2. Added deployment capability under pkg/driver/deployment/knative.go to correctly deploy vSwarm functions, and a yamls.tar.gz tarball that contains the YAML files for all the vSwarm functions. Documentation about this tarball is embedded inside the tarball.

This extends upon the work done previously on the invoker, to be able to execute vSwarm functions. With the merge of this PR, vSwarm functions will be able to be executed from the loader, by specifying config options.

Added parsing, and deployment functionality to run vSwarm functions using Invitro loader, based on mapper output.

External Dependencies 🍀

Breaking API Changes ⚠️

Simply specify none (N/A) if not applicable.

@aryans1204 aryans1204 requested review from leokondrashov and cvetkovic and removed request for leokondrashov February 10, 2025 11:57
Copy link
Contributor

@leokondrashov leokondrashov left a comment

Choose a reason for hiding this comment

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

Please explain in details why and what are you doing in the PR description. The template is there for a reason.

I don't see any documentation in the changes as well. There should be something about the feature, file formats (you are introducing several).

@aryans1204 aryans1204 requested a review from cvetkovic February 12, 2025 13:57
cvetkovic
cvetkovic previously approved these changes Feb 12, 2025
Copy link
Contributor

@leokondrashov leokondrashov left a comment

Choose a reason for hiding this comment

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

Please add docs on all files that you add as inputs to the execution. I have no idea how to get deploy_info and deployment yamls for a new set of benchmarks. What are the requirements for those benchmarks (use the relay from vSwarm, at least). Where is each file should be placed to be properly found by the loader.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain how I should create this file? Does something generate it? Because if my memory doesn't lie to me, previously, there were redeployment commands, not paths. Is there anything else that depends on the file formatted that way?

Copy link
Contributor Author

@aryans1204 aryans1204 Feb 17, 2025

Choose a reason for hiding this comment

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

So the deploy_info.json file exists inside the yamls.tar.gz tarball, where the file is generated using the generate_deploy_info.py file, also inside the tarball. Further, the docs to generate deploy info and its purpose are also inside the tarball. However, in order to generate the YAMLs, I just copied over the YAMLs from vSwarm into here, with some minor changes like changing the service name to the environment variable FUNC_NAME etc. In the future if someone needs to execute new benchmarks, they will have to generate the YAMLs and their locations by themself. I can add documentation for this in the tarball as well. Since the scope of this PR is to support vSwarm functions to be executed from a given mapper_output file, using the invitro loader, and not to generically execute any benchmark.

Copy link
Contributor

Choose a reason for hiding this comment

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

The docs inside the tarball is an extremely bad idea. Don't put it inside.

I want to see an explanation of each field used by the loader and how it affects the end result. Since you introduce a new deployment procedure, you should explain it here. If you have the script for that, explain it as well. If you expect someone else to do something, explain what needs to be done and what the requirements are for the outcome of these actions. With these docs, it should be possible to use the loader to run new benchmarks without looking into the code. It indeed doesn't need to explain all of the steps, but the user interface (file formats, effects of each field, etc)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, there are docs under docs/loader.md which explain all the steps required for a user to execute vSwarm benchmarks, ie. untar the YAML tarball, and then the execution command for executing under vSwarm mode. These docs are present under the Executing vSwarm functions and Single load generation subheadings of the loader docs. Please let me know whether they are adequate or some aspects are still unclear to the user about vSwarm based execution. As for the fields added, in terms of the experiment config, there is only one flag, ie. the vSwarm flag which is needed to turn on vSwarm execution. This documentation for the field is also added under docs/configuration.md. As for the deploy_info docs, I will move that doc from the tarball to the docs/ subdir. In terms of the changes to deployment procedure, there is an addition of a new script for deploying the databases for some vSwarm benchmarks. What would be a good way to create documentation for this? Under loader.md or a separate file under docs/?

Copy link
Contributor

@JooyoungPark73 JooyoungPark73 Feb 20, 2025

Choose a reason for hiding this comment

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

I think docs/generate_deploy_info_docs.md is okay. Please add schema and description of deploy_info.json

Also, pull all the Python scripts out of tarball and move to tools/mapper or sth, as it has to be transparent and can be viewed from Github webpage. Also, your tarball includes json file, so rename tarball to something more general.

Signed-off-by: aryans1204 <arshar1204@gmail.com>
Signed-off-by: aryans1204 <arshar1204@gmail.com>
Signed-off-by: aryans1204 <arshar1204@gmail.com>
Signed-off-by: aryans1204 <arshar1204@gmail.com>

Added tests to CI and formatted code

Signed-off-by: aryans1204 <arshar1204@gmail.com>

Fixing the E2E test for vSwarm loader

Signed-off-by: aryans1204 <arshar1204@gmail.com>

Extracted vSwarm mock server from standard workload

Signed-off-by: aryans1204 <arshar1204@gmail.com>
Signed-off-by: aryans1204 <arshar1204@gmail.com>
Signed-off-by: aryans1204 <arshar1204@gmail.com>
Signed-off-by: aryans1204 <arshar1204@gmail.com>
Signed-off-by: aryans1204 <arshar1204@gmail.com>
Signed-off-by: aryans1204 <arshar1204@gmail.com>
Signed-off-by: aryans1204 <arshar1204@gmail.com>
Signed-off-by: aryans1204 <arshar1204@gmail.com>
Signed-off-by: aryans1204 <arshar1204@gmail.com>
Signed-off-by: aryans1204 <arshar1204@gmail.com>
Signed-off-by: aryans1204 <arshar1204@gmail.com>
Signed-off-by: aryans1204 <arshar1204@gmail.com>
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.

4 participants