[FEAT] GHA workflow to perform tcph benchmarking #3184
Conversation
CodSpeed Performance ReportMerging #3184 will degrade performances by 34.59%Comparing Summary
Benchmarks breakdown
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3184 +/- ##
==========================================
- Coverage 76.54% 76.37% -0.17%
==========================================
Files 685 685
Lines 85269 85135 -134
==========================================
- Hits 65266 65020 -246
- Misses 20003 20115 +112 |
| with: | ||
| aws-region: us-west-2 | ||
| role-to-assume: ${{ secrets.ACTIONS_AWS_ROLE_ARN }} | ||
| role-session-name: daft-performance-comparisons |
There was a problem hiding this comment.
Why is this necessary -- Is this for S3 upload permissions?
There was a problem hiding this comment.
Yes, this was here for AWS S3 uploads. I believe writes to the bucket are private (whereas reads are public). May be wrong though.
raunakab
force-pushed
the
feat/infra
branch
5 times, most recently
from
0c8a35f to
a71c566
Compare
raunakab
changed the title
|
Example: Run by @desmondcheongzx. Run was submitted locally using the gh workflow run build-commit-run-tpch.yaml --ref $BRANCH_NAME -f skip_questions=$SKIP_QUESTIONS |
|
Tagging @colin-ho. You recently touched the |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
- slightly different than existing one
| - uv pip install pip ray[default] py-spy | ||
| # GitHub Actions workflow will replace all parameters between `<<...>>` with the | ||
| # actual values as determined dynamically during runtime of the actual workflow. | ||
| - uv pip install https://github-actions-artifacts-bucket.s3.us-west-2.amazonaws.com/builds/<<SHA>>/<<WHEEL>> |
There was a problem hiding this comment.
nit: I think {{SHA}} and {{WHEEL}} is more common here for templating
|
|
||
| for row in csv_reader: | ||
| row_str = make_md_row(row) | ||
| output.write(row_str) |
There was a problem hiding this comment.
Ok, but note that this was a nice-to-have that really should have been in a follow-on
| options: | ||
| - '2' | ||
| - '32' | ||
| - '32' |
There was a problem hiding this comment.
I think we should combine SF with partition size, since not all permutations have datasets?
I.e. SF10_part32, SF100_part512...
Or even simpler, just let people select the URL
There was a problem hiding this comment.
Ya, I was thinking of this as well.
Options could be:
2-210-32- etc.
| aws s3 sync \ | ||
| s3://eventual-dev-benchmarking-fixtures/uncompressed/tpch-dbgen/<<SCALE_FACTOR>>/<<PARTITION_SIZE>>/parquet/ \ | ||
| /tmp/data/<<SCALE_FACTOR>>/<<PARTITION_SIZE>>/parquet/ \ | ||
| --quiet |
There was a problem hiding this comment.
Is this intended behavior? Downloading 1TB of data is going to kill the machine. Are we benchmarking off s3 or local?
| type: string | ||
| description: The wheel artifact to use | ||
| required: false | ||
| default: getdaft-0.3.0.dev0-cp38-abi3-manylinux_2_31_x86_64.whl |
There was a problem hiding this comment.
What's this version of Daft and why is this not required?
There was a problem hiding this comment.
This is the name of the wheel which is inside of the directory in the github-actions-artifacts-bucket bucket. The name of the outputted wheel is determined via build job which then injects the name of the wheel to this step. If you want to run this step without having run the previous, you'll have to specify the name.
It's possible that we have an S3 List job to grab the name instead...
| aws-region: us-west-2 | ||
| role-session-name: run-tpch-workflow | ||
| - uses: ./.github/actions/install | ||
| - run: | |
There was a problem hiding this comment.
Split this into steps, this makes it difficult to debug if something goes wrong.
You probably also want the ray down step to "always run" regardless of success/failure of all the rest of the steps.
| --ray_job_dashboard_url http://localhost:8265 \ | ||
| --skip_warmup \ | ||
| --pickle_daft_module='false' | ||
| fi |
There was a problem hiding this comment.
Why not just leverage ray submit here?
| ray up .github/assets/benchmarking_ray_config.yaml -y | ||
| HEAD_NODE_IP=$(ray get-head-ip .github/assets/benchmarking_ray_config.yaml | tail -n 1) | ||
| ssh -o StrictHostKeyChecking=no -fN -L 8265:localhost:8265 -i ~/.ssh/ci-github-actions-ray-cluster-key.pem ubuntu@$HEAD_NODE_IP | ||
| export DAFT_ENABLE_RAY_TRACING=1 |
There was a problem hiding this comment.
You might want to make this configurable and default to true
| new_base=${base//:/_} | ||
| mv "$filepath" "$dir/$new_base" | ||
| done | ||
| ' _ {} + |
There was a problem hiding this comment.
This converts all of the instances of : in path names to _. Having : in path names will make the GitHub Actions upload-artifacts step fail.
|
Closing in favour of #3404. |
Overview
Create new GHA workflow for building a commit and running tpch against it.
Notes
There are 2 main workflows:
build-commit.yamlrun-tpch.yamlThe final workflow,
build-commit-run-tpch.yamljust runs the above two in a sequential order.I've also made some changes to
benchmarking/tpch/__main__.py. Namely:DAFTto the ray-runtime-env variables that's sent during ray-cluster initialization.daftmodule to ray-cluster during initialization.I've summarized the workflows individually down below:
build-commit workflow
run-tpch workflow
benchmarking.tpchbenchmarkbuild-commit-run-tpch workflow
build-commitjob (1st one) to the input of therun-tpchjob (2nd one)