1246008 - Add __lbheartbeat__ check specific to load balancers

Status: RESOLVED FIXED

(Cloud Services Graveyard :: Server: Token, defect)

Description

[Bob Micheletto [:bobm]]

The __heartbeat__ health check endpoint on this, and many of our services, return an error when backends and other upstream services are unavailable.  That's great for uptime monitoring of a full service, however it's not suitable for a load balancer membership test.

It can trigger a spiraling problem scenario where a load balancer marks otherwise healthy hosts as failing and reaps them when the problem is actually with a backend service such as a DB or caching server.

Adding a load balancer specific health check endpoint that only verifies health local to the instance, is one solution to this problem.  An alternate approach, might be to report upstream service outages in a payload but return a successful HTTP response.

Comment 1

[Ryan Kelly [:rfkelly]]

> The __heartbeat__ health check endpoint on this, and many of our services,
> return an error when backends and other upstream services are unavailable.

Are you sure that's the case for tokenserver?  Bug 996870 suggests that it's only checking the health of the web process itself, not its dependencies.

Comment 2

[Benson Wong [:mostlygeek]]

__heartbeat__ should be used for monitoring if the service is ok. This should trigger an alert to ops that something is wrong. 

__lbheartbeat__ should be used to monitor if the node is ok. This should trigger the ELB and ASG to stop sending traffic to the instance and replace it.

Comment 3

[Bob Micheletto [:bobm]]

(In reply to Ryan Kelly [:rfkelly] from comment #1)
 
> Are you sure that's the case for tokenserver?  Bug 996870 suggests that it's
> only checking the health of the web process itself, not its dependencies.

Good point.  But when it does, we'll need a load balancer specific check.

Comment 4

[Rémy Hubscher (:natim)]

Should we consider that if the node cannot contact the database anymore it is still ok to send traffic toward it?

Right now the strategy we had was to stop sending traffic to a node with no more database contact until it got its connection back.

When a node doesn't have a database connection it will return a 503.
When an ELB doesn't have any node it will return a 503.

The current strategy let us handle the case when not all the node can make contact with their database.

Thoughts?

Comment 5

[Ryan Kelly [:rfkelly]]

IIUC, the problem here is that the auto-scale group will reap the boxes if they're unhealthy and try to spin up news ones in their place, which is not productive when it's the db that's at fault.

Comment 6

[Rémy Hubscher (:natim)]

In the case of auto-scalling I totally agree then. I didn't know we had such strategy for loop-server.
BTW, we added a lbhealthcheck endpoint for loop-server and Kinto yesterday.

Comment 7

[Bob Micheletto [:bobm]]

:mostlygeek can you confirm that we can close this bug?

Comment 8

[Benson Wong [:mostlygeek]]

:rfkelly Did we add a __lbheartbeat__ endpoint to tokenserver? 
The __lbheartbeat__ only needs to respond with 200 OK.

Comment 9

[Ryan Kelly [:rfkelly]]

Nope, doesn't look like it:

  rfk@tangello:tokenserver(master)$ grep -r lbheartbeat tokenserver
  rfk@tangello:tokenserver(master)$ 

Any other services we need to add this on?

Comment 10

[Benson Wong [:mostlygeek]]

I think adding it Tokenserver would be enough for now.

Comment 11

[Ryan Kelly [:rfkelly]]

This got added \o/