Change 656251 had a related patch set uploaded (by MusikAnimal; owner: MusikAnimal):
[mediawiki/core@master] WatchedItemStore: Use REPLACE when inserting expiries after page move

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

Change 656251 merged by jenkins-bot:
[mediawiki/core@master] WatchedItemStore: Use REPLACE when inserting expiries after page move

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

I have not been able to reproduce the error on my local environment (MediaWiki 1.36.0-alpha (20e3a46)).

This change has been on group0 since yesterday. Looking at logstash, I cannot see this error occurring on group0 wikis since then. However, I don't know how to use logstash very well so perhaps someone who knows what they are doing could check.

@ifried I am moving this along because I think we have fixed the thing causing the production error. But, further to the questions for MusikAnimal below, there might be further bugs to be raised.

In T271971#6746180, @MusikAnimal wrote:

The impact of this bug is that it's possible users temporarily watching a page will end up permanently watching the new page, and an orphan row is left in the watchlist_expiry table. However the orphan row will eventually be deleted by WatchedItemStore::removeExpired(). And in my testing, the expiry is still preserved, both for me and others temporarily watching the page, on both the old title and new title. The error is also thrown in a deferred update so the user never sees it.

@MusikAnimal Speculating (with no evidence at the moment) as to the cause of this bug, could it be due to slow replication from the master to the replica databases?

If there is already a watchlist entry for $page and you move another page to $page, it replaces the entry in watchlist on master.

If updateExpiriesAfterMove runs a select on the replica database to find out the wl_id of the $page entry, it might get the old wl_id from before the replace.

This means the issue of orphan rows and pages incorrectly watched permanently you mentioned above could still happen, even if the production error does not occur.

I have tried to recreate this on my local environment, testwiki and mediawiki by repeatedly moving an article back and forward (e.g. from Article123 to Article123_moved and back again). But the temporary watch was maintained on both the pages. So I don't know if my hypothesis is actually accurate or possible.

Locally, it left behind a number of orphan watchlist_expiry rows. I guess if there is an existing row in watchlist which we replace we don't remove the old watchlist_expiry rows. But, as you say, the orphan rows will eventually be removed.

In T271971#6762121, @dom_walden wrote:

Speculating (with no evidence at the moment) as to the cause of this bug, could it be due to slow replication from the master to the replica databases?

If there is already a watchlist entry for $page and you move another page to $page, it replaces the entry in watchlist on master.

If updateExpiriesAfterMove runs a select on the replica database to find out the wl_id of the $page entry, it might get the old wl_id from before the replace.

updateExpiriesAfterMove should only be using DB_MASTER, so I don't think that's it.