fix(syncthing): apply folder migrations with temporary API/GUI server (#10330)

Prevent the feeling that nothing is happening / it's not starting.

Signed-off-by: Jakob Borg <[email protected]>

cmd/syncthing/main.go

@@ -479,11 +479,20 @@ func (c *serveCmd) syncthingMain() {
 		})
 	}
 
-	var tempApiAddress string
+	migratingAPICtx, migratingAPICancel := context.WithCancel(ctx)
 	if cfgWrapper.GUI().Enabled {
-		tempApiAddress = cfgWrapper.GUI().Address()
+		// Start a temporary API server during the migration. It will wait
+		// startDelay until actually starting, so that if we quickly pass
+		// through the migration steps (e.g., there was nothing to migrate)
+		// and cancel the context, it will never even start.
+		api := migratingAPI{
+			addr:       cfgWrapper.GUI().Address(),
+			startDelay: 5 * time.Second,
+		}
+		go api.Serve(migratingAPICtx)
 	}
-	if err := syncthing.TryMigrateDatabase(ctx, c.DBDeleteRetentionInterval, tempApiAddress); err != nil {
+
+	if err := syncthing.TryMigrateDatabase(ctx, c.DBDeleteRetentionInterval); err != nil {
 		slog.Error("Failed to migrate old-style database", slogutil.Error(err))
 		os.Exit(1)
 	}
@@ -494,6 +503,8 @@ func (c *serveCmd) syncthingMain() {
 		os.Exit(1)
 	}
 
+	migratingAPICancel() // we're done with the temporary API server
+
 	// Check if auto-upgrades is possible, and if yes, and it's enabled do an initial
 	// upgrade immediately. The auto-upgrade routine can only be started
 	// later after App is initialised.
@@ -1015,3 +1026,32 @@ func setConfigDataLocationsFromFlags(homeDir, confDir, dataDir string) error {
 	}
 	return nil
 }
+
+type migratingAPI struct {
+	addr       string
+	startDelay time.Duration
+}
+
+func (m migratingAPI) Serve(ctx context.Context) error {
+	srv := &http.Server{
+		Addr: m.addr,
+		Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.Header().Set("Content-Type", "text/plain")
+			w.Write([]byte("*** Database migration in progress ***\n\n"))
+			for _, line := range slogutil.GlobalRecorder.Since(time.Time{}) {
+				line.WriteTo(w)
+			}
+		}),
+	}
+	go func() {
+		select {
+		case <-time.After(m.startDelay):
+			slog.InfoContext(ctx, "Starting temporary GUI/API during migration", slogutil.Address(m.addr))
+			err := srv.ListenAndServe()
+			slog.InfoContext(ctx, "Temporary GUI/API closed", slogutil.Address(m.addr), slogutil.Error(err))
+		case <-ctx.Done():
+		}
+	}()
+	<-ctx.Done()
+	return srv.Close()
+}

internal/db/sqlite/db_open.go

@@ -86,6 +86,10 @@ func Open(path string, opts ...Option) (*DB, error) {
 		slog.Warn("Failed to clean dropped folders", slogutil.Error(err))
 	}
 
+	if err := db.startFolderDatabases(); err != nil {
+		return nil, wrap(err)
+	}
+
 	return db, nil
 }
 

internal/db/sqlite/db_update.go

@@ -7,6 +7,7 @@
 package sqlite
 
 import (
+	"errors"
 	"fmt"
 	"log/slog"
 	"os"
@@ -77,6 +78,22 @@ func (s *DB) cleanDroppedFolders() error {
 	return nil
 }
 
+// startFolderDatabases loads all existing folder databases, thus causing
+// migrations to apply.
+func (s *DB) startFolderDatabases() error {
+	ids, err := s.ListFolders()
+	if err != nil {
+		return wrap(err)
+	}
+	for _, id := range ids {
+		_, err := s.getFolderDB(id, false)
+		if err != nil && !errors.Is(err, errNoSuchFolder) {
+			return wrap(err)
+		}
+	}
+	return nil
+}
+
 // wrap returns the error wrapped with the calling function name and
 // optional extra context strings as prefix. A nil error wraps to nil.
 func wrap(err error, context ...string) error {

lib/syncthing/syncthing.go

@@ -191,10 +191,6 @@ func (a *App) startup() error {
 		if _, ok := cfgFolders[folder]; !ok {
 			slog.Info("Cleaning metadata for dropped folder", "folder", folder)
 			a.sdb.DropFolder(folder)
-		} else {
-			// Open the folder database, causing it to apply migrations
-			// early when appropriate.
-			_, _ = a.sdb.GetDeviceSequence(folder, protocol.LocalDeviceID)
 		}
 	}
 

lib/syncthing/utils.go

@@ -13,7 +13,6 @@ import (
 	"fmt"
 	"io"
 	"log/slog"
-	"net/http"
 	"os"
 	"sync"
 	"time"
@@ -159,7 +158,7 @@ func OpenDatabase(path string, deleteRetention time.Duration) (db.DB, error) {
 
 // Attempts migration of the old (LevelDB-based) database type to the new (SQLite-based) type
 // This will attempt to provide a temporary API server during the migration, if `apiAddr` is not empty.
-func TryMigrateDatabase(ctx context.Context, deleteRetention time.Duration, apiAddr string) error {
+func TryMigrateDatabase(ctx context.Context, deleteRetention time.Duration) error {
 	oldDBDir := locations.Get(locations.LegacyDatabase)
 	if _, err := os.Lstat(oldDBDir); err != nil {
 		// No old database
@@ -173,14 +172,6 @@ func TryMigrateDatabase(ctx context.Context, deleteRetention time.Duration, apiA
 	}
 	defer be.Close()
 
-	// Start a temporary API server during the migration
-	if apiAddr != "" {
-		api := migratingAPI{addr: apiAddr}
-		apiCtx, cancel := context.WithCancel(ctx)
-		defer cancel()
-		go api.Serve(apiCtx)
-	}
-
 	sdb, err := sqlite.OpenForMigration(locations.Get(locations.Database))
 	if err != nil {
 		return err
@@ -295,27 +286,3 @@ func TryMigrateDatabase(ctx context.Context, deleteRetention time.Duration, apiA
 	slog.Info("Migration complete", "files", totFiles, "blocks", totBlocks/1000, "duration", time.Since(t0).Truncate(time.Second))
 	return nil
 }
-
-type migratingAPI struct {
-	addr string
-}
-
-func (m migratingAPI) Serve(ctx context.Context) error {
-	srv := &http.Server{
-		Addr: m.addr,
-		Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			w.Header().Set("Content-Type", "text/plain")
-			w.Write([]byte("*** Database migration in progress ***\n\n"))
-			for _, line := range slogutil.GlobalRecorder.Since(time.Time{}) {
-				line.WriteTo(w)
-			}
-		}),
-	}
-	go func() {
-		slog.InfoContext(ctx, "Starting temporary GUI/API during migration", slogutil.Address(m.addr))
-		err := srv.ListenAndServe()
-		slog.InfoContext(ctx, "Temporary GUI/API closed", slogutil.Address(m.addr), slogutil.Error(err))
-	}()
-	<-ctx.Done()
-	return srv.Close()
-}