Create Comet docker file #675

comphead · 2024-07-16T16:58:29Z

Which issue does this PR close?

Closes #643 .

Rationale for this change

Create a Comet dockerfile to create a Spark based image with integrated Comet jar

What changes are included in this PR?

How are these changes tested?

andygrove · 2024-07-16T17:32:53Z

kube/Dockerfile

+FROM apache/spark:3.4.2
+
+USER root
+
+# Installing JDK11 as the image comes with JRE
+RUN apt update \
+    && apt install -y git \
+    && apt install -y curl \
+    && apt install -y openjdk-11-jdk \
+    && apt clean
+
+RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y
+ENV PATH="/root/.cargo/bin:${PATH}"
+ENV RUSTFLAGS="-C debuginfo=line-tables-only -C incremental=false"
+ENV SPARK_VERSION=3.4
+ENV SCALA_VERSION=2.12
+
+# Pick the JDK instead of JRE to compile Comet
+RUN cd /opt \
+    && git clone https://github.com/apache/datafusion-comet.git \
+    && cd datafusion-comet \
+    && JAVA_HOME=$(readlink -f $(which javac) | sed "s/\/bin\/javac//") make release PROFILES="-Pspark-$SPARK_VERSION -Pscala-$SCALA_VERSION"
+
+RUN cp /opt/datafusion-comet/spark/target/comet-spark-spark${SPARK_VERSION}_$SCALA_VERSION-0.1.0-SNAPSHOT.jar $SPARK_HOME/jars
+
+USER ${spark_uid}


We can reduce the size of the final image from ~8 GB to ~1 GB by using a multi stage build.

With the Dockerfile in this PR:

REPOSITORY TAG IMAGE ID CREATED SIZE andygrove/comet latest 223fb8a90579 21 seconds ago 8.42GB

With the version suggested below:

REPOSITORY TAG IMAGE ID CREATED SIZE andygrove/comet latest 44695163de5d 31 seconds ago 996MB

Suggested change

FROM apache/spark:3.4.2

USER root

# Installing JDK11 as the image comes with JRE

RUN apt update \

&& apt install -y git \

&& apt install -y curl \

&& apt install -y openjdk-11-jdk \

&& apt clean

RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y

ENV PATH="/root/.cargo/bin:${PATH}"

ENV RUSTFLAGS="-C debuginfo=line-tables-only -C incremental=false"

ENV SPARK_VERSION=3.4

ENV SCALA_VERSION=2.12

# Pick the JDK instead of JRE to compile Comet

RUN cd /opt \

&& git clone https://github.com/apache/datafusion-comet.git \

&& cd datafusion-comet \

&& JAVA_HOME=$(readlink -f $(which javac) | sed "s/\/bin\/javac//") make release PROFILES="-Pspark-$SPARK_VERSION -Pscala-$SCALA_VERSION"

RUN cp /opt/datafusion-comet/spark/target/comet-spark-spark${SPARK_VERSION}_$SCALA_VERSION-0.1.0-SNAPSHOT.jar $SPARK_HOME/jars

USER ${spark_uid}

FROM apache/spark:3.4.2 AS builder

USER root

# Installing JDK11 as the image comes with JRE

RUN apt update \

&& apt install -y git \

&& apt install -y curl \

&& apt install -y openjdk-11-jdk \

&& apt clean

RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y

ENV PATH="/root/.cargo/bin:${PATH}"

ENV RUSTFLAGS="-C debuginfo=line-tables-only -C incremental=false"

ENV SPARK_VERSION=3.4

ENV SCALA_VERSION=2.12

# Pick the JDK instead of JRE to compile Comet

RUN cd /opt \

&& git clone https://github.com/apache/datafusion-comet.git \

&& cd datafusion-comet \

&& JAVA_HOME=$(readlink -f $(which javac) | sed "s/\/bin\/javac//") make release PROFILES="-Pspark-$SPARK_VERSION -Pscala-$SCALA_VERSION"

FROM apache/spark:3.4.2

ENV SPARK_VERSION=3.4

ENV SCALA_VERSION=2.12

USER root

COPY --from=builder /opt/datafusion-comet/spark/target/comet-spark-spark${SPARK_VERSION}_$SCALA_VERSION-0.1.0-SNAPSHOT.jar $SPARK_HOME/jars

USER ${spark_uid}

Locally I am seeing this warning:

1 warning found (use --debug to expand): - UndefinedVar: Usage of undefined variable '$spark_uid' (line 33)

andygrove · 2024-07-16T17:33:27Z

kube/Dockerfile

@@ -0,0 +1,26 @@
+FROM apache/spark:3.4.2


Need to add ASF header

viirya · 2024-07-16T19:04:20Z

kube/Dockerfile

