feat(presto/bench): NUMA-aware Presto worker config generation - #391
Draft
misiugodfrey wants to merge 4 commits into
Draft
feat(presto/bench): NUMA-aware Presto worker config generation#391misiugodfrey 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. |
Based on kjmph's bench33 POC patch. Rewrites generate_presto_config.sh to discover host NUMA topology and size each worker to its share of the NUMA node(s) it's bound to, rather than dividing the full host uniformly. - discover_cpu_numa_topology / cpu_numa_node_for_worker helpers in common_functions.sh using lscpu, lsmem, and /proc/meminfo fallbacks - CPU_WORKERS_PER_HOST controls how many workers to spread per host - Per-worker CPU thread and RAM limits derived from the smallest NUMA node share assigned anywhere on the host (homogeneous sizing) - CPU_NUMA_WORKER_LAYOUT exported for downstream consumers - set_or_append_property helper for idempotent config file mutations - cuDF exchange guard restricted to GPU-only (was incorrectly gating CPU configurations)
misiugodfrey
force-pushed
the
misiug/bench-numa-config
branch
from
July 28, 2026 16:03
5eb4e2e to
dcfa364
Compare
- Document PHYSICAL_CORES/SMT_RATIO as diagnostic-only (not wired to any Presto property) to prevent future readers from treating them as dead code - Note set_or_append_property's sed metacharacter assumption at the call site so the constraint is visible to future callers - Add test_common_functions.sh: 39 unit tests for count_linux_cpu_list, discover_cpu_numa_topology, and cpu_numa_node_for_worker using synthetic sysfs trees via CPU_NUMA_SYSFS_ROOT; covers single-node, HBM-only proximity domain exclusion, symmetric 2-node, and balanced worker placement - Expand the ENVIRONMENT VARIABLES help section to document USE_NUMA, CPU_WORKERS_PER_HOST, NPROC, and RAM_GB
…r_configs single-node-execution-enabled and cudf.exchange are now fully reconciled by the post-duplication block on every run, making the equivalent sed calls inside duplicate_worker_configs redundant. Remove them. Convert the remaining sed calls for per-worker port and node.id to set_or_append_property for consistency and to handle templates that may not pre-declare these keys.
6 tasks
misiugodfrey
added a commit
that referenced
this pull request
Aug 5, 2026
- Remove jq download block and PATH prefix from run_queries; replace remaining jq-based wait_for_workers_to_register with python3 - Add exchange.http-client.max-content-length=128MB to GPU coordinator to pair with exchange.max-response-size=64MB (avoids client rejecting responses it requested at SF30K+) - Remove exchange.max-response-size from GPU and CPU worker configs; native workers do not read this property (not in Configs.h) - Fix libcudf cache mount target: /var/lib/presto/data/libcudf-cache matches LIBCUDF_KERNEL_CACHE_PATH set by PR #398 in the native image - Port full per-iteration profiler structure (PROFILE_ITERATIONS env var, combined and per-iter modes) with CTAS integration; collect metrics per-iteration after each per-iter profiler stop - Add java/etc_coordinator/catalog/hive.properties override to preserve SOFT_AFFINITY for Java benchmarks after it was removed from the shared template - Record PRESTO_SESSION_PROPERTIES in benchmark_result.json context as session_properties; add field to BenchmarkMetadata; document format in _session_properties_from_env docstring; reject empty property names - Reject --iterations < 1 at session start - Set CPU worker exchange/sink buffers to 512MB (safe multi-worker default until generate_presto_config.sh reconciliation lands in #391); remove misleading comment promising generator behavior not in this PR - Remove CPU_* generator promise comment from cpu/etc_worker template
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
Rewrites
generate_presto_config.shto discover host NUMA topology at config-generation time and size each worker to its proportional share of the NUMA node(s) it will be bound to. Previously all workers on a host received identical configs sized to the full host, which over-allocated CPU and memory for multi-worker-per-host CPU runs.Based on kjmph's bench33 POC patch.
discover_cpu_numa_topology(new helper incommon_functions.sh): useslscpu,lsmem, and/proc/meminfofallbacks to enumerate CPU NUMA nodes, their logical CPU counts, and their memory in GB; distinguishes CPU-hosting nodes from HBM-only memory nodes (relevant for Grace-Hopper hosts)cpu_numa_node_for_worker(new helper): maps a worker slot index to its NUMA node using balanced contiguous placement (yields 0; 0,1; 0,0,1,1 for 1/2/4 workers on a 2-socket host)count_linux_cpu_list(new helper): parses LinuxCpus_allowed_list-style range strings (e.g.0-47,96-143) to a countgenerate_presto_config.sh: NUMA topology discovery runs whenUSE_NUMA=1; per-worker CPU thread and RAM limits derived from the smallest NUMA node share;CPU_NUMA_WORKER_LAYOUTexported as<node>:<count>map for downstream consumers;set_or_append_propertyhelper for idempotent property file mutations; cuDF exchange guard restricted to GPU-only (was incorrectly gating CPU runs)Dependencies
Depends on PR #390 (
bench-fixes-tuning) for thecommon_functions.shjq→python3 fix inwait_for_worker_node_registration. Rebase after PR #390 merges.Verification
CPU_WORKERS_PER_HOST=2andUSE_NUMA=1, verifygenerate_presto_config.shproduces two configs with CPU count = (total logical CPUs on one socket / 2) and RAM = (HBM on that socket / 2)USE_NUMA=0, verify the generated config is unchanged from the pre-patch behavior (full-host sizing, single worker)CPU_NUMA_WORKER_LAYOUTis set correctly (e.g.0:1,1:1for 2 workers on a 2-socket host)discover_cpu_numa_topologycorrectly ignores HBM-only NUMA nodes as CPU-hosting nodes on Grace-Hopper (nodes withCPU_NUMA_NODE_CPU_COUNTS[i] == 0should not contribute CPU slots)set_or_append_propertyis idempotent: runninggenerate_presto_config.shtwice does not duplicate property entries./ci/check_style.shon changed files