feat: __lbheartbeat__ will return 500 if the connection pool is exhausted
#997
Conversation
There was a problem hiding this comment.
Would a combination of this and the previous method (checking intermittently in the pool periodic reporter but then ultimately reporting that status in the health check) potentially be more accurate?
Though I'm fine to try this assuming the probe can essentially do the same -- decide a pod is bad when this it sees lbheartbeat repeatedly report unhealthyness over some certain amount of time
| } | ||
| status_code = StatusCode::INTERNAL_SERVER_ERROR; |
There was a problem hiding this comment.
Maybe only set the 500 status in an else block? Might be safer avoiding healthy nodes who happen to hit the max conns only once reporting 500 here.
| } | |
| status_code = StatusCode::INTERNAL_SERVER_ERROR; | |
| } else { | |
| status_code = StatusCode::INTERNAL_SERVER_ERROR; | |
| } |
There was a problem hiding this comment.
Yeah, that was the reasoning behind adding the duration_ms. I'm kinda/sorta ok if a saturated node returns a 500 early, because it can't really process more. Things reset fairly quickly once things become desaturated, and I think it takes multiple 500s returns before a node is terminated. We might want to talk with @Micheletto to see if that matches what he wants. I'd also be fine returning the 500 only after the saturation passes some Duration.
There was a problem hiding this comment.
Based off of conversations with ops, returning 500 immediately is preferred. Monitoring will only kill a node once multiple 500 reports in a row are detected.
Closes #996
Description
Return a 500 error if the connection pool is exhausted
Testing
Includes unit test
Issue(s)
Closes #996