Actually do negative caching on failed discovery lookups (fixes #2434)

Cache the folderconfig Path() call

cmd/syncthing/usage_report.go

@@ -201,7 +201,7 @@ func reportData(cfg *config.Wrapper, m *model.Model) map[string]interface{} {
 	defaultRelayServers, otherRelayServers := 0, 0
 	for _, addr := range cfg.Options().RelayServers {
 		switch addr {
-		case "dynamic+https://relays.syncthing.net":
+		case "dynamic+https://relays.syncthing.net/endpoint":
 			defaultRelayServers++
 		default:
 			otherRelayServers++

lib/config/config.go

@@ -175,7 +175,7 @@ func (cfg *Configuration) prepare(myID protocol.DeviceID) {
 	cfg.Options.ListenAddress = uniqueStrings(cfg.Options.ListenAddress)
 	cfg.Options.GlobalAnnServers = uniqueStrings(cfg.Options.GlobalAnnServers)
 
-	if cfg.Version < OldestHandledVersion {
+	if cfg.Version > 0 && cfg.Version < OldestHandledVersion {
 		l.Warnf("Configuration version %d is deprecated. Attempting best effort conversion, but please verify manually.", cfg.Version)
 	}
 

lib/config/config_test.go

@@ -104,6 +104,13 @@ func TestDeviceConfig(t *testing.T) {
 			},
 		}
 
+		// The cachedPath will have been resolved to an absolute path,
+		// depending on where the tests are running. Zero it out so we don't
+		// fail based on that.
+		for i := range cfg.Folders {
+			cfg.Folders[i].cachedPath = ""
+		}
+
 		if runtime.GOOS != "windows" {
 			expectedFolders[0].RawPath += string(filepath.Separator)
 		}

lib/config/folderconfiguration.go

@@ -36,7 +36,8 @@ type FolderConfiguration struct {
 	PullerPauseS          int                         `xml:"pullerPauseS" json:"pullerPauseS"`
 	MaxConflicts          int                         `xml:"maxConflicts" json:"maxConflicts"`
 
-	Invalid string `xml:"-" json:"invalid"` // Set at runtime when there is an error, not saved
+	Invalid    string `xml:"-" json:"invalid"` // Set at runtime when there is an error, not saved
+	cachedPath string
 }
 
 type FolderDeviceConfiguration struct {
@@ -55,28 +56,11 @@ func (f FolderConfiguration) Path() string {
 	// This is intentionally not a pointer method, because things like
 	// cfg.Folders["default"].Path() should be valid.
 
-	// Attempt tilde expansion; leave unchanged in case of error
-	if path, err := osutil.ExpandTilde(f.RawPath); err == nil {
-		f.RawPath = path
+	if f.cachedPath == "" {
+		l.Infoln("bug: uncached path call (should only happen in tests)")
+		return f.cleanedPath()
 	}
-
-	// Attempt absolutification; leave unchanged in case of error
-	if !filepath.IsAbs(f.RawPath) {
-		// Abs() looks like a fairly expensive syscall on Windows, while
-		// IsAbs() is a whole bunch of string mangling. I think IsAbs() may be
-		// somewhat faster in the general case, hence the outer if...
-		if path, err := filepath.Abs(f.RawPath); err == nil {
-			f.RawPath = path
-		}
-	}
-
-	// Attempt to enable long filename support on Windows. We may still not
-	// have an absolute path here if the previous steps failed.
-	if runtime.GOOS == "windows" && filepath.IsAbs(f.RawPath) && !strings.HasPrefix(f.RawPath, `\\`) {
-		return `\\?\` + f.RawPath
-	}
-
-	return f.RawPath
+	return f.cachedPath
 }
 
 func (f *FolderConfiguration) CreateMarker() error {
@@ -129,6 +113,8 @@ func (f *FolderConfiguration) prepare() {
 		f.RawPath = f.RawPath + string(filepath.Separator)
 	}
 
+	f.cachedPath = f.cleanedPath()
+
 	if f.ID == "" {
 		f.ID = "default"
 	}
@@ -140,6 +126,33 @@ func (f *FolderConfiguration) prepare() {
 	}
 }
 
+func (f *FolderConfiguration) cleanedPath() string {
+	cleaned := f.RawPath
+
+	// Attempt tilde expansion; leave unchanged in case of error
+	if path, err := osutil.ExpandTilde(cleaned); err == nil {
+		cleaned = path
+	}
+
+	// Attempt absolutification; leave unchanged in case of error
+	if !filepath.IsAbs(cleaned) {
+		// Abs() looks like a fairly expensive syscall on Windows, while
+		// IsAbs() is a whole bunch of string mangling. I think IsAbs() may be
+		// somewhat faster in the general case, hence the outer if...
+		if path, err := filepath.Abs(cleaned); err == nil {
+			cleaned = path
+		}
+	}
+
+	// Attempt to enable long filename support on Windows. We may still not
+	// have an absolute path here if the previous steps failed.
+	if runtime.GOOS == "windows" && filepath.IsAbs(cleaned) && !strings.HasPrefix(f.RawPath, `\\`) {
+		return `\\?\` + cleaned
+	}
+
+	return cleaned
+}
+
 type FolderDeviceConfigurationList []FolderDeviceConfiguration
 
 func (l FolderDeviceConfigurationList) Less(a, b int) bool {

lib/db/benchmark_test.go

@@ -17,8 +17,6 @@ import (
 
 	"github.com/syncthing/syncthing/lib/db"
 	"github.com/syncthing/syncthing/lib/protocol"
-	"github.com/syndtr/goleveldb/leveldb"
-	"github.com/syndtr/goleveldb/leveldb/opt"
 )
 
 var files, oneFile, firstHalf, secondHalf []protocol.FileInfo
@@ -44,16 +42,16 @@ func init() {
 	fs.Replace(protocol.LocalDeviceID, firstHalf)
 }
 
-func tempDB() (*leveldb.DB, string) {
+func tempDB() (*db.Instance, string) {
 	dir, err := ioutil.TempDir("", "syncthing")
 	if err != nil {
 		panic(err)
 	}
-	db, err := leveldb.OpenFile(filepath.Join(dir, "db"), &opt.Options{OpenFilesCacheCapacity: 100})
+	dbi, err := db.Open(filepath.Join(dir, "db"))
 	if err != nil {
 		panic(err)
 	}
-	return db, dir
+	return dbi, dir
 }
 
 func BenchmarkReplaceAll(b *testing.B) {

lib/discover/cache.go

@@ -109,6 +109,12 @@ func (m *CachingMux) Lookup(deviceID protocol.DeviceID) (direct []string, relays
 				when:   time.Now(),
 				found:  len(td)+len(tr) > 0,
 			})
+		} else {
+			// Lookup returned error, add a negative cache entry.
+			m.caches[i].Set(deviceID, CacheEntry{
+				when:  time.Now(),
+				found: false,
+			})
 		}
 	}
 	m.mut.Unlock()