healthcheck: attempt to update primary only if the current tablet is serving

[deepthi]

Description

Fixes #8120

Related Issue(s)

Checklist

• Tests were added or are not required
• Documentation was added or is not required

Deployment Notes

[deepthi]

Before logs:

I0513 16:12:29.116383   29038 tablet_health_check.go:111] HealthCheckUpdate(Serving State): tablet: zone1-102 serving false => true for commerce/0 (RDONLY) reason: healthCheck update
I0513 16:12:51.023130   29038 tablet_health_check.go:111] HealthCheckUpdate(Serving State): tablet: zone1-100 serving true => false for commerce/0 (MASTER) reason: healthCheck update
E0513 16:12:51.465923   29038 healthcheck.go:478] Adding 1 to MasterPromoted counter for target: keyspace:"commerce" shard:"0" tablet_type:REPLICA , tablet: zone1-0000000101, tabletType: MASTER
W0513 16:12:56.023628   29038 healthcheck.go:441] not marking healthy master zone1-0000000100 as Up for commerce/0 because its MasterTermStartTime is smaller than the highest known timestamp from previous MASTERs zone1-0000000101: 1620947548 < 1620947571 
I0513 16:13:01.480794   29038 tablet_health_check.go:111] HealthCheckUpdate(Serving State): tablet: zone1-100 serving false => true for commerce/0 (REPLICA) reason: healthCheck update

After logs:

I0513 15:59:34.332740   25307 tablet_health_check.go:111] HealthCheckUpdate(Serving State): tablet: zone1-102 serving false => true for commerce/0 (RDONLY) reason: healthCheck update
I0513 16:07:51.464763   25307 tablet_health_check.go:111] HealthCheckUpdate(Serving State): tablet: zone1-100 serving true => false for commerce/0 (MASTER) reason: healthCheck update
E0513 16:07:52.192940   25307 healthcheck.go:478] Adding 1 to MasterPromoted counter for target: keyspace:"commerce" shard:"0" tablet_type:REPLICA , tablet: zone1-0000000101, tabletType: MASTER
I0513 16:08:02.202323   25307 tablet_health_check.go:111] HealthCheckUpdate(Serving State): tablet: zone1-100 serving false => true for commerce/0 (REPLICA) reason: healthCheck update

[5antelope]

lgtm

[5antelope]

nit: thc.Serving should not be false in practice (?) might want to add a warning log here when we change master to a tablet that is still down.

[deepthi]

It can be true or false, and this gets called for all tablet types. It is just that we only do anything with that value if the tablet_type is MASTER.
The healthy list update code inside the call to updateHealth is pretty solid, I think. It handles the true/false cases correctly. It will never replace with a down master.
Does this address your concern?

[5antelope]

ah got it. so this code actually "incorrectly" always claim isPrimaryUp=true in health update
this looks good