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.

Sign up for GitHub

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

Jump to bottom

Add liveness check #2180

Merged
merged 33 commits into from
Feb 13, 2020
Merged

Add liveness check #2180

merged 33 commits into from
Feb 13, 2020

Conversation

adelowo
Copy link
Contributor

@adelowo adelowo commented Mar 22, 2019 •
edited
Loading

  • Redis
  • Dashboard
  • Rpc

Fixes #857

@adelowo adelowo marked this pull request as ready for review March 22, 2019 12:32
matiasinsaurralde
matiasinsaurralde approved these changes Apr 4, 2019
View reviewed changes
Copy link
Contributor

@matiasinsaurralde matiasinsaurralde left a comment

Choose a reason for hiding this comment

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

Looks good!

@adelowo adelowo mentioned this pull request May 2, 2019
Closed
@excieve excieve mentioned this pull request May 10, 2019
Closed
buger
buger requested changes Jun 2, 2019
View reviewed changes
Copy link
Member

@buger buger left a comment

Choose a reason for hiding this comment

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

I see multiple issues in this approach.

  1. First of all health check API got new URL and it will break backward compatibility for all users. We need to keep the same url naming (and have it configurable).
  2. API becomes blocking if you have connectivity issues in either Redis for RPC layer, your status endpoint will block too, making it unusable for health checks. It should be a background goroutine instead, remember results, and status endpoint just immediately outputs these values. Also for RPC layer we already have a variable which checks if we have a connection or not.
  3. There is an RFC for implementing health checks, and looks solid:

@adelowo
Copy link
Contributor Author

adelowo commented Jun 2, 2019

Will take a look and update as needed

@adelowo
Copy link
Contributor Author

adelowo commented Jun 12, 2019 •
edited
Loading

Will update this according to the RFC and your comments today

@adelowo adelowo self-assigned this Sep 16, 2019
@adelowo
Copy link
Contributor Author

adelowo commented Dec 12, 2019

The currently available url is a “readiness check” while this one is a “liveness check.

#857 (comment)

buger
buger reviewed Jan 7, 2020
View reviewed changes
gateway/health_check.go Outdated Show resolved Hide resolved
buger
buger reviewed Jan 7, 2020
View reviewed changes
config/config.go Show resolved Hide resolved
@adelowo adelowo requested a review from buger January 10, 2020 15:36
@buger buger merged commit 0b59ca7 into TykTechnologies:master Feb 13, 2020
@buger
Copy link
Member

buger commented Feb 13, 2020

/release to release-2.10

@tykbot
Copy link

tykbot bot commented Feb 13, 2020

Working on it! Note that it can take a few minutes.

tykbot bot pushed a commit that referenced this pull request Feb 13, 2020
@adelowo
Add liveness check (#2180)
abb8011 
- [x] Redis
- [x] Dashboard
- [x] Rpc

Fixes #857 

(cherry picked from commit 0b59ca7)
@tykbot
Copy link

tykbot bot commented Feb 13, 2020

@buger Succesfully merged 0b59ca7188928f2087cb8f2b82dc5201270d4f40 to release-2.10 branch.

@adelowo adelowo deleted the liveness_check branch February 18, 2020 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matiasinsaurralde matiasinsaurralde Awaiting requested review from matiasinsaurralde

@buger buger Awaiting requested review from buger

Assignees

@adelowo adelowo

Labels
customer request Priority: Low
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improved healthcheck API
4 participants
@adelowo @buger @matiasinsaurralde @letzya