tenantcapabilitieswatcher: make the watcher react faster #114719
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Release note: None
Release note: None
Prior to this patch, the tenant info watcher would only react to changes to `system.tenants` upon rangefeed cache flushes, which could be (in default config) up to 3 seconds after the change is committed. This commit accelerates the behavior by processing updates as soon as the rangefeed observes the change. This new behavior is similar to the way that cluster settings changes are processed immediately in the settings watcher (pkg/settingswatcher). In order to handle deletions that occur during errors that aren't automatically retried inside the rangefeed library (and are instead retried by the watcher resulting in a new full scan), we emit any scan-generated rangefeed events at their scan timestamp, allowing us a means of clearing any stale data from the cache. Release note: None Co-authored-by: Raphael 'kena' Poss <[email protected]>
stevendanna
requested review from
msbutler and
yuzefovich
and removed request for
a team
|
This change is |
|
The main change since #112094 was a rebase and then a set of changes to ensure that all of the previous test cases still pass without substantive changes by using the scan timestamp to clear our cache. |
yuzefovich
approved these changes
Nov 20, 2023
There was a problem hiding this comment.
Do we want this to be backported too?
Reviewed 1 of 1 files at r1, 1 of 1 files at r2, 17 of 17 files at r3, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @msbutler)
Eventually I think we may so I've tagged it. But we almost always hit like 1 to 2 bugs anytime we touch one of these watchers so I also want to let it bake for a few days at least. |
stevendanna
added
the
backport-23.2.x
|
bors r=yuzefovich |
|
Build succeeded: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Needed for #111637.
Epic: CRDB-26691
Superceeds #112094
Prior to this patch, the tenant info watcher would only react to
changes to
system.tenantsupon rangefeed cache flushes, which couldbe (in default config) up to 3 seconds after the change is committed.
This commit accelerates the behavior by processing updates as soon as
the rangefeed observes the change.
This new behavior is similar to the way that cluster settings changes
are processed immediately in the settings
watcher (pkg/settingswatcher).
In order to handle deletions that occur during errors that aren't
automatically retried inside the rangefeed library (and are instead
retried by the watcher resulting in a new full scan), we emit any
scan-generated rangefeed events at their scan timestamp, allowing us a
means of clearing any stale data from the cache.
Release note: None