In ThrottleOverrideHooks::onPingLimiter, when a database lookup results in an expired record it is purged from the database. Current best practice is to avoid database modifications in response to a GET request as the GET verb is expected to be idempotent.
Description
Description
Details
Details
| Subject | Repo | Branch | Lines +/- | |
|---|---|---|---|---|
| Move expired record purge to a job | mediawiki/extensions/ThrottleOverride | master | +104 -44 |
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Open | None | T202759 Ease the bottleneck in wiki account creation at in-person events | |||
| Open | None | T27000 Deploy ThrottleOverride extension to Wikimedia wikis | |||
| Resolved | bd808 | T181025 Database purges should not be done for GET requests |
Event Timeline
Comment Actions
See comments on https://gerrit.wikimedia.org/r/#/c/392556/3/ThrottleOverride.hooks.php for more information/ideas from Aaron.
Comment Actions
I'd handle thr_expiry like ug_expiry in user_groups:
- SpecialThrottleOverrideList should filter out expired rows (it might already do, haven't checked)
- The hooks class should filter out expired rows (each row has to be checked against the current time anyways so that expired throttles do not apply).
- SpecialOverrideThrottle should purge expired rows whenever another (no matter whether a related one) override is edited/added/removed (the latter pending to be implemented). These operations are POST-requests anyways.
- The purging process should live in a in a job for performance reasons. DeferredUpdate would do as well, but according to the docs that's for time-critical stuff. If we just ignore expired rows, it doesn't matter whether those are removed from the db immediately or a few hours - or even a few days - later.
This will have expired overrides staying in the db for quite a while on small wikis (where SpecialOverrideThrottle isn't used regularly) though. But using "WHERE thr_expiry < $NOW" shouldn't be that hard, so this will never hit the mw layer and thus I'd say it can be neglected.
Comment Actions
Change 392778 had a related patch set uploaded (by BryanDavis; owner: Bryan Davis):
[mediawiki/extensions/ThrottleOverride@master] Move expired record purge to a job
Comment Actions
Change 392778 merged by jenkins-bot:
[mediawiki/extensions/ThrottleOverride@master] Move expired record purge to a job