docker-library / cassandra

Docker Official Image packaging for Cassandra
Apache License 2.0
262 stars 282 forks source link

update 4.0 Dockerfile to add -Xss512k to JVM_OPTS for ppc64le #252

Closed Mathieu-Ferraton closed 2 years ago

Mathieu-Ferraton commented 2 years ago

Fix issue #251 -Xss512k must be added to JVM_OTS for ppc64le. In 4.0, the value is set in $CASSANDRA_CONF/jvm-server.options. The 4.0 Dockerfile must reflect that change.

Mathieu-Ferraton commented 2 years ago

This PR doesn't update the Dockerfile.template file in parent directory. Should it be updated too?

tianon commented 2 years ago

If you don't want @docker-library-bot to revert it immediately after we merge, yeah :sweat_smile:

Looking at it, however, it's clear this block of code pre-dates #213, and that makes it easier for us to write this in a way that could help us make sure this doesn't break again in this same way in the future (because we'll know exactly what we expect to see in, say, 4.0 so we could write a more defensive check).

Do you mind if I take a stab at doing this a little differently?

Mathieu-Ferraton commented 2 years ago

I forced-push some changes to update the Dockerfile.template only right before your message was posted. I let you tell me what to do. Anf of course, feel free to try something else if you think it's worth

tianon commented 2 years ago

This is what I had in mind:

diff --git a/3.0/Dockerfile b/3.0/Dockerfile
index 3ed8f70..7a31570 100644
--- a/3.0/Dockerfile
+++ b/3.0/Dockerfile
@@ -144,16 +144,9 @@ RUN set -eux; \
        ppc64el) \
 # https://issues.apache.org/jira/browse/CASSANDRA-13345
 # "The stack size specified is too small, Specify at least 328k"
-           if grep -q -- '^-Xss' "$CASSANDRA_CONF/jvm.options"; then \
-# 3.11+ (jvm.options)
-               grep -- '^-Xss256k$' "$CASSANDRA_CONF/jvm.options"; \
-               sed -ri 's/^-Xss256k$/-Xss512k/' "$CASSANDRA_CONF/jvm.options"; \
-               grep -- '^-Xss512k$' "$CASSANDRA_CONF/jvm.options"; \
-           elif grep -q -- '-Xss256k' "$CASSANDRA_CONF/cassandra-env.sh"; then \
-# 3.0 (cassandra-env.sh)
-               sed -ri 's/-Xss256k/-Xss512k/g' "$CASSANDRA_CONF/cassandra-env.sh"; \
-               grep -- '-Xss512k' "$CASSANDRA_CONF/cassandra-env.sh"; \
-           fi; \
+           grep -q -- '-Xss256k' "$CASSANDRA_CONF/cassandra-env.sh"; \
+           sed -ri 's/-Xss256k/-Xss512k/g' "$CASSANDRA_CONF/cassandra-env.sh"; \
+           grep -- '-Xss512k' "$CASSANDRA_CONF/cassandra-env.sh"; \
            ;; \
    esac; \
    \
diff --git a/3.11/Dockerfile b/3.11/Dockerfile
index 0e74c03..954233b 100644
--- a/3.11/Dockerfile
+++ b/3.11/Dockerfile
@@ -144,16 +144,9 @@ RUN set -eux; \
        ppc64el) \
 # https://issues.apache.org/jira/browse/CASSANDRA-13345
 # "The stack size specified is too small, Specify at least 328k"
-           if grep -q -- '^-Xss' "$CASSANDRA_CONF/jvm.options"; then \
-# 3.11+ (jvm.options)
-               grep -- '^-Xss256k$' "$CASSANDRA_CONF/jvm.options"; \
-               sed -ri 's/^-Xss256k$/-Xss512k/' "$CASSANDRA_CONF/jvm.options"; \
-               grep -- '^-Xss512k$' "$CASSANDRA_CONF/jvm.options"; \
-           elif grep -q -- '-Xss256k' "$CASSANDRA_CONF/cassandra-env.sh"; then \
-# 3.0 (cassandra-env.sh)
-               sed -ri 's/-Xss256k/-Xss512k/g' "$CASSANDRA_CONF/cassandra-env.sh"; \
-               grep -- '-Xss512k' "$CASSANDRA_CONF/cassandra-env.sh"; \
-           fi; \
+           grep -- '^-Xss256k$' "$CASSANDRA_CONF/jvm.options"; \
+           sed -ri 's/^-Xss256k$/-Xss512k/' "$CASSANDRA_CONF/jvm.options"; \
+           grep -- '^-Xss512k$' "$CASSANDRA_CONF/jvm.options"; \
            ;; \
    esac; \
    \
