Skip to content

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

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

Merged
merged 1 commit into from
Jun 3, 2022

Conversation

Mathieu-Ferraton
Copy link

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
Copy link
Author

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

@tianon
Copy link
Member

tianon commented Jun 3, 2022

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

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
Copy link
Author

Mathieu-Ferraton commented Jun 3, 2022

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
Copy link
Member

tianon commented Jun 3, 2022

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 👀)

@tianon
Copy link
Member

tianon commented Jun 3, 2022

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

@tianon tianon merged commit 5cde9b1 into docker-library:master Jun 3, 2022
@tianon
Copy link
Member

tianon commented Jun 3, 2022

Thank you!

docker-library-bot added a commit to docker-library-bot/official-images that referenced this pull request Jun 3, 2022
Changes:

- docker-library/cassandra@5cde9b1: Merge pull request docker-library/cassandra#252 from Mathieu-Ferraton/4.0-ppc64le
- docker-library/cassandra@1a9d5f3: update 4.0 Dockerfile to add -Xss512k to JVM_OPTS for ppc64le
- docker-library/cassandra@dee264d: Remove arch exclusions -- 4.0 is now working on s390x!
docker-library-bot added a commit to docker-library-bot/official-images that referenced this pull request Jun 3, 2022
Changes:

- docker-library/cassandra@9315e44: Remove errant "-q"
- docker-library/cassandra@5cde9b1: Merge pull request docker-library/cassandra#252 from Mathieu-Ferraton/4.0-ppc64le
- docker-library/cassandra@1a9d5f3: update 4.0 Dockerfile to add -Xss512k to JVM_OPTS for ppc64le
- docker-library/cassandra@dee264d: Remove arch exclusions -- 4.0 is now working on s390x!
BaurzhanSakhariev pushed a commit to crate/official-images that referenced this pull request Jul 18, 2022
Changes:

- docker-library/cassandra@9315e44: Remove errant "-q"
- docker-library/cassandra@5cde9b1: Merge pull request docker-library/cassandra#252 from Mathieu-Ferraton/4.0-ppc64le
- docker-library/cassandra@1a9d5f3: update 4.0 Dockerfile to add -Xss512k to JVM_OPTS for ppc64le
- docker-library/cassandra@dee264d: Remove arch exclusions -- 4.0 is now working on s390x!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants