-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
release-23.2: tenantcapabilitieswatcher: make the watcher react faster #114818
release-23.2: tenantcapabilitieswatcher: make the watcher react faster #114818
Conversation
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]>
938224a
to
bba4f04
Compare
Thanks for opening a backport. Please check the backport criteria before merging:
If your backport adds new functionality, please ensure that the following additional criteria are satisfied:
Also, please add a brief release justification to the body of your PR to justify this |
0b7557d
to
05f294d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree about letting it bake on master for a week or so.
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! 0 of 0 LGTMs obtained (waiting on @dt and @msbutler)
Reminder: it has been 3 weeks please merge or close your backport! |
@stevendanna still want to merge this? |
@msbutler Thanks for the reminder. I'll see if we had any follow-up fixes that should be added to this backport and then rebase it. |
Backport 3/3 commits from #114719 on behalf of @stevendanna.
/cc @cockroachdb/release
Needed for #111637.
Epic: CRDB-26691
Superceeds #112094
Prior to this patch, the tenant info watcher would only react to
changes to
system.tenants
upon 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
Release justification: Substantial improvement that reduces the rate of timeouts and flakes in many tests.