From b779e22205585ca1a05dc2e87adc82ec3b47d24a Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Mon, 13 Jun 2016 17:44:03 +0000 Subject: [PATCH] lib/model: Don't set ignore bit when it's already set This adds a metric for "committed items" to the database instance that I use in the test code, and a couple of tests that ensure that scans that don't change anything also don't commit anything. There was a case in the scanner where we set the invalid bit on files that are ignored, even though they were already ignored and had the invalid bit set. I had assumed this would result in an extra database commit, but it was in fact filtered out by the Set... Anyway, I think we can save some work on not pushing that change to the Set at all. GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3298 --- lib/db/leveldb_dbinstance.go | 7 +++++ lib/db/leveldb_transactions.go | 16 +++++++---- lib/db/set_test.go | 33 +++++++++++++++++++++ lib/model/model.go | 2 +- lib/model/model_test.go | 52 ++++++++++++++++++++++++++++++++++ 5 files changed, 103 insertions(+), 7 deletions(-) diff --git a/lib/db/leveldb_dbinstance.go b/lib/db/leveldb_dbinstance.go index 6f321b554..4281bdf91 100644 --- a/lib/db/leveldb_dbinstance.go +++ b/lib/db/leveldb_dbinstance.go @@ -13,6 +13,7 @@ import ( "path/filepath" "sort" "strings" + "sync/atomic" "github.com/syncthing/syncthing/lib/osutil" "github.com/syncthing/syncthing/lib/protocol" @@ -31,6 +32,7 @@ type Instance struct { *leveldb.DB folderIdx *smallIndex deviceIdx *smallIndex + committed int64 } const ( @@ -91,6 +93,11 @@ func newDBInstance(db *leveldb.DB) *Instance { return i } +// Committed returns the number of items committed to the database since startup +func (db *Instance) Committed() int64 { + return atomic.LoadInt64(&db.committed) +} + func (db *Instance) genericReplace(folder, device []byte, fs []protocol.FileInfo, localSize, globalSize *sizeTracker, deleteFn deletionHandler) int64 { sort.Sort(fileList(fs)) // sort list on name, same as in the database diff --git a/lib/db/leveldb_transactions.go b/lib/db/leveldb_transactions.go index 40d97f8bc..e157101ab 100644 --- a/lib/db/leveldb_transactions.go +++ b/lib/db/leveldb_transactions.go @@ -8,6 +8,7 @@ package db import ( "bytes" + "sync/atomic" "github.com/syncthing/syncthing/lib/protocol" "github.com/syndtr/goleveldb/leveldb" @@ -55,21 +56,24 @@ func (db *Instance) newReadWriteTransaction() readWriteTransaction { } func (t readWriteTransaction) close() { - if err := t.db.Write(t.Batch, nil); err != nil { - panic(err) - } + t.flush() t.readOnlyTransaction.close() } func (t readWriteTransaction) checkFlush() { if t.Batch.Len() > batchFlushSize { - if err := t.db.Write(t.Batch, nil); err != nil { - panic(err) - } + t.flush() t.Batch.Reset() } } +func (t readWriteTransaction) flush() { + if err := t.db.Write(t.Batch, nil); err != nil { + panic(err) + } + atomic.AddInt64(&t.db.committed, int64(t.Batch.Len())) +} + func (t readWriteTransaction) insertFile(folder, device []byte, file protocol.FileInfo) int64 { l.Debugf("insert; folder=%q device=%v %v", folder, protocol.DeviceIDFromBytes(device), file) diff --git a/lib/db/set_test.go b/lib/db/set_test.go index cfe363b2c..d297cbd57 100644 --- a/lib/db/set_test.go +++ b/lib/db/set_test.go @@ -624,6 +624,39 @@ func TestLongPath(t *testing.T) { } } +func TestCommitted(t *testing.T) { + // Verify that the Committed counter increases when we change things and + // doesn't increase when we don't. + + ldb := db.OpenMemory() + + s := db.NewFileSet("test", ldb) + + local := []protocol.FileInfo{ + {Name: string("file"), Version: protocol.Vector{{ID: myID, Value: 1000}}}, + } + + // Adding a file should increase the counter + + c0 := ldb.Committed() + + s.Replace(protocol.LocalDeviceID, local) + + c1 := ldb.Committed() + if c1 <= c0 { + t.Errorf("committed data didn't increase; %d <= %d", c1, c0) + } + + // Updating with something identical should not do anything + + s.Update(protocol.LocalDeviceID, local) + + c2 := ldb.Committed() + if c2 > c1 { + t.Errorf("replace with same contents should do nothing but %d > %d", c2, c1) + } +} + func BenchmarkUpdateOneFile(b *testing.B) { local0 := fileList{ protocol.FileInfo{Name: "a", Version: protocol.Vector{{ID: myID, Value: 1000}}, Blocks: genBlocks(1)}, diff --git a/lib/model/model.go b/lib/model/model.go index 7e0ec2fe2..efa69959f 100644 --- a/lib/model/model.go +++ b/lib/model/model.go @@ -1551,7 +1551,7 @@ func (m *Model) internalScanFolderSubdirs(folder string, subs []string) error { batch = batch[:0] } - if ignores.Match(f.Name).IsIgnored() || symlinkInvalid(folder, f) { + if !f.IsInvalid() && (ignores.Match(f.Name).IsIgnored() || symlinkInvalid(folder, f)) { // File has been ignored or an unsupported symlink. Set invalid bit. l.Debugln("setting invalid bit on ignored", f) nf := protocol.FileInfo{ diff --git a/lib/model/model_test.go b/lib/model/model_test.go index 839209dd2..04b810fdd 100644 --- a/lib/model/model_test.go +++ b/lib/model/model_test.go @@ -1455,6 +1455,58 @@ func TestIssue3164(t *testing.T) { } } +func TestScanNoDatabaseWrite(t *testing.T) { + // When scanning, nothing should be committed to database unless + // something actually changed. + + db := db.OpenMemory() + m := NewModel(defaultConfig, protocol.LocalDeviceID, "device", "syncthing", "dev", db, nil) + m.AddFolder(defaultFolderConfig) + m.StartFolder("default") + m.ServeBackground() + + // Start with no ignores, and restore the previous state when the test completes + + curIgn, _, err := m.GetIgnores("default") + if err != nil { + t.Fatal(err) + } + defer m.SetIgnores("default", curIgn) + m.SetIgnores("default", nil) + + // Scan the folder twice. The second scan should be a no-op database wise + + m.ScanFolder("default") + c0 := db.Committed() + + m.ScanFolder("default") + c1 := db.Committed() + + if c1 != c0 { + t.Errorf("scan should not commit data when nothing changed but %d != %d", c1, c0) + } + + // Ignore a file we know exists. It'll be updated in the database. + + m.SetIgnores("default", []string{"foo"}) + + m.ScanFolder("default") + c2 := db.Committed() + + if c2 <= c1 { + t.Errorf("scan should commit data when something got ignored but %d <= %d", c2, c1) + } + + // Scan again. Nothing should happen. + + m.ScanFolder("default") + c3 := db.Committed() + + if c3 != c2 { + t.Errorf("scan should not commit data when nothing changed (with ignores) but %d != %d", c3, c2) + } +} + type fakeAddr struct{} func (fakeAddr) Network() string {