From 0d9c8248de8b5d67180622482597223b4a2f5a94 Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Fri, 24 Oct 2025 16:08:49 -0700 Subject: [PATCH 01/23] Split Java config files --- .../config/template/etc_common/jvm.config | 41 +------------------ .../template/etc_shared/jvm_java.config | 40 ++++++++++++++++++ .../template/etc_shared/jvm_native.config | 40 ++++++++++++++++++ 3 files changed, 81 insertions(+), 40 deletions(-) create mode 100644 presto/docker/config/template/etc_shared/jvm_java.config create mode 100644 presto/docker/config/template/etc_shared/jvm_native.config diff --git a/presto/docker/config/template/etc_common/jvm.config b/presto/docker/config/template/etc_common/jvm.config index 7ea5d695..cb8f2a63 100644 --- a/presto/docker/config/template/etc_common/jvm.config +++ b/presto/docker/config/template/etc_common/jvm.config @@ -1,40 +1 @@ -# Enable JVM server mode for better JIT optimization on long-running servers. --server -# Maximum Java heap size; templated to match container memory. --Xmx{{ .HeapSizeGb }}G -# Initial Java heap size; equal to max to avoid heap resizing pauses. --Xms{{ .HeapSizeGb }}G -# Use the G1 garbage collector for predictable pause times. --XX:+UseG1GC -# Tune G1 region size to balance GC throughput and fragmentation. --XX:G1HeapRegionSize=32M -# Abort when GC overhead becomes excessive to prevent hangs. --XX:+UseGCOverheadLimit -# Make System.gc() invoke concurrent collections to reduce pauses. --XX:+ExplicitGCInvokesConcurrent -# Create heap dumps on OOM for postmortem analysis. --XX:+HeapDumpOnOutOfMemoryError -# Exit the JVM on OOM so orchestration can restart the process. --XX:+ExitOnOutOfMemoryError -# Cap NIO direct buffer cache to limit retained off-heap memory. --Djdk.nio.maxCachedBufferSize=2000000 -# Allow self-attach for profilers (e.g., async-profiler) during debugging. --Djdk.attach.allowAttachSelf=true -# Open JDK internals for reflection required by Presto and dependencies under Java 11+ modules. ---add-opens=java.base/java.io=ALL-UNNAMED ---add-opens=java.base/java.lang=ALL-UNNAMED ---add-opens=java.base/java.lang.ref=ALL-UNNAMED ---add-opens=java.base/java.lang.reflect=ALL-UNNAMED ---add-opens=java.base/java.net=ALL-UNNAMED ---add-opens=java.base/java.nio=ALL-UNNAMED ---add-opens=java.base/java.security=ALL-UNNAMED ---add-opens=java.base/javax.security.auth=ALL-UNNAMED ---add-opens=java.base/javax.security.auth.login=ALL-UNNAMED ---add-opens=java.base/java.text=ALL-UNNAMED ---add-opens=java.base/java.util=ALL-UNNAMED ---add-opens=java.base/java.util.concurrent=ALL-UNNAMED ---add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED ---add-opens=java.base/java.util.regex=ALL-UNNAMED ---add-opens=java.base/jdk.internal.loader=ALL-UNNAMED ---add-opens=java.base/sun.security.action=ALL-UNNAMED ---add-opens=java.security.jgss/sun.security.krb5=ALL-UNNAMED +# This file will be overridden by a coordinator or worker specific configuration file. diff --git a/presto/docker/config/template/etc_shared/jvm_java.config b/presto/docker/config/template/etc_shared/jvm_java.config new file mode 100644 index 00000000..7ea5d695 --- /dev/null +++ b/presto/docker/config/template/etc_shared/jvm_java.config @@ -0,0 +1,40 @@ +# Enable JVM server mode for better JIT optimization on long-running servers. +-server +# Maximum Java heap size; templated to match container memory. +-Xmx{{ .HeapSizeGb }}G +# Initial Java heap size; equal to max to avoid heap resizing pauses. +-Xms{{ .HeapSizeGb }}G +# Use the G1 garbage collector for predictable pause times. +-XX:+UseG1GC +# Tune G1 region size to balance GC throughput and fragmentation. +-XX:G1HeapRegionSize=32M +# Abort when GC overhead becomes excessive to prevent hangs. +-XX:+UseGCOverheadLimit +# Make System.gc() invoke concurrent collections to reduce pauses. +-XX:+ExplicitGCInvokesConcurrent +# Create heap dumps on OOM for postmortem analysis. +-XX:+HeapDumpOnOutOfMemoryError +# Exit the JVM on OOM so orchestration can restart the process. +-XX:+ExitOnOutOfMemoryError +# Cap NIO direct buffer cache to limit retained off-heap memory. +-Djdk.nio.maxCachedBufferSize=2000000 +# Allow self-attach for profilers (e.g., async-profiler) during debugging. +-Djdk.attach.allowAttachSelf=true +# Open JDK internals for reflection required by Presto and dependencies under Java 11+ modules. +--add-opens=java.base/java.io=ALL-UNNAMED +--add-opens=java.base/java.lang=ALL-UNNAMED +--add-opens=java.base/java.lang.ref=ALL-UNNAMED +--add-opens=java.base/java.lang.reflect=ALL-UNNAMED +--add-opens=java.base/java.net=ALL-UNNAMED +--add-opens=java.base/java.nio=ALL-UNNAMED +--add-opens=java.base/java.security=ALL-UNNAMED +--add-opens=java.base/javax.security.auth=ALL-UNNAMED +--add-opens=java.base/javax.security.auth.login=ALL-UNNAMED +--add-opens=java.base/java.text=ALL-UNNAMED +--add-opens=java.base/java.util=ALL-UNNAMED +--add-opens=java.base/java.util.concurrent=ALL-UNNAMED +--add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED +--add-opens=java.base/java.util.regex=ALL-UNNAMED +--add-opens=java.base/jdk.internal.loader=ALL-UNNAMED +--add-opens=java.base/sun.security.action=ALL-UNNAMED +--add-opens=java.security.jgss/sun.security.krb5=ALL-UNNAMED diff --git a/presto/docker/config/template/etc_shared/jvm_native.config b/presto/docker/config/template/etc_shared/jvm_native.config new file mode 100644 index 00000000..7ea5d695 --- /dev/null +++ b/presto/docker/config/template/etc_shared/jvm_native.config @@ -0,0 +1,40 @@ +# Enable JVM server mode for better JIT optimization on long-running servers. +-server +# Maximum Java heap size; templated to match container memory. +-Xmx{{ .HeapSizeGb }}G +# Initial Java heap size; equal to max to avoid heap resizing pauses. +-Xms{{ .HeapSizeGb }}G +# Use the G1 garbage collector for predictable pause times. +-XX:+UseG1GC +# Tune G1 region size to balance GC throughput and fragmentation. +-XX:G1HeapRegionSize=32M +# Abort when GC overhead becomes excessive to prevent hangs. +-XX:+UseGCOverheadLimit +# Make System.gc() invoke concurrent collections to reduce pauses. +-XX:+ExplicitGCInvokesConcurrent +# Create heap dumps on OOM for postmortem analysis. +-XX:+HeapDumpOnOutOfMemoryError +# Exit the JVM on OOM so orchestration can restart the process. +-XX:+ExitOnOutOfMemoryError +# Cap NIO direct buffer cache to limit retained off-heap memory. +-Djdk.nio.maxCachedBufferSize=2000000 +# Allow self-attach for profilers (e.g., async-profiler) during debugging. +-Djdk.attach.allowAttachSelf=true +# Open JDK internals for reflection required by Presto and dependencies under Java 11+ modules. +--add-opens=java.base/java.io=ALL-UNNAMED +--add-opens=java.base/java.lang=ALL-UNNAMED +--add-opens=java.base/java.lang.ref=ALL-UNNAMED +--add-opens=java.base/java.lang.reflect=ALL-UNNAMED +--add-opens=java.base/java.net=ALL-UNNAMED +--add-opens=java.base/java.nio=ALL-UNNAMED +--add-opens=java.base/java.security=ALL-UNNAMED +--add-opens=java.base/javax.security.auth=ALL-UNNAMED +--add-opens=java.base/javax.security.auth.login=ALL-UNNAMED +--add-opens=java.base/java.text=ALL-UNNAMED +--add-opens=java.base/java.util=ALL-UNNAMED +--add-opens=java.base/java.util.concurrent=ALL-UNNAMED +--add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED +--add-opens=java.base/java.util.regex=ALL-UNNAMED +--add-opens=java.base/jdk.internal.loader=ALL-UNNAMED +--add-opens=java.base/sun.security.action=ALL-UNNAMED +--add-opens=java.security.jgss/sun.security.krb5=ALL-UNNAMED From fd55b13f6cfc734c798497fbfd2b2cf837bf5ca0 Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Fri, 24 Oct 2025 16:08:59 -0700 Subject: [PATCH 02/23] Map split Java config files --- presto/docker/docker-compose.common.yml | 1 + presto/docker/docker-compose.java.yml | 2 ++ presto/docker/docker-compose.native-cpu.yml | 1 + presto/docker/docker-compose.native-gpu.yml | 1 + 4 files changed, 5 insertions(+) diff --git a/presto/docker/docker-compose.common.yml b/presto/docker/docker-compose.common.yml index 6165006d..1901498e 100644 --- a/presto/docker/docker-compose.common.yml +++ b/presto/docker/docker-compose.common.yml @@ -33,3 +33,4 @@ services: 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 + - ./config/generated/etc_shared/jvm_native.config:/opt/presto-server/etc/jvm.config diff --git a/presto/docker/docker-compose.java.yml b/presto/docker/docker-compose.java.yml index f25fb508..22e85630 100644 --- a/presto/docker/docker-compose.java.yml +++ b/presto/docker/docker-compose.java.yml @@ -5,6 +5,7 @@ services: service: presto-base-coordinator volumes: - ./config/generated/etc_coordinator/config_java.properties:/opt/presto-server/etc/config.properties + - ./config/generated/etc_shared/jvm_java.config:/opt/presto-server/etc/jvm.config presto-java-worker: extends: @@ -15,5 +16,6 @@ services: 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/etc_shared/jvm_java.config:/opt/presto-server/etc/jvm.config depends_on: - presto-coordinator diff --git a/presto/docker/docker-compose.native-cpu.yml b/presto/docker/docker-compose.native-cpu.yml index 09fd5a87..ac57ad8a 100644 --- a/presto/docker/docker-compose.native-cpu.yml +++ b/presto/docker/docker-compose.native-cpu.yml @@ -5,6 +5,7 @@ services: service: presto-base-coordinator volumes: - ./config/generated/etc_coordinator/config_native.properties:/opt/presto-server/etc/config.properties + - ./config/generated/etc_shared/jvm_native.config:/opt/presto-server/etc/jvm.config presto-native-worker-cpu: extends: diff --git a/presto/docker/docker-compose.native-gpu.yml b/presto/docker/docker-compose.native-gpu.yml index 376c5167..ec003f2c 100644 --- a/presto/docker/docker-compose.native-gpu.yml +++ b/presto/docker/docker-compose.native-gpu.yml @@ -5,6 +5,7 @@ services: service: presto-base-coordinator volumes: - ./config/generated/etc_coordinator/config_native.properties:/opt/presto-server/etc/config.properties + - ./config/generated/etc_shared/jvm_native.config:/opt/presto-server/etc/jvm.config presto-native-worker-gpu: extends: From 10b9fe2449a1b217120e1dac6030464a9198d924 Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Fri, 24 Oct 2025 16:11:15 -0700 Subject: [PATCH 03/23] Java mode JVM heap always 16GB --- presto/docker/config/template/etc_shared/jvm_java.config | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/presto/docker/config/template/etc_shared/jvm_java.config b/presto/docker/config/template/etc_shared/jvm_java.config index 7ea5d695..0a7bfcaa 100644 --- a/presto/docker/config/template/etc_shared/jvm_java.config +++ b/presto/docker/config/template/etc_shared/jvm_java.config @@ -1,9 +1,9 @@ # Enable JVM server mode for better JIT optimization on long-running servers. -server # Maximum Java heap size; templated to match container memory. --Xmx{{ .HeapSizeGb }}G +-Xmx16G # Initial Java heap size; equal to max to avoid heap resizing pauses. --Xms{{ .HeapSizeGb }}G +-Xms16G # Use the G1 garbage collector for predictable pause times. -XX:+UseG1GC # Tune G1 region size to balance GC throughput and fragmentation. From c8220ec4a2e72105753d97fa90ac4da4323c3591 Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Fri, 24 Oct 2025 16:13:37 -0700 Subject: [PATCH 04/23] etc_common_java is better --- .../template/{etc_shared => etc_common_java}/jvm_java.config | 0 .../{etc_shared => etc_common_java}/jvm_native.config | 0 presto/docker/docker-compose.common.yml | 2 +- presto/docker/docker-compose.java.yml | 4 ++-- presto/docker/docker-compose.native-cpu.yml | 2 +- presto/docker/docker-compose.native-gpu.yml | 2 +- 6 files changed, 5 insertions(+), 5 deletions(-) rename presto/docker/config/template/{etc_shared => etc_common_java}/jvm_java.config (100%) rename presto/docker/config/template/{etc_shared => etc_common_java}/jvm_native.config (100%) diff --git a/presto/docker/config/template/etc_shared/jvm_java.config b/presto/docker/config/template/etc_common_java/jvm_java.config similarity index 100% rename from presto/docker/config/template/etc_shared/jvm_java.config rename to presto/docker/config/template/etc_common_java/jvm_java.config diff --git a/presto/docker/config/template/etc_shared/jvm_native.config b/presto/docker/config/template/etc_common_java/jvm_native.config similarity index 100% rename from presto/docker/config/template/etc_shared/jvm_native.config rename to presto/docker/config/template/etc_common_java/jvm_native.config diff --git a/presto/docker/docker-compose.common.yml b/presto/docker/docker-compose.common.yml index 1901498e..2d7bfe27 100644 --- a/presto/docker/docker-compose.common.yml +++ b/presto/docker/docker-compose.common.yml @@ -33,4 +33,4 @@ services: 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 - - ./config/generated/etc_shared/jvm_native.config:/opt/presto-server/etc/jvm.config + - ./config/generated/etc_common_java/jvm_native.config:/opt/presto-server/etc/jvm.config diff --git a/presto/docker/docker-compose.java.yml b/presto/docker/docker-compose.java.yml index 22e85630..fec692a5 100644 --- a/presto/docker/docker-compose.java.yml +++ b/presto/docker/docker-compose.java.yml @@ -5,7 +5,7 @@ services: service: presto-base-coordinator volumes: - ./config/generated/etc_coordinator/config_java.properties:/opt/presto-server/etc/config.properties - - ./config/generated/etc_shared/jvm_java.config:/opt/presto-server/etc/jvm.config + - ./config/generated/etc_common_java/jvm_java.config:/opt/presto-server/etc/jvm.config presto-java-worker: extends: @@ -16,6 +16,6 @@ services: 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/etc_shared/jvm_java.config:/opt/presto-server/etc/jvm.config + - ./config/generated/etc_common_java/jvm_java.config:/opt/presto-server/etc/jvm.config depends_on: - presto-coordinator diff --git a/presto/docker/docker-compose.native-cpu.yml b/presto/docker/docker-compose.native-cpu.yml index ac57ad8a..958acc01 100644 --- a/presto/docker/docker-compose.native-cpu.yml +++ b/presto/docker/docker-compose.native-cpu.yml @@ -5,7 +5,7 @@ services: service: presto-base-coordinator volumes: - ./config/generated/etc_coordinator/config_native.properties:/opt/presto-server/etc/config.properties - - ./config/generated/etc_shared/jvm_native.config:/opt/presto-server/etc/jvm.config + - ./config/generated/etc_common_java/jvm_native.config:/opt/presto-server/etc/jvm.config presto-native-worker-cpu: extends: diff --git a/presto/docker/docker-compose.native-gpu.yml b/presto/docker/docker-compose.native-gpu.yml index ec003f2c..d7f4c526 100644 --- a/presto/docker/docker-compose.native-gpu.yml +++ b/presto/docker/docker-compose.native-gpu.yml @@ -5,7 +5,7 @@ services: service: presto-base-coordinator volumes: - ./config/generated/etc_coordinator/config_native.properties:/opt/presto-server/etc/config.properties - - ./config/generated/etc_shared/jvm_native.config:/opt/presto-server/etc/jvm.config + - ./config/generated/etc_common_java/jvm_native.config:/opt/presto-server/etc/jvm.config presto-native-worker-gpu: extends: From 3bc9d7de2f165483da69d87a1bf5dd08e3828e54 Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Fri, 24 Oct 2025 16:16:35 -0700 Subject: [PATCH 05/23] Set JVM heap to 24GB as before, 16GB was an optimistic mistake --- presto/docker/config/template/etc_common_java/jvm_java.config | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/presto/docker/config/template/etc_common_java/jvm_java.config b/presto/docker/config/template/etc_common_java/jvm_java.config index 0a7bfcaa..e2c1f74e 100644 --- a/presto/docker/config/template/etc_common_java/jvm_java.config +++ b/presto/docker/config/template/etc_common_java/jvm_java.config @@ -1,9 +1,9 @@ # Enable JVM server mode for better JIT optimization on long-running servers. -server # Maximum Java heap size; templated to match container memory. --Xmx16G +-Xmx24G # Initial Java heap size; equal to max to avoid heap resizing pauses. --Xms16G +-Xms24G # Use the G1 garbage collector for predictable pause times. -XX:+UseG1GC # Tune G1 region size to balance GC throughput and fragmentation. From efc4b3cc41a6e6e4b6e0cf9ad0c2f631df02001a Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Mon, 20 Oct 2025 18:33:57 -0700 Subject: [PATCH 06/23] WIP --- .github/workflows/presto-nightly-upstream.yml | 3 +++ .github/workflows/presto-nightly.yml | 1 + .github/workflows/presto-test-composite.yml | 10 ++++++++++ .github/workflows/presto-test.yml | 8 ++++++++ presto/scripts/checkout_velox_to_match_presto.sh | 13 +++++++++++++ 5 files changed, 35 insertions(+) create mode 100644 presto/scripts/checkout_velox_to_match_presto.sh diff --git a/.github/workflows/presto-nightly-upstream.yml b/.github/workflows/presto-nightly-upstream.yml index 108b54be..b9fc7fcc 100644 --- a/.github/workflows/presto-nightly-upstream.yml +++ b/.github/workflows/presto-nightly-upstream.yml @@ -1,6 +1,8 @@ name: Presto Nightly Test (Upstream) on: + schedule: + - cron: '0 4 * * *' # daily at 04:00 UTC workflow_dispatch: jobs: @@ -15,4 +17,5 @@ jobs: run_cpu_tests: true run_gpu_tests: true set_velox_backward_compatible: false + build_upstream: true secrets: inherit diff --git a/.github/workflows/presto-nightly.yml b/.github/workflows/presto-nightly.yml index e7106f30..f0ace12f 100644 --- a/.github/workflows/presto-nightly.yml +++ b/.github/workflows/presto-nightly.yml @@ -17,4 +17,5 @@ jobs: run_cpu_tests: true run_gpu_tests: true set_velox_backward_compatible: ${{ vars.SET_PRESTO_VELOX_BACKWARD_COMPATIBLE == 'true' }} + build_upstream: false secrets: inherit diff --git a/.github/workflows/presto-test-composite.yml b/.github/workflows/presto-test-composite.yml index 52f9c424..6e068e70 100644 --- a/.github/workflows/presto-test-composite.yml +++ b/.github/workflows/presto-test-composite.yml @@ -40,6 +40,10 @@ on: description: 'Set VELOX_ENABLE_BACKWARD_COMPATIBLE in Velox build' type: string required: false + build_upstream: + description: 'Build Presto Upstream Default' + type: boolean + default: false jobs: build-and-test: @@ -58,6 +62,12 @@ jobs: repository: ${{ inputs.velox_repository }} ref: ${{ inputs.velox_commit }} path: velox + - name: Change Velox to match upstream Presto + if: ${{ inputs.build_upstream == 'true' }} + run: | + pushd velox-testing/presto/scripts + ./checkout_velox_to_match_presto.sh + popd - name: Checkout Velox Testing uses: actions/checkout@v4 with: diff --git a/.github/workflows/presto-test.yml b/.github/workflows/presto-test.yml index 0e41b999..806cd38d 100644 --- a/.github/workflows/presto-test.yml +++ b/.github/workflows/presto-test.yml @@ -44,6 +44,10 @@ on: description: 'Set VELOX_ENABLE_BACKWARD_COMPATIBLE in Velox build' type: boolean default: false + build_upstream: &build_upstream + description: 'Build Presto Upstream Default' + type: boolean + default: false workflow_call: inputs: @@ -56,6 +60,7 @@ on: run_cpu_tests: *run_cpu_tests run_gpu_tests: *run_gpu_tests set_velox_backward_compatible: *set_velox_backward_compatible + build_upstream: *build_upstream jobs: java: @@ -70,6 +75,7 @@ jobs: velox_commit: ${{ inputs.velox_commit }} velox_testing_commit: ${{ inputs.velox_testing_commit }} set_velox_backward_compatible: false + build_upstream: ${{ inputs.build_upstream }} secrets: inherit native-cpu: if: ${{ inputs.run_cpu_tests }} @@ -83,6 +89,7 @@ jobs: velox_commit: ${{ inputs.velox_commit }} velox_testing_commit: ${{ inputs.velox_testing_commit }} set_velox_backward_compatible: ${{ inputs.set_velox_backward_compatible }} + build_upstream: ${{ inputs.build_upstream }} secrets: inherit native-gpu: if: ${{ inputs.run_gpu_tests }} @@ -96,4 +103,5 @@ jobs: velox_commit: ${{ inputs.velox_commit }} velox_testing_commit: ${{ inputs.velox_testing_commit }} set_velox_backward_compatible: ${{ inputs.set_velox_backward_compatible }} + build_upstream: ${{ inputs.build_upstream }} secrets: inherit diff --git a/presto/scripts/checkout_velox_to_match_presto.sh b/presto/scripts/checkout_velox_to_match_presto.sh new file mode 100644 index 00000000..be69f01b --- /dev/null +++ b/presto/scripts/checkout_velox_to_match_presto.sh @@ -0,0 +1,13 @@ +#!/bin/bash + +set -e + +# get SHA of Presto Velox submodule +pushd ../../presto/presto-native-execution/velox +SHA=$(git rev-parse HEAD) +popd + +# checkout sibling Velox to that SHA +pushd ../../velox +get checkout ${SHA} +popd From ae0eb6297f05c8c4405e03c7aa6f474e7dafbd04 Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Mon, 20 Oct 2025 18:40:16 -0700 Subject: [PATCH 07/23] Oops. Check as bool. --- .github/workflows/presto-test-composite.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/presto-test-composite.yml b/.github/workflows/presto-test-composite.yml index 6e068e70..df2bcb0f 100644 --- a/.github/workflows/presto-test-composite.yml +++ b/.github/workflows/presto-test-composite.yml @@ -63,7 +63,7 @@ jobs: ref: ${{ inputs.velox_commit }} path: velox - name: Change Velox to match upstream Presto - if: ${{ inputs.build_upstream == 'true' }} + if: ${{ inputs.build_upstream }} run: | pushd velox-testing/presto/scripts ./checkout_velox_to_match_presto.sh From 8cc7b7430c01cdcc28039fa620fba875b298beb3 Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Thu, 23 Oct 2025 10:21:12 -0700 Subject: [PATCH 08/23] Fix embarrassing typo --- presto/scripts/checkout_velox_to_match_presto.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/presto/scripts/checkout_velox_to_match_presto.sh b/presto/scripts/checkout_velox_to_match_presto.sh index be69f01b..9a4a815f 100644 --- a/presto/scripts/checkout_velox_to_match_presto.sh +++ b/presto/scripts/checkout_velox_to_match_presto.sh @@ -9,5 +9,5 @@ popd # checkout sibling Velox to that SHA pushd ../../velox -get checkout ${SHA} +git checkout ${SHA} popd From 3b358fb310e2d3eeb43d8a8827ea76690aa06acf Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Thu, 23 Oct 2025 10:37:16 -0700 Subject: [PATCH 09/23] Add logging to the script --- presto/scripts/checkout_velox_to_match_presto.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/presto/scripts/checkout_velox_to_match_presto.sh b/presto/scripts/checkout_velox_to_match_presto.sh index 9a4a815f..6d211438 100644 --- a/presto/scripts/checkout_velox_to_match_presto.sh +++ b/presto/scripts/checkout_velox_to_match_presto.sh @@ -2,6 +2,8 @@ set -e +echo "Checking out version of Velox which matches Presto pinned version" + # get SHA of Presto Velox submodule pushd ../../presto/presto-native-execution/velox SHA=$(git rev-parse HEAD) @@ -11,3 +13,5 @@ popd pushd ../../velox git checkout ${SHA} popd + +echo "Velox checked out to ${SHA}" From a2bd959ad27ad6f563fb5f6684c32fa0f278ccf3 Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Thu, 23 Oct 2025 11:07:08 -0700 Subject: [PATCH 10/23] Reorder steps --- .github/workflows/presto-test-composite.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/presto-test-composite.yml b/.github/workflows/presto-test-composite.yml index df2bcb0f..4ad35aa1 100644 --- a/.github/workflows/presto-test-composite.yml +++ b/.github/workflows/presto-test-composite.yml @@ -62,18 +62,18 @@ jobs: repository: ${{ inputs.velox_repository }} ref: ${{ inputs.velox_commit }} path: velox - - name: Change Velox to match upstream Presto - if: ${{ inputs.build_upstream }} - run: | - pushd velox-testing/presto/scripts - ./checkout_velox_to_match_presto.sh - popd - name: Checkout Velox Testing uses: actions/checkout@v4 with: repository: rapidsai/velox-testing ref: ${{ inputs.velox_testing_commit }} path: velox-testing + - name: Change Velox to match upstream Presto + if: ${{ inputs.build_upstream }} + run: | + pushd velox-testing/presto/scripts + ./checkout_velox_to_match_presto.sh + popd - name: Download Presto Dependencies Container Image if: ${{ inputs.presto_worker_type != 'java' }} env: From e011b5aa8c42b63a83c214d1125bb226eb9fb36e Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Thu, 23 Oct 2025 11:10:43 -0700 Subject: [PATCH 11/23] Make script executable --- presto/scripts/checkout_velox_to_match_presto.sh | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 presto/scripts/checkout_velox_to_match_presto.sh diff --git a/presto/scripts/checkout_velox_to_match_presto.sh b/presto/scripts/checkout_velox_to_match_presto.sh old mode 100644 new mode 100755 From 842f7b9760009c55123b3222f29a6251ccf9b514 Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Thu, 23 Oct 2025 11:20:56 -0700 Subject: [PATCH 12/23] Match branch --- .github/workflows/presto-test-composite.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/presto-test-composite.yml b/.github/workflows/presto-test-composite.yml index 4ad35aa1..5274fcce 100644 --- a/.github/workflows/presto-test-composite.yml +++ b/.github/workflows/presto-test-composite.yml @@ -62,6 +62,9 @@ jobs: repository: ${{ inputs.velox_repository }} ref: ${{ inputs.velox_commit }} path: velox + - name: Report requested Velox Testing commit + run: | + echo "Using Velox Testing commit/ref: ${{ inputs.velox_testing_commit }}" - name: Checkout Velox Testing uses: actions/checkout@v4 with: From 650770818ec8e4b457c39ce03f826966c0d8c505 Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Thu, 23 Oct 2025 11:26:47 -0700 Subject: [PATCH 13/23] Up one more level --- presto/scripts/checkout_velox_to_match_presto.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/presto/scripts/checkout_velox_to_match_presto.sh b/presto/scripts/checkout_velox_to_match_presto.sh index 6d211438..abba22ec 100755 --- a/presto/scripts/checkout_velox_to_match_presto.sh +++ b/presto/scripts/checkout_velox_to_match_presto.sh @@ -5,12 +5,12 @@ set -e echo "Checking out version of Velox which matches Presto pinned version" # get SHA of Presto Velox submodule -pushd ../../presto/presto-native-execution/velox +pushd ../../../presto/presto-native-execution/velox SHA=$(git rev-parse HEAD) popd # checkout sibling Velox to that SHA -pushd ../../velox +pushd ../../../velox git checkout ${SHA} popd From 8b6b6158ccf2c8aada4ea3fd4e27e82f518929ad Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Thu, 23 Oct 2025 11:36:56 -0700 Subject: [PATCH 14/23] Debug --- presto/scripts/checkout_velox_to_match_presto.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/presto/scripts/checkout_velox_to_match_presto.sh b/presto/scripts/checkout_velox_to_match_presto.sh index abba22ec..76378655 100755 --- a/presto/scripts/checkout_velox_to_match_presto.sh +++ b/presto/scripts/checkout_velox_to_match_presto.sh @@ -6,11 +6,17 @@ echo "Checking out version of Velox which matches Presto pinned version" # get SHA of Presto Velox submodule pushd ../../../presto/presto-native-execution/velox +pwd +ls -l SHA=$(git rev-parse HEAD) popd +echo "Presto pinned version is ${SHA}" + # checkout sibling Velox to that SHA pushd ../../../velox +pwd +ls -l git checkout ${SHA} popd From b3b8b231a1b02de14df763cfebc213995a141222 Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Thu, 23 Oct 2025 11:37:54 -0700 Subject: [PATCH 15/23] Skip for Java build --- .github/workflows/presto-test-composite.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/presto-test-composite.yml b/.github/workflows/presto-test-composite.yml index 5274fcce..62273492 100644 --- a/.github/workflows/presto-test-composite.yml +++ b/.github/workflows/presto-test-composite.yml @@ -72,7 +72,7 @@ jobs: ref: ${{ inputs.velox_testing_commit }} path: velox-testing - name: Change Velox to match upstream Presto - if: ${{ inputs.build_upstream }} + if: ${{ inputs.build_upstream && inputs.presto_worker_type != 'java' }} run: | pushd velox-testing/presto/scripts ./checkout_velox_to_match_presto.sh From 3e5c2576edaa21b84fce3004da39b2c90d6b8689 Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Thu, 23 Oct 2025 11:44:04 -0700 Subject: [PATCH 16/23] Run make submodules first! --- presto/scripts/checkout_velox_to_match_presto.sh | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/presto/scripts/checkout_velox_to_match_presto.sh b/presto/scripts/checkout_velox_to_match_presto.sh index 76378655..c6ce2e57 100755 --- a/presto/scripts/checkout_velox_to_match_presto.sh +++ b/presto/scripts/checkout_velox_to_match_presto.sh @@ -5,9 +5,10 @@ set -e echo "Checking out version of Velox which matches Presto pinned version" # get SHA of Presto Velox submodule -pushd ../../../presto/presto-native-execution/velox -pwd -ls -l +# must do make submodules first as sub-module clone is not automatic +pushd ../../../presto/presto-native-execution +make submodules +cd velox SHA=$(git rev-parse HEAD) popd @@ -15,8 +16,6 @@ echo "Presto pinned version is ${SHA}" # checkout sibling Velox to that SHA pushd ../../../velox -pwd -ls -l git checkout ${SHA} popd From 3aa167addd50aaa337ee7998a93013b40d26af6b Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Thu, 23 Oct 2025 11:52:49 -0700 Subject: [PATCH 17/23] Remove velox_testing_commit --- .github/workflows/presto-test-composite.yml | 11 ++--------- .github/workflows/presto-test.yml | 9 --------- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/.github/workflows/presto-test-composite.yml b/.github/workflows/presto-test-composite.yml index 62273492..a88df31c 100644 --- a/.github/workflows/presto-test-composite.yml +++ b/.github/workflows/presto-test-composite.yml @@ -23,11 +23,6 @@ on: type: string required: false default: 'main' - velox_testing_commit: - description: 'Velox Testing Commit SHA or Branch' - type: string - required: false - default: 'main' presto_worker_type: description: 'Type of Presto Worker to use' type: string @@ -62,14 +57,12 @@ jobs: repository: ${{ inputs.velox_repository }} ref: ${{ inputs.velox_commit }} path: velox - - name: Report requested Velox Testing commit - run: | - echo "Using Velox Testing commit/ref: ${{ inputs.velox_testing_commit }}" - name: Checkout Velox Testing + # checkout branch github.ref_name to automatically match the workflow branch uses: actions/checkout@v4 with: repository: rapidsai/velox-testing - ref: ${{ inputs.velox_testing_commit }} + ref: ${{ github.ref_name }} path: velox-testing - name: Change Velox to match upstream Presto if: ${{ inputs.build_upstream && inputs.presto_worker_type != 'java' }} diff --git a/.github/workflows/presto-test.yml b/.github/workflows/presto-test.yml index 806cd38d..709439bc 100644 --- a/.github/workflows/presto-test.yml +++ b/.github/workflows/presto-test.yml @@ -23,11 +23,6 @@ on: type: string required: false default: 'main' - velox_testing_commit: &velox_testing_commit - description: 'Velox Testing Commit SHA or Branch' - type: string - required: false - default: 'main' run_java_tests: &run_java_tests description: 'Run tests with Java Worker' type: boolean @@ -55,7 +50,6 @@ on: presto_commit: *presto_commit velox_repository: *velox_repository velox_commit: *velox_commit - velox_testing_commit: *velox_testing_commit run_java_tests: *run_java_tests run_cpu_tests: *run_cpu_tests run_gpu_tests: *run_gpu_tests @@ -73,7 +67,6 @@ jobs: presto_commit: ${{ inputs.presto_commit }} velox_repository: ${{ inputs.velox_repository }} velox_commit: ${{ inputs.velox_commit }} - velox_testing_commit: ${{ inputs.velox_testing_commit }} set_velox_backward_compatible: false build_upstream: ${{ inputs.build_upstream }} secrets: inherit @@ -87,7 +80,6 @@ jobs: presto_commit: ${{ inputs.presto_commit }} velox_repository: ${{ inputs.velox_repository }} velox_commit: ${{ inputs.velox_commit }} - velox_testing_commit: ${{ inputs.velox_testing_commit }} set_velox_backward_compatible: ${{ inputs.set_velox_backward_compatible }} build_upstream: ${{ inputs.build_upstream }} secrets: inherit @@ -101,7 +93,6 @@ jobs: presto_commit: ${{ inputs.presto_commit }} velox_repository: ${{ inputs.velox_repository }} velox_commit: ${{ inputs.velox_commit }} - velox_testing_commit: ${{ inputs.velox_testing_commit }} set_velox_backward_compatible: ${{ inputs.set_velox_backward_compatible }} build_upstream: ${{ inputs.build_upstream }} secrets: inherit From be603298177365f2521ba4f58030e66ef6494dc8 Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Thu, 23 Oct 2025 11:53:37 -0700 Subject: [PATCH 18/23] More debug --- presto/scripts/checkout_velox_to_match_presto.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/presto/scripts/checkout_velox_to_match_presto.sh b/presto/scripts/checkout_velox_to_match_presto.sh index c6ce2e57..418e34cd 100755 --- a/presto/scripts/checkout_velox_to_match_presto.sh +++ b/presto/scripts/checkout_velox_to_match_presto.sh @@ -16,6 +16,9 @@ echo "Presto pinned version is ${SHA}" # checkout sibling Velox to that SHA pushd ../../../velox +pwd +ls -l +git rev-parse HEAD git checkout ${SHA} popd From 0f5e509645a4b5ea7ef0c4695a00b0bc7026e5ef Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Thu, 23 Oct 2025 12:00:20 -0700 Subject: [PATCH 19/23] Add git fsck --- presto/scripts/checkout_velox_to_match_presto.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/presto/scripts/checkout_velox_to_match_presto.sh b/presto/scripts/checkout_velox_to_match_presto.sh index 418e34cd..65857b82 100755 --- a/presto/scripts/checkout_velox_to_match_presto.sh +++ b/presto/scripts/checkout_velox_to_match_presto.sh @@ -16,8 +16,7 @@ echo "Presto pinned version is ${SHA}" # checkout sibling Velox to that SHA pushd ../../../velox -pwd -ls -l +git fsck git rev-parse HEAD git checkout ${SHA} popd From 32d7c1f3d95e86bd6f01dccca7404d73085e048c Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Thu, 23 Oct 2025 12:05:23 -0700 Subject: [PATCH 20/23] Yet more debug --- presto/scripts/checkout_velox_to_match_presto.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/presto/scripts/checkout_velox_to_match_presto.sh b/presto/scripts/checkout_velox_to_match_presto.sh index 65857b82..1ebbbc05 100755 --- a/presto/scripts/checkout_velox_to_match_presto.sh +++ b/presto/scripts/checkout_velox_to_match_presto.sh @@ -16,8 +16,11 @@ echo "Presto pinned version is ${SHA}" # checkout sibling Velox to that SHA pushd ../../../velox +echo "DEBUG: running git fsck" git fsck +echo "DEBUG: running git rev-parse HEAD" git rev-parse HEAD +echo "DEBUG: running git checkout" git checkout ${SHA} popd From 25acf453a30a78ad0e0bc81d9654e47461af8402 Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Thu, 23 Oct 2025 12:40:48 -0700 Subject: [PATCH 21/23] New technique --- .github/workflows/presto-test-composite.yml | 24 ++++++++++------- .../scripts/checkout_velox_to_match_presto.sh | 27 ------------------- 2 files changed, 14 insertions(+), 37 deletions(-) delete mode 100755 presto/scripts/checkout_velox_to_match_presto.sh diff --git a/.github/workflows/presto-test-composite.yml b/.github/workflows/presto-test-composite.yml index a88df31c..9cf7d49a 100644 --- a/.github/workflows/presto-test-composite.yml +++ b/.github/workflows/presto-test-composite.yml @@ -50,26 +50,30 @@ jobs: repository: ${{ inputs.presto_repository }} ref: ${{ inputs.presto_commit }} path: presto + - name: Get Presto Pinned Velox Version + id: get-presto-pinned-velox-version + if: ${{ inputs.build_upstream && inputs.presto_worker_type != 'java' }} + run: | + pushd presto/presto-native-execution + make submodules + cd velox + PRESTO_PINNED_VELOX_SHA=$(git rev-parse HEAD) + echo "Found Presto pinned Velox SHA: ${PRESTO_PINNED_VELOX_SHA}" + echo "PRESTO_PINNED_VELOX_SHA=${PRESTO_PINNED_VELOX_SHA}" >> $GITHUB_OUTPUT + popd - name: Checkout Velox if: ${{ inputs.presto_worker_type != 'java' }} uses: actions/checkout@v4 with: - repository: ${{ inputs.velox_repository }} - ref: ${{ inputs.velox_commit }} + repository: ${{ inputs.build_upstream && 'facebookincubator/velox' || inputs.velox_repository }} + ref: ${{ inputs.build_upstream && steps.get-presto-pinned-velox-version.outputs.PRESTO_PINNED_VELOX_SHA || inputs.velox_commit }} path: velox - name: Checkout Velox Testing - # checkout branch github.ref_name to automatically match the workflow branch uses: actions/checkout@v4 with: repository: rapidsai/velox-testing - ref: ${{ github.ref_name }} + ref: ${{ github.ref_name }} # automatically match the workflow branch path: velox-testing - - name: Change Velox to match upstream Presto - if: ${{ inputs.build_upstream && inputs.presto_worker_type != 'java' }} - run: | - pushd velox-testing/presto/scripts - ./checkout_velox_to_match_presto.sh - popd - name: Download Presto Dependencies Container Image if: ${{ inputs.presto_worker_type != 'java' }} env: diff --git a/presto/scripts/checkout_velox_to_match_presto.sh b/presto/scripts/checkout_velox_to_match_presto.sh deleted file mode 100755 index 1ebbbc05..00000000 --- a/presto/scripts/checkout_velox_to_match_presto.sh +++ /dev/null @@ -1,27 +0,0 @@ -#!/bin/bash - -set -e - -echo "Checking out version of Velox which matches Presto pinned version" - -# get SHA of Presto Velox submodule -# must do make submodules first as sub-module clone is not automatic -pushd ../../../presto/presto-native-execution -make submodules -cd velox -SHA=$(git rev-parse HEAD) -popd - -echo "Presto pinned version is ${SHA}" - -# checkout sibling Velox to that SHA -pushd ../../../velox -echo "DEBUG: running git fsck" -git fsck -echo "DEBUG: running git rev-parse HEAD" -git rev-parse HEAD -echo "DEBUG: running git checkout" -git checkout ${SHA} -popd - -echo "Velox checked out to ${SHA}" From 8e44bce1108bb7e9cbb2f8c811fad18ac8a5928f Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Fri, 24 Oct 2025 14:36:09 -0700 Subject: [PATCH 22/23] Dump status and logs on timeout --- presto/scripts/common_functions.sh | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/presto/scripts/common_functions.sh b/presto/scripts/common_functions.sh index 9c7cc4df..1ad46ded 100644 --- a/presto/scripts/common_functions.sh +++ b/presto/scripts/common_functions.sh @@ -14,6 +14,22 @@ # See the License for the specific language governing permissions and # limitations under the License. +function print_presto_container_status_and_logs() { + # get all Presto containers + export CONTAINERS = $(docker ps -a --format '{{.Names}}') + + # log their status + echo "Docker Container Status:" + echo ${CONTAINERS} + + # dump each container's log + echo "Docker Logs:" + while IFS= read -r CONTAINER; do + echo "############ Log for Container '${CONTAINER}' ############" + docker logs ${CONTAINER} + done <<< ${CONTAINERS} +} + function wait_for_worker_node_registration() { trap "rm -rf node_response.json" RETURN @@ -28,6 +44,7 @@ function wait_for_worker_node_registration() { (( $(jq length node_response.json) > 0 )); do if (( $retry_count >= $MAX_RETRIES )); then echo "Error: Worker node not registered after 60s. Exiting." + print_presto_container_status_and_logs exit 1 fi sleep 5 From f14e59d2d2710874a5818a99a057b6685d4e4c4f Mon Sep 17 00:00:00 2001 From: Simon Eves Date: Fri, 24 Oct 2025 15:29:52 -0700 Subject: [PATCH 23/23] Fix --- presto/scripts/common_functions.sh | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/presto/scripts/common_functions.sh b/presto/scripts/common_functions.sh index 1ad46ded..55edff37 100644 --- a/presto/scripts/common_functions.sh +++ b/presto/scripts/common_functions.sh @@ -15,19 +15,18 @@ # limitations under the License. function print_presto_container_status_and_logs() { - # get all Presto containers - export CONTAINERS = $(docker ps -a --format '{{.Names}}') - - # log their status - echo "Docker Container Status:" - echo ${CONTAINERS} + # log container status (whether running or not) + echo "############ Docker Container Status ############" + docker ps -a # dump each container's log - echo "Docker Logs:" + echo "############ Docker Logs ############" + local CONTAINERS=$(docker ps -a --format '{{.Names}}') while IFS= read -r CONTAINER; do echo "############ Log for Container '${CONTAINER}' ############" docker logs ${CONTAINER} done <<< ${CONTAINERS} + echo "############ End of Docker Logs ############" } function wait_for_worker_node_registration() {