diff --git a/4.0/Dockerfile b/4.0/Dockerfile
index 88d7949..1a5197f 100644
--- a/4.0/Dockerfile
+++ b/4.0/Dockerfile
@@ -144,16 +144,9 @@ RUN set -eux; \
        ppc64el) \
 # https://issues.apache.org/jira/browse/CASSANDRA-13345
 # "The stack size specified is too small, Specify at least 328k"
-           if grep -q -- '^-Xss' "$CASSANDRA_CONF/jvm.options"; then \
-# 3.11+ (jvm.options)
-               grep -- '^-Xss256k$' "$CASSANDRA_CONF/jvm.options"; \
-               sed -ri 's/^-Xss256k$/-Xss512k/' "$CASSANDRA_CONF/jvm.options"; \
-               grep -- '^-Xss512k$' "$CASSANDRA_CONF/jvm.options"; \
-           elif grep -q -- '-Xss256k' "$CASSANDRA_CONF/cassandra-env.sh"; then \
-# 3.0 (cassandra-env.sh)
-               sed -ri 's/-Xss256k/-Xss512k/g' "$CASSANDRA_CONF/cassandra-env.sh"; \
-               grep -- '-Xss512k' "$CASSANDRA_CONF/cassandra-env.sh"; \
-           fi; \
+           grep -- '^-Xss256k$' "$CASSANDRA_CONF/jvm-server.options"; \
+           sed -ri 's/^-Xss256k$/-Xss512k/' "$CASSANDRA_CONF/jvm-server.options"; \
+           grep -- '^-Xss512k$' "$CASSANDRA_CONF/jvm-server.options"; \
            ;; \
    esac; \
    \
diff --git a/Dockerfile.template b/Dockerfile.template
index aa6b739..a8ac488 100644
--- a/Dockerfile.template
+++ b/Dockerfile.template
@@ -1,3 +1,6 @@
+{{
+   def major: env.version | split(".")[0] | tonumber
+-}}
 FROM eclipse-temurin:{{ .java }}-jre-focal

 # explicitly set user/group IDs
@@ -16,7 +19,7 @@ RUN set -eux; \
 {{
    # python3 is only supported in 4.0+
    # https://issues.apache.org/jira/browse/CASSANDRA-10190
-   if env.version | split(".") | .[0] | tonumber < 4 then (
+   if major < 4 then (
 -}}
        python \
 {{ ) else ( -}}
@@ -146,16 +149,19 @@ RUN set -eux; \
        ppc64el) \
 # https://issues.apache.org/jira/browse/CASSANDRA-13345
 # "The stack size specified is too small, Specify at least 328k"
-           if grep -q -- '^-Xss' "$CASSANDRA_CONF/jvm.options"; then \
-# 3.11+ (jvm.options)
-               grep -- '^-Xss256k$' "$CASSANDRA_CONF/jvm.options"; \
-               sed -ri 's/^-Xss256k$/-Xss512k/' "$CASSANDRA_CONF/jvm.options"; \
-               grep -- '^-Xss512k$' "$CASSANDRA_CONF/jvm.options"; \
-           elif grep -q -- '-Xss256k' "$CASSANDRA_CONF/cassandra-env.sh"; then \
-# 3.0 (cassandra-env.sh)
-               sed -ri 's/-Xss256k/-Xss512k/g' "$CASSANDRA_CONF/cassandra-env.sh"; \
-               grep -- '-Xss512k' "$CASSANDRA_CONF/cassandra-env.sh"; \
-           fi; \
+{{ if env.version == "3.0" then ( -}}
+           grep -q -- '-Xss256k' "$CASSANDRA_CONF/cassandra-env.sh"; \
+           sed -ri 's/-Xss256k/-Xss512k/g' "$CASSANDRA_CONF/cassandra-env.sh"; \
+           grep -- '-Xss512k' "$CASSANDRA_CONF/cassandra-env.sh"; \
+{{ ) elif major == 3 then ( -}}
+           grep -- '^-Xss256k$' "$CASSANDRA_CONF/jvm.options"; \
+           sed -ri 's/^-Xss256k$/-Xss512k/' "$CASSANDRA_CONF/jvm.options"; \
+           grep -- '^-Xss512k$' "$CASSANDRA_CONF/jvm.options"; \
+{{ ) else ( -}}
+           grep -- '^-Xss256k$' "$CASSANDRA_CONF/jvm-server.options"; \
+           sed -ri 's/^-Xss256k$/-Xss512k/' "$CASSANDRA_CONF/jvm-server.options"; \
+           grep -- '^-Xss512k$' "$CASSANDRA_CONF/jvm-server.options"; \
+{{ ) end -}}
            ;; \
    esac; \
    \

(Essentially converting our version check to generate-time detection instead of runtime detection so we can have very explicit verification that will fail the build and give us a better signal that something's wrong :eyes:)

tianon commented 2 years ago

I've now successfully built all three versions on ppc64le to make sure this works. :smile:

tianon commented 2 years ago

Thank you!