Skip to content

[VL] Use the dynamic-link image for the dev container - #12778

Merged
philo-he merged 1 commit into
apache:mainfrom
felipepessoto:devcontainer-dynamic-link
Aug 22, 2026
Merged

[VL] Use the dynamic-link image for the dev container#12778
philo-he merged 1 commit into
apache:mainfrom
felipepessoto:devcontainer-dynamic-link

Conversation

@felipepessoto

@felipepessoto felipepessoto commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

What changes are proposed in this pull request?

Follow-up to #12125, which added the Dev Container. As configured there, the container runs dev/ci-velox-buildstatic-centos-9.sh as its postCreateCommand on top of apache/gluten:vcpkg-centos-9. That is a CI packaging script, and running it at container creation is a poor fit:

  • It sets export NUM_THREADS=2. That is right for a CI runner and wrong for a workstation: it pins the whole Velox build to 2-way parallelism no matter how many cores the container has. This is the main reason it runs for hours.
  • It also enables --enable_s3 --enable_gcs --enable_hdfs --enable_abfs and static linking, which serve release packaging rather than day-to-day development.
  • The static link step is memory hungry; Velox.md recommends 64 GB.
  • vcpkg caches per port, so a moved checkout normally rebuilds only the ports it changed plus their dependents. But the ABI hash also covers the toolchain, so an environment change invalidates every port at once and vcpkg does not explain why it started rebuilding. In a postCreateCommand that looks like a container hanging with no output.
  • A postCreateCommand that runs for hours stalls container creation and leaves a half-built tree behind whenever the editor disconnects or a Codespace times out.

Static linking exists to produce portable release jars, which a developer never needs. This switches to apache/gluten:centos-9-jdk8, the dynamically linked image the cpp/UDF test job already runs on: it ships Velox's third-party libraries and Arrow under /usr/local, a pre-warmed Maven repository, and the Spark distributions the unit tests need under /opt/shims. The vcpkg image never runs install-spark-resources.sh, so gluten-ut cannot run there at all, and it ships JDK 17 only.

The native build is no longer automatic. postCreateCommand now runs .devcontainer/post-create.sh, which:

  • installs JDK 17 alongside JDK 8, needed by Spark 4.x and by the Spark unit tests, which CI runs on JDK 17;
  • installs clang-format 15, which dev/format-cpp-code.sh requires under that exact name and tries to install with apt, and the regex module dev/check.py imports;
  • sizes NUM_THREADS from memory rather than cores, and exports it;
  • prints the build and test commands to run.

The NUM_THREADS sizing is what keeps a build from being OOM-killed. The build scripts default it to nproc --ignore=2 while Velox's heavier translation units peak at roughly 3.5 GB resident each, so on a 32-core, 62 GB container the default asks for 30 jobs and about 100 GB, and the OOM killer takes down the build and the container with it. Reserving a few GB and allowing ~4 GB per job gives 13 jobs there. The value is recomputed per shell, so it follows a resized Codespace.

devcontainer.json also:

  • puts GCC 12 on PATH through remoteEnv, since the image's default GCC 11 cannot compile Velox's C++20 sources and the build scripts do not enable the toolset themselves;
  • declares hostRequirements, where the 64 GB storage floor is what rules out machine types too small for the image plus the Velox build tree;
  • keeps the ccache and the Maven repository in named volumes so they survive a rebuild;
  • excludes the build trees from the editor's file watcher, which otherwise exhausts inotify handles once a build has produced more than 10 GB of artifacts.

docs/developers/dev-container.md is rewritten to match, including a warning that the unit test command must not use -pl gluten-ut: gluten-ut is an aggregator POM, -pl does not select its children, and the build then finishes in seconds with no suite in the reactor.

How was this patch tested?

Dev Container configuration and documentation only; no product code is touched.

