From 00fa77dd479a0e8533b89ffbb1ff02ee7f7795be Mon Sep 17 00:00:00 2001 From: Simon Frei Date: Sun, 20 Jan 2019 08:47:20 +0100 Subject: [PATCH] lib/db: Consistent use of buffers (#5470) --- lib/db/instance.go | 26 +++++++------- lib/db/schemaupdater.go | 8 ++--- lib/db/transactions.go | 77 +++++++++++++++++++++++------------------ 3 files changed, 59 insertions(+), 52 deletions(-) diff --git a/lib/db/instance.go b/lib/db/instance.go index 603b82f3..d670e7c8 100644 --- a/lib/db/instance.go +++ b/lib/db/instance.go @@ -34,7 +34,7 @@ func (db *instance) updateFiles(folder, device []byte, fs []protocol.FileInfo, m t := db.newReadWriteTransaction() defer t.close() - var dk, gk []byte + var dk, gk, keyBuf []byte devID := protocol.DeviceIDFromBytes(device) for _, f := range fs { name := []byte(f.Name) @@ -53,7 +53,7 @@ func (db *instance) updateFiles(folder, device []byte, fs []protocol.FileInfo, m t.insertFile(dk, folder, device, f) gk = db.keyer.GenerateGlobalVersionKey(gk, folder, name) - t.updateGlobal(gk, folder, device, f, meta) + keyBuf, _ = t.updateGlobal(gk, keyBuf, folder, device, f, meta) // Write out and reuse the batch every few records, to avoid the batch // growing too large and thus allocating unnecessarily much memory. @@ -156,7 +156,7 @@ func (db *instance) withAllFolderTruncated(folder []byte, fn func(device []byte, dbi := t.NewIterator(util.BytesPrefix(db.keyer.GenerateDeviceFileKey(nil, folder, nil, nil).WithoutNameAndDevice()), nil) defer dbi.Release() - var gk []byte + var gk, buf []byte for dbi.Next() { device, ok := db.keyer.DeviceFromDeviceFileKey(dbi.Key()) if !ok { @@ -181,7 +181,7 @@ func (db *instance) withAllFolderTruncated(folder []byte, fn func(device []byte, l.Infof("Dropping invalid filename %q from database", f.Name) name := []byte(f.Name) gk = db.keyer.GenerateGlobalVersionKey(gk, folder, name) - t.removeFromGlobal(gk, folder, device, name, nil) + buf = t.removeFromGlobal(gk, buf, folder, device, name, nil) t.Delete(dbi.Key()) t.checkFlush() continue @@ -202,8 +202,7 @@ func (db *instance) getFileDirty(folder, device, file []byte) (protocol.FileInfo func (db *instance) getGlobalDirty(folder, file []byte, truncate bool) (FileIntf, bool) { t := db.newReadOnlyTransaction() defer t.close() - - _, _, f, ok := t.getGlobalInto(nil, nil, folder, file, truncate) + _, f, ok := t.getGlobal(nil, folder, file, truncate) return f, ok } @@ -219,7 +218,7 @@ func (db *instance) withGlobal(folder, prefix []byte, truncate bool, fn Iterator prefix = append(prefix, '/') } - if _, _, f, ok := t.getGlobalInto(nil, nil, folder, unslashedPrefix, truncate); ok && !fn(f) { + if _, f, ok := t.getGlobal(nil, folder, unslashedPrefix, truncate); ok && !fn(f) { return } } @@ -363,11 +362,11 @@ func (db *instance) withNeedLocal(folder []byte, truncate bool, fn Iterator) { dbi := t.NewIterator(util.BytesPrefix(db.keyer.GenerateNeedFileKey(nil, folder, nil).WithoutName()), nil) defer dbi.Release() - var gk, dk []byte + var buf []byte var f FileIntf var ok bool for dbi.Next() { - gk, dk, f, ok = t.getGlobalInto(gk, dk, folder, db.keyer.NameFromGlobalVersionKey(dbi.Key()), truncate) + buf, f, ok = t.getGlobal(buf, folder, db.keyer.NameFromGlobalVersionKey(dbi.Key()), truncate) if !ok { continue } @@ -402,13 +401,12 @@ func (db *instance) dropDeviceFolder(device, folder []byte, meta *metadataTracke dbi := t.NewIterator(util.BytesPrefix(db.keyer.GenerateDeviceFileKey(nil, folder, device, nil)), nil) defer dbi.Release() - var gk []byte + var gk, buf []byte for dbi.Next() { - key := dbi.Key() - name := db.keyer.NameFromDeviceFileKey(key) + name := db.keyer.NameFromDeviceFileKey(dbi.Key()) gk = db.keyer.GenerateGlobalVersionKey(gk, folder, name) - t.removeFromGlobal(gk, folder, device, name, meta) - t.Delete(key) + buf = t.removeFromGlobal(gk, buf, folder, device, name, meta) + t.Delete(dbi.Key()) t.checkFlush() } } diff --git a/lib/db/schemaupdater.go b/lib/db/schemaupdater.go index ac1d05c0..acb6b892 100644 --- a/lib/db/schemaupdater.go +++ b/lib/db/schemaupdater.go @@ -101,7 +101,7 @@ func (db *schemaUpdater) updateSchema0to1() { changedFolders := make(map[string]struct{}) ignAdded := 0 meta := newMetadataTracker() // dummy metadata tracker - var gk []byte + var gk, buf []byte for dbi.Next() { folder, ok := db.keyer.FolderFromDeviceFileKey(dbi.Key()) @@ -126,7 +126,7 @@ func (db *schemaUpdater) updateSchema0to1() { changedFolders[string(folder)] = struct{}{} } gk = db.keyer.GenerateGlobalVersionKey(gk, folder, name) - t.removeFromGlobal(gk, folder, device, nil, nil) + buf = t.removeFromGlobal(gk, buf, folder, device, nil, nil) t.Delete(dbi.Key()) t.checkFlush() continue @@ -156,8 +156,8 @@ func (db *schemaUpdater) updateSchema0to1() { // Add invalid files to global list if f.IsInvalid() { gk = db.keyer.GenerateGlobalVersionKey(gk, folder, name) - if t.updateGlobal(gk, folder, device, f, meta) { - if _, ok := changedFolders[string(folder)]; !ok { + if buf, ok = t.updateGlobal(gk, buf, folder, device, f, meta); ok { + if _, ok = changedFolders[string(folder)]; !ok { changedFolders[string(folder)] = struct{}{} } ignAdded++ diff --git a/lib/db/transactions.go b/lib/db/transactions.go index a7775431..e0c97c6a 100644 --- a/lib/db/transactions.go +++ b/lib/db/transactions.go @@ -56,7 +56,6 @@ func (t readOnlyTransaction) getFileTrunc(key []byte, trunc bool) (FileIntf, boo l.Debugln("surprise error:", err) return nil, false } - f, err := unmarshalTrunc(bs, trunc) if err != nil { l.Debugln("unmarshal error:", err) @@ -65,25 +64,25 @@ func (t readOnlyTransaction) getFileTrunc(key []byte, trunc bool) (FileIntf, boo return f, true } -func (t readOnlyTransaction) getGlobalInto(gk, dk, folder, file []byte, truncate bool) ([]byte, []byte, FileIntf, bool) { - gk = t.db.keyer.GenerateGlobalVersionKey(gk, folder, file) +func (t readOnlyTransaction) getGlobal(keyBuf, folder, file []byte, truncate bool) ([]byte, FileIntf, bool) { + keyBuf = t.db.keyer.GenerateGlobalVersionKey(keyBuf, folder, file) - bs, err := t.Get(gk, nil) + bs, err := t.Get(keyBuf, nil) if err != nil { - return gk, dk, nil, false + return keyBuf, nil, false } vl, ok := unmarshalVersionList(bs) if !ok { - return gk, dk, nil, false + return keyBuf, nil, false } - dk = t.db.keyer.GenerateDeviceFileKey(dk, folder, vl.Versions[0].Device, file) - if fi, ok := t.getFileTrunc(dk, truncate); ok { - return gk, dk, fi, true + keyBuf = t.db.keyer.GenerateDeviceFileKey(keyBuf, folder, vl.Versions[0].Device, file) + if fi, ok := t.getFileTrunc(keyBuf, truncate); ok { + return keyBuf, fi, true } - return gk, dk, nil, false + return keyBuf, nil, false } // A readWriteTransaction is a readOnlyTransaction plus a batch for writes. @@ -129,7 +128,7 @@ func (t readWriteTransaction) insertFile(fk, folder, device []byte, file protoco // updateGlobal 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 (t readWriteTransaction) updateGlobal(gk, folder, device []byte, file protocol.FileInfo, meta *metadataTracker) bool { +func (t readWriteTransaction) updateGlobal(gk, keyBuf, folder, device []byte, file protocol.FileInfo, meta *metadataTracker) ([]byte, bool) { l.Debugf("update global; folder=%q device=%v file=%q version=%v invalid=%v", folder, protocol.DeviceIDFromBytes(device), file.Name, file.Version, file.IsInvalid()) var fl VersionList @@ -139,7 +138,7 @@ func (t readWriteTransaction) updateGlobal(gk, folder, device []byte, file proto fl, removedFV, removedAt, insertedAt := fl.update(folder, device, file, t.readOnlyTransaction) if insertedAt == -1 { l.Debugln("update global; same version, global unchanged") - return false + return keyBuf, false } name := []byte(file.Name) @@ -148,19 +147,22 @@ func (t readWriteTransaction) updateGlobal(gk, folder, device []byte, file proto if insertedAt == 0 { // Inserted a new newest version global = file - } else if new, ok := t.getFile(folder, fl.Versions[0].Device, name); ok { - global = new } else { - panic("This file must exist in the db") + keyBuf = t.db.keyer.GenerateDeviceFileKey(keyBuf, folder, fl.Versions[0].Device, name) + if new, ok := t.getFileByKey(keyBuf); ok { + global = new + } else { + panic("This file must exist in the db") + } } // Fixup the list of files we need. - t.updateLocalNeed(folder, name, fl, global) + keyBuf = t.updateLocalNeed(keyBuf, folder, name, fl, global) if removedAt != 0 && insertedAt != 0 { l.Debugf(`new global for "%v" after update: %v`, file.Name, fl) t.Put(gk, mustMarshal(&fl)) - return true + return keyBuf, true } // Remove the old global from the global size counter @@ -171,7 +173,8 @@ func (t readWriteTransaction) updateGlobal(gk, folder, device []byte, file proto // The previous newest version is now at index 1 oldGlobalFV = fl.Versions[1] } - if oldFile, ok := t.getFile(folder, oldGlobalFV.Device, name); ok { + keyBuf = t.db.keyer.GenerateDeviceFileKey(keyBuf, folder, oldGlobalFV.Device, name) + if oldFile, ok := t.getFileByKey(keyBuf); ok { // A failure to get the file here is surprising and our // global size data will be incorrect until a restart... meta.removeFile(protocol.GlobalDeviceID, oldFile) @@ -183,24 +186,25 @@ func (t readWriteTransaction) updateGlobal(gk, folder, device []byte, file proto l.Debugf(`new global for "%v" after update: %v`, file.Name, fl) t.Put(gk, mustMarshal(&fl)) - return true + return keyBuf, true } -// updateLocalNeeds checks whether the given file is still needed on the local +// updateLocalNeed checks whether the given file is still needed on the local // device according to the version list and global FileInfo given and updates // the db accordingly. -func (t readWriteTransaction) updateLocalNeed(folder, name []byte, fl VersionList, global protocol.FileInfo) { - nk := t.db.keyer.GenerateNeedFileKey(nil, folder, name) - hasNeeded, _ := t.db.Has(nk, nil) +func (t readWriteTransaction) updateLocalNeed(keyBuf, folder, name []byte, fl VersionList, global protocol.FileInfo) []byte { + keyBuf = t.db.keyer.GenerateNeedFileKey(keyBuf, folder, name) + hasNeeded, _ := t.Has(keyBuf, nil) if localFV, haveLocalFV := fl.Get(protocol.LocalDeviceID[:]); need(global, haveLocalFV, localFV.Version) { if !hasNeeded { l.Debugf("local need insert; folder=%q, name=%q", folder, name) - t.Put(nk, nil) + t.Put(keyBuf, nil) } } else if hasNeeded { l.Debugf("local need delete; folder=%q, name=%q", folder, name) - t.Delete(nk) + t.Delete(keyBuf) } + return keyBuf } func need(global FileIntf, haveLocal bool, localVersion protocol.Vector) bool { @@ -222,54 +226,59 @@ func need(global FileIntf, haveLocal bool, localVersion protocol.Vector) bool { // removeFromGlobal 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. -func (t readWriteTransaction) removeFromGlobal(gk, folder, device, file []byte, meta *metadataTracker) { +func (t readWriteTransaction) removeFromGlobal(gk, keyBuf, folder, device []byte, file []byte, meta *metadataTracker) []byte { l.Debugf("remove from global; folder=%q device=%v file=%q", folder, protocol.DeviceIDFromBytes(device), file) svl, err := t.Get(gk, nil) if err != nil { // We might be called to "remove" a global version that doesn't exist // if the first update for the file is already marked invalid. - return + return keyBuf } var fl VersionList err = fl.Unmarshal(svl) if err != nil { l.Debugln("unmarshal error:", err) - return + return keyBuf } fl, _, removedAt := fl.pop(device) if removedAt == -1 { // There is no version for the given device - return + return keyBuf } if removedAt == 0 { // A failure to get the file here is surprising and our // global size data will be incorrect until a restart... - if f, ok := t.getFile(folder, device, file); ok { + keyBuf = t.db.keyer.GenerateDeviceFileKey(keyBuf, folder, device, file) + if f, ok := t.getFileByKey(keyBuf); ok { meta.removeFile(protocol.GlobalDeviceID, f) } } if len(fl.Versions) == 0 { - t.Delete(t.db.keyer.GenerateNeedFileKey(nil, folder, file)) + keyBuf = t.db.keyer.GenerateNeedFileKey(keyBuf, folder, file) + t.Delete(keyBuf) t.Delete(gk) - return + return keyBuf } if removedAt == 0 { - global, ok := t.getFile(folder, fl.Versions[0].Device, file) + keyBuf = t.db.keyer.GenerateDeviceFileKey(keyBuf, folder, fl.Versions[0].Device, file) + global, ok := t.getFileByKey(keyBuf) if !ok { panic("This file must exist in the db") } - t.updateLocalNeed(folder, file, fl, global) + keyBuf = t.updateLocalNeed(keyBuf, folder, file, fl, global) meta.addFile(protocol.GlobalDeviceID, global) } l.Debugf("new global after remove: %v", fl) t.Put(gk, mustMarshal(&fl)) + + return keyBuf } func (t readWriteTransaction) deleteKeyPrefix(prefix []byte) {