Conversation
patdevinwilson
commented
Oct 29, 2025
- Create comprehensive README for presto/ directory
- Include links to main repo README and NVIDIA developer blog
- Document current testing status (TPC-H as primary benchmark)
- Provide quick start guide and setup instructions
- Add sections on testing, benchmarking, and configuration
- Create comprehensive README for presto/ directory - Include links to main repo README and NVIDIA developer blog - Document current testing status (TPC-H as primary benchmark) - Provide quick start guide and setup instructions - Add sections on testing, benchmarking, and configuration
simoneves
left a comment
There was a problem hiding this comment.
I think this is an excellent first bash. Some bits definitely need emphasizing and de-emphasizing.
presto/README.md
Outdated
| ├─ velox | ||
| ``` | ||
|
|
||
| All three repositories must be checked out as sibling directories. |
There was a problem hiding this comment.
More info about the actual GH repos to get these from?
There was a problem hiding this comment.
Something about versions and compatibility? A public equivalent of the Canvas. Not sure how to express that at this time, though.
presto/README.md
Outdated
| ```bash | ||
| ./build_centos_deps_image.sh | ||
| # OR fetch a pre-built image (requires credentials) | ||
| ./fetch_centos_deps_image.sh |
There was a problem hiding this comment.
Probably best to bury this even more, as third-party users won't be able to do a fetch yet.
presto/README.md
Outdated
|
|
||
| ```bash | ||
| cd velox-testing/presto/scripts | ||
| ./generate_presto_config.sh |
There was a problem hiding this comment.
Not sure we need to document this specifically, as it runs automatically now, even more so after #109 lands.
paul-aiyedun
left a comment
There was a problem hiding this comment.
Changes overall look good to me. However, I had a couple of corrections and suggested update for the section about testing different scale factors.
presto/README.md
Outdated
| ```bash | ||
| cd velox-testing/presto/scripts | ||
| ./run_integ_test.sh --help # See all options | ||
| ./run_integ_test.sh --test-suite tpch |
There was a problem hiding this comment.
This should be --benchmark-type (
--test-suite.
presto/README.md
Outdated
| 4. Run benchmarks: | ||
| ```bash | ||
| ./run_benchmark.sh --help # See all options | ||
| ./run_benchmark.sh --benchmark tpch |
There was a problem hiding this comment.
--schema-name has to be set when executing this script.
|
|
||
| Use the provided scripts to generate data at various scale factors or set up tables on existing data. | ||
|
|
||
| ## Testing Different Scale Factors |
There was a problem hiding this comment.
Using the setup_benchmark_data_and_tables.sh or setup_benchmark_tables.sh script and then executing the run_integ_test.sh script with the --schema-name option is another way of doing this and may be preferred in some cases, since it allows switching between different scale factors without having to regenerate data.
Updated README to improve clarity and structure, including repository structure, testing instructions, and configuration management.