Validated from inside the container that the documented commands work end to end:

  • Clean build after wiping cpp/build, ep/build-velox/build/velox_ep/_build and ep/_ep, using the documented command: BUILD SUCCESS, producing cpp/build/releases/libgluten.so, libvelox.so and package/target/gluten-package-1.8.0-SNAPSHOT-3.5.jar.
  • cd cpp/build && ctest: 5675 of 5675 tests passed, confirming --build_tests=ON produces working test binaries.
  • Memory: the computed parallelism held a 41 GB peak of 62 GB with 20 GB still available, where the core-count default was OOM-killed at 707 of 961 Velox objects.
  • The corrected unit test command puts Gluten Unit Test Spark35 in the Maven reactor, where the previous -pl gluten-ut form selected only the aggregator POM and ran no suite.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: GitHub Copilot CLI claude-opus-5

Copilot AI lite review requested due to automatic review settings August 14, 2026 20:49
@github-actions github-actions Bot added the DOCS label Aug 14, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates Gluten’s Velox dev container to use the dynamically linked CentOS 9 image and moves setup work into a lightweight post-create script, so container creation is fast and doesn’t trigger a long native build. Documentation is rewritten to match the new workflow and provide correct build/test commands and sizing guidance.

Changes:

  • Switch .devcontainer/devcontainer.json from the static-link vcpkg image to apache/gluten:centos-9-jdk8, add host requirements, mounts, GCC toolset env, and watcher excludes.
  • Add .devcontainer/post-create.sh to install missing dev tools (JDK 17, clang-format 15, regex), size/export NUM_THREADS, and print recommended commands.
  • Rewrite docs/developers/dev-container.md to document the new image choice, non-automatic native build, and correct build/test invocations.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
docs/developers/dev-container.md Rewrites dev-container docs to match the dynamic-link image workflow and correct commands/constraints.
.devcontainer/post-create.sh New post-create setup script for tool installation, NUM_THREADS sizing, and guidance output.
.devcontainer/devcontainer.json Switches devcontainer to dynamic image and adds env/mounts/hostRequirements and editor excludes.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .devcontainer/post-create.sh
Comment thread .devcontainer/post-create.sh
Copilot AI review requested due to automatic review settings August 14, 2026 20:59
@felipepessoto
felipepessoto force-pushed the devcontainer-dynamic-link branch from ca8918e to cf01f2e Compare August 14, 2026 20:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@felipepessoto

Copy link
Copy Markdown
Contributor Author

@philo-he, @zhouyuan you have reviewed the first version, when you have a chance could you check these improvements?

@philo-he

Copy link
Copy Markdown
Member

vcpkg reuses pre-built binaries only when the manifest in the checkout hashes identically to the image's. The images are rebuilt weekly from main, so a checkout that has moved rebuilds all ~144 ports from source, OpenSSL, Boost, gRPC and the AWS and Azure SDKs included.

@felipepessoto, according to my understanding, vcpkg maintains its local binary cache per port, so it usually skips building unchanged dependencies and only rebuilds modified ones. Exceptional cases do happen, for instance, after a GCC upgrade, cached binaries won't match the output of the new toolchain, but this is relatively rare.

@FelixYBW

Copy link
Copy Markdown
Contributor

vcpkg reuses pre-built binaries only when the manifest in the checkout hashes identically to the image's. The images are rebuilt weekly from main, so a checkout that has moved rebuilds all ~144 ports from source, OpenSSL, Boost, gRPC and the AWS and Azure SDKs included.

@felipepessoto, according to my understanding, vcpkg maintains its local binary cache per port, so it usually skips building unchanged dependencies and only rebuilds modified ones. Exceptional cases do happen, for instance, after a GCC upgrade, cached binaries won't match the output of the new toolchain, but this is relatively rare.

Yes. We noted that vcpkg is a bit sensitive to the system environment. The problem is that once the environment is changed, vcpkg silently rebuilds the packages but doesn't show the reason. The only way is to try a fresh image and then add the script one by one.

postCreateCommand ran dev/ci-velox-buildstatic-centos-9.sh on top of
apache/gluten:vcpkg-centos-9. That is a CI packaging script, and running it
at container creation is a poor fit:

