Comment 3 for bug 507557

Revision history for this message
Gareth White (gwhite-deactivatedaccount) wrote :

I've done some more investigation into this issue. Here's what I've found so far:

1. The first 9 commits each create a single-revision pack (call them p1-p9) and add them to pack-names.
2. The 10th commit creates p10 but does not yet add it to pack-names.
3. The 11th commit (on a different branch) creates p11 but does not yet add it to pack-names.
4. We're now at step 5 in the description above, where both processes are stopped at _create_pack_from_packs(). There are 11 single-revision packs on disk (p1-p11), but only p1-p9 are in pack-names at this stage.
5. After the first process has continued up to _obsolete_packs(), there are 12 packs on disk. These are p1-p11 plus a new 10-revision pack (call it q1) which contains the revisions in p1-p10. At this stage pack-names has been updated to contain just q1.
6a. If the first process now continues, p1-p10 will be moved into obsolete_packs, leaving just q1 and p11. The second process will then try to create a new 10-rev pack, fail because it can't find p1-p9, retry the autopack, determine nothing needs doing and instead just add p11 to pack-names. Both commits succeed and everyone is happy.
6b. If the second process continues *before* the first process calls obsolete_packs() (as in the example at the start), things go a bit differently. A second 10-rev pack (q2) is created on disk containing p1-p9 plus p11, and pack-names now references q1 and q2.
7. At this stage on disk there are p1-p11, q1 and q2.
8. The first process obsoletes p1-p10, leaving just p11, q1 and q2.
9. The second process tries to obsolete p1-p9 plus p11 but fails because p1-p9 aren't there.
10. The second commit fails, leaving p11 "orphaned" on disk. The revisions in p1-p9 are now in two separate pack files (q1 & q2), both of which are referenced in pack-names.

Some possible solutions:
1. If it's OK to have some revisions duplicated in different packs then we could:
    (a) make obsolete_pack() ignore NoSuchFile and just assume someone else had already obsoleted the file, or
    (b) be smarter about which packs we try to obsolete. We may be able to determine this in _diff_pack_names() by calculating the intersection of deleted_nodes and disk_nodes.
2. If it's not OK then we could somehow detect that revisions were being duplicated and back out the second autopack. Perhaps _save_pack_names() could fail if packs it was expecting to remove had already been removed from pack-names.