Tablet throttler: starvation fix and consolidation of logic.

[shlomi-noach]

Description

This PR addresses a starvation scenario in the tablet throttler. The scenario actually doesn't take place, due to an unintentional change. We fix the unintentional change & we fix the starvation scenario.

We also consolidate two different measurements of "last check" or "recent check":

1. recentCheckValue
2. lastCheckTimeNano

They are superfluous, because both serve to answer a similar question: "when was this throttler last checked?"

lastCheckTimeNano was updated in check.go at every check, and that's also where the aforementioned unintentional behavior takes place: this value should not be updated when the app is "vitess". Right now in main this value does get updated when the app is "vitess", which means the throttler's self-checks keep that value always recent. In turn, this prevents the throttler from going into dormant mode.

We introduce a new, single tracker for the checks, in the form of recentCheckRateLimiter. RateLimiter now not only produces ticks, and not only rate limits some action, but can also tell us the logical clock diff since last action invocation.

Using this, we can easily determine:

• Whether a check was made in the last 1-2 seconds
• Whether a check was made in the past 1min

Starvation prevention through PRIMARY stimulation

The fix to the starvation scenario is for the replica to proactively let the PRIMARY know that it has been checked. The PRIMARY still gets that information from the replicas when probing them. But now also, the replica invokes a CheckThrottler() directly on the PRIMARY with "throttler-stimulator" app name. This "stimulates" the PRIMARY, as it sees that it has just been checked, and therefore immediately renews heartbeat lease.

However, the replica does not overload the PRIMARY with these stimulations. Stimulations are rate limited to exactly dormant period (1min at this time).

Unit tests

We add unit tests to validate:

• Dormant mode is correct.
• "vitess" app checks does not affect "recently checked" state, neither or PRIMARY nor on replicas.
• "throttler-stimulator" affects "recently checked" state and causes heartbeat renewals.
• Replica checks invoke CheckThrottler on PRIMARY with "throttler-stimulator" app name.

Related Issue(s)

#15397

Checklist

• "Backport to:" labels have been added if this change should be back-ported to release branches
• If this change is to be back-ported to previous releases, a justification is included in the PR description
• Tests were added or are not required
• Did the new or modified tests pass consistently locally and on CI?
• Documentation was added or is not required

Deployment Notes

[vitess-bot]

Review Checklist

Hello reviewers! 👋 Please follow this checklist when reviewing this Pull Request.

General

• Ensure that the Pull Request has a descriptive title.
• Ensure there is a link to an issue (except for internal cleanup and flaky test fixes), new features should have an RFC that documents use cases and test cases.

Tests

• Bug fixes should have at least one unit or end-to-end test, enhancement and new features should have a sufficient number of tests.

Documentation

• Apply the release notes (needs details) label if users need to know about this change.
• New features should be documented.
• There should be some code comments as to why things are implemented the way they are.
• There should be a comment at the top of each new or modified test to explain what the test does.

New flags

• Is this flag really necessary?
• Flag names must be clear and intuitive, use dashes (-), and have a clear help text.

If a workflow is added or modified:

• Each item in Jobs should be named in order to mark it as required.
• If the workflow needs to be marked as required, the maintainer team must be notified.

Backward compatibility

• Protobuf changes should be wire-compatible.
• Changes to _vt tables and RPCs need to be backward compatible.
• RPC changes should be compatible with vitess-operator
• If a flag is removed, then it should also be removed from vitess-operator and arewefastyet, if used there.
• vtctl command output order should be stable and awk-able.

[shlomi-noach]

Removed from check.,go. Delegated to, and consolidated in checkStore() in throttler.go.

[shlomi-noach]

No metrics for "vitess" app.

[shlomi-noach]

Temprarily converting to Draft while running more checks.

[codecov]

Codecov Report

Attention: Patch coverage is 93.05556% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 65.65%. Comparing base (696fe0e) to head (1f4f73f).
Report is 99 commits behind head on main.

Files	Patch %	Lines
go/vt/vttablet/tabletserver/throttle/throttler.go	90.56%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #15398      +/-   ##
==========================================
- Coverage   67.41%   65.65%   -1.77%     
==========================================
  Files        1560     1562       +2     
  Lines      192752   194366    +1614     
==========================================
- Hits       129952   127611    -2341     
- Misses      62800    66755    +3955     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[shlomi-noach]

Now that this PR fixes the dormant issue, we also notice that any throttler should be collecting their own already-probed metrics. This comes at nearly no-cost. dormancy was made for the PRIMARY to not overwhelm network traffic when not needed, whilst collecting metrics from the replicas.

[shlomi-noach]

Upon adding b58bac6, I realize this PR now solves a different starvation issue, perhaps even more dominant than the one it was meant to solve. Will open a new issue to describe it.

[shlomi-noach]

2nd starvation issue: #15433