KAFKA-16827: Integrate kafka native-image with system tests #16046
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
omkreddy
reviewed
May 28, 2024
| @@ -208,7 +208,7 @@ fi | |||
|
|
|||
| # JMX settings | |||
| if [ -z "$KAFKA_JMX_OPTS" ]; then | |||
| KAFKA_JMX_OPTS="-Dcom.sun.management.jmxremote -Dcom.sun.management.jmxremote.authenticate=false -Dcom.sun.management.jmxremote.ssl=false " | |||
| KAFKA_JMX_OPTS="-Dcom.sun.management.jmxremote=true -Dcom.sun.management.jmxremote.authenticate=false -Dcom.sun.management.jmxremote.ssl=false " | |||
There was a problem hiding this comment.
nit: maynot be required as default is true for this system property
There was a problem hiding this comment.
Yes, that is correct for JVM.
But the native binary requires it explicitly. Hence added this.
bin/kafka-run-class.sh
Outdated
| # Launch mode | ||
| if [ "x$DAEMON_MODE" = "xtrue" ]; then | ||
| nohup "$JAVA" $KAFKA_HEAP_OPTS $KAFKA_JVM_PERFORMANCE_OPTS $KAFKA_GC_LOG_OPTS $KAFKA_JMX_OPTS $KAFKA_LOG4J_CMD_OPTS -cp "$CLASSPATH" $KAFKA_OPTS "$@" > "$CONSOLE_OUTPUT_FILE" 2>&1 < /dev/null & | ||
| if [ "$KAFKA_MODE" = "native" ]; then |
There was a problem hiding this comment.
Can we use the existing convention to test the string like if [ "x$KAFKA_MODE" = "xnative" ]
| if [ "x$DAEMON_MODE" = "xtrue" ]; then | ||
| nohup "$JAVA" $KAFKA_HEAP_OPTS $KAFKA_JVM_PERFORMANCE_OPTS $KAFKA_GC_LOG_OPTS $KAFKA_JMX_OPTS $KAFKA_LOG4J_CMD_OPTS -cp "$CLASSPATH" $KAFKA_OPTS "$@" > "$CONSOLE_OUTPUT_FILE" 2>&1 < /dev/null & | ||
| if [ "$KAFKA_MODE" = "native" ]; then | ||
| exec $base_dir/kafka.Kafka start --config "$2" $KAFKA_LOG4J_CMD_OPTS $KAFKA_JMX_OPTS $KAFKA_OPTS |
There was a problem hiding this comment.
Dont we need to pass $KAFKA_HEAP_OPTS to native run?
There was a problem hiding this comment.
As per the native-image docs
When executing a native image, suitable Java heap settings will be determined automatically based on the system configuration and the used GC.
Though they have given mechanism to override it explicitly, I am making use of the automatic mechanism.
kagarwal06
commented
May 28, 2024
| @@ -926,7 +932,7 @@ def run_features_command(self, op, new_version): | |||
| def pids(self, node): | |||
| """Return process ids associated with running processes on the given node.""" | |||
| try: | |||
| cmd = "jcmd | grep -e %s | awk '{print $1}'" % self.java_class_name() | |||
There was a problem hiding this comment.
jcmd command won't work if Kafka is running in the native mode.
Replacing it with ps ax.
kagarwal06
commented
May 28, 2024
| @@ -994,7 +1000,7 @@ def thread_dump(self, node): | |||
| def clean_node(self, node): | |||
| JmxMixin.clean_node(self, node) | |||
| self.security_config.clean_node(node) | |||
| node.account.kill_java_processes(self.java_class_name(), | |||
There was a problem hiding this comment.
kill_java_processes command won't work if Kafka is running in the native mode.
Replacing it with kill_process.
kagarwal06
commented
May 28, 2024
| @@ -55,6 +55,7 @@ KafkaServer { | |||
| useKeyTab=true | |||
| storeKey=true | |||
| keyTab="/mnt/security/keytab" | |||
| refreshKrb5Config=true | |||
There was a problem hiding this comment.
This change is needed because of an issue in GraalVm native-image tool. They will fix it in upcoming releases.
I raised this in their community slack channel.
This is a workaround for now.
Conversation thread: https://graalvm.slack.com/archives/CN9KSFB40/p1700246576831259
omkreddy
reviewed
May 28, 2024
tests/docker/run_tests.sh
Outdated
| else | ||
| export KAFKA_MODE="jvm" | ||
| fi | ||
| } |
There was a problem hiding this comment.
Can we print the mode here
tests/docker/run_tests.sh
Outdated
| get_mode | ||
| cleanup && mkdir "${TMP_NATIVE_DIR}" | ||
| if [ "$KAFKA_MODE" == "native" ]; then | ||
| kafka_tarball_filename=(core/build/distributions/kafka*SNAPSHOT.tgz) |
There was a problem hiding this comment.
Can we print a line stating building for native image
omkreddy
approved these changes
May 30, 2024
chiacyu
pushed a commit
to chiacyu/kafka
that referenced
this pull request
Jun 1, 2024
…6046) This PR does following things System tests should bring up Kafka broker in the native mode System tests should run on Kafka broker in native mode Extract out native build command so that it can be reused. Allow system tests to run on Native Kafka broker using Docker mechanism To run system tests by bringing up Kafka in native mode: Pass kafka_mode as native in the ducktape globals:--globals '{\"kafka_mode\":\"native\"}' Running system tests by bringing up kafka in native mode via docker mechanism _DUCKTAPE_OPTIONS="--globals '{\"kafka_mode\":\"native\"}'" TC_PATHS="tests/kafkatest/tests/" bash tests/docker/run_tests.sh To only bring up ducker nodes to cater native kafka bash tests/docker/ducker-ak up -m native Reviewers: Manikumar Reddy <[email protected]>
wernerdv
pushed a commit
to wernerdv/kafka
that referenced
this pull request
Jun 3, 2024
…6046) This PR does following things System tests should bring up Kafka broker in the native mode System tests should run on Kafka broker in native mode Extract out native build command so that it can be reused. Allow system tests to run on Native Kafka broker using Docker mechanism To run system tests by bringing up Kafka in native mode: Pass kafka_mode as native in the ducktape globals:--globals '{\"kafka_mode\":\"native\"}' Running system tests by bringing up kafka in native mode via docker mechanism _DUCKTAPE_OPTIONS="--globals '{\"kafka_mode\":\"native\"}'" TC_PATHS="tests/kafkatest/tests/" bash tests/docker/run_tests.sh To only bring up ducker nodes to cater native kafka bash tests/docker/ducker-ak up -m native Reviewers: Manikumar Reddy <[email protected]>
TaiJuWu
pushed a commit
to TaiJuWu/kafka
that referenced
this pull request
Jun 8, 2024
…6046) This PR does following things System tests should bring up Kafka broker in the native mode System tests should run on Kafka broker in native mode Extract out native build command so that it can be reused. Allow system tests to run on Native Kafka broker using Docker mechanism To run system tests by bringing up Kafka in native mode: Pass kafka_mode as native in the ducktape globals:--globals '{\"kafka_mode\":\"native\"}' Running system tests by bringing up kafka in native mode via docker mechanism _DUCKTAPE_OPTIONS="--globals '{\"kafka_mode\":\"native\"}'" TC_PATHS="tests/kafkatest/tests/" bash tests/docker/run_tests.sh To only bring up ducker nodes to cater native kafka bash tests/docker/ducker-ak up -m native Reviewers: Manikumar Reddy <[email protected]>
gongxuanzhang
pushed a commit
to gongxuanzhang/kafka
that referenced
this pull request
Jun 12, 2024
…6046) This PR does following things System tests should bring up Kafka broker in the native mode System tests should run on Kafka broker in native mode Extract out native build command so that it can be reused. Allow system tests to run on Native Kafka broker using Docker mechanism To run system tests by bringing up Kafka in native mode: Pass kafka_mode as native in the ducktape globals:--globals '{\"kafka_mode\":\"native\"}' Running system tests by bringing up kafka in native mode via docker mechanism _DUCKTAPE_OPTIONS="--globals '{\"kafka_mode\":\"native\"}'" TC_PATHS="tests/kafkatest/tests/" bash tests/docker/run_tests.sh To only bring up ducker nodes to cater native kafka bash tests/docker/ducker-ak up -m native Reviewers: Manikumar Reddy <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR does following things
bin/kafka-run-class.shtests/kafkatest/services/kafka/kafka.pytests/kafkatest/services/security/templates/jaas.confdocker/native/Dockerfiledocker/native/native_command.shtests/docker/run_tests.shtests/docker/Dockerfiletests/docker/ducker-akTo run system tests by bringing up Kafka in native mode:
kafka_modeasnativein the ducktape globals:--globals '{\"kafka_mode\":\"native\"}'TESTING
Ran system tests job 3 ways
We can see the tests results are consistent.
Committer Checklist (excluded from commit message)