- It sets NUM_THREADS=2, which is right for a CI runner and wrong for a
  workstation: the whole Velox build is pinned to 2-way parallelism no
  matter how many cores the container has. This is the main reason it runs
  for hours.
- It also enables S3, GCS, HDFS, ABFS and static linking, which serve
  release packaging rather than day-to-day development.
- The static link step is memory hungry; Velox.md recommends 64 GB.
- vcpkg caches per port, so a moved checkout normally rebuilds only what it
  changed, but the ABI hash also covers the toolchain: an environment change
  invalidates every port at once and vcpkg does not say why. In a
  postCreateCommand that looks like a container hanging with no output.
- A postCreateCommand that runs for hours stalls container creation and
  leaves a half-built tree behind whenever the editor disconnects or a
  Codespace times out.

Static linking exists to produce portable release jars, which a developer
never needs. Switch to apache/gluten:centos-9-jdk8, the dynamically linked
image the cpp/UDF test job already runs on: it ships Velox's third-party
libraries and Arrow under /usr/local, a pre-warmed Maven repository, and the
Spark distributions the unit tests need under /opt/shims. The vcpkg image
never runs install-spark-resources.sh, so gluten-ut cannot run there at all,
and it ships JDK 17 only.

The native build is no longer automatic. postCreateCommand now runs
.devcontainer/post-create.sh, which:

- installs JDK 17 alongside JDK 8, needed by Spark 4.x and by the Spark
  unit tests, which CI runs on JDK 17;
- installs clang-format 15, which dev/format-cpp-code.sh requires under that
  exact name and tries to install with apt, and the regex module
  dev/check.py imports;
- sizes NUM_THREADS from memory rather than cores, and exports it;
- prints the build and test commands to run.

The NUM_THREADS sizing is what keeps a build from being OOM-killed. The
build scripts default it to "nproc --ignore=2" while Velox's heavier
translation units peak at roughly 3.5 GB resident each, so on a 32-core,
62 GB container the default asks for 30 jobs and about 100 GB, and the OOM
killer takes down the build and the container with it. Reserving a few GB
and allowing ~4 GB per job gives 13 jobs there. The value is recomputed per
shell so it follows a resized Codespace.

devcontainer.json also:

- puts GCC 12 on PATH through remoteEnv, since the image's default GCC 11
  cannot compile Velox's C++20 sources and the build scripts do not enable
  the toolset themselves;
- declares hostRequirements, where the 64 GB storage floor is what rules out
  machine types too small for the image plus the Velox build tree;
- keeps the ccache and the Maven repository in named volumes so they survive
  a rebuild;
- excludes the build trees from the editor's file watcher, which otherwise
  exhausts inotify handles once a build has produced more than 10 GB of
  artifacts.

docs/developers/dev-container.md covers all of it, including that the test
command must not use "-pl gluten-ut": gluten-ut is an aggregator POM, "-pl"
does not select its children, and the build then finishes in seconds with no
suite in the reactor.

Generated-by: GitHub Copilot CLI claude-opus-5

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 18, 2026 19:37
@felipepessoto
felipepessoto force-pushed the devcontainer-dynamic-link branch from cf01f2e to 3992572 Compare August 18, 2026 19:37
@felipepessoto

Copy link
Copy Markdown
Contributor Author

@philo-he @FelixYBW you are both right, and my wording in the description was wrong. Thanks for catching it.

I had described vcpkg's caching as all-or-nothing against the manifest. It isn't: the ABI hash is computed per port, and Dockerfile.centos9-static-build bakes in VCPKG_BINARY_SOURCES=clear;files,/var/cache/vcpkg,readwrite, so that cache ships inside the image and unchanged ports are restored from it. Only changed ports and their dependents rebuild. I have corrected the PR description and the doc.

