feat(presto/bench): config buffer tuning and benchmark robustness fixes - #390
Open
misiugodfrey wants to merge 4 commits into
Open
feat(presto/bench): config buffer tuning and benchmark robustness fixes#390misiugodfrey wants to merge 4 commits into
misiugodfrey wants to merge 4 commits into
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
misiugodfrey
force-pushed
the
misiug/bench-fixes-tuning
branch
from
July 27, 2026 22:05
4d119ee to
4fd0ca2
Compare
Based on kjmph's bench33 POC patch. - run_benchmark.sh: add --maxfail=1 when --profile is set; add PRESTO_BENCHMARK_KEEP_VENV escape hatch to skip venv teardown - common_functions.sh: replace jq-based worker registration wait with python3 JSON parse; fix while-loop so it actually retries correctly - common_fixtures.py: fix error handling for non-Presto exceptions in failed_queries_dict; move REST metrics collection outside profiler interval; add PRESTO_SESSION_PROPERTIES env var support - hive.properties: comment out SOFT_AFFINITY in base template; keep in CPU-specific override only - Config overrides: exchange.max-response-size=64MB for CPU coordinator; lower buffer defaults to 32MB for CPU workers; add local-exchange and largest-size-class-pages knobs; tune GPU exchange/sink buffer sizes - .gitignore: add result_dir_*/ and latest_result_dir.txt
misiugodfrey
force-pushed
the
misiug/bench-fixes-tuning
branch
from
July 28, 2026 16:03
4fd0ca2 to
47b0a20
Compare
_session_properties_from_env() calls os.environ.get() but os was not imported, causing NameError on every presto_cursor fixture initialization.
The native worker rejects DataSize suffixes (MB) for this property; express 512MB as raw bytes instead.
…iner The container root is read-only squashfs so /root/.libcudf is not writable, causing libcudf JIT/RTC cache creation to fail. Bind-mount a per-worker host directory to provide the writable path.
misiugodfrey
commented
Jul 29, 2026
|
|
||
| # CPU UCX row pages and local exchange were tuned together. The generator | ||
| # reapplies these as overridable CPU_* values on every run. | ||
| local-exchange.max-buffer-size=536870912 |
Contributor
Author
There was a problem hiding this comment.
Note that these values can't have units associated with them (MB for instance), so we need to leave this as a byte value.
| while true; do | ||
| if curl -s -f -o node_response.json "${COORDINATOR_URL}/v1/node"; then | ||
| node_count=$(python3 -c \ | ||
| 'import json, sys; print(len(json.load(open(sys.argv[1]))))' \ |
Contributor
Author
There was a problem hiding this comment.
replaced jq with python parsing since jq is not installed in the cluster/coordinator image by default.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Low-risk correctness fixes and config tuning for Presto native benchmarks. Good first merge of this series.
Based on kjmph's bench33 POC patch.
--maxfail=1when--profileis set (avoids waiting for all queries when one fails during profiling runs); addPRESTO_BENCHMARK_KEEP_VENVescape hatchjq-basedwait_for_worker_node_registrationwithpython3JSON parse; fix broken while-loop that was not actually retrying correctlyfailed_queries_dict; move REST metrics collection outside the profiler interval so profiling overhead doesn't skew results; addPRESTO_SESSION_PROPERTIESenv var for per-run session overridesSOFT_AFFINITYin base template; keep it in the CPU-specific override (overrides/cpu/) only — GPU experiments should opt in explicitlyexchange.max-response-size=64MBfor CPU coordinator; lower CPU worker buffer defaults to 32MB; addlocal-exchange.max-buffer-sizeandlargest-size-class-pages; tune GPU exchange/sink buffer sizesresult_dir_*/andlatest_result_dir.txt(introduced by PR Land @bdice prototype workflows in main for testing #3 of this series)Dependencies
None. This PR is self-contained and should merge first.
Verification
result_dir_<jobid>/andlatest_result_dir.txtis written--profile; verify benchmark aborts after the first query failure instead of continuing through all 22 queriespresto/scripts/run_benchmark.shand confirm thePRESTO_BENCHMARK_KEEP_VENV=1escape hatch skips venv teardownwait_for_worker_node_registrationretries correctly when workers are slow to register (previously the loop exited immediately on the first iteration)common_fixtures.pymetrics are collected after the profiler window closes (check thatQueryStatsREST calls don't appear inside the nsys/perf capture interval in logs)./ci/check_style.sh(ruff + codespell) on changed Python files