[GLUTEN-4827][UT] Add Golden Files for TPC-H Spark34 + Gluten Execution Plan #4828
Conversation
|
Run Gluten Clickhouse CI |
1 similar comment
|
Run Gluten Clickhouse CI |
|
Run Gluten Clickhouse CI |
|
Run Gluten Clickhouse CI |
.github/workflows/velox_be.yml
Outdated
| - name: Clean temp golden files | ||
| if: failure() | ||
| run: | | ||
| rm -rf /tmp/$GITHUB_RUN_ID/spark34/tpch-approved-plan |
There was a problem hiding this comment.
Can this be moved to the above "Upload golden files" GHA task ? If feasible, maybe we can unify them (the above two or three tasks) into one task.
There was a problem hiding this comment.
I got your point, i will go futher to find how to resolve this.
|
Run Gluten Clickhouse CI |
.github/workflows/velox_be.yml
Outdated
| runs-on: velox-self-hosted | ||
| needs: [ubuntu2004-test-spark32, ubuntu2004-test-spark33, ubuntu2004-test-spark34] | ||
| steps: | ||
| - name: Upload golden files |
There was a problem hiding this comment.
Right, we should trigger this upload job when the previous job fail.
And according to Github Action Doc, we should add this condition on job instead of adding on each step.
|
Run Gluten Clickhouse CI |
1 similar comment
|
Run Gluten Clickhouse CI |
|
Run Gluten Clickhouse CI |
7 similar comments
|
Run Gluten Clickhouse CI |
|
Run Gluten Clickhouse CI |
|
Run Gluten Clickhouse CI |
|
Run Gluten Clickhouse CI |
|
Run Gluten Clickhouse CI |
|
Run Gluten Clickhouse CI |
|
Run Gluten Clickhouse CI |
|
@zwangsheng is there any block on this pr ? |
I'm still testing merge upload step. But IMO, we can leave merge step job in the following PR. WDYT @ulysses-you @PHILO-HE |
OK to me to separate the work into two or more PRs. |
|
I'm fine to seperate pr. @PHILO-HE IIUC the current action should work, just some code cleanup leave to another pr. |
|
Run Gluten Clickhouse CI |
|
Thanks for both @ulysses-you @PHILO-HE, i will revert this commit to focus on Spark 34 Golden Files, after some test, will turn this PR ready. |
|
Run Gluten Clickhouse CI |
.github/workflows/velox_be.yml
Outdated
| @@ -670,3 +673,12 @@ jobs: | |||
| if: ${{ always() }} | |||
| run: | | |||
| $PATH_TO_GLUTEN_TE/$OS_IMAGE_NAME/gha/gha-checkout/clean.sh | |||
|
|
|||
| clean-up-tmp: | |||
There was a problem hiding this comment.
@zwangsheng It seems a bit overkill to add a common job to do cleanup. The github action become complex.. I prefer to revert this change.
There was a problem hiding this comment.
IMO, it is acceptable to add a new job to do the cleanup, especially since we will later incorporate the upload operations into this job.
There was a problem hiding this comment.
I think we should merge upload and cleanup into one job and it seems unnecessary to pull out upload to a new job. There exists many similar jobs in gluten CI, if we want to make it clear, we should consider other jobs.
|
Run Gluten Clickhouse CI |
|
===== Performance report for TPCH SF2000 with Velox backend, for reference only ====
|
What changes were proposed in this pull request?
Close #4827
As title, follow up to add TPC-H + Spark33 Golden Files.
How was this patch tested?
unit tests