Skip to content

Docker build parameters#7370

Merged
monoid merged 7 commits intodevfrom
docker-build-parameters-more
Oct 10, 2025
Merged

Docker build parameters#7370
monoid merged 7 commits intodevfrom
docker-build-parameters-more

Conversation

@monoid
Copy link
Contributor

@monoid monoid commented Oct 9, 2025

  1. Add Docker ARGs TARGET_CPU and JEMALLOC_SYS_WITH_LG_PAGE;
  2. Document Docker ARGs in docs/DEVELOPMENT.md.

All Submissions:

  • Contributions should target the dev branch. Did you create your branch from dev?
  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

monoid added 3 commits October 9, 2025 11:00
@monoid
Add ARG TARGET_CPU to Dockerfile
b1b09d3
@monoid
Add ARG JEMALLOC_SYS_WITH_LG_PAGE to Dockerfile
f71542d 
It allows redefining jemalloc's page size for image targeting specific
ARM machines.
@monoid
Document Docker build parameters
c2ad835
@monoid monoid self-assigned this Oct 9, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 9, 2025 •
edited
Loading

📝 Walkthrough

Walkthrough

Adds two new Docker build arguments: TARGET_CPU and JEMALLOC_SYS_WITH_LG_PAGE. Dockerfile changes conditionally append -C target-cpu=<TARGET_CPU> to RUSTFLAGS in two build steps when TARGET_CPU is provided, export JEMALLOC_SYS_WITH_LG_PAGE into the build environment, and forward both args to the xx-cargo build invocations. Updates docs/DEVELOPMENT.md with a "Docker image build parameters" section documenting build-arg options (FEATURES, RUSTFLAGS, LINKER, TARGET_CPU, JEMALLOC_SYS_WITH_LG_PAGE, GPU) and sample docker build commands. No runtime code changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly captures the primary change of introducing Docker build parameters. It directly reflects the main modifications in both the Dockerfile and related documentation without any extraneous detail. Its concise phrasing makes the PR’s intent immediately clear to reviewers.
Description Check ✅ Passed The description clearly outlines both the addition of the two Docker ARGs and the corresponding documentation updates in DEVELOPMENT.md. It is directly related to the changes in the Dockerfile and developer guide and provides sufficient context for the review. The included checklist confirms adherence to contribution guidelines without introducing unrelated information.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch docker-build-parameters-more
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4aa2dd3 and 732dd38.

📒 Files selected for processing (2)
  • Dockerfile (3 hunks)
  • docs/DEVELOPMENT.md (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/DEVELOPMENT.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: lint
  • GitHub Check: e2e-tests
  • GitHub Check: test-consistency
  • GitHub Check: test-shard-snapshot-api-s3-minio
  • GitHub Check: integration-tests-consensus
  • GitHub Check: rust-tests (macos-latest)
  • GitHub Check: test-consensus-compose
  • GitHub Check: rust-tests (windows-latest)
  • GitHub Check: rust-tests (ubuntu-latest)
  • GitHub Check: rust-tests-no-rocksdb (ubuntu-latest)
  • GitHub Check: integration-tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai bot reviewed Oct 9, 2025
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Dockerfile (1)

120-124: Propagate TARGET_CPU and jemalloc env to the final build.

The second build RUN drops both TARGET_CPU and JEMALLOC_SYS_WITH_LG_PAGE, so the final xx-cargo build ignores these new knobs. Extend this command the same way as the earlier chef cook step:

 RUN PKG_CONFIG="/usr/bin/$(xx-info)-pkg-config" \
     PATH="$PATH:/opt/mold/bin" \
-    RUSTFLAGS="${LINKER:+-C link-arg=-fuse-ld=}$LINKER $RUSTFLAGS" \
+    RUSTFLAGS="${LINKER:+-C link-arg=-fuse-ld=}$LINKER ${TARGET_CPU:+-C target-cpu=}$TARGET_CPU $RUSTFLAGS" \
+    JEMALLOC_SYS_WITH_LG_PAGE="${JEMALLOC_SYS_WITH_LG_PAGE}" \
     xx-cargo build --profile $PROFILE ${FEATURES:+--features} $FEATURES --features=stacktrace ${GPU:+--features=gpu} --bin qdrant \
     && PROFILE_DIR=$(if [ "$PROFILE" = dev ]; then echo debug; else echo $PROFILE; fi) \
     && mv target/$(xx-cargo --print-target-triple)/$PROFILE_DIR/qdrant /qdrant/qdrant
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3459653 and c2ad835.

📒 Files selected for processing (2)
  • Dockerfile (2 hunks)
  • docs/DEVELOPMENT.md (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: test-consensus-compose
  • GitHub Check: integration-tests-consensus
  • GitHub Check: test-consistency
  • GitHub Check: integration-tests
  • GitHub Check: test-shard-snapshot-api-s3-minio
  • GitHub Check: e2e-tests
  • GitHub Check: rust-tests (macos-latest)
  • GitHub Check: rust-tests (windows-latest)
  • GitHub Check: rust-tests (ubuntu-latest)
  • GitHub Check: rust-tests-no-rocksdb (ubuntu-latest)
  • GitHub Check: lint

@monoid monoid changed the title Docker build parameters more Docker build parameters Oct 9, 2025
monoid added 2 commits October 9, 2025 13:06
@monoid
Fix docs
1f85347 
jemalloc page size var name was incorrect.
@monoid
Fix build call params
4aa2dd3
ffuugoo
ffuugoo approved these changes Oct 9, 2025
View reviewed changes
monoid added 2 commits October 9, 2025 15:03
@monoid
Fix jemalloc compilation
4c3c49e 
It cannot handle empty `JEMALLOC_SYS_WITH_LG_PAGE` value.  So we have to
skip it completely if it is empty.
@monoid
Extend documentation
732dd38
ffuugoo
ffuugoo reviewed Oct 9, 2025
View reviewed changes
@monoid monoid merged commit 4a732a1 into dev Oct 10, 2025
15 checks passed
@monoid monoid deleted the docker-build-parameters-more branch October 10, 2025 09:35
timvisee pushed a commit that referenced this pull request Nov 14, 2025
@monoid @timvisee
Docker build parameters (#7370)
494880b 
* Add ARG `TARGET_CPU` to Dockerfile

* Add ARG `JEMALLOC_SYS_WITH_LG_PAGE` to Dockerfile

It allows redefining jemalloc's page size for image targeting specific
ARM machines.

Refs #3831.

* Document Docker build parameters
@timvisee timvisee mentioned this pull request Nov 14, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ffuugoo ffuugoo ffuugoo approved these changes

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@timvisee timvisee timvisee approved these changes

@generall generall Awaiting requested review from generall

@IvanPleshkov IvanPleshkov Awaiting requested review from IvanPleshkov

Assignees

@monoid monoid

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@monoid @timvisee @ffuugoo