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

Adds rationale for healthcheck config #2812

Merged
merged 1 commit into from
Sep 21, 2019
Merged

Adds rationale for healthcheck config #2812

merged 1 commit into from
Sep 21, 2019

Conversation

codefromthecrypt
Copy link
Member

As we routinely get FUD about slow startup or memory usage, this
documents health check parameters in efforts to avoid them being read
literally as "zipkin takes 30s to startup"

See #2808

As we routinely get FUD about slow startup or memory usage, this
documents health check parameters in efforts to avoid them being read
literally as "zipkin takes 30s to startup"

See #2808
# simultaneously bootstrapping. If in production, you have a 30s startup, please
# report to https://gitter.im/openzipkin/zipkin including the values of the
# /health and /info endpoints as this would be unexpected.
#
HEALTHCHECK --interval=5s --start-period=30s --timeout=5s CMD wget --quiet http://localhost:9411/health || exit 1
Copy link
Member Author

Choose a reason for hiding this comment

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

@anuraaga FWIW with the above comments, I'm fine with the parameters. This is mainly defense, as mentioned a few different ways. Java raises antibodies for a lot of people who have a different favorite language.

I think you've recalled over the years people who don't like Java getting obsessed over image size, memory (with sometimes dodgy accounting of it), and startup time. It has been my experience that anything that can be used against us will. With the above comments, I think someone would have to be quite unprofessional to leap to the conclusion that we expect our server to startup in 30s. This was my goal here.. to reduce the amount of time lost, and potential sites over the FUD spread by folks who hate java.

Copy link
Contributor

@anuraaga anuraaga left a comment

Choose a reason for hiding this comment

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

Thanks that's a good guard

Copy link
Member

@basvanbeek basvanbeek left a comment

Choose a reason for hiding this comment

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

This helps setting expectations straight.

@codefromthecrypt codefromthecrypt merged commit d5d6781 into master Sep 21, 2019
@codefromthecrypt codefromthecrypt deleted the rationale branch September 21, 2019 22:27
@codefromthecrypt
Copy link
Member Author

thanks for the look rag n bas

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.

None yet

3 participants