Page MenuHomePhabricator

Watchlist Expiry: Don't run database UPDATEs to re-watch a page when the expiry is the same
Open, Needs TriagePublicWed, Oct 21

Description

As someone concerned about performance, I want to make sure no UPDATEs are ran when watching a page unless the watched state or the expiry changed. If I watch a page with the expiry 9999-01-23T00:00Z, and then re-watch again with the same expiry, it should not attempt any database transactions since nothing changed.

Background: The logic up until now was to run an UPDATE if either the watched state changed or any expiry value is provided. It did not actually compare the stored expiry with the new one, meaning if I could re-watch a page with the same expiry and it'd unnecessarily run an UPDATE every time.

Acceptance criteria:

  • No UPDATE is performed unless it has to.

NOTE: this may be difficult to test manually; you could try observing the SQL log (with $wgDebugDumpSql set), or perhaps add log/var_dump statements to WatchedItemStore::addWatchBatchForUser()

Event Timeline

Restricted Application added a project: Community-Tech. ยท View Herald TranscriptJul 22 2020, 8:10 PM
Restricted Application added a subscriber: Aklapper. ยท View Herald Transcript

Change 615555 had a related patch set uploaded (by MusikAnimal; owner: MusikAnimal):
[mediawiki/core@master] WatchAction: avoid unnecessary UPDATEs when expiry is unchanged

https://gerrit.wikimedia.org/r/615555

Change 615555 merged by jenkins-bot:
[mediawiki/core@master] WatchAction: avoid unnecessary UPDATEs when expiry is unchanged

https://gerrit.wikimedia.org/r/615555

Note again that this may be difficult to test manually. You could try observing the SQL log (with $wgDebugDumpSql set), or perhaps add logging/var_dump statements to WatchedItemStore::addWatchBatchForUser().

Change 620674 had a related patch set uploaded (by MusikAnimal; owner: MusikAnimal):
[mediawiki/core@REL1_35] WatchAction: avoid unnecessary UPDATEs when expiry is unchanged

https://gerrit.wikimedia.org/r/620674

Change 620674 merged by jenkins-bot:
[mediawiki/core@REL1_35] WatchAction: avoid unnecessary UPDATEs when expiry is unchanged

https://gerrit.wikimedia.org/r/620674

@MusikAnimal I find when the page is already permanently watched and I choose a permanent expiry value (e.g. "infinite") it is doing some INSERTs/DELETEs.

I am assuming because ExpiryDef::normalizeExpiry( $oldWatchedItem->getExpiry() ) returns null but ExpiryDef::normalizeExpiry( $expiry ); returns "infinity".

For example, editing a permanently watched page, keeping the edit expiry dropdown as "Permanent":

[DBQuery] WatchedItemStore::addWatchBatchForUser [0s] 127.0.0.1: INSERT IGNORE INTO `watchlist` (wl_user,wl_namespace,wl_title,wl_notificationtimestamp) VALUES (4,0,'Vnphyyblfr17',NULL),(4,1,'Vnphyyblfr17',NULL)
[DBQuery] WatchedItemStore::updateOrDeleteExpiries [0.001s] 127.0.0.1: DELETE `watchlist_expiry` FROM `watchlist_expiry`, `watchlist` WHERE we_item=wl_id  AND ((wl_user = 4 AND wl_namespace = 0 AND wl_title = 'Vnphyyblfr17') OR (wl_user = 4 AND wl_namespace = 1 AND wl_title = 'Vnphyyblfr17'))
ARamirez_WMF set Due Date to Wed, Oct 21, 4:00 AM.Thu, Oct 8, 10:07 PM
ARamirez_WMF changed the subtype of this task from "Task" to "Deadline".

Change 633830 had a related patch set uploaded (by MusikAnimal; owner: MusikAnimal):
[mediawiki/core@master] WatchAction: avoid UPDATE when old and new watch period is indefinite

https://gerrit.wikimedia.org/r/633830

Change 633830 merged by jenkins-bot:
[mediawiki/core@master] WatchAction: avoid UPDATE when old and new watch period is indefinite

https://gerrit.wikimedia.org/r/633830

@dom_walden Ready for QA again! I thought I'd ping you since this was incorrectly in your column for almost 2 months... now it's intentionally there :)

Change 633765 had a related patch set uploaded (by Samwilson; owner: MusikAnimal):
[mediawiki/core@REL1_35] WatchAction: avoid UPDATE when old and new watch period is indefinite

https://gerrit.wikimedia.org/r/633765

Change 633765 merged by jenkins-bot:
[mediawiki/core@REL1_35] WatchAction: avoid UPDATE when old and new watch period is indefinite

https://gerrit.wikimedia.org/r/633765

@dom_walden Ready for QA again! I thought I'd ping you since this was incorrectly in your column for almost 2 months... now it's intentionally there :)

Thanks!

The issue in T258649#6393420 has now been fixed.

The watchlist_expiry database table is only updated when the expiry changes.

I tested changing the expiry and watch status via Source Editor and Api:Edit.

As mentioned in T258649#6390877, I don't think you can see this in action on production.

Test Environment: Local vagrant MediaWiki 1.36.0-alpha (8e68310).