What is left after dropping that claim still argues for the change, and I think it is a better argument than the one I made:

  • dev/ci-velox-buildstatic-centos-9.sh sets export NUM_THREADS=2. That is the right choice for a CI runner and the wrong one for a workstation: it pins the whole Velox build to 2-way parallelism no matter how many cores the container has. That, not vcpkg, is the main reason it runs for hours as a postCreateCommand.
  • The script also turns on --enable_s3 --enable_gcs --enable_hdfs --enable_abfs and static linking, which serve release packaging rather than day-to-day development.
  • @FelixYBW's point is exactly the failure mode I hit and worded badly. Since the ABI hash covers the toolchain as well as the ports, an environment change invalidates everything at once, and vcpkg silently rebuilds without saying why. That is tolerable in CI, where there are logs and a time limit, but in postCreateCommand it looks like a container that hangs with no output, and a disconnect leaves a half-built tree behind.

Independently of vcpkg, apache/gluten:vcpkg-centos-9 is missing things a developer needs: install-spark-resources.sh is never run in that image, so there is no /opt/shims and gluten-ut cannot run, and it ships JDK 17 only, while the dynamic image ships JDK 8 and post-create.sh adds 17 for the whole Spark matrix.

Static linking is still the right default for release jars, and the doc keeps instructions for switching back to the vcpkg image when someone needs to reproduce a static-link problem. The comparison table now says the cache works per port and flags the toolchain caveat, rather than claiming a full rebuild.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

.devcontainer/post-create.sh:58

  • After pip3 install clang-format==15.0.7 succeeds, the script assumes clang-format is now discoverable on PATH and silently does nothing if it is not (leaving clang-format-15 missing with no warning). It would be safer to warn when clang-format cannot be found after a successful install, so failures don’t show up much later when ./dev/format-cpp-code.sh is run.
    if pip3 install --quiet --retries 1 clang-format==15.0.7; then
        CLANG_FORMAT=$(command -v clang-format)
        if [ -n "$CLANG_FORMAT" ]; then
            ln -sf "$CLANG_FORMAT" /usr/local/bin/clang-format-15
        fi

@zhouyuan

Copy link
Copy Markdown
Member

@felipepessoto would you do a check if we could adopt to use multiple dev containers here?
https://github.com/madebygps/multiple-dev-container-vscode

@felipepessoto

Copy link
Copy Markdown
Contributor Author

@felipepessoto would you do a check if we could adopt to use multiple dev containers here? https://github.com/madebygps/multiple-dev-container-vscode

@zhouyuan I checked it, and multiple configurations are supported. Alternatives can be placed under .devcontainer/<name>/devcontainer.json; VS Code presents a configuration picker, and Codespaces also lets users choose one when creating a codespace.

Multiple configurations do not change why this PR replaces the current default. The current configuration uses the static-link vcpkg image and runs a CI packaging script during postCreateCommand. That script pins the build to two threads, enables static linking and all cloud-filesystem dependencies, and can block container creation for hours. The image also lacks /opt/shims, so the Spark unit tests cannot run there. The dynamic-link configuration is the better default for daily development because it starts quickly, includes the Spark distributions, and leaves the native build under the developer's control.

For this PR, I suggest keeping the validated Velox dynamic-link configuration as the default. If a static-link alternative is needed, it should be redesigned for development rather than preserve the current configuration unchanged:

  • open quickly in the vcpkg image without an automatic native build;
  • print a manual build command with memory-aware parallelism, a selected Spark version, and optional cloud-filesystem dependencies;
  • use static-specific environment settings rather than copying VELOX_BUILD_SHARED=ON from the dynamic configuration;
  • either install /opt/shims for Spark tests or clearly identify the configuration as intended only for static packaging and reproduction.

I have not evaluated ClickHouse or GPU configurations because I am not familiar enough with those backends to define and validate their environments reliably. Those configurations would need input and ownership from contributors who work on those backends.

@philo-he philo-he left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks.

@felipepessoto

Copy link
Copy Markdown
Contributor Author

@zhouyuan, I'm tracking multi dev container here: #12850

@philo-he
philo-he merged commit 57a167a into apache:main Aug 22, 2026
5 checks passed
@felipepessoto
felipepessoto deleted the devcontainer-dynamic-link branch August 22, 2026 00:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants