Healthcheck - limit healthchecks by keyspace

[spark4]

Overview

Currently, vtgates support a keyspaces_to_watch flag which can enable data isolation by keyspace. However, vtgates still perform healthchecks against all tablets regardless of the value provided to this flag.

This PR introduces changes to healthchecks such that when the keyspaces_to_watch flag is utilized, vtgates will only maintain healthchecks on tablets in keyspaces that the vtgate has access to.

--
This closes #5387.
Relevant PR: #4420

Implementation

1. Introduce a new TabletRecorder filter, FilterByKeyspace, which will only add tablets from keyspaces that the vtgate has access to
2. Update the discovery gateway to use this filter when values for keyspaces_to_watch is passed in

Testing

In addition to unit testing, I performed the following manual tests:

• Confirmed that vtgates that use the keyspaces_to_watch flag were only performing healthchecks on tablets from the desired keyspace
• Confirmed that vtgates that do not use the keyspaces_to_watch flag performed health checks on the expected tablets
• Confirmed on a vtctld that various keyspace commands continue to work as expected
• Confirmed that backups continue to work as expected

[rafael]

Serry, this is looking good to me. Added some minor comments.

[rafael]

The check for: fbk.isIncluded(new) might be redundant. A tablet shouldn't be able to change keyspaces. So if it was included in the old it should be included in new.

[deepthi]

In that case, maybe we should check for those failure conditions - isIncluded(new) && !isIncluded(old) and the reverse - and return a FAILED_PRECONDITION error.

[spark4]

if it was included in the old it should be included in new.

☝️ I'm probably missing some important context here -- can you expand more about what you mean here? My understanding is that given that tablets can't change keyspaces, the fbk.isIncluded(old) is the unnecessary check here (as opposed to fbk.isIncluded(new)).

[spark4]

Looks like I posted the above comment on a non-refreshed page - just now seeing @deepthi's comment.

@deepthi it looks like the callers of ReplaceTablet do not have mechanisms for handling these types of errors. Do we know what the expected behavior for FAILED_PRECONDITION errors during tablet replacement is?

Given the low likelihood of us hitting this case, would logging be sufficient here?

[deepthi]

Oh I see.. the interface won't let you return an error.
Yes, logging it would be fine.

[deepthi]

Nicely written PR description!
Can you clarify the following?

• Confirmed that vtgates that use the keyspaces_to_watch flag were only performing healthchecks on tablets from the desired keyspace.
• Confirmed that vtgates that do use the keyspaces_to_watch flag performed health checks on the expected tablets.

These two look similar. Maybe the second case was for vtgates that don't use the keyspaces_to_watch flag?

[spark4]

Hey @deepthi - good catch. Yes, the second one was meant to say do not use. I will update the PR description now!

[deepthi]

LGTM

[rafael]

LGTM