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

Add a HEALTHCHECK #474

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

timdorr
Copy link

@timdorr timdorr commented Mar 28, 2019

Fixes #167

I swiped the script from @knordstrom and @Lukkie. The general gist is that it asks Zookeeper for the brokers it knows about and checks that it is in that list. So, a little better than just a port check with netcat.

If anyone has ideas for improvement, please let me know and I'll add it to this PR.

@timdorr timdorr changed the title Health☑️ Add a HEALTHCHECK Mar 28, 2019
@sscaling
Copy link
Collaborator

sscaling commented Mar 29, 2019

The response in my comment to this issues still applies - #167 (comment)

Health checks don't really provide much use other than an a queryable label on the running container.

It'd be great to understand what real-world use-case this particular solution solves to know on whether to proceed or not.

As an aside - i'm not sure this is a great approach to health check, all it does is validate that the broker has connected to zookeeper and been assigned a znode - it has no real indication of whether Kafka is healthy or not, the only half decent way of doing so is via a successful query to the brokers. Even with that said, if your brokers are in a distinct network from your clients and communication is via an external port a health check from inside the broker network becomes misleading, i.e. it could appear healthy but be completely unusable.

@fvigotti
Copy link

fvigotti commented Apr 4, 2019

In my installation which may be slightly different but probably not so much.. the
shell commands ( ie : zookeeper-shell.sh ) use same env variables as kafka..
with the provided script there could be problems ie: if the pod is limited in memory to something less than 200% of the -Xmx or if the kafka is exporting JMX maybe would be usefull to reset/reimport from another name those variables

KAFKA_HEAP_OPTS="-Xms200M -Xmx500M"
## avoid conflicts on jmx ports with kafka 
KAFKA_JMX_OPTS=""
JMX_PORT=""

#! /bin/bash

r=`$KAFKA_HOME/bin/zookeeper-shell.sh $KAFKA_ZOOKEEPER_CONNECT <<< "ls /brokers/ids" | tail -1 | jq '.[]'`
ids=( $r )
Copy link

Choose a reason for hiding this comment

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

trailing whitespace on this line and the previous line

also fwiw, using "health☑️" as a branch name makes it difficult for others to use your branch.

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.

4 participants