fix: track invalid files in LocalFlags to fix global count (#10170)

Move the "invalid" bit to a local flag, making it easier to track in counts etc.

internal/db/counts.go

@@ -19,9 +19,9 @@ type Counts struct {
 	Symlinks    int
 	Deleted     int
 	Bytes       int64
-	Sequence    int64             // zero for the global state
-	DeviceID    protocol.DeviceID // device ID for remote devices, or special values for local/global
-	LocalFlags  uint32            // the local flag for this count bucket
+	Sequence    int64              // zero for the global state
+	DeviceID    protocol.DeviceID  // device ID for remote devices, or special values for local/global
+	LocalFlags  protocol.FlagLocal // the local flag for this count bucket
 }
 
 func (c Counts) Add(other Counts) Counts {

internal/db/interface.go

@@ -100,7 +100,7 @@ type FileMetadata struct {
 	Sequence   int64
 	ModNanos   int64
 	Size       int64
-	LocalFlags int64
+	LocalFlags protocol.FlagLocal
 	Type       protocol.FileInfoType
 	Deleted    bool
 	Invalid    bool

internal/db/sqlite/basedb.go

@@ -103,6 +103,7 @@ func openBase(path string, maxConns int, pragmas, schemaScripts, migrationScript
 		"FlagLocalReceiveOnly": protocol.FlagLocalReceiveOnly,
 		"FlagLocalGlobal":      protocol.FlagLocalGlobal,
 		"FlagLocalNeeded":      protocol.FlagLocalNeeded,
+		"LocalInvalidFlags":    protocol.LocalInvalidFlags,
 		"SyncthingVersion":     build.LongVersion,
 	}
 

internal/db/sqlite/db_global_test.go

@@ -268,6 +268,57 @@ func TestDontNeedIgnored(t *testing.T) {
 	}
 }
 
+func TestDontNeedRemoteInvalid(t *testing.T) {
+	t.Parallel()
+
+	db, err := OpenTemp()
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() {
+		if err := db.Close(); err != nil {
+			t.Fatal(err)
+		}
+	})
+
+	// A remote file with the invalid bit set
+	files := []protocol.FileInfo{
+		genFile("test1", 1, 103),
+	}
+	files[0].LocalFlags = protocol.FlagLocalRemoteInvalid
+	err = db.Update(folderID, protocol.DeviceID{42}, files)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// It's not part of the global size
+	s, err := db.CountGlobal(folderID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if s.Bytes != 0 || s.Files != 0 {
+		t.Log(s)
+		t.Error("bad global")
+	}
+
+	// We don't need it
+	s, err = db.CountNeed(folderID, protocol.LocalDeviceID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if s.Bytes != 0 || s.Files != 0 {
+		t.Log(s)
+		t.Error("bad need")
+	}
+
+	// It shouldn't show up in the need list
+	names := mustCollect[protocol.FileInfo](t)(db.AllNeededGlobalFiles(folderID, protocol.LocalDeviceID, config.PullOrderAlphabetic, 0, 0))
+	if len(names) != 0 {
+		t.Log(names)
+		t.Error("need no files")
+	}
+}
+
 func TestRemoteDontNeedLocalIgnored(t *testing.T) {
 	t.Parallel()
 

internal/db/sqlite/folderdb_counts.go

@@ -39,13 +39,10 @@ func (s *folderDB) CountNeed(device protocol.DeviceID) (db.Counts, error) {
 }
 
 func (s *folderDB) CountGlobal() (db.Counts, error) {
-	// Exclude ignored and receive-only changed files from the global count
-	// (legacy expectation? it's a bit weird since those files can in fact
-	// be global and you can get them with GetGlobal etc.)
 	var res []countsRow
 	err := s.stmt(`
 		SELECT s.type, s.count, s.size, s.local_flags, s.deleted FROM counts s
-		WHERE s.local_flags & {{.FlagLocalGlobal}} != 0 AND s.local_flags & {{or .FlagLocalReceiveOnly .FlagLocalIgnored}} = 0
+		WHERE s.local_flags & {{.FlagLocalGlobal}} != 0 AND s.local_flags & {{.LocalInvalidFlags}} = 0
 	`).Select(&res)
 	if err != nil {
 		return db.Counts{}, wrap(err)
@@ -84,7 +81,7 @@ func (s *folderDB) needSizeRemote(device protocol.DeviceID) (db.Counts, error) {
 	// See neededGlobalFilesRemote for commentary as that is the same query without summing
 	if err := s.stmt(`
 		SELECT g.type, count(*) as count, sum(g.size) as size, g.local_flags, g.deleted FROM files g
-		WHERE g.local_flags & {{.FlagLocalGlobal}} != 0 AND NOT g.deleted AND NOT g.invalid AND NOT EXISTS (
+		WHERE g.local_flags & {{.FlagLocalGlobal}} != 0 AND NOT g.deleted AND g.local_flags & {{.LocalInvalidFlags}} = 0 AND NOT EXISTS (
 			SELECT 1 FROM FILES f
 			INNER JOIN devices d ON d.idx = f.device_idx
 			WHERE f.name = g.name AND f.version = g.version AND d.device_id = ?
@@ -94,10 +91,10 @@ func (s *folderDB) needSizeRemote(device protocol.DeviceID) (db.Counts, error) {
 		UNION ALL
 
 		SELECT g.type, count(*) as count, sum(g.size) as size, g.local_flags, g.deleted FROM files g
-		WHERE g.local_flags & {{.FlagLocalGlobal}} != 0 AND g.deleted AND NOT g.invalid AND EXISTS (
+		WHERE g.local_flags & {{.FlagLocalGlobal}} != 0 AND g.deleted AND g.local_flags & {{.LocalInvalidFlags}} = 0 AND EXISTS (
 			SELECT 1 FROM FILES f
 			INNER JOIN devices d ON d.idx = f.device_idx
-			WHERE f.name = g.name AND d.device_id = ? AND NOT f.deleted AND NOT f.invalid
+			WHERE f.name = g.name AND d.device_id = ? AND NOT f.deleted AND f.local_flags & {{.LocalInvalidFlags}} = 0
 		)
 		GROUP BY g.type, g.local_flags, g.deleted
 	`).Select(&res, device.String(),

internal/db/sqlite/folderdb_global.go

@@ -74,7 +74,7 @@ func (s *folderDB) GetGlobalAvailability(file string) ([]protocol.DeviceID, erro
 
 func (s *folderDB) AllGlobalFiles() (iter.Seq[db.FileMetadata], func() error) {
 	it, errFn := iterStructs[db.FileMetadata](s.stmt(`
-		SELECT f.sequence, f.name, f.type, f.modified as modnanos, f.size, f.deleted, f.invalid, f.local_flags as localflags FROM files f
+		SELECT f.sequence, f.name, f.type, f.modified as modnanos, f.size, f.deleted, f.local_flags as localflags FROM files f
 		WHERE f.local_flags & {{.FlagLocalGlobal}} != 0
 		ORDER BY f.name
 	`).Queryx())
@@ -93,7 +93,7 @@ func (s *folderDB) AllGlobalFilesPrefix(prefix string) (iter.Seq[db.FileMetadata
 	end := prefixEnd(prefix)
 
 	it, errFn := iterStructs[db.FileMetadata](s.stmt(`
-		SELECT f.sequence, f.name, f.type, f.modified as modnanos, f.size, f.deleted, f.invalid, f.local_flags as localflags FROM files f
+		SELECT f.sequence, f.name, f.type, f.modified as modnanos, f.size, f.deleted, f.local_flags as localflags FROM files f
 		WHERE f.name >= ? AND f.name < ? AND f.local_flags & {{.FlagLocalGlobal}} != 0
 		ORDER BY f.name
 	`).Queryx(prefix, end))
@@ -158,7 +158,7 @@ func (s *folderDB) neededGlobalFilesRemote(device protocol.DeviceID, selectOpts
 		SELECT fi.fiprotobuf, bl.blprotobuf, g.name, g.size, g.modified FROM fileinfos fi
 		INNER JOIN files g on fi.sequence = g.sequence
 		LEFT JOIN blocklists bl ON bl.blocklist_hash = g.blocklist_hash
-		WHERE g.local_flags & {{.FlagLocalGlobal}} != 0 AND NOT g.deleted AND NOT g.invalid AND NOT EXISTS (
+		WHERE g.local_flags & {{.FlagLocalGlobal}} != 0 AND NOT g.deleted AND g.local_flags & {{.LocalInvalidFlags}} = 0 AND NOT EXISTS (
 			SELECT 1 FROM FILES f
 			INNER JOIN devices d ON d.idx = f.device_idx
 			WHERE f.name = g.name AND f.version = g.version AND d.device_id = ?
@@ -169,10 +169,10 @@ func (s *folderDB) neededGlobalFilesRemote(device protocol.DeviceID, selectOpts
 		SELECT fi.fiprotobuf, bl.blprotobuf, g.name, g.size, g.modified FROM fileinfos fi
 		INNER JOIN files g on fi.sequence = g.sequence
 		LEFT JOIN blocklists bl ON bl.blocklist_hash = g.blocklist_hash
-		WHERE g.local_flags & {{.FlagLocalGlobal}} != 0 AND g.deleted AND NOT g.invalid AND EXISTS (
+		WHERE g.local_flags & {{.FlagLocalGlobal}} != 0 AND g.deleted AND g.local_flags & {{.LocalInvalidFlags}} = 0 AND EXISTS (
 			SELECT 1 FROM FILES f
 			INNER JOIN devices d ON d.idx = f.device_idx
-			WHERE f.name = g.name AND d.device_id = ? AND NOT f.deleted AND NOT f.invalid
+			WHERE f.name = g.name AND d.device_id = ? AND NOT f.deleted AND f.local_flags & {{.LocalInvalidFlags}} = 0 
 		)
 	`+selectOpts).Queryx(
 		device.String(),

internal/db/sqlite/folderdb_local.go

@@ -89,7 +89,7 @@ func (s *folderDB) AllLocalFilesWithPrefix(device protocol.DeviceID, prefix stri
 
 func (s *folderDB) AllLocalFilesWithBlocksHash(h []byte) (iter.Seq[db.FileMetadata], func() error) {
 	return iterStructs[db.FileMetadata](s.stmt(`
-		SELECT f.sequence, f.name, f.type, f.modified as modnanos, f.size, f.deleted, f.invalid, f.local_flags as localflags FROM files f
+		SELECT f.sequence, f.name, f.type, f.modified as modnanos, f.size, f.deleted, f.local_flags as localflags FROM files f
 		WHERE f.device_idx = {{.LocalDeviceIdx}} AND f.blocklist_hash = ?
 	`).Queryx(h))
 }

internal/db/sqlite/folderdb_update.go

@@ -46,8 +46,8 @@ func (s *folderDB) Update(device protocol.DeviceID, fs []protocol.FileInfo) erro
 
 	//nolint:sqlclosecheck
 	insertFileStmt, err := txp.Preparex(`
-		INSERT OR REPLACE INTO files (device_idx, remote_sequence, name, type, modified, size, version, deleted, invalid, local_flags, blocklist_hash)
-		VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
+		INSERT OR REPLACE INTO files (device_idx, remote_sequence, name, type, modified, size, version, deleted, local_flags, blocklist_hash)
+		VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
 		RETURNING sequence
 	`)
 	if err != nil {
@@ -101,7 +101,7 @@ func (s *folderDB) Update(device protocol.DeviceID, fs []protocol.FileInfo) erro
 			remoteSeq = &f.Sequence
 		}
 		var localSeq int64
-		if err := insertFileStmt.Get(&localSeq, deviceIdx, remoteSeq, f.Name, f.Type, f.ModTime().UnixNano(), f.Size, f.Version.String(), f.IsDeleted(), f.IsInvalid(), f.LocalFlags, blockshash); err != nil {
+		if err := insertFileStmt.Get(&localSeq, deviceIdx, remoteSeq, f.Name, f.Type, f.ModTime().UnixNano(), f.Size, f.Version.String(), f.IsDeleted(), f.LocalFlags, blockshash); err != nil {
 			return wrap(err, "insert file")
 		}
 
@@ -329,7 +329,7 @@ func (s *folderDB) recalcGlobalForFolder(txp *txPreparedStmts) error {
 func (s *folderDB) recalcGlobalForFile(txp *txPreparedStmts, file string) error {
 	//nolint:sqlclosecheck
 	selStmt, err := txp.Preparex(`
-		SELECT name, device_idx, sequence, modified, version, deleted, invalid, local_flags FROM files
+		SELECT name, device_idx, sequence, modified, version, deleted, local_flags FROM files
 		WHERE name = ?
 	`)
 	if err != nil {
@@ -350,7 +350,7 @@ func (s *folderDB) recalcGlobalForFile(txp *txPreparedStmts, file string) error
 	// The global version is the first one in the list that is not invalid,
 	// or just the first one in the list if all are invalid.
 	var global fileRow
-	globIdx := slices.IndexFunc(es, func(e fileRow) bool { return !e.Invalid })
+	globIdx := slices.IndexFunc(es, func(e fileRow) bool { return !e.IsInvalid() })
 	if globIdx < 0 {
 		globIdx = 0
 	}
@@ -368,7 +368,7 @@ func (s *folderDB) recalcGlobalForFile(txp *txPreparedStmts, file string) error
 	// Set the global flag on the global entry. Set the need flag if the
 	// local device needs this file, unless it's invalid.
 	global.LocalFlags |= protocol.FlagLocalGlobal
-	if hasLocal || global.Invalid {
+	if hasLocal || global.IsInvalid() {
 		global.LocalFlags &= ^protocol.FlagLocalNeeded
 	} else {
 		global.LocalFlags |= protocol.FlagLocalNeeded
@@ -426,9 +426,8 @@ type fileRow struct {
 	Sequence   int64
 	Modified   int64
 	Size       int64
-	LocalFlags int64 `db:"local_flags"`
+	LocalFlags protocol.FlagLocal `db:"local_flags"`
 	Deleted    bool
-	Invalid    bool
 }
 
 func (e fileRow) Compare(other fileRow) int {
@@ -436,8 +435,8 @@ func (e fileRow) Compare(other fileRow) int {
 	vc := e.Version.Compare(other.Version.Vector)
 	switch vc {
 	case protocol.Equal:
-		if e.Invalid != other.Invalid {
-			if e.Invalid {
+		if e.IsInvalid() != other.IsInvalid() {
+			if e.IsInvalid() {
 				return 1
 			}
 			return -1
@@ -453,8 +452,8 @@ func (e fileRow) Compare(other fileRow) int {
 	case protocol.Lesser: // we are older
 		return 1
 	case protocol.ConcurrentGreater, protocol.ConcurrentLesser: // there is a conflict
-		if e.Invalid != other.Invalid {
-			if e.Invalid { // we are invalid, we lose
+		if e.IsInvalid() != other.IsInvalid() {
+			if e.IsInvalid() { // we are invalid, we lose
 				return 1
 			}
 			return -1 // they are invalid, we win
@@ -477,6 +476,10 @@ func (e fileRow) Compare(other fileRow) int {
 	}
 }
 
+func (e fileRow) IsInvalid() bool {
+	return e.LocalFlags.IsInvalid()
+}
+
 func (s *folderDB) periodicCheckpointLocked(fs []protocol.FileInfo) {
 	// Induce periodic checkpoints. We add points for each file and block,
 	// and checkpoint when we've written more than a threshold of points.

internal/db/sqlite/sql/schema/folder/20-files.sql

@@ -31,7 +31,6 @@ CREATE TABLE IF NOT EXISTS files (
     size INTEGER NOT NULL,
     version TEXT NOT NULL COLLATE BINARY,
     deleted INTEGER NOT NULL, -- boolean
-    invalid INTEGER NOT NULL, -- boolean
     local_flags INTEGER NOT NULL,
     blocklist_hash BLOB, -- null when there are no blocks
     FOREIGN KEY(device_idx) REFERENCES devices(idx) ON DELETE CASCADE