-
Notifications
You must be signed in to change notification settings - Fork 636
perf(docker): reuse runtime layers and scope Maven builds #3194
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,14 +28,14 @@ ARG MAVEN_ARGS | |
| ARG SOURCE_REVISION=local | ||
|
|
||
| RUN --mount=type=cache,id=hugegraph-maven-${SOURCE_REVISION},target=/root/.m2,sharing=locked \ | ||
| mvn install $MAVEN_ARGS -e -B -ntp -Dmaven.test.skip=true -Dmaven.javadoc.skip=true \ | ||
| mvn install -pl hugegraph-server/hugegraph-dist,hugegraph-pd/hg-pd-dist,hugegraph-store/hg-store-dist \ | ||
| -am $MAVEN_ARGS -e -B -ntp -Dmaven.test.skip=true -Dmaven.javadoc.skip=true \ | ||
| && rm ./hugegraph-server/*.tar.gz ./hugegraph-pd/*.tar.gz ./hugegraph-store/*.tar.gz | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 The reactor is scoped but the build context is not, so the caching win is smaller than the benchmark suggests. A follow-up rather than a change here, but worth recording. One caveat for whoever picks it up: these directories cannot be ignored wholesale, because the root pom lists them in |
||
|
|
||
| # 2nd stage: runtime env | ||
| # Note: ZGC (The Z Garbage Collector) is only supported on ARM-Mac with java > 13 | ||
| FROM eclipse-temurin:11-jre-jammy | ||
|
|
||
| COPY --from=build /pkg/hugegraph-server/apache-hugegraph-server-*/ /hugegraph-server/ | ||
| LABEL maintainer="HugeGraph Docker Maintainers <dev@hugegraph.apache.org>" | ||
|
|
||
| # TODO: use g1gc or zgc as default | ||
|
|
@@ -46,7 +46,7 @@ ENV JAVA_OPTS="-XX:+UnlockExperimentalVMOptions -XX:+UseContainerSupport -XX:Max | |
|
|
||
| WORKDIR /hugegraph-server/ | ||
|
|
||
| # 1. Install runtime dependencies and configure server | ||
| # 1. Install runtime dependencies | ||
| # Note: iproute2 provides `ss`, which the bin/util.sh port preflight needs. The | ||
| # jammy base image ships neither ss nor netstat, so without it the preflight is | ||
| # permanently inconclusive and a duplicate start is no longer caught. It | ||
|
|
@@ -59,8 +59,10 @@ RUN apt-get -q update \ | |
| iproute2 \ | ||
| vim \ | ||
| && apt-get clean \ | ||
| && rm -rf /var/lib/apt/lists/* \ | ||
| && sed -i "s/^restserver.url.*$/restserver.url=http:\/\/0.0.0.0:8080/g" ./conf/rest-server.properties | ||
| && rm -rf /var/lib/apt/lists/* | ||
|
|
||
| COPY --from=build /pkg/hugegraph-server/apache-hugegraph-server-*/ /hugegraph-server/ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This only bites on a registry-cache flow, and that flow is out of tree: Could you add a documented way to force a reinstall, for example |
||
| RUN sed -i "s/^restserver.url.*$/restserver.url=http:\/\/0.0.0.0:8080/g" ./conf/rest-server.properties | ||
|
|
||
| # 2. Init docker script | ||
| COPY hugegraph-server/hugegraph-dist/docker/scripts/remote-connect.groovy ./scripts | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -28,18 +28,14 @@ ARG MAVEN_ARGS | |||||||||||||||
| ARG SOURCE_REVISION=local | ||||||||||||||||
|
|
||||||||||||||||
| RUN --mount=type=cache,id=hugegraph-maven-${SOURCE_REVISION},target=/root/.m2,sharing=locked \ | ||||||||||||||||
| mvn install $MAVEN_ARGS -e -B -ntp -Dmaven.test.skip=true -Dmaven.javadoc.skip=true \ | ||||||||||||||||
| mvn install -pl hugegraph-server/hugegraph-dist,hugegraph-pd/hg-pd-dist,hugegraph-store/hg-store-dist \ | ||||||||||||||||
| -am $MAVEN_ARGS -e -B -ntp -Dmaven.test.skip=true -Dmaven.javadoc.skip=true \ | ||||||||||||||||
| && rm ./hugegraph-server/*.tar.gz ./hugegraph-pd/*.tar.gz ./hugegraph-store/*.tar.gz | ||||||||||||||||
|
|
||||||||||||||||
| # 2nd stage: runtime env | ||||||||||||||||
| # Note: ZGC (The Z Garbage Collector) is only supported on ARM-Mac with java > 13 | ||||||||||||||||
| FROM eclipse-temurin:11-jre-jammy | ||||||||||||||||
|
|
||||||||||||||||
| COPY --from=build /pkg/hugegraph-server/apache-hugegraph-server-*/ /hugegraph-server/ | ||||||||||||||||
| # remove hugegraph.properties and rename hstore.properties.template for default hstore backend | ||||||||||||||||
| RUN cd /hugegraph-server/conf/graphs \ | ||||||||||||||||
| && rm hugegraph.properties && mv hstore.properties.template hugegraph.properties | ||||||||||||||||
|
|
||||||||||||||||
| LABEL maintainer="HugeGraph Docker Maintainers <dev@hugegraph.apache.org>" | ||||||||||||||||
|
|
||||||||||||||||
| # TODO: use g1gc or zgc as default | ||||||||||||||||
|
|
@@ -48,7 +44,7 @@ ENV JAVA_OPTS="-XX:+UnlockExperimentalVMOptions -XX:+UseContainerSupport -XX:Max | |||||||||||||||
|
|
||||||||||||||||
| WORKDIR /hugegraph-server/ | ||||||||||||||||
|
|
||||||||||||||||
| # 1. Install runtime dependencies and configure server | ||||||||||||||||
| # 1. Install runtime dependencies | ||||||||||||||||
| # Note: iproute2 provides `ss`, which the bin/util.sh port preflight needs. The | ||||||||||||||||
| # jammy base image ships neither ss nor netstat, so without it the preflight is | ||||||||||||||||
| # permanently inconclusive and a duplicate start is no longer caught. It | ||||||||||||||||
|
|
@@ -61,8 +57,13 @@ RUN apt-get -q update \ | |||||||||||||||
| iproute2 \ | ||||||||||||||||
| vim \ | ||||||||||||||||
| && apt-get clean \ | ||||||||||||||||
| && rm -rf /var/lib/apt/lists/* \ | ||||||||||||||||
| && sed -i "s/^restserver.url.*$/restserver.url=http:\/\/0.0.0.0:8080/g" ./conf/rest-server.properties | ||||||||||||||||
| && rm -rf /var/lib/apt/lists/* | ||||||||||||||||
|
|
||||||||||||||||
| COPY --from=build /pkg/hugegraph-server/apache-hugegraph-server-*/ /hugegraph-server/ | ||||||||||||||||
| # remove hugegraph.properties and rename hstore.properties.template for default hstore backend | ||||||||||||||||
| RUN cd /hugegraph-server/conf/graphs \ | ||||||||||||||||
| && rm hugegraph.properties && mv hstore.properties.template hugegraph.properties | ||||||||||||||||
| RUN sed -i "s/^restserver.url.*$/restserver.url=http:\/\/0.0.0.0:8080/g" ./conf/rest-server.properties | ||||||||||||||||
|
Comment on lines
+64
to
+66
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Both of these
Suggested change
The comment on line 63 then covers only part of what the merged |
||||||||||||||||
|
|
||||||||||||||||
| # 2. Init docker script | ||||||||||||||||
| COPY hugegraph-server/hugegraph-dist/docker/scripts/remote-connect.groovy ./scripts | ||||||||||||||||
|
|
||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 The module list is hardcoded ahead of
$MAVEN_ARGS, which is the only Maven knobdocker/bake.hclexposes. Maven accumulates repeated-plvalues rather than letting a later one replace an earlier one, soMAVEN_ARGScan still add modules or drop them with a!prefix, but it can no longer set the scope outright. The PR description notes that the fork experiments "enabled the same module selection throughMAVEN_ARGS"; that route closes here.Drift between the four copies is already covered by the
docker-bake-checkjob in.github/workflows/docker-build-ci.yml, so this is only about overridability.Suggested change: hoist the list into a build arg beside the existing
ARG MAVEN_ARGSand pass it through_common.argsindocker/bake.hcl.with the command becoming
mvn install -pl "$MAVEN_PROJECTS" -am $MAVEN_ARGS .... All four files stay byte-identical, so the CI identity check still passes.