Include jvmkill in Docker image#13736
Conversation
core/docker/default/etc/jvm.config
Outdated
There was a problem hiding this comment.
Since we compile from source I believe it would work on all 3 of x64, arm64 and ppcle64 arches?
There was a problem hiding this comment.
Yes, that’s why I did it this way. I wish there was an easier way to package this so we don’t need to build every time.
There was a problem hiding this comment.
I could add a workflow in the jvmkill repo that would create releases with binaries for different architectures.
There was a problem hiding this comment.
Existing is ok for me. Also serves as an example for others who want to add similar in their own docker images.
There was a problem hiding this comment.
@nineinchnick I like that idea. Then we could just download them from the GitHub releases URL?
core/docker/Dockerfile
Outdated
There was a problem hiding this comment.
Thanks for teaching that we can have un-named stages (numbered starting from 0).
| COPY --chown=trino:trino --from=0 /tmp/jvmkill/libjvmkill.so /usr/lib/trino/bin | |
| COPY --chown=trino:trino --from=builder /tmp/jvmkill/libjvmkill.so /usr/lib/trino/bin |
Then change L14 to be FROM eclipse-temurin:17-jdk AS builder. More idiomatic and familiar to most people AFAIK.
There was a problem hiding this comment.
Nice, that's definitely easier to understand. The --from=0 makes me think it's reading from stdin. I had copied that from some other Dockerfile.
|
A question and two nits. |
core/docker/Dockerfile
Outdated
There was a problem hiding this comment.
Should we pin this to some commit? I expect jvmkill to rarely change but still it would prevent accidentally breaking workflows here.
There was a problem hiding this comment.
jvmkill has been stable since its initial commit in 2015, so pointing to master seems good, since the only changes are bug fixes (like adding _exit() as an alternative way to die). I do like your idea of doing releases through GHA, then we could point to an exact release.
core/docker/Dockerfile
Outdated
There was a problem hiding this comment.
How much larger does this make the image? We shouldn't keep these tools beyond what's needed to build the image.
There was a problem hiding this comment.
This is a separate builder image. We just copy the (tiny) library into the final image.
4 participants
Description
Include the jvmkill agent to ensure the JVM is terminated on any resource exhaustion event, such as unable to create a native thread. This is important, because various code such as
BoundedExecutorcan become unusable after this occurs.Documentation
(x) No documentation is needed.
Release notes
(x) Release notes entries required with the following suggested text: