Skip to content
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

Support message headers with null values #139

Merged
merged 1 commit into from
Jun 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/main/docker/kafdrop.sh
Original file line number Diff line number Diff line change
Expand Up @@ -64,5 +64,4 @@ ARGS="--add-opens=java.base/sun.nio.ch=ALL-UNNAMED -Xss256K \
$HEAP_ARGS \
$JVM_OPTS"

java $ARGS -jar /kafdrop*/kafdrop*jar $CMD_ARGS

exec java $ARGS -jar /kafdrop*/kafdrop*jar ${CMD_ARGS}
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need exec here to replace the pid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Docker container is designed to run single process. Run of many processes can be supported by init system like tini that handles signals and sends them to child process. Bash scripts for example do not handle and emit signals properly.

I think we need to replace process PID to 1 here so that the java application can handle the SIGTERM/SIGKILL signals sent to the container.

Usfull link: https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#entrypoint

This script uses the exec Bash command so that the final running application becomes the container’s PID 1. This allows the application to receive any Unix signals sent to the container. > For more, see the ENTRYPOINT reference.

3 changes: 2 additions & 1 deletion src/main/java/kafdrop/service/KafkaMonitorImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,8 @@ public List<MessageVO> getMessages(TopicPartition topicPartition, long offset, i
private static Map<String, String> headersToMap(Headers headers) {
final var map = new TreeMap<String, String>();
for (var header : headers) {
map.put(header.key(), new String(header.value()));
final var value = header.value();
map.put(header.key(), (value == null) ? null : new String(value));
}
return map;
}
Expand Down