In several places in the codebase, timestamps are used to determine the order of edits and other actions, which can lead to issues in situations where several actions happen at nearly the same time. One example of this is countRevisionsBetween in Title.php. If multiple revisions of a page are made within 2 seconds, it will return an incorrect count when used on those revisions. Shouldn't this and all similar uses be changed to compare based on the table's primary key instead (revision ID in this case)?
Description
Details
- Reference
- bz59609
Status | Subtype | Assigned | Task | ||
---|---|---|---|---|---|
Open | None | T72163 Edit conflicts (tracking) | |||
Open | None | T61609 Over-reliance on timestamps can lead to incorrect counts | |||
Open | None | T61694 Edit form should keep track of what the latest revision was when editing began |
Event Timeline
(In reply to comment #0)
In several places in the codebase
Thanks for reporting this. If you are aware of more places, please list them.
I've just now noticed that edit conflict detection in EditPage.php does the same thing.
624dfd884e683dddc3fb2d86e6764277e8546bef by @aaron changed countRevisionsBetween and countAuthorsBetween to use rev_timestamp instead of rev_id.
It took me some time to figure out why diffs like this don't show the multinotice.
The reason our code often uses timestamps (instead of primary keys) is because revisions can be inserted out of sequence. Most commonly as a result of importing revisions and pages via Special:Import.
It may also happen as result of a history merge (unsure?).
Timestamps tend be to be preferred because they match the way we view revisions on the history page.
As described, I think the task should be declined. But it would be reasonable to extend the resolution of timestamps to the point where they are unique, for example, using a time UUID like UIDGenerator::newTimestampedUID88(). Revision IDs are not monotonic in time and should never be used to select ranges for display.