+# Installing JDK11 as the image comes with JRE
+RUN apt update \
+    && apt install -y git \
+    && apt install -y curl \
+    && apt install -y openjdk-11-jdk \
+    && apt clean
+


Doesn't apache/spark:3.4.2 have Java installed?

I think it has JRE but not JDK based on the comment

Yeah, its only JRE but the folder name is confusing and is said it is openjdk, but in fact no javac in the bin folder

andygrove

The Dockerfile builds without error for me. I have not tried deploying it in k8s.

Should there be a README as well, or any update to the user or contributor guides?

comphead · 2024-07-16T22:20:03Z

Thanks @andygrove I'll add docs later when I wrap up with local kubectl. I'm not sure how useful just a raw docker file, the user most likely wants to run it in the cluster?

andygrove · 2024-07-16T22:36:50Z

Thanks @andygrove I'll add docs later when I wrap up with local kubectl. I'm not sure how useful just a raw docker file, the user most likely wants to run it in the cluster?

Personally, I'm fine with adding the Dockerfile first and docs later.

viirya · 2024-07-16T23:26:25Z

kube/Dockerfile

+# limitations under the License.
+#
+
+FROM apache/spark:3.4.2 AS builder


We are using 3.4.3.

It should be good to keep this consistent with our main branch.

kazuyukitanimura · 2024-07-17T00:06:21Z

kube/Dockerfile

+ENV SPARK_VERSION=3.4
+ENV SCALA_VERSION=2.12
+USER root
+COPY --from=builder  /opt/datafusion-comet/spark/target/comet-spark-spark${SPARK_VERSION}_$SCALA_VERSION-0.1.0-SNAPSHOT.jar $SPARK_HOME/jars


Perhaps we should change 0.1.0-SNAPSHOT to ENV?

we going to parametrize variables in follow up PR

parthchandra · 2024-07-17T00:48:30Z

kube/Dockerfile

+# limitations under the License.
+#
+
+FROM apache/spark:3.4.2 AS builder


We could use ARG to parameterize this -

ARG BASE_IMAGE=apache/spark-3.4.3 FROM ${BASE_IMAGE}

Then the user can specify a different version of the Spark image -

docker build -f kube/Dockerfile --platform linux/amd64 \ --build-arg BASE_IMAGE=MY_BASE_IMAGE \ ...

I don't think we need to parameterize this as part of this PR. But potentially we could parameterize a bunch of the ENV variables similarly.

See also: https://docs.docker.com/build/guide/build-args/

That a neat idea, I'll do a parametrization in followup PR, it also requires some documentation

* Create Comet docker file

Create Comet docker file

c5060b2

andygrove reviewed Jul 16, 2024

View reviewed changes

kube/Dockerfile Outdated

@@ -0,0 +1,26 @@

FROM apache/spark:3.4.2

Copy link

Member

andygrove Jul 16, 2024

Choose a reason for hiding this comment

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

Need to add ASF header

comphead reacted with thumbs up emoji

viirya reviewed Jul 16, 2024

View reviewed changes

comphead marked this pull request as draft July 16, 2024 19:28

Create Comet docker file. Comments

8be9630

comphead marked this pull request as ready for review July 16, 2024 21:47

comphead requested review from andygrove and viirya July 16, 2024 21:47

andygrove approved these changes Jul 16, 2024

View reviewed changes

viirya reviewed Jul 16, 2024

View reviewed changes

viirya approved these changes Jul 16, 2024

View reviewed changes

kazuyukitanimura reviewed Jul 17, 2024

View reviewed changes

parthchandra reviewed Jul 17, 2024

View reviewed changes

Create Comet docker file. Comments

ec4c573

kazuyukitanimura approved these changes Jul 17, 2024

View reviewed changes

comphead merged commit 7ac2fb9 into apache:main Jul 17, 2024
74 checks passed

himadripal pushed a commit to himadripal/datafusion-comet that referenced this pull request Sep 7, 2024

Create Comet docker file (apache#675)

ed62c72

* Create Comet docker file

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create Comet docker file #675

Create Comet docker file #675

comphead commented Jul 16, 2024

andygrove Jul 16, 2024

comphead Jul 16, 2024

andygrove Jul 16, 2024

viirya Jul 16, 2024

andygrove Jul 16, 2024

comphead Jul 16, 2024

andygrove left a comment

comphead commented Jul 16, 2024

andygrove commented Jul 16, 2024

viirya Jul 16, 2024

viirya Jul 16, 2024

kazuyukitanimura Jul 17, 2024

comphead Jul 17, 2024

parthchandra Jul 17, 2024

comphead Jul 17, 2024

Create Comet docker file #675

Create Comet docker file #675

Conversation

comphead commented Jul 16, 2024

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

How are these changes tested?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andygrove left a comment

Choose a reason for hiding this comment

comphead commented Jul 16, 2024

andygrove commented Jul 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment