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

ci: tpcds benchmarking #3597

Closed
wants to merge 9 commits into from
Closed

ci: tpcds benchmarking #3597

wants to merge 9 commits into from

Conversation

raunakab
Copy link
Contributor

Overview

This PR introduces a new script called tpcds.py which runs the build-commit workflow (only if a pre-built wheel does not exist), and then runs the run-cluster workflow on the built wheel.

Note

The build-commit.yaml workflow has been changed to not perform any checking if python wheel exists; that logic has been moved into the tpcds.py script.

Usage

uv run tools/tpcds.py --ref $MY_BRANCH --force

@raunakab raunakab requested a review from jaychia December 18, 2024 03:53
@github-actions github-actions bot added the ci label Dec 18, 2024
Copy link

codspeed-hq bot commented Dec 18, 2024

CodSpeed Performance Report

Merging #3597 will improve performances by 42.54%

Comparing ci/tpcds (9fba5d2) with main (6602502)

Summary

⚡ 1 improvements
✅ 26 untouched benchmarks

Benchmarks breakdown

Benchmark main ci/tpcds Change
test_iter_rows_first_row[100 Small Files] 236.5 ms 165.9 ms +42.54%

Copy link

codecov bot commented Dec 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.93%. Comparing base (cc5ad00) to head (9fba5d2).
Report is 18 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3597      +/-   ##
==========================================
+ Coverage   77.85%   77.93%   +0.07%     
==========================================
  Files         719      720       +1     
  Lines       88455    88480      +25     
==========================================
+ Hits        68871    68960      +89     
+ Misses      19584    19520      -64     

see 29 files with indirect coverage changes

Copy link
Contributor

@jaychia jaychia left a comment

Choose a reason for hiding this comment

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

Logic for checking if the wheel exists/triggering the workflow should not be in TPC-DS workflow.

It's re-usable for other jobs as well (I'd want to use it for my jobs). Likely belongs in the run-cluster workflow, or a wrapper of that which runs one after the other.

@raunakab raunakab marked this pull request as ready for review December 19, 2024 18:16
@raunakab
Copy link
Contributor Author

Closing in favour of #3619.

@raunakab raunakab closed this Dec 19, 2024
@raunakab raunakab deleted the ci/tpcds branch December 19, 2024 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants