lib/db: Don't get blocklists on drop and missing continue (ref #6457) (#6502)

This reverts commit c101a04179.

cmd/syncthing/main.go

@@ -139,12 +139,10 @@ The following are valid values for the STTRACE variable:
 %s`
 )
 
+// Environment options
 var (
-	// Environment options
 	innerProcess    = os.Getenv("STNORESTART") != "" || os.Getenv("STMONITORED") != ""
 	noDefaultFolder = os.Getenv("STNODEFAULTFOLDER") != ""
-
-	errConcurrentUpgrade = errors.New("upgrade prevented by other running Syncthing instance")
 )
 
 type RuntimeOptions struct {
@@ -361,24 +359,14 @@ func main() {
 	}
 
 	if options.doUpgradeCheck {
-		if _, err := checkUpgrade(); err != nil {
-			l.Warnln("Checking for upgrade:", err)
-			os.Exit(exitCodeForUpgrade(err))
-		}
+		checkUpgrade()
 		return
 	}
 
 	if options.doUpgrade {
-		release, err := checkUpgrade()
-		if err == nil {
-			err = performUpgrade(release)
-		}
-		if err != nil {
-			l.Warnln("Upgrade:", err)
-			os.Exit(exitCodeForUpgrade(err))
-		}
-		l.Infof("Upgraded to %q", release.Tag)
-		os.Exit(syncthing.ExitUpgrade.AsInt())
+		release := checkUpgrade()
+		performUpgrade(release)
+		return
 	}
 
 	if options.resetDatabase {
@@ -474,47 +462,45 @@ func debugFacilities() string {
 	return b.String()
 }
 
-type errNoUpgrade struct {
-	current, latest string
-}
-
-func (e errNoUpgrade) Error() string {
-	return fmt.Sprintf("no upgrade available (current %q >= latest %q).", e.current, e.latest)
-}
-
-func checkUpgrade() (upgrade.Release, error) {
+func checkUpgrade() upgrade.Release {
 	cfg, _ := loadOrDefaultConfig(protocol.EmptyDeviceID, events.NoopLogger)
 	opts := cfg.Options()
 	release, err := upgrade.LatestRelease(opts.ReleasesURL, build.Version, opts.UpgradeToPreReleases)
 	if err != nil {
-		return upgrade.Release{}, err
+		l.Warnln("Upgrade:", err)
+		os.Exit(syncthing.ExitError.AsInt())
 	}
 
 	if upgrade.CompareVersions(release.Tag, build.Version) <= 0 {
-		return upgrade.Release{}, errNoUpgrade{build.Version, release.Tag}
+		noUpgradeMessage := "No upgrade available (current %q >= latest %q)."
+		l.Infof(noUpgradeMessage, build.Version, release.Tag)
+		os.Exit(syncthing.ExitNoUpgradeAvailable.AsInt())
 	}
 
 	l.Infof("Upgrade available (current %q < latest %q)", build.Version, release.Tag)
-	return release, nil
+	return release
 }
 
-func performUpgradeDirect(release upgrade.Release) error {
+func performUpgrade(release upgrade.Release) {
 	// Use leveldb database locks to protect against concurrent upgrades
-	if _, err := syncthing.OpenDBBackend(locations.Get(locations.Database), config.TuningAuto); err != nil {
-		return errConcurrentUpgrade
-	}
-	return upgrade.To(release)
-}
-
-func performUpgrade(release upgrade.Release) error {
-	if err := performUpgradeDirect(release); err != nil {
-		if err != errConcurrentUpgrade {
-			return err
+	_, err := syncthing.OpenDBBackend(locations.Get(locations.Database), config.TuningAuto)
+	if err == nil {
+		err = upgrade.To(release)
+		if err != nil {
+			l.Warnln("Upgrade:", err)
+			os.Exit(syncthing.ExitError.AsInt())
 		}
+		l.Infof("Upgraded to %q", release.Tag)
+	} else {
 		l.Infoln("Attempting upgrade through running Syncthing...")
-		return upgradeViaRest()
+		err = upgradeViaRest()
+		if err != nil {
+			l.Warnln("Upgrade:", err)
+			os.Exit(syncthing.ExitError.AsInt())
+		}
+		l.Infoln("Syncthing upgrading")
+		os.Exit(syncthing.ExitUpgrade.AsInt())
 	}
-	return nil
 }
 
 func upgradeViaRest() error {
@@ -582,45 +568,6 @@ func syncthingMain(runtimeOptions RuntimeOptions) {
 		os.Exit(syncthing.ExitError.AsInt())
 	}
 
-	// Candidate builds should auto upgrade. Make sure the option is set,
-	// unless we are in a build where it's disabled or the STNOUPGRADE
-	// environment variable is set.
-
-	if build.IsCandidate && !upgrade.DisabledByCompilation && !runtimeOptions.NoUpgrade {
-		l.Infoln("Automatic upgrade is always enabled for candidate releases.")
-		if opts := cfg.Options(); opts.AutoUpgradeIntervalH == 0 || opts.AutoUpgradeIntervalH > 24 {
-			opts.AutoUpgradeIntervalH = 12
-			// Set the option into the config as well, as the auto upgrade
-			// loop expects to read a valid interval from there.
-			cfg.SetOptions(opts)
-			cfg.Save()
-		}
-		// We don't tweak the user's choice of upgrading to pre-releases or
-		// not, as otherwise they cannot step off the candidate channel.
-	}
-
-	// Check if auto-upgrades should be done and if yes, do an initial
-	// upgrade immedately. The auto-upgrade routine can only be started
-	// later after App is initialised.
-
-	shouldAutoUpgrade := shouldUpgrade(cfg, runtimeOptions)
-	if shouldAutoUpgrade {
-		// Try to do upgrade directly
-		release, err := checkUpgrade()
-		if err == nil {
-			if err = performUpgradeDirect(release); err == nil {
-				l.Infof("Upgraded to %q, exiting now.", release.Tag)
-				os.Exit(syncthing.ExitUpgrade.AsInt())
-			}
-		}
-		// Log the error if relevant.
-		if err != nil {
-			if _, ok := err.(errNoUpgrade); !ok {
-				l.Infoln("Initial automatic upgrade:", err)
-			}
-		}
-	}
-
 	if runtimeOptions.unpaused {
 		setPauseState(cfg, false)
 	} else if runtimeOptions.paused {
@@ -645,10 +592,6 @@ func syncthingMain(runtimeOptions RuntimeOptions) {
 
 	app := syncthing.New(cfg, ldb, evLogger, cert, appOpts)
 
-	if shouldAutoUpgrade {
-		go autoUpgrade(cfg, app, evLogger)
-	}
-
 	setupSignalHandling(app)
 
 	if len(os.Getenv("GOMAXPROCS")) == 0 {
@@ -671,6 +614,31 @@ func syncthingMain(runtimeOptions RuntimeOptions) {
 		go standbyMonitor(app)
 	}
 
+	// Candidate builds should auto upgrade. Make sure the option is set,
+	// unless we are in a build where it's disabled or the STNOUPGRADE
+	// environment variable is set.
+
+	if build.IsCandidate && !upgrade.DisabledByCompilation && !runtimeOptions.NoUpgrade {
+		l.Infoln("Automatic upgrade is always enabled for candidate releases.")
+		if opts := cfg.Options(); opts.AutoUpgradeIntervalH == 0 || opts.AutoUpgradeIntervalH > 24 {
+			opts.AutoUpgradeIntervalH = 12
+			// Set the option into the config as well, as the auto upgrade
+			// loop expects to read a valid interval from there.
+			cfg.SetOptions(opts)
+			cfg.Save()
+		}
+		// We don't tweak the user's choice of upgrading to pre-releases or
+		// not, as otherwise they cannot step off the candidate channel.
+	}
+
+	if opts := cfg.Options(); opts.AutoUpgradeIntervalH > 0 {
+		if runtimeOptions.NoUpgrade {
+			l.Infof("No automatic upgrades; STNOUPGRADE environment variable defined.")
+		} else {
+			go autoUpgrade(cfg, app, evLogger)
+		}
+	}
+
 	if err := app.Start(); err != nil {
 		os.Exit(syncthing.ExitError.AsInt())
 	}
@@ -803,20 +771,6 @@ func standbyMonitor(app *syncthing.App) {
 	}
 }
 
-func shouldUpgrade(cfg config.Wrapper, runtimeOptions RuntimeOptions) bool {
-	if upgrade.DisabledByCompilation {
-		return false
-	}
-	if opts := cfg.Options(); opts.AutoUpgradeIntervalH < 0 {
-		return false
-	}
-	if runtimeOptions.NoUpgrade {
-		l.Infof("No automatic upgrades; STNOUPGRADE environment variable defined.")
-		return false
-	}
-	return true
-}
-
 func autoUpgrade(cfg config.Wrapper, app *syncthing.App, evLogger events.Logger) {
 	timer := time.NewTimer(0)
 	sub := evLogger.Subscribe(events.DeviceConnected)
@@ -939,10 +893,3 @@ func setPauseState(cfg config.Wrapper, paused bool) {
 		os.Exit(syncthing.ExitError.AsInt())
 	}
 }
-
-func exitCodeForUpgrade(err error) int {
-	if _, ok := err.(errNoUpgrade); ok {
-		return syncthing.ExitNoUpgradeAvailable.AsInt()
-	}
-	return syncthing.ExitError.AsInt()
-}

lib/db/db_test.go

@@ -261,11 +261,11 @@ func TestRepairSequence(t *testing.T) {
 	short := protocol.LocalDeviceID.Short()
 
 	files := []protocol.FileInfo{
-		{Name: "fine"},
-		{Name: "duplicate"},
-		{Name: "missing"},
-		{Name: "overwriting"},
-		{Name: "inconsistent"},
+		{Name: "fine", Blocks: genBlocks(1)},
+		{Name: "duplicate", Blocks: genBlocks(2)},
+		{Name: "missing", Blocks: genBlocks(3)},
+		{Name: "overwriting", Blocks: genBlocks(4)},
+		{Name: "inconsistent", Blocks: genBlocks(5)},
 	}
 	for i, f := range files {
 		files[i].Version = f.Version.Update(short)
@@ -282,7 +282,7 @@ func TestRepairSequence(t *testing.T) {
 		if err != nil {
 			t.Fatal(err)
 		}
-		if err := trans.putFile(dk, f); err != nil {
+		if err := trans.putFile(dk, f, false); err != nil {
 			t.Fatal(err)
 		}
 		sk, err := trans.keyer.GenerateSequenceKey(nil, folder, seq)
@@ -352,7 +352,7 @@ func TestRepairSequence(t *testing.T) {
 	// Loading the metadata for the first time means a "re"calculation happens,
 	// along which the sequences get repaired too.
 	db.gcMut.RLock()
-	_ = loadMetadataTracker(db, folderStr)
+	_ = db.loadMetadataTracker(folderStr)
 	db.gcMut.RUnlock()
 	if err != nil {
 		t.Fatal(err)
@@ -399,16 +399,20 @@ func TestRepairSequence(t *testing.T) {
 	}
 	defer it.Release()
 	for it.Next() {
-		fi, ok, err := ro.getFileTrunc(it.Value(), true)
+		intf, ok, err := ro.getFileTrunc(it.Value(), false)
 		if err != nil {
 			t.Fatal(err)
 		}
+		fi := intf.(protocol.FileInfo)
 		seq := ro.keyer.SequenceFromSequenceKey(it.Key())
 		if !ok {
 			t.Errorf("Sequence entry %v points at nothing", seq)
 		} else if fi.SequenceNo() != seq {
 			t.Errorf("Inconsistent sequence entry for %v: %v != %v", fi.FileName(), fi.SequenceNo(), seq)
 		}
+		if len(fi.Blocks) == 0 {
+			t.Error("Missing blocks in", fi.FileName())
+		}
 	}
 	if err := it.Error(); err != nil {
 		t.Fatal(err)
@@ -440,3 +444,40 @@ func TestDowngrade(t *testing.T) {
 		t.Fatalf("Error has %v as min Syncthing version, expected %v", err.minSyncthingVersion, dbMinSyncthingVersion)
 	}
 }
+
+func TestCheckGlobals(t *testing.T) {
+	db := NewLowlevel(backend.OpenMemory())
+	defer db.Close()
+
+	fs := NewFileSet("test", fs.NewFilesystem(fs.FilesystemTypeFake, ""), db)
+
+	// Add any file
+	name := "foo"
+	fs.Update(protocol.LocalDeviceID, []protocol.FileInfo{
+		{
+			Name:    name,
+			Type:    protocol.FileInfoTypeFile,
+			Version: protocol.Vector{Counters: []protocol.Counter{{ID: 1, Value: 1001}}},
+		},
+	})
+
+	// Remove just the file entry
+	if err := db.dropPrefix([]byte{KeyTypeDevice}); err != nil {
+		t.Fatal(err)
+	}
+
+	// Clean up global entry of the now missing file
+	if err := db.checkGlobals([]byte(fs.folder), fs.meta); err != nil {
+		t.Fatal(err)
+	}
+
+	// Check that the global entry is gone
+	gk, err := db.keyer.GenerateGlobalVersionKey(nil, []byte(fs.folder), []byte(name))
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = db.Get(gk)
+	if !backend.IsNotFound(err) {
+		t.Error("Expected key missing error, got", err)
+	}
+}

lib/db/lowlevel.go

@@ -114,7 +114,7 @@ func (db *Lowlevel) updateRemoteFiles(folder, device []byte, fs []protocol.FileI
 		meta.addFile(devID, f)
 
 		l.Debugf("insert; folder=%q device=%v %v", folder, devID, f)
-		if err := t.putFile(dk, f); err != nil {
+		if err := t.putFile(dk, f, false); err != nil {
 			return err
 		}
 
@@ -201,7 +201,7 @@ func (db *Lowlevel) updateLocalFiles(folder []byte, fs []protocol.FileInfo, meta
 		meta.addFile(protocol.LocalDeviceID, f)
 
 		l.Debugf("insert (local); folder=%q %v", folder, f)
-		if err := t.putFile(dk, f); err != nil {
+		if err := t.putFile(dk, f, false); err != nil {
 			return err
 		}
 
@@ -385,6 +385,7 @@ func (db *Lowlevel) checkGlobals(folder []byte, meta *metadataTracker) error {
 			if err := t.Delete(dbi.Key()); err != nil {
 				return err
 			}
+			continue
 		}
 
 		// Check the global version list for consistency. An issue in previous
@@ -409,7 +410,7 @@ func (db *Lowlevel) checkGlobals(folder []byte, meta *metadataTracker) error {
 			newVL.Versions = append(newVL.Versions, version)
 
 			if i == 0 {
-				if fi, ok, err := t.getFileByKey(dk); err != nil {
+				if fi, ok, err := t.getFileTrunc(dk, true); err != nil {
 					return err
 				} else if ok {
 					meta.addFile(protocol.GlobalDeviceID, fi)
@@ -417,7 +418,11 @@ func (db *Lowlevel) checkGlobals(folder []byte, meta *metadataTracker) error {
 			}
 		}
 
-		if len(newVL.Versions) != len(vl.Versions) {
+		if newLen := len(newVL.Versions); newLen == 0 {
+			if err := t.Delete(dbi.Key()); err != nil {
+				return err
+			}
+		} else if newLen != len(vl.Versions) {
 			if err := t.Put(dbi.Key(), mustMarshal(&newVL)); err != nil {
 				return err
 			}
@@ -623,6 +628,117 @@ func (db *Lowlevel) gcIndirect() error {
 	return db.Compact()
 }
 
+// CheckRepair checks folder metadata and sequences for miscellaneous errors.
+func (db *Lowlevel) CheckRepair() {
+	for _, folder := range db.ListFolders() {
+		_ = db.getMetaAndCheck(folder)
+	}
+}
+
+func (db *Lowlevel) getMetaAndCheck(folder string) *metadataTracker {
+	db.gcMut.RLock()
+	defer db.gcMut.RUnlock()
+
+	meta, err := db.recalcMeta(folder)
+	if err == nil {
+		var fixed int
+		fixed, err = db.repairSequenceGCLocked(folder, meta)
+		if fixed != 0 {
+			l.Infof("Repaired %d sequence entries in database", fixed)
+		}
+	}
+
+	if backend.IsClosed(err) {
+		return nil
+	} else if err != nil {
+		panic(err)
+	}
+
+	return meta
+}
+
+func (db *Lowlevel) loadMetadataTracker(folder string) *metadataTracker {
+	meta := newMetadataTracker()
+	if err := meta.fromDB(db, []byte(folder)); err != nil {
+		l.Infof("No stored folder metadata for %q; recalculating", folder)
+		return db.getMetaAndCheck(folder)
+	}
+
+	curSeq := meta.Sequence(protocol.LocalDeviceID)
+	if metaOK := db.verifyLocalSequence(curSeq, folder); !metaOK {
+		l.Infof("Stored folder metadata for %q is out of date after crash; recalculating", folder)
+		return db.getMetaAndCheck(folder)
+	}
+
+	if age := time.Since(meta.Created()); age > databaseRecheckInterval {
+		l.Infof("Stored folder metadata for %q is %v old; recalculating", folder, age)
+		return db.getMetaAndCheck(folder)
+	}
+
+	return meta
+}
+
+func (db *Lowlevel) recalcMeta(folder string) (*metadataTracker, error) {
+	meta := newMetadataTracker()
+	if err := db.checkGlobals([]byte(folder), meta); err != nil {
+		return nil, err
+	}
+
+	t, err := db.newReadWriteTransaction()
+	if err != nil {
+		return nil, err
+	}
+	defer t.close()
+
+	var deviceID protocol.DeviceID
+	err = t.withAllFolderTruncated([]byte(folder), func(device []byte, f FileInfoTruncated) bool {
+		copy(deviceID[:], device)
+		meta.addFile(deviceID, f)
+		return true
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	meta.SetCreated()
+	if err := meta.toDB(t, []byte(folder)); err != nil {
+		return nil, err
+	}
+	if err := t.Commit(); err != nil {
+		return nil, err
+	}
+	return meta, nil
+}
+
+// Verify the local sequence number from actual sequence entries. Returns
+// true if it was all good, or false if a fixup was necessary.
+func (db *Lowlevel) verifyLocalSequence(curSeq int64, folder string) bool {
+	// Walk the sequence index from the current (supposedly) highest
+	// sequence number and raise the alarm if we get anything. This recovers
+	// from the occasion where we have written sequence entries to disk but
+	// not yet written new metadata to disk.
+	//
+	// Note that we can have the same thing happen for remote devices but
+	// there it's not a problem -- we'll simply advertise a lower sequence
+	// number than we've actually seen and receive some duplicate updates
+	// and then be in sync again.
+
+	t, err := db.newReadOnlyTransaction()
+	if err != nil {
+		panic(err)
+	}
+	ok := true
+	if err := t.withHaveSequence([]byte(folder), curSeq+1, func(fi FileIntf) bool {
+		ok = false // we got something, which we should not have
+		return false
+	}); err != nil && !backend.IsClosed(err) {
+		panic(err)
+	}
+	t.close()
+
+	return ok
+}
+
 // repairSequenceGCLocked makes sure the sequence numbers in the sequence keys
 // match those in the corresponding file entries. It returns the amount of fixed
 // entries.
@@ -675,7 +791,7 @@ func (db *Lowlevel) repairSequenceGCLocked(folderStr string, meta *metadataTrack
 			if err := t.Put(sk, it.Key()); err != nil {
 				return 0, err
 			}
-			if err := t.putFile(it.Key(), fi.copyToFileInfo()); err != nil {
+			if err := t.putFile(it.Key(), fi.copyToFileInfo(), true); err != nil {
 				return 0, err
 			}
 		}

lib/db/schemaupdater.go

@@ -465,7 +465,7 @@ func (db *schemaUpdater) updateSchemato9(prev int) error {
 		if fi.Blocks == nil {
 			continue
 		}
-		if err := t.putFile(it.Key(), fi); err != nil {
+		if err := t.putFile(it.Key(), fi, false); err != nil {
 			return err
 		}
 		if err := t.Checkpoint(); err != nil {

lib/db/set.go

@@ -75,112 +75,11 @@ func NewFileSet(folder string, fs fs.Filesystem, db *Lowlevel) *FileSet {
 		folder:      folder,
 		fs:          fs,
 		db:          db,
-		meta:        loadMetadataTracker(db, folder),
+		meta:        db.loadMetadataTracker(folder),
 		updateMutex: sync.NewMutex(),
 	}
 }
 
-func loadMetadataTracker(db *Lowlevel, folder string) *metadataTracker {
-	recalc := func() *metadataTracker {
-		db.gcMut.RLock()
-		defer db.gcMut.RUnlock()
-		meta, err := recalcMeta(db, folder)
-		if err == nil {
-			var fixed int
-			fixed, err = db.repairSequenceGCLocked(folder, meta)
-			if fixed != 0 {
-				l.Infoln("Repaired %v sequence entries in database", fixed)
-			}
-		}
-		if backend.IsClosed(err) {
-			return nil
-		} else if err != nil {
-			panic(err)
-		}
-		return meta
-	}
-
-	meta := newMetadataTracker()
-	if err := meta.fromDB(db, []byte(folder)); err != nil {
-		l.Infof("No stored folder metadata for %q; recalculating", folder)
-		return recalc()
-	}
-
-	curSeq := meta.Sequence(protocol.LocalDeviceID)
-	if metaOK := verifyLocalSequence(curSeq, db, folder); !metaOK {
-		l.Infof("Stored folder metadata for %q is out of date after crash; recalculating", folder)
-		return recalc()
-	}
-
-	if age := time.Since(meta.Created()); age > databaseRecheckInterval {
-		l.Infof("Stored folder metadata for %q is %v old; recalculating", folder, age)
-		return recalc()
-	}
-
-	return meta
-}
-
-func recalcMeta(db *Lowlevel, folder string) (*metadataTracker, error) {
-	meta := newMetadataTracker()
-	if err := db.checkGlobals([]byte(folder), meta); err != nil {
-		return nil, err
-	}
-
-	t, err := db.newReadWriteTransaction()
-	if err != nil {
-		return nil, err
-	}
-	defer t.close()
-
-	var deviceID protocol.DeviceID
-	err = t.withAllFolderTruncated([]byte(folder), func(device []byte, f FileInfoTruncated) bool {
-		copy(deviceID[:], device)
-		meta.addFile(deviceID, f)
-		return true
-	})
-	if err != nil {
-		return nil, err
-	}
-
-	meta.SetCreated()
-	if err := meta.toDB(t, []byte(folder)); err != nil {
-		return nil, err
-	}
-	if err := t.Commit(); err != nil {
-		return nil, err
-	}
-	return meta, nil
-}
-
-// Verify the local sequence number from actual sequence entries. Returns
-// true if it was all good, or false if a fixup was necessary.
-func verifyLocalSequence(curSeq int64, db *Lowlevel, folder string) bool {
-	// Walk the sequence index from the current (supposedly) highest
-	// sequence number and raise the alarm if we get anything. This recovers
-	// from the occasion where we have written sequence entries to disk but
-	// not yet written new metadata to disk.
-	//
-	// Note that we can have the same thing happen for remote devices but
-	// there it's not a problem -- we'll simply advertise a lower sequence
-	// number than we've actually seen and receive some duplicate updates
-	// and then be in sync again.
-
-	t, err := db.newReadOnlyTransaction()
-	if err != nil {
-		panic(err)
-	}
-	ok := true
-	if err := t.withHaveSequence([]byte(folder), curSeq+1, func(fi FileIntf) bool {
-		ok = false // we got something, which we should not have
-		return false
-	}); err != nil && !backend.IsClosed(err) {
-		panic(err)
-	}
-	t.close()
-
-	return ok
-}
-
 func (s *FileSet) Drop(device protocol.DeviceID) {
 	l.Debugf("%s Drop(%v)", s.folder, device)
 

lib/db/structs.go

@@ -257,14 +257,13 @@ func (vl VersionList) insertAt(i int, v FileVersion) VersionList {
 // as the removed FileVersion and the position, where that FileVersion was.
 // If there is no FileVersion for the given device, the position is -1.
 func (vl VersionList) pop(device []byte) (VersionList, FileVersion, int) {
-	removedAt := -1
 	for i, v := range vl.Versions {
 		if bytes.Equal(v.Device, device) {
 			vl.Versions = append(vl.Versions[:i], vl.Versions[i+1:]...)
 			return vl, v, i
 		}
 	}
-	return vl, FileVersion{}, removedAt
+	return vl, FileVersion{}, -1
 }
 
 func (vl VersionList) Get(device []byte) (FileVersion, bool) {

lib/db/transactions.go

@@ -8,19 +8,13 @@ package db
 
 import (
 	"bytes"
-	"fmt"
+	"errors"
 
 	"github.com/syncthing/syncthing/lib/db/backend"
 	"github.com/syncthing/syncthing/lib/protocol"
 )
 
-type errDeviceEntryMissing struct {
-	name string
-}
-
-func (err errDeviceEntryMissing) Error() string {
-	return fmt.Sprintf("device present in global list but missing as device/fileinfo entry: %s", err.name)
-}
+var errEntryFromGlobalMissing = errors.New("device present in global list but missing as device/fileinfo entry")
 
 // A readOnlyTransaction represents a database snapshot.
 type readOnlyTransaction struct {
@@ -364,7 +358,7 @@ func (t *readOnlyTransaction) withNeed(folder, device []byte, truncate bool, fn
 			return err
 		}
 		if !ok {
-			return errDeviceEntryMissing{string(name)}
+			return errEntryFromGlobalMissing
 		}
 		if !need(gf, have, haveFV.Version) {
 			continue
@@ -438,13 +432,19 @@ func (t readWriteTransaction) close() {
 	t.WriteTransaction.Release()
 }
 
-func (t readWriteTransaction) putFile(fkey []byte, fi protocol.FileInfo) error {
+// putFile stores a file in the database, taking care of indirected fields.
+// Set the truncated flag when putting a file that deliberatly can have an
+// empty block list but a non-empty block list hash. This should normally be
+// false.
+func (t readWriteTransaction) putFile(fkey []byte, fi protocol.FileInfo, truncated bool) error {
 	var bkey []byte
 
-	// Always set the blocks hash when there are blocks.
+	// Always set the blocks hash when there are blocks. Leave the blocks
+	// hash alone when there are no blocks and we might be putting a
+	// "truncated" FileInfo (no blocks, but the hash reference is live).
 	if len(fi.Blocks) > 0 {
 		fi.BlocksHash = protocol.BlocksHash(fi.Blocks)
-	} else {
+	} else if !truncated {
 		fi.BlocksHash = nil
 	}
 
@@ -485,14 +485,10 @@ func (t readWriteTransaction) updateGlobal(gk, keyBuf, folder, device []byte, fi
 	if err != nil {
 		return nil, false, err
 	}
-	if insertedAt == -1 {
-		l.Debugln("update global; same version, global unchanged")
-		return keyBuf, false, nil
-	}
 
 	name := []byte(file.Name)
 
-	var global protocol.FileInfo
+	var global FileIntf
 	if insertedAt == 0 {
 		// Inserted a new newest version
 		global = file
@@ -501,7 +497,7 @@ func (t readWriteTransaction) updateGlobal(gk, keyBuf, folder, device []byte, fi
 		if err != nil {
 			return nil, false, err
 		}
-		new, ok, err := t.getFileByKey(keyBuf)
+		new, ok, err := t.getFileTrunc(keyBuf, true)
 		if err != nil || !ok {
 			return keyBuf, false, err
 		}
@@ -534,7 +530,7 @@ func (t readWriteTransaction) updateGlobal(gk, keyBuf, folder, device []byte, fi
 	if err != nil {
 		return nil, false, err
 	}
-	oldFile, ok, err := t.getFileByKey(keyBuf)
+	oldFile, ok, err := t.getFileTrunc(keyBuf, true)
 	if err != nil {
 		return nil, false, err
 	}
@@ -558,7 +554,7 @@ func (t readWriteTransaction) updateGlobal(gk, keyBuf, folder, device []byte, fi
 // 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(keyBuf, folder, name []byte, fl VersionList, global protocol.FileInfo) ([]byte, error) {
+func (t readWriteTransaction) updateLocalNeed(keyBuf, folder, name []byte, fl VersionList, global FileIntf) ([]byte, error) {
 	var err error
 	keyBuf, err = t.keyer.GenerateNeedFileKey(keyBuf, folder, name)
 	if err != nil {
@@ -635,8 +631,8 @@ func (t readWriteTransaction) removeFromGlobal(gk, keyBuf, folder, device []byte
 		if err != nil {
 			return nil, err
 		}
-		if f, ok, err := t.getFileByKey(keyBuf); err != nil {
-			return keyBuf, nil
+		if f, ok, err := t.getFileTrunc(keyBuf, true); err != nil {
+			return nil, err
 		} else if ok {
 			meta.removeFile(protocol.GlobalDeviceID, f)
 		}
@@ -661,9 +657,12 @@ func (t readWriteTransaction) removeFromGlobal(gk, keyBuf, folder, device []byte
 		if err != nil {
 			return nil, err
 		}
-		global, ok, err := t.getFileByKey(keyBuf)
-		if err != nil || !ok {
-			return keyBuf, err
+		global, ok, err := t.getFileTrunc(keyBuf, true)
+		if err != nil {
+			return nil, err
+		}
+		if !ok {
+			return nil, errEntryFromGlobalMissing
 		}
 		keyBuf, err = t.updateLocalNeed(keyBuf, folder, file, fl, global)
 		if err != nil {

lib/syncthing/syncthing.go

@@ -35,6 +35,7 @@ import (
 	"github.com/syncthing/syncthing/lib/rand"
 	"github.com/syncthing/syncthing/lib/sha256"
 	"github.com/syncthing/syncthing/lib/tlsutil"
+	"github.com/syncthing/syncthing/lib/upgrade"
 	"github.com/syncthing/syncthing/lib/ur"
 )
 
@@ -224,7 +225,7 @@ func (a *App) startup() error {
 
 	prevParts := strings.Split(prevVersion, "-")
 	curParts := strings.Split(build.Version, "-")
-	if prevParts[0] != curParts[0] {
+	if rel := upgrade.CompareVersions(prevParts[0], curParts[0]); rel != upgrade.Equal {
 		if prevVersion != "" {
 			l.Infoln("Detected upgrade from", prevVersion, "to", build.Version)
 		}
@@ -237,6 +238,14 @@ func (a *App) startup() error {
 		miscDB.PutString("prevVersion", build.Version)
 	}
 
+	// Check and repair metadata and sequences on every upgrade including RCs.
+	prevParts = strings.Split(prevVersion, "+")
+	curParts = strings.Split(build.Version, "+")
+	if rel := upgrade.CompareVersions(prevParts[0], curParts[0]); rel != upgrade.Equal {
+		l.Infoln("Checking db due to upgrade - this may take a while...")
+		a.ll.CheckRepair()
+	}
+
 	m := model.NewModel(a.cfg, a.myID, "syncthing", build.Version, a.ll, protectedFiles, a.evLogger)
 
 	if a.opts.DeadlockTimeoutS > 0 {

lib/ur/usage_report.go

@@ -385,15 +385,17 @@ func (s *Service) sendUsageReport(ctx context.Context) error {
 		},
 	}
 	req, err := http.NewRequest("POST", s.cfg.Options().URURL, &b)
-	if err == nil {
-		req.Header.Set("Content-Type", "application/json")
-		req.Cancel = ctx.Done()
-		var resp *http.Response
-		resp, err = client.Do(req)
-		resp.Body.Close()
+	if err != nil {
+		return err
 	}
-
-	return err
+	req.Header.Set("Content-Type", "application/json")
+	req.Cancel = ctx.Done()
+	resp, err := client.Do(req)
+	if err != nil {
+		return err
+	}
+	resp.Body.Close()
+	return nil
 }
 
 func (s *Service) serve(ctx context.Context) {