From 9776c8360ca72bce56995574274d58f710559fd3 Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Mon, 27 Oct 2025 08:48:35 -0700 Subject: [PATCH 1/3] More robust handling of any existing config --- presto/scripts/generate_presto_config.sh | 61 +++++++++++++------ .../scripts/start_presto_helper_parse_args.sh | 6 ++ 2 files changed, 48 insertions(+), 19 deletions(-) diff --git a/presto/scripts/generate_presto_config.sh b/presto/scripts/generate_presto_config.sh index 35249a0a..8b321863 100755 --- a/presto/scripts/generate_presto_config.sh +++ b/presto/scripts/generate_presto_config.sh @@ -17,21 +17,25 @@ set -euo pipefail RED='\033[0;31m' +YELLOW='\033[1;33m' GREEN='\033[0;32m' NC='\033[0m' # No Color function echo_error { - echo -e "${RED}$1${NC}" - exit 1 + echo -e "${RED}$1${NC}" + exit 1 +} + +function echo_warning { + echo -e "${YELLOW}$1${NC}" } function echo_success { - echo -e "${GREEN}$1${NC}" + echo -e "${GREEN}$1${NC}" } if [ ! -x ../pbench/pbench ]; then - echo "ERROR: generate_presto_config.sh script must only be run from presto:presto/scripts" - exit 1 + echo_error "ERROR: generate_presto_config.sh script must only be run from presto:presto/scripts" fi # get host values @@ -39,13 +43,10 @@ NPROC=`nproc` # lsmem will report in SI. Make sure we get values in GB. RAM_GB=$(lsmem -b | grep "Total online memory" | awk '{print int($4 / (1024*1024*1024)); }') -echo "Generating Presto Config files for ${NPROC} CPU cores and ${RAM_GB}GB RAM" - # variant-specific behavior # for GPU you must set vcpu_per_worker to a small number, not the CPU count if [[ -z ${VARIANT_TYPE} || ! ${VARIANT_TYPE} =~ ^(cpu|gpu|java)$ ]]; then - echo "Error: VARIANT_TYPE must be set to a valid variant type (cpu, gpu, java)." - exit 1 + echo_error "ERROR: VARIANT_TYPE must be set to a valid variant type (cpu, gpu, java)." fi if [[ "${VARIANT_TYPE}" == "gpu" ]]; then VCPU_PER_WORKER=2 @@ -59,10 +60,14 @@ pushd ../docker/config > /dev/null # always move back even on failure trap "popd > /dev/null" EXIT -# (re-)generate the config.json file -rm -rf generated -mkdir -p generated -cat > generated/config.json << EOF +# generate only if no existing config or overwrite flag is set +if [[ ! -d generated || "${OVERWRITE_CONFIG}" == "true" ]]; then + echo "Generating Presto Config files for ${NPROC} CPU cores and ${RAM_GB}GB RAM" + + # (re-)generate the config.json file + rm -rf generated + mkdir -p generated + cat > generated/config.json << EOF { "cluster_size": "small", "coordinator_instance_type": "${NPROC}-core CPU and ${RAM_GB}GB RAM", @@ -77,10 +82,28 @@ cat > generated/config.json << EOF } EOF -# run pbench to generate the config files -# hide default pbench logging which goes to stderr so we only see any errors -if ../../pbench/pbench genconfig -p params.json -t template generated 2>&1 | grep '\{\"level":"error"'; then - echo_error "ERROR in pbench genconfig. Configs were not generated successfully" -fi + # run pbench to generate the config files + # hide default pbench logging which goes to stderr so we only see any errors + if ../../pbench/pbench genconfig -p params.json -t template generated 2>&1 | grep '\{\"level":"error"'; then + echo_error "ERROR: Errors reported by pbench genconfig. Configs were not generated successfully." + fi + + # write variant to file for future checks + echo "${VARIANT_TYPE}" > generated/variant_type.txt -echo_success "Configs were generated successfully" + # success message + echo_success "Configs were generated successfully" +else + # avoid reuse of wrong variant config + if [[ -f generated/variant_type.txt ]]; then + EXISTING_VARIANT_TYPE=$(cat generated/variant_type.txt) + if [[ "${EXISTING_VARIANT_TYPE}" != "${VARIANT_TYPE}" ]]; then + echo_error "ERROR: Found config for '${EXISTING_VARIANT_TYPE}'. Use --overwrite-config option regenerate config for '${VARIANT_TYPE}'." + fi + else + echo_warning "WARNING: Existing config found but variant type is unknown. Use --overwrite-config option to regenerate config." + fi + + # otherwise, reuse existing config + echo_success "Reusing existing Presto Config files" +fi diff --git a/presto/scripts/start_presto_helper_parse_args.sh b/presto/scripts/start_presto_helper_parse_args.sh index dd5060e0..a1d58f6b 100644 --- a/presto/scripts/start_presto_helper_parse_args.sh +++ b/presto/scripts/start_presto_helper_parse_args.sh @@ -38,6 +38,7 @@ OPTIONS: -p, --profile Launch the Presto server with profiling enabled. --profile-args Arguments to pass to the profiler when it launches the Presto server. This will override the default arguments. + --overwrite-config Force config to be regenerated (will overwrite local changes). EXAMPLES: $SCRIPT_NAME --no-cache @@ -53,6 +54,7 @@ EOF NUM_THREADS=$(($(nproc) / 2)) BUILD_TYPE=release ALL_CUDA_ARCHS=false +export OVERWRITE_CONFIG=false export PROFILE=OFF parse_args() { while [[ $# -gt 0 ]]; do @@ -110,6 +112,10 @@ parse_args() { ALL_CUDA_ARCHS=true shift ;; + --overwrite-config) + OVERWRITE_CONFIG=true + shift + ;; *) echo "Error: Unknown argument $1" print_help From 15ab87de40a58aad702f74df61d9e420b051aa96 Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Wed, 29 Oct 2025 20:10:01 -0700 Subject: [PATCH 2/3] Separate configs per variant --- presto/scripts/generate_presto_config.sh | 31 ++++++++---------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/presto/scripts/generate_presto_config.sh b/presto/scripts/generate_presto_config.sh index 9d551281..85c8996e 100755 --- a/presto/scripts/generate_presto_config.sh +++ b/presto/scripts/generate_presto_config.sh @@ -60,14 +60,16 @@ pushd ../docker/config > /dev/null # always move back even on failure trap "popd > /dev/null" EXIT +CONFIG_DIR=generated/${VARIANT_TYPE} + # generate only if no existing config or overwrite flag is set -if [[ ! -d generated || "${OVERWRITE_CONFIG}" == "true" ]]; then - echo "Generating Presto Config files for ${NPROC} CPU cores and ${RAM_GB}GB RAM" +if [[ ! -d ${CONFIG_DIR} || "${OVERWRITE_CONFIG}" == "true" ]]; then + echo "Generating Presto Config files for '${VARIANT_TYPE}' for host with ${NPROC} CPU cores and ${RAM_GB}GB RAM" # (re-)generate the config.json file - rm -rf generated - mkdir -p generated - cat > generated/config.json << EOF + rm -rf ${CONFIG_DIR} + mkdir -p ${CONFIG_DIR} + cat > ${CONFIG_DIR}/config.json << EOF { "cluster_size": "small", "coordinator_instance_type": "${NPROC}-core CPU and ${RAM_GB}GB RAM", @@ -84,7 +86,7 @@ EOF # run pbench to generate the config files # hide default pbench logging which goes to stderr so we only see any errors - if ../../pbench/pbench genconfig -p params.json -t template generated 2>&1 | grep '\{\"level":"error"'; then + if ../../pbench/pbench genconfig -p params.json -t template ${CONFIG_DIR} 2>&1 | grep '\{\"level":"error"'; then echo_error "ERROR: Errors reported by pbench genconfig. Configs were not generated successfully." fi @@ -93,26 +95,13 @@ EOF # for GPU variant, uncomment these optimizer settings # optimizer.joins-not-null-inference-strategy=USE_FUNCTION_METADATA # optimizer.default-filter-factor-enabled=true - COORD_CONFIG="generated/etc_coordinator/config_native.properties" + COORD_CONFIG="${CONFIG_DIR}/etc_coordinator/config_native.properties" sed -i 's/\#optimizer/optimizer/g' ${COORD_CONFIG} fi - # write variant to file for future checks - echo "${VARIANT_TYPE}" > generated/variant_type.txt - # success message echo_success "Configs were generated successfully" else - # avoid reuse of wrong variant config - if [[ -f generated/variant_type.txt ]]; then - EXISTING_VARIANT_TYPE=$(cat generated/variant_type.txt) - if [[ "${EXISTING_VARIANT_TYPE}" != "${VARIANT_TYPE}" ]]; then - echo_error "ERROR: Found config for '${EXISTING_VARIANT_TYPE}'. Use --overwrite-config option regenerate config for '${VARIANT_TYPE}'." - fi - else - echo_warning "WARNING: Existing config found but variant type is unknown. Use --overwrite-config option to regenerate config." - fi - # otherwise, reuse existing config - echo_success "Reusing existing Presto Config files" + echo_success "Reusing existing Presto Config files for '${VARIANT_TYPE}'" fi From c0cca8eec1c4701505f321fe806e65bcf21b4cbb Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Wed, 29 Oct 2025 20:10:13 -0700 Subject: [PATCH 3/3] Map separate configs per variant --- presto/docker/docker-compose.common.yml | 6 ------ presto/docker/docker-compose.java.yml | 9 ++++++--- presto/docker/docker-compose.native-cpu.yml | 8 +++++++- presto/docker/docker-compose.native-gpu.yml | 8 +++++++- 4 files changed, 20 insertions(+), 11 deletions(-) diff --git a/presto/docker/docker-compose.common.yml b/presto/docker/docker-compose.common.yml index 6165006d..7191d5ab 100644 --- a/presto/docker/docker-compose.common.yml +++ b/presto/docker/docker-compose.common.yml @@ -1,7 +1,6 @@ services: presto-base-volumes: volumes: - - ./config/generated/etc_common:/opt/presto-server/etc - ./.hive_metastore:/var/lib/presto/data/hive/metastore - ../testing/integration_tests/data:/var/lib/presto/data/hive/data/integration_test - ${PRESTO_DATA_DIR:-/dev/null}:/var/lib/presto/data/hive/data/user_data @@ -19,8 +18,6 @@ services: image: presto-coordinator:latest ports: - 8080:8080 - volumes: - - ./config/generated/etc_coordinator/node.properties:/opt/presto-server/etc/node.properties presto-base-native-worker: extends: @@ -30,6 +27,3 @@ services: dockerfile: velox-testing/presto/docker/native_build.dockerfile environment: - GLOG_logtostderr=1 - volumes: - - ./config/generated/etc_worker/node.properties:/opt/presto-server/etc/node.properties - - ./config/generated/etc_worker/config_native.properties:/opt/presto-server/etc/config.properties diff --git a/presto/docker/docker-compose.java.yml b/presto/docker/docker-compose.java.yml index f25fb508..9eb0fb43 100644 --- a/presto/docker/docker-compose.java.yml +++ b/presto/docker/docker-compose.java.yml @@ -4,7 +4,9 @@ services: file: docker-compose.common.yml service: presto-base-coordinator volumes: - - ./config/generated/etc_coordinator/config_java.properties:/opt/presto-server/etc/config.properties + - ./config/generated/java/etc_common:/opt/presto-server/etc + - ./config/generated/java/etc_coordinator/config_java.properties:/opt/presto-server/etc/config. + - ./config/generated/java/etc_coordinator/node.properties:/opt/presto-server/etc/node.properties presto-java-worker: extends: @@ -13,7 +15,8 @@ services: container_name: presto-java-worker image: presto-java-worker:latest volumes: - - ./config/generated/etc_worker/config_java.properties:/opt/presto-server/etc/config.properties - - ./config/generated/etc_worker/node.properties:/opt/presto-server/etc/node.properties + - ./config/generated/java/etc_common:/opt/presto-server/etc + - ./config/generated/java/etc_worker/config_java.properties:/opt/presto-server/etc/config.properties + - ./config/generated/java/etc_worker/node.properties:/opt/presto-server/etc/node.properties depends_on: - presto-coordinator diff --git a/presto/docker/docker-compose.native-cpu.yml b/presto/docker/docker-compose.native-cpu.yml index 09fd5a87..830e59ac 100644 --- a/presto/docker/docker-compose.native-cpu.yml +++ b/presto/docker/docker-compose.native-cpu.yml @@ -4,7 +4,9 @@ services: file: docker-compose.common.yml service: presto-base-coordinator volumes: - - ./config/generated/etc_coordinator/config_native.properties:/opt/presto-server/etc/config.properties + - ./config/generated/cpu/etc_common:/opt/presto-server/etc + - ./config/generated/cpu/etc_coordinator/config_native.properties:/opt/presto-server/etc/config.properties + - ./config/generated/cpu/etc_coordinator/node.properties:/opt/presto-server/etc/node.properties presto-native-worker-cpu: extends: @@ -17,3 +19,7 @@ services: - GPU=OFF depends_on: - presto-coordinator + volumes: + - ./config/generated/cpu/etc_common:/opt/presto-server/etc + - ./config/generated/cpu/etc_worker/node.properties:/opt/presto-server/etc/node.properties + - ./config/generated/cpu/etc_worker/config_native.properties:/opt/presto-server/etc/config.properties diff --git a/presto/docker/docker-compose.native-gpu.yml b/presto/docker/docker-compose.native-gpu.yml index 376c5167..edfc499d 100644 --- a/presto/docker/docker-compose.native-gpu.yml +++ b/presto/docker/docker-compose.native-gpu.yml @@ -4,7 +4,9 @@ services: file: docker-compose.common.yml service: presto-base-coordinator volumes: - - ./config/generated/etc_coordinator/config_native.properties:/opt/presto-server/etc/config.properties + - ./config/generated/gpu/etc_common:/opt/presto-server/etc + - ./config/generated/gpu/etc_coordinator/config_native.properties:/opt/presto-server/etc/config.properties + - ./config/generated/gpu/etc_coordinator/node.properties:/opt/presto-server/etc/node.properties presto-native-worker-gpu: extends: @@ -22,3 +24,7 @@ services: - PROFILE_ARGS=${PROFILE_ARGS} depends_on: - presto-coordinator + volumes: + - ./config/generated/gpu/etc_common:/opt/presto-server/etc + - ./config/generated/gpu/etc_worker/node.properties:/opt/presto-server/etc/node.properties + - ./config/generated/gpu/etc_worker/config_native.properties:/opt/presto-server/etc/config.properties