mvcc: bound syncWatchers scan to deliverable revisions - #22265
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: waterWang The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @waterWang. Thanks for your PR. I'm waiting for a etcd-io member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Description
syncWatchers()currently callsrangeEventswith[minRev, curRev+1)— the full window from the oldest unsynced watcher to the current tip. When a watcher is far behind (e.g., 500,000+ revisions due to a Kubernetes API server being unsynced), every 100mssyncWatchersloop re-scans the entire backlog, butnewWatcherBatchonly delivers at mostwatchBatchMaxRevs(1000) distinct revisions per watcher per pass.This means the backlog is re-read roughly
number_of_revisions_behind / 1000times. Each scan holdswatchableStore.mufor the full duration, andwatchableStoreTxnWrite.End()needs the same lock to publish events, stalling the apply loop.Fix
Bound the scan window to
[minRev, minRev+watchBatchMaxRevs+1)so each pass reads only the revisions that can actually be delivered to the watchers. This eliminates the read amplification entirely — the scan is always at most 1001 revisions wide, regardless of how far behind the watcher is.Reproducer output comparison
Before (on
main, recovering a single watcher 20,000 revisions behind):After (with this fix):
The fix reduces total revisions read by ~10x and eliminates the long lock hold times.
Fixes #22264