Page MenuHomePhabricator

Simplify "builtin file" management and recover from races
ClosedPublic

Authored by epriestley on Jul 11 2016, 4:19 PM.

Details

Summary

Fixes T11307. Fixes T8124. Currently, builtin files are tracked by using a special transform with an invalid source ID.

Just use a dedicated column instead. The transform thing is too clever/weird/hacky and exposes us to issues with the "file" and "transform" tables getting out of sync (possibly the issue in T11307?) and with race conditions.

Test Plan
  • Loaded profile "edit picture" page, saw builtins.
  • Deleted all builtin files, put 3 second sleep in the storage engine write, loaded profile page in two windows.
    • Before patch: one of them failed with a race.
    • After patch: both of them loaded.

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley updated this revision to Diff 39138.Jul 11 2016, 4:19 PM
epriestley retitled this revision from to Simplify "builtin file" management and recover from races.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.
chad accepted this revision.Jul 11 2016, 4:24 PM
chad edited edge metadata.
This revision is now accepted and ready to land.Jul 11 2016, 4:24 PM
This revision was automatically updated to reflect the committed changes.