From fc34cb64072bafffc0bbe37b3dfbd6b20fe7718a Mon Sep 17 00:00:00 2001 From: misiugodfrey Date: Wed, 24 Sep 2025 11:37:46 -0700 Subject: [PATCH 1/3] Created common script argument parsing --- presto/scripts/run_integ_test.sh | 95 ++------------ ...rk_helper_check_instance_and_parse_args.sh | 48 ++----- presto/scripts/start_presto_helper.sh | 4 +- .../scripts/start_presto_helper_parse_args.sh | 45 ++----- .../scripts/generate_test_files.sh | 43 ++---- scripts/helper_functions.sh | 79 ++++++++++++ scripts/run_py_script.sh | 48 +++---- velox/scripts/benchmark_velox.sh | 122 ++++-------------- velox/scripts/test_velox.sh | 30 +---- 9 files changed, 174 insertions(+), 340 deletions(-) create mode 100644 scripts/helper_functions.sh diff --git a/presto/scripts/run_integ_test.sh b/presto/scripts/run_integ_test.sh index a94c1f55..9ddda9b2 100755 --- a/presto/scripts/run_integ_test.sh +++ b/presto/scripts/run_integ_test.sh @@ -49,88 +49,19 @@ EOF KEEP_TABLES=false -parse_args() { - while [[ $# -gt 0 ]]; do - case $1 in - -h|--help) - print_help - exit 0 - ;; - -b|--benchmark-type) - if [[ -n $2 ]]; then - BENCHMARK_TYPE=$2 - shift 2 - else - echo "Error: --benchmark-type requires a value" - exit 1 - fi - ;; - -q|--queries) - if [[ -n $2 ]]; then - QUERIES=$2 - shift 2 - else - echo "Error: --queries requires a value" - exit 1 - fi - ;; - -k|--keep-tables) - KEEP_TABLES=true - shift - ;; - -h|--hostname) - if [[ -n $2 ]]; then - HOST_NAME=$2 - shift 2 - else - echo "Error: --hostname requires a value" - exit 1 - fi - ;; - -p|--port) - if [[ -n $2 ]]; then - PORT=$2 - shift 2 - else - echo "Error: --port requires a value" - exit 1 - fi - ;; - -u|--user) - if [[ -n $2 ]]; then - USER=$2 - shift 2 - else - echo "Error: --user requires a value" - exit 1 - fi - ;; - -s|--schema-name) - if [[ -n $2 ]]; then - SCHEMA_NAME=$2 - shift 2 - else - echo "Error: --schema-name requires a value" - exit 1 - fi - ;; - -f|--scale-factor) - if [[ -n $2 ]]; then - SCALE_FACTOR=$2 - shift 2 - else - echo "Error: --scale-factor requires a value" - exit 1 - fi - ;; - *) - echo "Error: Unknown argument $1" - print_help - exit 1 - ;; - esac - done -} +source ../../scripts/helper_functions.sh + +declare -A OPTION_MAP=( ["-b"]="--benchmark-type" + ["-q"]="--queries" + ["-h"]="--hostname" + ["-p"]="--port" + ["-u"]="--user" + ["-s"]="--schema-name" + ["-f"]="--scale-factor" ) +make_options "OPTION_MAP" + +declare -A FLAG_MAP=( ["-k"]="--keep-tables" ) +make_flags "FLAG_MAP" parse_args "$@" diff --git a/presto/scripts/setup_benchmark_helper_check_instance_and_parse_args.sh b/presto/scripts/setup_benchmark_helper_check_instance_and_parse_args.sh index cb939b25..58d21032 100644 --- a/presto/scripts/setup_benchmark_helper_check_instance_and_parse_args.sh +++ b/presto/scripts/setup_benchmark_helper_check_instance_and_parse_args.sh @@ -57,44 +57,18 @@ if [[ -z $PRESTO_DATA_DIR ]]; then exit 1 fi -source ./common_functions.sh +source ../../scripts/helper_functions.sh -wait_for_worker_node_registration +declare -A OPTION_MAP=( ["-b"]="--benchmark-type" + ["-d"]="--data-dir-name" + ["-s"]="--schema-name" ) +make_options "OPTION_MAP" -parse_args() { +custom_parse_args() { while [[ $# -gt 0 ]]; do case $1 in - -h|--help) - print_help - exit 0 - ;; - -b|--benchmark-type) - if [[ -n $2 ]]; then - BENCHMARK_TYPE=$2 - shift 2 - else - echo "Error: --benchmark-type requires a value" - exit 1 - fi - ;; - -s|--schema-name) - if [[ -n $2 ]]; then - SCHEMA_NAME=$2 - shift 2 - else - echo "Error: --schema-name requires a value" - exit 1 - fi - ;; - -d|--data-dir-name) - if [[ -n $2 ]]; then - DATA_DIR_NAME=$2 - shift 2 - else - echo "Error: --data-dir-name requires a value" - exit 1 - fi - ;; + -h|--help) print_help; exit 0;; + $OPTIONS) parse_option $1 $2; shift 2;; -c|--convert-decimals-to-floats) CONVERT_DECIMALS_TO_FLOATS_ARG="--convert-decimals-to-floats" shift @@ -118,7 +92,11 @@ parse_args() { done } -parse_args "$@" +custom_parse_args "$@" + +source ./common_functions.sh + +wait_for_worker_node_registration if [[ -z ${BENCHMARK_TYPE} || ! ${BENCHMARK_TYPE} =~ ^tpc(h|ds)$ ]]; then echo "Error: A valid benchmark type (tpch or tpcds) is required. Use the -b or --benchmark-type argument." diff --git a/presto/scripts/start_presto_helper.sh b/presto/scripts/start_presto_helper.sh index 2d57a7b6..d81b9d97 100755 --- a/presto/scripts/start_presto_helper.sh +++ b/presto/scripts/start_presto_helper.sh @@ -16,6 +16,8 @@ set -e +source ./start_presto_helper_parse_args.sh + if [[ -z ${VARIANT_TYPE} || ! ${VARIANT_TYPE} =~ ^(cpu|gpu|java)$ ]]; then echo "Internal error: A valid variant type (cpu, gpu, or java) is required. Set VARIANT_TYPE to an appropriate value." exit 1 @@ -29,8 +31,6 @@ fi # Validate repo layout using shared script ../../scripts/validate_directories_exist.sh "../../../presto" "../../../velox" -source ./start_presto_helper_parse_args.sh - COORDINATOR_SERVICE="presto-coordinator" COORDINATOR_IMAGE=${COORDINATOR_SERVICE}:latest JAVA_WORKER_SERVICE="presto-java-worker" diff --git a/presto/scripts/start_presto_helper_parse_args.sh b/presto/scripts/start_presto_helper_parse_args.sh index 59bddaae..e8b498d3 100644 --- a/presto/scripts/start_presto_helper_parse_args.sh +++ b/presto/scripts/start_presto_helper_parse_args.sh @@ -44,48 +44,25 @@ EOF NUM_THREADS=$(($(nproc) / 2)) -parse_args() { +source ../../scripts/helper_functions.sh +declare -A OPTION_MAP=( ["-b"]="--build-target" ["-j"]="--num-threads" ) +make_options "OPTION_MAP" + +custom_parse_args() { while [[ $# -gt 0 ]]; do case $1 in - -h|--help) - print_help - exit 0 - ;; - -n|--no-cache) - SKIP_CACHE_ARG="--no-cache" - shift - ;; - -b|--build) - if [[ -n $2 ]]; then - BUILD_TARGET=$2 - shift 2 - else - echo "Error: --build requires a value" - exit 1 - fi - ;; - -j|--num-threads) - if [[ -n $2 ]]; then - NUM_THREADS=$2 - shift 2 - else - echo "Error: --num-threads requires a value" - exit 1 - fi - ;; - *) - echo "Error: Unknown argument $1" - print_help - exit 1 - ;; + -h|--help) print_help; exit 0 ;; + -n|--no-cache) SKIP_CACHE_ARG="--no-cache"; shift ;; + $OPTIONS) parse_option $1 $2; shift 2 ;; + *) echo "Error: Unknown argument $1"; print_help; exit 1 ;; esac done } -parse_args "$@" +custom_parse_args "$@" if [[ -n ${BUILD_TARGET} && ! ${BUILD_TARGET} =~ ^(coordinator|c|worker|w|all|a)$ ]]; then - echo "Error: invalid --build value." + echo "Error: invalid --build-target value." print_help exit 1 fi diff --git a/presto/testing/integration_tests/scripts/generate_test_files.sh b/presto/testing/integration_tests/scripts/generate_test_files.sh index 0339a908..4c65b3dc 100755 --- a/presto/testing/integration_tests/scripts/generate_test_files.sh +++ b/presto/testing/integration_tests/scripts/generate_test_files.sh @@ -47,49 +47,26 @@ EOF SCALE_FACTOR=0.01 CONVERT_DECIMALS_TO_FLOATS=false -parse_args() { +source ../../scripts/helper_functions.sh +declare -A OPTION_MAP=( ["-b"]="--benchmark-type" ["-f"]="--scale-factor" ) +make_options "OPTION_MAP" + +custom_parse_args() { while [[ $# -gt 0 ]]; do case $1 in - -h|--help) - print_help - exit 0 - ;; - -b|--benchmark-type) - if [[ -n $2 ]]; then - BENCHMARK_TYPE=$2 - shift 2 - else - echo "Error: --benchmark-type requires a value" - exit 1 - fi - ;; - -s|--scale-factor) - if [[ -n $2 ]]; then - SCALE_FACTOR=$2 - shift 2 - else - echo "Error: --scale-factor requires a value" - exit 1 - fi - ;; + -h|--help) print_help; exit 0 ;; + $OPTIONS) parse_option $1 $2; shift 2;; -c|--convert-decimals-to-floats) CONVERT_DECIMALS_TO_FLOATS=true shift ;; - -v|--verbose) - VERBOSE="--verbose" - shift - ;; - *) - echo "Error: Unknown argument $1" - print_help - exit 1 - ;; + -v|--verbose) VERBOSE="--verbose"; shift ;; + *) echo "Error: Unknown argument $1"; print_help; exit 1;; esac done } -parse_args "$@" +custom_parse_args "$@" if [[ -z $BENCHMARK_TYPE ]]; then BENCHMARK_TYPES_TO_GENERATE=("tpch" "tpcds") diff --git a/scripts/helper_functions.sh b/scripts/helper_functions.sh new file mode 100644 index 00000000..20cb0f00 --- /dev/null +++ b/scripts/helper_functions.sh @@ -0,0 +1,79 @@ +#!/bin/bash + +# Copyright (c) 2025, NVIDIA CORPORATION. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +shopt -s extglob # needed for paramaterized case statement + +function make_options() { + local -n LOCAL_OPTION_MAP="$1" + declare -g OPTIONS="@(" + SEPARATOR="" + for key in "${!LOCAL_OPTION_MAP[@]}"; do + OPTIONS+="${SEPARATOR}$key|${LOCAL_OPTION_MAP[$key]}" + SEPARATOR="|" + done + OPTIONS+=")" +} + +function make_flags() { + local -n LOCAL_FLAG_MAP="$1" + FLAGS="@(" + SEPARATOR="" + for key in "${!LOCAL_FLAG_MAP[@]}"; do + FLAGS+="${SEPARATOR}$key|${LOCAL_FLAG_MAP[$key]}" + SEPARATOR="|" + done + FLAGS+=")" +} + +function parse_option() { + local option=$1 + [[ -v OPTION_MAP["$option"] ]] && option=${OPTION_MAP["$option"]} + [[ $# -lt 2 || -z $2 ]] && echo "Error: $option requires a value" >&2 && exit 1 + option=${option//--/} # remove double-dash + option=${option//-/_} # replace dash with underscore + option=$(echo "${option^^}") # capitalize + declare -g "$option=$2" +} + +function parse_flag() { + local flag=$1 + [[ -v FLAG_MAP["$flag"] ]] && flag=${FLAG_MAP["$flag"]} + flag=${flag//--/} # remove double-dash + flag=${flag//-/_} # replace dash with underscore + flag=$(echo "${flag^^}") # capitalize + declare -g "$flag=true" +} + +parse_args() { + while [[ $# -gt 0 ]]; do + case $1 in + $OPTIONS) parse_option "$@"; shift 2;; + $FLAGS) parse_flag $1; shift 1;; + -h|--help) print_help; exit 0;; + *) echo "Error: Unknown argument $1"; print_help; exit 1;; + esac + done +} + +parse_args_no_flags() { + while [[ $# -gt 0 ]]; do + case $1 in + $OPTIONS) parse_option "$@"; shift 2;; + -h|--help) print_help; exit 0;; + *) echo "Error: Unknown argument $1"; print_help; exit 1;; + esac + done +} diff --git a/scripts/run_py_script.sh b/scripts/run_py_script.sh index 545b24c0..bb024806 100755 --- a/scripts/run_py_script.sh +++ b/scripts/run_py_script.sh @@ -21,7 +21,7 @@ print_help() { Usage: $0 [OPTIONS] -This script sets up a new python virtual environment, installs dependencies, +This script sets up a new python virtual environment, installs dependencies, runs the given python script, and then deletes the created virtual environment. OPTIONS: @@ -39,51 +39,33 @@ EXAMPLES: EOF } +source ./helper_functions.sh + +declare -A OPTION_MAP=( ["-p"]="--python-script-path" ["-r"]="--requirements-file-path" ) +make_options "OPTION_MAP" + SCRIPT_ARGS=() -parse_args() { +custom_parse_args() { while [[ $# -gt 0 ]]; do case $1 in - -h|--help) - print_help - exit 0 - ;; - -p|--python-script-path) - if [[ -n $2 ]]; then - SCRIPT_PATH=$2 - shift 2 - else - echo "Error: --python-script-path requires a value" - exit 1 - fi - ;; - -r|--requirements-file-path) - if [[ -n $2 ]]; then - REQUIREMENTS_FILE_PATH=$2 - shift 2 - else - echo "Error: --requirements-file-path requires a value" - exit 1 - fi - ;; - *) - SCRIPT_ARGS+=($1) - shift - ;; + -h|--help) print_help; exit 0;; + $OPTIONS) parse_option $1 $2; shift 2;; + *) SCRIPT_ARGS+=($1); shift 1;; esac done } -parse_args "$@" +custom_parse_args "$@" -if [[ -z $SCRIPT_PATH ]]; then +if [[ -z $PYTHON_SCRIPT_PATH ]]; then echo "Error: --python-script-path must be set" print_help exit 1 fi if [[ -z $REQUIREMENTS_FILE_PATH ]]; then - REQUIREMENTS_FILE_PATH="$(dirname $SCRIPT_PATH)/requirements.txt" + REQUIREMENTS_FILE_PATH="$(dirname $PYTHON_SCRIPT_PATH)/requirements.txt" fi source "$(dirname $(readlink -f $0))/py_env_functions.sh" @@ -96,6 +78,6 @@ init_python_virtual_env echo "Running pip install for requirements file: $REQUIREMENTS_FILE_PATH" pip install -q -r $REQUIREMENTS_FILE_PATH -echo -e "\nRunning python script with args:\n$SCRIPT_PATH ${SCRIPT_ARGS[@]}\n" -python $SCRIPT_PATH ${SCRIPT_ARGS[@]} +echo -e "\nRunning python script with args:\n$PYTHON_SCRIPT_PATH ${SCRIPT_ARGS[@]}\n" +python $PYTHON_SCRIPT_PATH ${SCRIPT_ARGS[@]} echo "Finished running python script" diff --git a/velox/scripts/benchmark_velox.sh b/velox/scripts/benchmark_velox.sh index 1951fb2e..a9ac3ed6 100755 --- a/velox/scripts/benchmark_velox.sh +++ b/velox/scripts/benchmark_velox.sh @@ -75,102 +75,6 @@ Output: EOF } -parse_args() { - - # Parse general arguments - while [[ $# -gt 0 ]]; do - case $1 in - -b|--benchmark-type) - if [[ -n "${2:-}" ]]; then - BENCHMARK_TYPE="$2" - shift 2 - else - echo "ERROR: --benchmark-type requires a benchmark type argument" >&2 - exit 1 - fi - ;; - -q|--queries) - if [[ -n "${2:-}" ]]; then - QUERIES="$2" - shift 2 - else - echo "ERROR: --queries requires a query list argument" >&2 - exit 1 - fi - ;; - -d|--device-type) - if [[ -n "${2:-}" ]]; then - DEVICE_TYPE="$2" - shift 2 - else - echo "ERROR: --device-type requires a device type argument" >&2 - exit 1 - fi - ;; - -p|--profile) - if [[ -n "${2:-}" ]]; then - PROFILE="$2" - shift 2 - else - echo "ERROR: --profile requires true or false argument" >&2 - exit 1 - fi - ;; - -o|--output) - if [[ -n "${2:-}" ]]; then - BENCHMARK_RESULTS_OUTPUT="$2" - shift 2 - else - echo "ERROR: --output requires a directory argument" >&2 - exit 1 - fi - ;; - --data-dir) - if [[ -n "${2:-}" ]]; then - DATA_DIR="$2" - shift 2 - else - echo "ERROR: --data-dir requires a directory argument" >&2 - exit 1 - fi - ;; - --num-repeats) - if [[ -n "${2:-}" ]]; then - NUM_REPEATS="$2" - shift 2 - else - echo "ERROR: --num-repeats requires a number argument" >&2 - exit 1 - fi - ;; - -h|--help) - print_help - exit 0 - ;; - *) - echo "ERROR: Unknown option: $1" >&2 - echo "Use --help for usage information" >&2 - exit 1 - ;; - esac - done - - # Set benchmark-specific defaults for queries if not provided, NOTE: changes with `RUN` commands do not persist - if [[ -z "$QUERIES" ]]; then - case "$BENCHMARK_TYPE" in - "tpch") - QUERIES="$(get_tpch_default_queries)" - ;; - *) - echo "ERROR: Unknown benchmark type: $BENCHMARK_TYPE" >&2 - echo "Supported benchmark types: tpch" >&2 - exit 1 - ;; - esac - fi - -} - # Helper function to create/update environment file for Docker Compose create_docker_env_file() { local env_file="./.env" @@ -285,8 +189,32 @@ run_benchmark() { done } +source ../../scripts/helper_functions.sh +declare -A OPTION_MAP=( ["-b"]="--benchmark-type" + ["-q"]="--queries" + ["-d"]="--device-type" + ["-p"]="--profile" + ["-o"]="--benchmark-results-output" + ["-D"]="--data-dir" + ["-n"]="--num-repeats" ) +make_options "OPTION_MAP" + # Parse arguments -parse_args "$@" +parse_args_no_flags "$@" + +# Set benchmark-specific defaults for queries if not provided, NOTE: changes with `RUN` commands do not persist +if [[ -z "$QUERIES" ]]; then + case "$BENCHMARK_TYPE" in + "tpch") + QUERIES="$(get_tpch_default_queries)" + ;; + *) + echo "ERROR: Unknown benchmark type: $BENCHMARK_TYPE" >&2 + echo "Supported benchmark types: tpch" >&2 + exit 1 + ;; + esac +fi echo "" echo "Velox Benchmark Runner" diff --git a/velox/scripts/test_velox.sh b/velox/scripts/test_velox.sh index bcf174b7..a703b947 100755 --- a/velox/scripts/test_velox.sh +++ b/velox/scripts/test_velox.sh @@ -37,31 +37,13 @@ By default, uses 3/4 of available CPU cores for parallel test execution. EOF } -parse_args() { - while [[ $# -gt 0 ]]; do - case $1 in - -j|--num-threads) - if [[ -z "${2:-}" || "${2}" =~ ^- ]]; then - echo "Error: --num-threads requires a value" - exit 1 - fi - NUM_THREADS="$2" - shift 2 - ;; - -h|--help) - print_help - exit 0 - ;; - *) - echo "Unknown option: $1" - echo "Use -h or --help for usage information" - exit 1 - ;; - esac - done -} +source ../../scripts/helper_functions.sh + +declare -A OPTION_MAP=( ["-j"]="--num-threads" ) +make_options "OPTION_MAP" +FLAGS="" -parse_args "$@" +parse_args_no_flags "$@" echo "Running tests on Velox adapters..." echo "" From 2850a721a7022c5132f2e0b18328338f68de9110 Mon Sep 17 00:00:00 2001 From: misiugodfrey Date: Wed, 24 Sep 2025 11:48:34 -0700 Subject: [PATCH 2/3] removed unnecessary declaration --- velox/scripts/test_velox.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/velox/scripts/test_velox.sh b/velox/scripts/test_velox.sh index a703b947..e635b23f 100755 --- a/velox/scripts/test_velox.sh +++ b/velox/scripts/test_velox.sh @@ -41,7 +41,6 @@ source ../../scripts/helper_functions.sh declare -A OPTION_MAP=( ["-j"]="--num-threads" ) make_options "OPTION_MAP" -FLAGS="" parse_args_no_flags "$@" From b91addcf2c5fff88ac4e8d8ffe7fac62d6530150 Mon Sep 17 00:00:00 2001 From: misiugodfrey Date: Thu, 25 Sep 2025 11:18:03 -0700 Subject: [PATCH 3/3] Added colour-coded echo options --- scripts/helper_functions.sh | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/scripts/helper_functions.sh b/scripts/helper_functions.sh index 20cb0f00..5d83365b 100644 --- a/scripts/helper_functions.sh +++ b/scripts/helper_functions.sh @@ -16,6 +16,21 @@ shopt -s extglob # needed for paramaterized case statement +# --- Print error messages in red --- +function echo_error() { + echo -e "\033[0;31m$*\033[0m" >&2 +} + +# --- Print warning messages in yellow --- +function echo_warning() { + echo -e "\033[0;33m$*\033[0m: " >&2 +} + +# --- Print warning messages in green --- +function echo_success() { + echo -e "\033[0;32m$*\033[0m: " +} + function make_options() { local -n LOCAL_OPTION_MAP="$1" declare -g OPTIONS="@(" @@ -41,7 +56,7 @@ function make_flags() { function parse_option() { local option=$1 [[ -v OPTION_MAP["$option"] ]] && option=${OPTION_MAP["$option"]} - [[ $# -lt 2 || -z $2 ]] && echo "Error: $option requires a value" >&2 && exit 1 + [[ $# -lt 2 || -z $2 ]] && echo_error "Error: $option requires a value" && exit 1 option=${option//--/} # remove double-dash option=${option//-/_} # replace dash with underscore option=$(echo "${option^^}") # capitalize @@ -63,7 +78,7 @@ parse_args() { $OPTIONS) parse_option "$@"; shift 2;; $FLAGS) parse_flag $1; shift 1;; -h|--help) print_help; exit 0;; - *) echo "Error: Unknown argument $1"; print_help; exit 1;; + *) echo_error "Error: Unknown argument $1"; print_help; exit 1;; esac done } @@ -73,7 +88,7 @@ parse_args_no_flags() { case $1 in $OPTIONS) parse_option "$@"; shift 2;; -h|--help) print_help; exit 0;; - *) echo "Error: Unknown argument $1"; print_help; exit 1;; + *) echo_error "Error: Unknown argument $1"; print_help; exit 1;; esac done }