diff --git a/e2e/multiroot_scenarios_test.go b/e2e/multiroot_scenarios_test.go index 16615064..e6a4e54d 100644 --- a/e2e/multiroot_scenarios_test.go +++ b/e2e/multiroot_scenarios_test.go @@ -966,3 +966,158 @@ func TestIgnore_ExcludedFilesInExtraLocationsAreStillSnapshotted(t *testing.T) { "rollback empties the folder first, so anything missing from the snapshot is destroyed", got) } } + +// The same deletion when the peer's report that it took the location never +// arrived: on a loaded machine it can time out ("could not tell ... that ... +// is sync-complete: context deadline exceeded"), and then this device has no +// record that the peer holds those files. It must still not take the deleted +// file back from the peer as one it never had. +func TestMultiRootScenario_ADeletionSurvivesALostReport(t *testing.T) { + a, b, gameID, aConfig, bConfig := twoLocationPair(t, "LocLostReport", map[string]string{ + "settings.ini": "fullscreen=1", + "keys.cfg": "jump=space", + }) + pauseAutoSync(t, a, b, gameID) + + // Where A stands when the report is lost: it handed the location over and + // recorded that, and nothing since says the peer took it. + pushed, err := delta.BuildManifest(aConfig) + if err != nil { + t.Fatal(err) + } + store := a.Daemon.Store + if err := store.SetSyncStateForRoot(gameID, b.NodeID(), "config", nil, nil); err != nil { + t.Fatal(err) + } + if err := store.SetAgreedHashForRoot(gameID, b.NodeID(), "config", ""); err != nil { + t.Fatal(err) + } + if err := store.SetPushedHashForRoot(gameID, b.NodeID(), "config", pushed.RootHash(delta.PrimaryRoot)); err != nil { + t.Fatal(err) + } + + if err := os.Remove(filepath.Join(aConfig, "keys.cfg")); err != nil { + t.Fatal(err) + } + time.Sleep(syncSettleWindow) + a.API(http.MethodPost, "/api/games/"+gameID+"/sync", nil, nil) + + if !testutil.WaitFor(45*time.Second, func() bool { + return readIn(bConfig, "keys.cfg") == "" + }) { + t.Fatalf("the deletion never reached the peer: keys.cfg there=%q, here=%q", + readIn(bConfig, "keys.cfg"), readIn(aConfig, "keys.cfg")) + } + if got := readIn(aConfig, "keys.cfg"); got != "" { + t.Errorf("the deleted file came back here: %q", got) + } + if got := readIn(bConfig, "settings.ini"); got != "fullscreen=1" { + t.Errorf("an untouched file on the peer changed: %q", got) + } +} + +// The report that the peer took the location arrives, and the refresh it +// starts in the background runs only after a file has been deleted here — a +// moment's delay on a loaded machine. The refresh must not drop the deleted +// file from the record of what both hold, or the next sync fetches it back. +func TestMultiRootScenario_ADeletionSurvivesALateRefresh(t *testing.T) { + a, b, gameID, aConfig, bConfig := twoLocationPair(t, "LocLateRefresh", map[string]string{ + "settings.ini": "fullscreen=1", + "keys.cfg": "jump=space", + }) + pauseAutoSync(t, a, b, gameID) + if !testutil.WaitFor(45*time.Second, func() bool { + files, _, _ := a.Daemon.Store.GetSyncStateForRoot(gameID, b.NodeID(), "config") + for _, f := range files { + if f == "keys.cfg" { + return true + } + } + return false + }) { + t.Fatal("setup: A never recorded that B holds keys.cfg") + } + + if err := os.Remove(filepath.Join(aConfig, "keys.cfg")); err != nil { + t.Fatal(err) + } + var peer syncengine.Peer + for _, p := range a.Daemon.P2P.OnlinePeers() { + if p.ID == b.NodeID() { + peer = p + } + } + if peer.ID == "" { + t.Fatal("B is not online for A") + } + // The refresh the report started, running late. + a.Daemon.P2P.Sync.RefreshLineage(context.Background(), gameID, peer) + + time.Sleep(syncSettleWindow) + a.API(http.MethodPost, "/api/games/"+gameID+"/sync", nil, nil) + if !testutil.WaitFor(45*time.Second, func() bool { + return readIn(bConfig, "keys.cfg") == "" + }) { + t.Fatalf("the deletion never reached the peer: keys.cfg there=%q, here=%q", + readIn(bConfig, "keys.cfg"), readIn(aConfig, "keys.cfg")) + } + if got := readIn(aConfig, "keys.cfg"); got != "" { + t.Errorf("the deleted file came back here: %q", got) + } +} + +// "Put them back" for an emptied extra location, with no snapshot to restore, +// fetches its files even when a lost report has left the location's base +// behind. The push-proof repair records what both devices held; while the +// game is held as emptied it must leave alone the lineage put back takes the +// files out of, or they read as deleted here and the peer loses them too. +func TestMultiRootScenario_PutBackOfAnEmptiedLocationSurvivesALostReport(t *testing.T) { + files := map[string]string{"settings.ini": "shared v1", "keys.cfg": "jump=space"} + a, b, gameID, aConfig, bConfig := twoLocationPair(t, "LocPutBack", files) + testutil.SettleSync(t, gameID, a, b) + + emptyFolder(t, aConfig) + syncBothWays(t, gameID, a, b) + if e := emptiedState(t, a, gameID); e == nil { + t.Fatal("setup: emptying the location did not hold the game back") + } + var snaps map[string]struct { + Branches map[string]struct { + Snapshots []struct { + ID string `json:"id"` + } `json:"snapshots"` + } `json:"branches"` + } + a.API(http.MethodGet, "/api/games", nil, &snaps) + for _, br := range snaps[gameID].Branches { + for _, s := range br.Snapshots { + if err := a.Daemon.Store.SetSnapshotCheck(s.ID, time.Now().UnixMilli(), "damaged for this test"); err != nil { + t.Fatal(err) + } + } + } + a.API(http.MethodPost, "/api/games/"+gameID+"/emptied", map[string]string{"answer": "restore"}, nil) + + // What a lost report leaves for the location: the base behind what the + // peer holds, and the push still on record. + m, err := delta.BuildManifest(bConfig) + if err != nil { + t.Fatal(err) + } + if err := a.Daemon.Store.SetAgreedHashForRoot(gameID, b.NodeID(), "config", "a base from before"); err != nil { + t.Fatal(err) + } + if err := a.Daemon.Store.SetPushedHashForRoot(gameID, b.NodeID(), "config", m.RootHash(delta.PrimaryRoot)); err != nil { + t.Fatal(err) + } + + syncBothWays(t, gameID, a, b) + for rel, want := range files { + if got := readIn(bConfig, rel); got != want { + t.Errorf("the other device: config/%s is %q, want %q", rel, got, want) + } + if got := readIn(aConfig, rel); got != want { + t.Errorf("the emptied device, from the other: config/%s is %q, want %q", rel, got, want) + } + } +} diff --git a/internal/p2p/syncengine/multiroot.go b/internal/p2p/syncengine/multiroot.go index 6810e402..81a64fea 100644 --- a/internal/p2p/syncengine/multiroot.go +++ b/internal/p2p/syncengine/multiroot.go @@ -119,6 +119,38 @@ func (e *Engine) syncOneRoot(ctx context.Context, gameID string, game store.Game remote = filterManifest(remote, rules) } + // A push is proven once the peer is seen holding exactly what was handed + // over, whatever became of its report — the repair the main folder makes + // (see SyncWithPeer), and for the same reason. The push was recorded below + // and nothing ever read it, so a lost report left this location's base + // behind both devices, and the next edit on one of them read as both + // having moved: a conflict over a change only one device made. + // + // The lineage is repaired with it, before anything is classified: the + // report is also what records which files both sides hold. Without that, + // a file deleted here after a lost report read as one the peer had and + // this device never did, and it was fetched back — the deletion undone + // (TestMultiRootScenario_ADeletionSurvivesALostReport). Both held exactly + // the pushed state, so its files are the lineage. + // + // Not while the game is held as emptied (hold.go): the lineage is then + // kept on purpose. "Put them back" with no snapshot takes the files out of + // it so that the next sync fetches them; written back here, they read as + // deleted on this device, and the deletions went to the peer — both + // devices lost the location's files + // (TestMultiRootScenario_PutBackOfAnEmptiedLocationSurvivesALostReport). + // + // Both hashes are of the filtered view, which is what the push recorded. + if pushed := e.Store.GetPushedHashForRoot(gameID, peer.ID, sr.root.Name); pushed != "" { + if remoteHash := remote.RootHash(delta.PrimaryRoot); remoteHash == pushed && + e.Store.GetAgreedHashForRoot(gameID, peer.ID, sr.root.Name) != remoteHash { + if _, held, err := e.Store.GetDeletionHold(gameID); err == nil && !held { + e.persistRootLineage(gameID, peer.ID, sr.root.Name, remote, remote) + } + _ = e.Store.SetAgreedHashForRoot(gameID, peer.ID, sr.root.Name, remoteHash) + } + } + fileList, dirList, err := e.Store.GetSyncStateForRoot(gameID, peer.ID, sr.root.Name) if err != nil { return err @@ -132,24 +164,22 @@ func (e *Engine) syncOneRoot(ctx context.Context, gameID string, game store.Game lineageFiles = filterLineage(lineageFiles, rules) lineageDirs = filterLineage(lineageDirs, rules) } + // And the deletions this device recorded in this location, as the main + // folder uses them (see SyncWithPeer): a recorded deletion does not depend + // on the lineage surviving, and is acted on only while the peer's copy is + // still exactly what was deleted. + deleted := e.recordedDeletions(gameID, sr.root.Name) + if !rules.Empty() { + for path := range deleted { + if rules.Match(path) { + delete(deleted, path) + } + } + } // The same base the conflict check below uses. Compute used to be called // with an empty base, so a named location ranked files by mtime even when // one side was still the confirmed version. base := e.Store.GetAgreedHashForRoot(gameID, peer.ID, sr.root.Name) - // A push is proven once the peer is seen holding exactly what was handed - // over, whatever became of its report — the repair the main folder makes - // (see SyncWithPeer), and for the same reason. The push was recorded below - // and nothing ever read it, so a lost report left this location's base - // behind both devices, and the next edit on one of them read as both - // having moved: a conflict over a change only one device made. - // - // Both hashes are of the filtered view, which is what the push recorded. - if pushed := e.Store.GetPushedHashForRoot(gameID, peer.ID, sr.root.Name); pushed != "" { - if remoteHash := remote.RootHash(delta.PrimaryRoot); base != remoteHash && remoteHash == pushed { - _ = e.Store.SetAgreedHashForRoot(gameID, peer.ID, sr.root.Name, remoteHash) - base = remoteHash - } - } // A base recorded before the rules existed was hashed over everything, so // it can equal neither filtered side — and a base matching neither reads // as both having moved, which is a conflict on the first sync after anyone @@ -164,7 +194,7 @@ func (e *Engine) syncOneRoot(ctx context.Context, gameID string, game store.Game base = remote.RootHash(delta.PrimaryRoot) } } - decision := ComputeWithBase(local, remote, lineageFiles, lineageDirs, base) + decision := ComputeWithDeletions(local, remote, lineageFiles, lineageDirs, base, deleted) if !decision.HasChanges() { e.persistRootLineage(gameID, peer.ID, sr.root.Name, local, remote) _ = e.Store.SetAgreedHashForRoot(gameID, peer.ID, sr.root.Name, local.RootHash(delta.PrimaryRoot)) @@ -258,6 +288,22 @@ func (e *Engine) syncOneRoot(ctx context.Context, gameID string, game store.Game func (e *Engine) persistRootLineage(gameID, peerID, root string, local, remote delta.Manifest) { files, dirs := IntersectLineage(local, remote) + // As persistLineage does for the main folder: a path both held before + // that one side holds no longer is a deletion in flight, and its record is + // what lets the next sync send it on rather than fetch the file back. The + // refresh after a peer's report runs in the background, so a file deleted + // here before it ran dropped out of the record — and came back + // (TestMultiRootScenario_ADeletionSurvivesALateRefresh). + if oldFiles, oldDirs, err := e.Store.GetSyncStateForRoot(gameID, peerID, root); err == nil { + files = keepPendingDeletions(files, oldFiles, local.Files, remote.Files) + dirs = keepPendingDirDeletions(dirs, oldDirs, local.Dirs, remote.Dirs) + } + // And excluded paths stay out of it, as in the main folder: the refresh + // rebuilds this from unfiltered manifests. + if rules := e.rulesFor(gameID); !rules.Empty() { + files = filterPathList(files, rules) + dirs = filterPathList(dirs, rules) + } if err := e.Store.SetSyncStateForRoot(gameID, peerID, root, files, dirs); err != nil { e.Log("warn", fmt.Sprintf("persist lineage for the %q location failed: %v", root, err)) }