From 670a9809fa2d8173e43cf63cbdf1592587cd0d2e Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Mon, 6 Apr 2020 09:53:37 +0200 Subject: [PATCH 1/3] lib/ur: Correct freaky error handling (fixes #6499) (#6500) --- lib/ur/usage_report.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/ur/usage_report.go b/lib/ur/usage_report.go index fa65b7307..30282d199 100644 --- a/lib/ur/usage_report.go +++ b/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) { From 0275cbd66af249fda64cc7325087c360d7e55c3e Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Tue, 7 Apr 2020 12:55:25 +0200 Subject: [PATCH 2/3] Revert "cmd/syncthing: Do auto-upgrade before startup (fixes #6384) (#6385)" This reverts commit c101a04179595dc625f7e05d309de26a529940af. --- cmd/syncthing/main.go | 159 ++++++++++++++---------------------------- 1 file changed, 53 insertions(+), 106 deletions(-) diff --git a/cmd/syncthing/main.go b/cmd/syncthing/main.go index ed45c7d9e..d5bd24710 100644 --- a/cmd/syncthing/main.go +++ b/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() -} From 0f532a5607d0bdd5d1906aa268f845c16c3246b4 Mon Sep 17 00:00:00 2001 From: Simon Frei Date: Tue, 7 Apr 2020 13:13:18 +0200 Subject: [PATCH 3/3] lib/db: Don't get blocklists on drop and missing continue (ref #6457) (#6502) --- lib/db/lowlevel.go | 3 ++- lib/db/transactions.go | 12 ++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/db/lowlevel.go b/lib/db/lowlevel.go index 9c97e6bc1..67984c88b 100644 --- a/lib/db/lowlevel.go +++ b/lib/db/lowlevel.go @@ -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) diff --git a/lib/db/transactions.go b/lib/db/transactions.go index 234d548f6..e006e89c0 100644 --- a/lib/db/transactions.go +++ b/lib/db/transactions.go @@ -488,7 +488,7 @@ func (t readWriteTransaction) updateGlobal(gk, keyBuf, folder, device []byte, fi name := []byte(file.Name) - var global protocol.FileInfo + var global FileIntf if insertedAt == 0 { // Inserted a new newest version global = file @@ -497,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 } @@ -530,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 } @@ -554,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 { @@ -631,7 +631,7 @@ func (t readWriteTransaction) removeFromGlobal(gk, keyBuf, folder, device []byte if err != nil { return nil, err } - if f, ok, err := t.getFileByKey(keyBuf); err != nil { + if f, ok, err := t.getFileTrunc(keyBuf, true); err != nil { return nil, err } else if ok { meta.removeFile(protocol.GlobalDeviceID, f) @@ -657,7 +657,7 @@ func (t readWriteTransaction) removeFromGlobal(gk, keyBuf, folder, device []byte if err != nil { return nil, err } - global, ok, err := t.getFileByKey(keyBuf) + global, ok, err := t.getFileTrunc(keyBuf, true) if err != nil { return nil, err }