From 253049a4cccacaed6ebb7aba1d35f0c20d155731 Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Tue, 12 Jan 2021 16:25:21 +0100 Subject: [PATCH] lib/api, lib/model: Avoid contention on filesystem for DB status call (ref #7270) (#7271) This splits the ignore getting to two methods, one that loads from disk (the old one) and one that just returns whatever is already loaded (the new one). The folder summary service which is just interested in stats now uses the latter method. This means that it, and API calls that call it, does not get blocked by folder I/O. --- lib/api/api.go | 2 +- lib/api/mocked_model_test.go | 6 +++++- lib/model/folder_summary.go | 2 +- lib/model/model.go | 32 ++++++++++++++++++++++++++++---- lib/model/model_test.go | 10 +++++----- 5 files changed, 40 insertions(+), 12 deletions(-) diff --git a/lib/api/api.go b/lib/api/api.go index ec8100d08..a94083281 100644 --- a/lib/api/api.go +++ b/lib/api/api.go @@ -1218,7 +1218,7 @@ func (s *service) getDBIgnores(w http.ResponseWriter, r *http.Request) { folder := qs.Get("folder") - lines, patterns, err := s.model.GetIgnores(folder) + lines, patterns, err := s.model.LoadIgnores(folder) if err != nil && !ignore.IsParseError(err) { http.Error(w, err.Error(), 500) return diff --git a/lib/api/mocked_model_test.go b/lib/api/mocked_model_test.go index 8d73bbc52..58b605f3d 100644 --- a/lib/api/mocked_model_test.go +++ b/lib/api/mocked_model_test.go @@ -80,7 +80,11 @@ func (m *mockedModel) Availability(folder string, file protocol.FileInfo, block return nil } -func (m *mockedModel) GetIgnores(folder string) ([]string, []string, error) { +func (m *mockedModel) LoadIgnores(folder string) ([]string, []string, error) { + return nil, nil, nil +} + +func (m *mockedModel) CurrentIgnores(folder string) ([]string, []string, error) { return nil, nil, nil } diff --git a/lib/model/folder_summary.go b/lib/model/folder_summary.go index d5253758a..b6d5949ef 100644 --- a/lib/model/folder_summary.go +++ b/lib/model/folder_summary.go @@ -142,7 +142,7 @@ func (c *folderSummaryService) Summary(folder string) (map[string]interface{}, e res["version"] = ourSeq + remoteSeq // legacy res["sequence"] = ourSeq + remoteSeq // new name - ignorePatterns, _, _ := c.model.GetIgnores(folder) + ignorePatterns, _, _ := c.model.CurrentIgnores(folder) res["ignorePatterns"] = false for _, line := range ignorePatterns { if len(line) > 0 && !strings.HasPrefix(line, "//") { diff --git a/lib/model/model.go b/lib/model/model.go index 0de3f82f5..458380c6c 100644 --- a/lib/model/model.go +++ b/lib/model/model.go @@ -83,7 +83,8 @@ type Model interface { Override(folder string) Revert(folder string) BringToFront(folder, file string) - GetIgnores(folder string) ([]string, []string, error) + LoadIgnores(folder string) ([]string, []string, error) + CurrentIgnores(folder string) ([]string, []string, error) SetIgnores(folder string, content []string) error GetFolderVersions(folder string) (map[string][]versioner.FileVersion, error) @@ -648,7 +649,7 @@ func (m *model) UsageReportingStats(report *contract.Report, version int, previe // Ignore stats var seenPrefix [3]bool for folder := range m.cfg.Folders() { - lines, _, err := m.GetIgnores(folder) + lines, _, err := m.CurrentIgnores(folder) if err != nil { continue } @@ -1971,7 +1972,9 @@ func (m *model) Connection(deviceID protocol.DeviceID) (protocol.Connection, boo return cn, ok } -func (m *model) GetIgnores(folder string) ([]string, []string, error) { +// LoadIgnores loads or refreshes the ignore patterns from disk, if the +// folder is healthy, and returns the refreshed lines and patterns. +func (m *model) LoadIgnores(folder string) ([]string, []string, error) { m.fmut.RLock() cfg, cfgOk := m.folderCfgs[folder] ignores, ignoresOk := m.folderIgnores[folder] @@ -1990,7 +1993,7 @@ func (m *model) GetIgnores(folder string) ([]string, []string, error) { } if !ignoresOk { - ignores = ignore.New(fs.NewFilesystem(cfg.FilesystemType, cfg.Path)) + ignores = ignore.New(cfg.Filesystem()) } err := ignores.Load(".stignore") @@ -2004,6 +2007,27 @@ func (m *model) GetIgnores(folder string) ([]string, []string, error) { return ignores.Lines(), ignores.Patterns(), err } +// CurrentIgnores returns the currently loaded set of ignore patterns, +// whichever it may be. No attempt is made to load or refresh ignore +// patterns from disk. +func (m *model) CurrentIgnores(folder string) ([]string, []string, error) { + m.fmut.RLock() + _, cfgOk := m.folderCfgs[folder] + ignores, ignoresOk := m.folderIgnores[folder] + m.fmut.RUnlock() + + if !cfgOk { + return nil, nil, fmt.Errorf("folder %s does not exist", folder) + } + + if !ignoresOk { + // Empty ignore patterns + return []string{}, []string{}, nil + } + + return ignores.Lines(), ignores.Patterns(), nil +} + func (m *model) SetIgnores(folder string, content []string) error { cfg, ok := m.cfg.Folder(folder) if !ok { diff --git a/lib/model/model_test.go b/lib/model/model_test.go index cc9b9d029..ff33eab98 100644 --- a/lib/model/model_test.go +++ b/lib/model/model_test.go @@ -1405,7 +1405,7 @@ func changeIgnores(t *testing.T, m *testModel, expected []string) { return true } - ignores, _, err := m.GetIgnores("default") + ignores, _, err := m.LoadIgnores("default") if err != nil { t.Error(err) } @@ -1421,7 +1421,7 @@ func changeIgnores(t *testing.T, m *testModel, expected []string) { t.Error(err) } - ignores2, _, err := m.GetIgnores("default") + ignores2, _, err := m.LoadIgnores("default") if err != nil { t.Error(err) } @@ -1441,7 +1441,7 @@ func changeIgnores(t *testing.T, m *testModel, expected []string) { t.Error(err) } - ignores, _, err = m.GetIgnores("default") + ignores, _, err = m.LoadIgnores("default") if err != nil { t.Error(err) } @@ -1472,7 +1472,7 @@ func TestIgnores(t *testing.T) { changeIgnores(t, m, expected) - _, _, err := m.GetIgnores("doesnotexist") + _, _, err := m.LoadIgnores("doesnotexist") if err == nil { t.Error("No error") } @@ -1490,7 +1490,7 @@ func TestIgnores(t *testing.T) { m.folderIgnores[fcfg.ID] = ignores m.fmut.Unlock() - _, _, err = m.GetIgnores("fresh") + _, _, err = m.LoadIgnores("fresh") if err == nil { t.Error("No error") }