Add support for aggregation fuzzer to call Presto java and setup CI run. #7947
Conversation
✅ Deploy Preview for meta-velox canceled.
|
facebook-github-bot
added
the
CLA Signed
| @@ -43,6 +44,11 @@ DEFINE_string( | |||
| "this comma separated list of function names " | |||
| "(e.g: --only \"min\" or --only \"sum,avg\")."); | |||
|
|
|||
| DEFINE_string( | |||
| prestoCoordinatorUri, | |||
There was a problem hiding this comment.
consistency: presto_url (let's not include coordinator as it makes it very long)
| DEFINE_string( | ||
| prestoCoordinatorUri, | ||
| "", | ||
| "Presto coordinator Uri, along with port. If this is set we"); |
There was a problem hiding this comment.
Unfinished sentence. Please, include an example.
| "kurtosis", | ||
| "entropy", | ||
| }); | ||
| std::unique_ptr<facebook::velox::exec::test::ReferenceQueryRunner> runner; |
There was a problem hiding this comment.
nit: perhaps, extract the logic of createing a query runner into a helper function
auto queryRunner = setupReferenceQueryRunner();
.github/workflows/experimental.yml
Outdated
| - name: Upload spark fuzzer | ||
| uses: actions/upload-artifact@v3 | ||
| with: | ||
| name: spark |
There was a problem hiding this comment.
Why do we need Spark here?
.github/workflows/experimental.yml
Outdated
| name: aggregation | ||
| path: velox/_build/debug/velox/exec/tests/velox_aggregation_fuzzer_test | ||
|
|
||
| - name: Upload join fuzzer |
There was a problem hiding this comment.
Why do we need join fuzzer? Let's remove unnecessary steps.
There was a problem hiding this comment.
nit: These arent steps perse and arent run, this can be invoked when we are running join fuzzer to upload the fuzzer artifacts. I added these because I think we will be using exprimental for future experimental flags for all these jobs. However I will remove it since consensus is to only add when being used.
There was a problem hiding this comment.
for future reference you can disable steps by adding if: false to it, useful for testing and such ^^
kgpai
force-pushed
the
agg_fuzzer_query_runner
branch
from
d8b4822
to
a6c872b
Compare
|
@kgpai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
| DEFINE_string( | ||
| presto_url, | ||
| "", | ||
| "Presto coordinator Uri, along with port. If this is set we use Presto " |
There was a problem hiding this comment.
nit: Presto coordinator URI along with port. If set, use Presto as source of truth. Otherwise, use DuckDB. Example: --presto_url=http://127.0.0.1:8080
| with: | ||
| name: aggregation | ||
|
|
||
| - name: "Run Aggregate Fuzzer" |
There was a problem hiding this comment.
nit: Run Aggregate Fuzzer using Presto as source of truth
Do you have a link to a sample job run? Curious if it succeeded or failed.
There was a problem hiding this comment.
I ran it locally - Will come up with a job run on CI shortly.
|
@kgpai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
kgpai
force-pushed
the
agg_fuzzer_query_runner
branch
from
de4f1b1
to
3a31722
Compare
|
@kgpai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
There was a problem hiding this comment.
LGTM (the build is failing at compile but I think that can be solved via rebase) When this has run for a bit and proven useful we can integrate it into scheduled.yml, if there is a reason for separate runs we could integrate it and just run at a different time.
…un. (facebookincubator#7947) Summary: This PR: 1. Sets up Aggregation Fuzzer to call Presto Java 2. Sets up an experimental scheduled run for the agg fuzzer. Reviewed By: mbasmanova Differential Revision: D52040998 Pulled By: kgpai
kgpai
force-pushed
the
agg_fuzzer_query_runner
branch
from
3a31722
to
2824b40
Compare
|
This pull request was exported from Phabricator. Differential Revision: D52040998 |
…un. (facebookincubator#7947) Summary: This PR: 1. Sets up Aggregation Fuzzer to call Presto Java 2. Sets up an experimental scheduled run for the agg fuzzer. Pull Request resolved: facebookincubator#7947 Reviewed By: mbasmanova Differential Revision: D52040998 Pulled By: kgpai fbshipit-source-id: 020282eef5f2c950c000cebed388fb160aba726b
This PR: