Page MenuHomePhabricator

Quickly importing two versions of a file leads to timing issues in LocalFile.php
Closed, ResolvedPublic

Description

While working on the FileImporter extension I add an integration test that simulates the import of a file with two text- and two file-revisions using the complete core chain behind these operations. [1]

The test fails when I run it locally because it seems to be executed quite fast and it runs into a name conflict in the LocalFile.php. [2] The CI seems to be to slow so it does not occur there, but another colleague had the same problems locally.

I tried to fix the issue because one code path calling publishTo comes from uploadOld in OldLocalFile.php and there we already have a archive name that could be used ( and that does not collide with a current timestamp ) but this then leads to a different naming conflict deeper down in FileRepo with the copy and store operations. - That's where my brain stopped working and I thought it might be better putting this into a ticket.

So some questions:

  • Do we need to generate these archive names there?
  • Why does it seem we can't take the existing archive name there?
  • I don't get the difference between store and copy in the FileRepo why does every file triggers both?
  • Any ideas how to fix this?

[1] https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/FileImporter/+/445632/
[2] https://gerrit.wikimedia.org/g/mediawiki/core/+/06b70bbb47108545105be9065fe74302d1f82f05/includes/filerepo/file/LocalFile.php#1863

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 19 2018, 12:36 PM
Restricted Application added a project: TCB-Team. · View Herald TranscriptJul 19 2018, 12:43 PM

Change 445632 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/FileImporter@master] Extend Importer integration test using multiple revisions

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

WMDE-Fisch updated the task description. (Show Details)Jan 2 2019, 11:13 AM

Change 481860 had a related patch set uploaded (by WMDE-Fisch; owner: Thiemo Kreuz (WMDE)):
[mediawiki/core@master] Do not create new archive file names for old files

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

Change 445632 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@master] Extend Importer integration test using multiple revisions

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

Change 481860 had a related patch set uploaded (by Marostegui; owner: Thiemo Kreuz (WMDE)):
[mediawiki/core@master] Do not create new archive file names for old files

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

Change 481860 merged by jenkins-bot:
[mediawiki/core@master] Do not create new archive file names for old files

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

WMDE-Fisch closed this task as Resolved.Mar 22 2019, 1:34 PM
WMDE-Fisch claimed this task.

I cannot reproduce the issue anymore and with the above patch the timestamp strangeness in the code is also fixed.