feat: continuous s3 benchmarking #2355
Merged
Conversation
This file contains 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
danking
force-pushed
the
dk/continuous-s3-benchmarking
branch
from
86f3073 to
677f504
Compare
Benchmarks: TPC-H on S3Table of Results
|
danking
force-pushed
the
dk/continuous-s3-benchmarking
branch
from
085546c to
b5ff4a2
Compare
Deploying vortex-bench with
|
| Latest commit: |
37c740d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4822acd2.vortex-bench.pages.dev |
| Branch Preview URL: | https://dk-continuous-s3-benchmarkin.vortex-bench.pages.dev |
danking
force-pushed
the
dk/continuous-s3-benchmarking
branch
from
b5ff4a2 to
36aa83a
Compare
danking
commented
Feb 13, 2025
.github/workflows/bench-pr.yml
Outdated
| run: | | ||
| cargo run --bin tpch_benchmark --release -- \ | ||
| --use-remote-data-dir s3://vortex-bench-dev/tpch-sf1/ \ | ||
| --exclude-queries 15 \ |
There was a problem hiding this comment.
Query 15 basically has a mismatched row count (0 instead of the correct 1) on Parquet in S3.
danking
commented
Feb 13, 2025
| @@ -241,6 +253,7 @@ async fn bench_main( | |||
| .zip_eq(EXPECTED_ROW_COUNTS) | |||
| .enumerate() | |||
| .filter(|(idx, _)| queries.as_ref().map(|q| q.contains(idx)).unwrap_or(true)) | |||
| .filter(|(idx, _)| exclude_queries.as_ref().map(|excluded| !excluded.contains(idx)).unwrap_or(true)) | |||
There was a problem hiding this comment.
This has always been missing. If you exclude a query its row count is zero which fails the check unless the expected row count is zero.
danking
commented
Feb 13, 2025
benchmarks-website/code.js
Outdated
|
|
||
| "vortex-file-compressed": '#23d100', | ||
| "vortex-file-compressed (nvme)": '#23d100', | ||
| "vortex-file-compressed (s3)": '#7ad169', |
There was a problem hiding this comment.
The S3 colors are less saturated versions of the NVME colors.
Benchmarks: TPC-H on NVMETable of Results
|
Benchmarks: random_access |
Benchmarks: Clickbench on NVMETable of Results
|
Benchmarks: compressTable of Results
|
The TPC-H benchmark JSONs now include a "storage" field which is either "s3", "nvme", or not present. When we show PR comments, we join on the field so that we don't compare s3 to nvme. When we show benchmarks in plots, we split nvme and s3 into distinct series.
danking
force-pushed
the
dk/continuous-s3-benchmarking
branch
from
2ad6d8c to
78204e9
Compare
|
@robert3005 I think you were the one who asked to be tagged. |
AdamGS
reviewed
Feb 17, 2025
.github/workflows/bench-pr.yml
Outdated
| run: | | ||
| echo "TMPDIR=/work" >> $GITHUB_ENV | ||
|
|
||
| - name: Run TPC-H benchmark |
There was a problem hiding this comment.
this moved to a different file
AdamGS
reviewed
Feb 17, 2025
Comment on lines
212
to
218
| let storage = match url.scheme() { | ||
| "s3" => "s3", | ||
| "gcs" => "gcs", | ||
| "file" => "nvme", | ||
| otherwise => { | ||
| println!("unknown URL scheme: {}", otherwise); | ||
| return ExitCode::FAILURE; |
There was a problem hiding this comment.
nit - there must be a nicer way, maybe supported @ "s3" | "gcs" | "file" => support.to_owned() ...
There was a problem hiding this comment.
I want file to become nvme though :/
robert3005
reviewed
Feb 17, 2025
.github/workflows/bench-pr.yml
Outdated
| @@ -218,7 +156,7 @@ jobs: | |||
| run: | | |||
| echo "TMPDIR=/work" >> $GITHUB_ENV | |||
|
|
|||
| - name: Run TPC-H benchmark | |||
| - name: Run benchmark | |||
There was a problem hiding this comment.
you can add ${{ matrix.benchmark.id }} here
|
I made it so. 
|
danking
added a commit
that referenced
this pull request
Feb 18, 2025
This slipped in in my continuous S3 benchmarks PR #2355.
danking
added a commit
that referenced
this pull request
Feb 18, 2025
This slipped in in my continuous S3 benchmarks PR #2355.
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.
No description provided.