diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index 29d7fe993..82be30d7a 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -35,7 +35,7 @@ }, { "ImportPath": "github.com/syncthing/protocol", - "Rev": "7996ef0d45b7743ff930048b6413b37b2c33cd85" + "Rev": "fcbd12b42176237d48985c1bfafbc43b2107aa31" }, { "ImportPath": "github.com/syndtr/goleveldb/leveldb", diff --git a/Godeps/_workspace/src/github.com/syncthing/protocol/conflict_test.go b/Godeps/_workspace/src/github.com/syncthing/protocol/conflict_test.go new file mode 100644 index 000000000..ef5c44d7e --- /dev/null +++ b/Godeps/_workspace/src/github.com/syncthing/protocol/conflict_test.go @@ -0,0 +1,23 @@ +// Copyright (C) 2015 The Protocol Authors. + +package protocol + +import "testing" + +func TestWinsConflict(t *testing.T) { + testcases := [][2]FileInfo{ + // The first should always win over the second + {{Modified: 42}, {Modified: 41}}, + {{Modified: 41}, {Modified: 42, Flags: FlagDeleted}}, + {{Modified: 41, Version: Vector{{42, 2}, {43, 1}}}, {Modified: 41, Version: Vector{{42, 1}, {43, 2}}}}, + } + + for _, tc := range testcases { + if !tc[0].WinsConflict(tc[1]) { + t.Errorf("%v should win over %v", tc[0], tc[1]) + } + if tc[1].WinsConflict(tc[0]) { + t.Errorf("%v should not win over %v", tc[1], tc[0]) + } + } +} diff --git a/Godeps/_workspace/src/github.com/syncthing/protocol/message.go b/Godeps/_workspace/src/github.com/syncthing/protocol/message.go index c2d898944..49df7d4fa 100644 --- a/Godeps/_workspace/src/github.com/syncthing/protocol/message.go +++ b/Godeps/_workspace/src/github.com/syncthing/protocol/message.go @@ -58,6 +58,31 @@ func (f FileInfo) HasPermissionBits() bool { return f.Flags&FlagNoPermBits == 0 } +// WinsConflict returns true if "f" is the one to choose when it is in +// conflict with "other". +func (f FileInfo) WinsConflict(other FileInfo) bool { + // If a modification is in conflict with a delete, we pick the + // modification. + if !f.IsDeleted() && other.IsDeleted() { + return true + } + if f.IsDeleted() && !other.IsDeleted() { + return false + } + + // The one with the newer modification time wins. + if f.Modified > other.Modified { + return true + } + if f.Modified < other.Modified { + return false + } + + // The modification times were equal. Use the device ID in the version + // vector as tie breaker. + return f.Version.Compare(other.Version) == ConcurrentGreater +} + type BlockInfo struct { Offset int64 // noencode (cache only) Size int32 diff --git a/internal/db/leveldb.go b/internal/db/leveldb.go index f7a69fb9d..762da6efe 100644 --- a/internal/db/leveldb.go +++ b/internal/db/leveldb.go @@ -236,7 +236,7 @@ func ldbGenericReplace(db *leveldb.DB, folder, device []byte, fs []protocol.File if fs[fsi].IsInvalid() { ldbRemoveFromGlobal(snap, batch, folder, device, newName) } else { - ldbUpdateGlobal(snap, batch, folder, device, newName, fs[fsi].Version) + ldbUpdateGlobal(snap, batch, folder, device, fs[fsi]) } fsi++ @@ -259,7 +259,7 @@ func ldbGenericReplace(db *leveldb.DB, folder, device []byte, fs []protocol.File if fs[fsi].IsInvalid() { ldbRemoveFromGlobal(snap, batch, folder, device, newName) } else { - ldbUpdateGlobal(snap, batch, folder, device, newName, fs[fsi].Version) + ldbUpdateGlobal(snap, batch, folder, device, fs[fsi]) } } else if debugDB { l.Debugln("generic replace; equal - ignore") @@ -348,7 +348,7 @@ func ldbReplaceWithDelete(db *leveldb.DB, folder, device []byte, fs []protocol.F } batch.Put(dbi.Key(), bs) mtimeRepo.DeleteMtime(tf.Name) - ldbUpdateGlobal(db, batch, folder, device, deviceKeyName(dbi.Key()), f.Version) + ldbUpdateGlobal(db, batch, folder, device, f) return ts } return 0 @@ -392,7 +392,7 @@ func ldbUpdate(db *leveldb.DB, folder, device []byte, fs []protocol.FileInfo) in if f.IsInvalid() { ldbRemoveFromGlobal(snap, batch, folder, device, name) } else { - ldbUpdateGlobal(snap, batch, folder, device, name, f.Version) + ldbUpdateGlobal(snap, batch, folder, device, f) } continue } @@ -411,7 +411,7 @@ func ldbUpdate(db *leveldb.DB, folder, device []byte, fs []protocol.FileInfo) in if f.IsInvalid() { ldbRemoveFromGlobal(snap, batch, folder, device, name) } else { - ldbUpdateGlobal(snap, batch, folder, device, name, f.Version) + ldbUpdateGlobal(snap, batch, folder, device, f) } } @@ -464,11 +464,12 @@ func ldbInsert(batch dbWriter, folder, device []byte, file protocol.FileInfo) in // ldbUpdateGlobal adds this device+version to the version list for the given // file. If the device is already present in the list, the version is updated. // If the file does not have an entry in the global list, it is created. -func ldbUpdateGlobal(db dbReader, batch dbWriter, folder, device, file []byte, version protocol.Vector) bool { +func ldbUpdateGlobal(db dbReader, batch dbWriter, folder, device []byte, file protocol.FileInfo) bool { if debugDB { - l.Debugf("update global; folder=%q device=%v file=%q version=%d", folder, protocol.DeviceIDFromBytes(device), file, version) + l.Debugf("update global; folder=%q device=%v file=%q version=%d", folder, protocol.DeviceIDFromBytes(device), file.Name, file.Version) } - gk := globalKey(folder, file) + name := []byte(file.Name) + gk := globalKey(folder, name) svl, err := db.Get(gk, nil) if err != nil && err != leveldb.ErrNotFound { panic(err) @@ -485,7 +486,7 @@ func ldbUpdateGlobal(db dbReader, batch dbWriter, folder, device, file []byte, v for i := range fl.versions { if bytes.Compare(fl.versions[i].device, device) == 0 { - if fl.versions[i].version.Equal(version) { + if fl.versions[i].version.Equal(file.Version) { // No need to do anything return false } @@ -497,21 +498,38 @@ func ldbUpdateGlobal(db dbReader, batch dbWriter, folder, device, file []byte, v nv := fileVersion{ device: device, - version: version, + version: file.Version, } + + // Find a position in the list to insert this file. The file at the front + // of the list is the newer, the "global". for i := range fl.versions { - // We compare against ConcurrentLesser as well here because we need - // to enforce a consistent ordering of versions even in the case of - // conflicts. - if comp := fl.versions[i].version.Compare(version); comp == protocol.Equal || comp == protocol.Lesser || comp == protocol.ConcurrentLesser { - t := append(fl.versions, fileVersion{}) - copy(t[i+1:], t[i:]) - t[i] = nv - fl.versions = t + switch fl.versions[i].version.Compare(file.Version) { + case protocol.Equal, protocol.Lesser: + // The version at this point in the list is equal to or lesser + // ("older") than us. We insert ourselves in front of it. + fl.versions = insertVersion(fl.versions, i, nv) goto done + + case protocol.ConcurrentLesser, protocol.ConcurrentGreater: + // The version at this point is in conflict with us. We must pull + // the actual file metadata to determine who wins. If we win, we + // insert ourselves in front of the loser here. (The "Lesser" and + // "Greater" in the condition above is just based on the device + // IDs in the version vector, which is not the only thing we use + // to determine the winner.) + of, ok := ldbGet(db, folder, fl.versions[i].device, name) + if !ok { + panic("file referenced in version list does not exist") + } + if file.WinsConflict(of) { + fl.versions = insertVersion(fl.versions, i, nv) + goto done + } } } + // We didn't find a position for an insert above, so append to the end. fl.versions = append(fl.versions, nv) done: @@ -524,6 +542,13 @@ done: return true } +func insertVersion(vl []fileVersion, i int, v fileVersion) []fileVersion { + t := append(vl, fileVersion{}) + copy(t[i+1:], t[i:]) + t[i] = v + return t +} + // ldbRemoveFromGlobal removes the device from the global version list for the // given file. If the version list is empty after this, the file entry is // removed entirely. @@ -644,7 +669,7 @@ func ldbWithAllFolderTruncated(db *leveldb.DB, folder []byte, fn func(device []b } } -func ldbGet(db *leveldb.DB, folder, device, file []byte) (protocol.FileInfo, bool) { +func ldbGet(db dbReader, folder, device, file []byte) (protocol.FileInfo, bool) { nk := deviceKey(folder, device, file) bs, err := db.Get(nk, nil) if err == leveldb.ErrNotFound { diff --git a/test/conflict_test.go b/test/conflict_test.go index 63ecbeb5a..72cbe1a84 100644 --- a/test/conflict_test.go +++ b/test/conflict_test.go @@ -9,6 +9,7 @@ package integration import ( + "bytes" "io/ioutil" "log" "os" @@ -156,15 +157,23 @@ func TestConflictsDefault(t *testing.T) { } rc.AwaitSync("default", sender, receiver) - // The conflict should manifest on the s2 side again, where we should have - // moved the file to a conflict copy instead of just deleting it. + // The conflict is resolved to the advantage of the edit over the delete. + // As such, we get the edited content synced back to s1 where it was + // removed. files, err = osutil.Glob("s2/*sync-conflict*") if err != nil { t.Fatal(err) } - if len(files) != 2 { - t.Errorf("Expected 2 conflicted files instead of %d", len(files)) + if len(files) != 1 { + t.Errorf("Expected 1 conflicted files instead of %d", len(files)) + } + bs, err := ioutil.ReadFile("s1/testfile.txt") + if err != nil { + t.Error("reading file:", err) + } + if !bytes.Contains(bs, []byte("more text added to s2")) { + t.Error("s1/testfile.txt should contain data added in s2") } }