diff --git a/app/discovery/discovery.go b/app/discovery/discovery.go index cf052a71..8a7cc25d 100644 --- a/app/discovery/discovery.go +++ b/app/discovery/discovery.go @@ -5,6 +5,7 @@ package discovery import ( + "container/list" "context" "fmt" "net/http" @@ -21,16 +22,25 @@ import ( // Service implements discovery with multiple providers and url matcher type Service struct { - providers []Provider - mappers map[string][]URLMapper - mappersCache map[string][]URLMapper - lock sync.RWMutex + providers []Provider + mappers map[string][]URLMapper + mappersCache map[string]*list.Element + cacheOrder *list.List + serverRegexps map[string]*regexp.Regexp + lock sync.RWMutex // cacheLock guards mappersCache on the lazy read/write path in findMatchingMappers, which runs under // lock.RLock; the wholesale cache reset in Run runs under the exclusive lock.Lock and so needs no cacheLock cacheLock sync.RWMutex interval time.Duration } +const mappersCacheCapacity = 1024 + +type mapperCacheEntry struct { + server string + mappers []URLMapper +} + // URLMapper contains all info about source and destination routes type URLMapper struct { Server string @@ -159,10 +169,23 @@ func (s *Service) Run(ctx context.Context) error { } s.lock.Lock() s.mappers = make(map[string][]URLMapper) - s.mappersCache = make(map[string][]URLMapper) + s.mappersCache = make(map[string]*list.Element) + s.cacheOrder = list.New() + s.serverRegexps = make(map[string]*regexp.Regexp) for _, m := range lst { s.mappers[m.Server] = append(s.mappers[m.Server], m) } + for server := range s.mappers { + if isDefaultServer(server) || strings.HasPrefix(server, "*.") { + continue + } + re, err := regexp.Compile(server) + if err != nil { + log.Printf("[WARN] invalid regexp %s: %s", server, err) + continue + } + s.serverRegexps[server] = re + } s.lock.Unlock() } } @@ -281,9 +304,7 @@ func (s *Service) findMatchingMappers(srvName string) []URLMapper { // order is nondeterministic, so if the host matches 2+ server patterns the resolvers may select different // mapper slices and the cached value is last-writer-wins. this is consistent with the pre-existing // nondeterministic match-selection order, not a new defect - s.cacheLock.RLock() - cachedMapper, isCached := s.mappersCache[srvName] - s.cacheLock.RUnlock() + cachedMapper, isCached := s.cachedMappers(srvName) if isCached { return cachedMapper } @@ -298,24 +319,19 @@ func (s *Service) findMatchingMappers(srvName string) []URLMapper { if strings.HasPrefix(mapperServer, "*.") { domainPattern := mapperServer[1:] // strip the '*' if strings.HasSuffix(srvName, domainPattern) { - s.cacheLock.Lock() - s.mappersCache[srvName] = mapper - s.cacheLock.Unlock() + s.cacheMappers(srvName, mapper) return mapper } continue } - re, err := regexp.Compile(mapperServer) - if err != nil { - log.Printf("[WARN] invalid regexp %s: %s", mapperServer, err) + re, ok := s.serverRegexps[mapperServer] + if !ok { continue } if re.MatchString(srvName) { - s.cacheLock.Lock() - s.mappersCache[srvName] = mapper - s.cacheLock.Unlock() + s.cacheMappers(srvName, mapper) return mapper } } @@ -323,6 +339,39 @@ func (s *Service) findMatchingMappers(srvName string) []URLMapper { return nil } +func (s *Service) cachedMappers(server string) ([]URLMapper, bool) { + s.cacheLock.Lock() + defer s.cacheLock.Unlock() + + elem, ok := s.mappersCache[server] + if !ok { + return nil, false + } + s.cacheOrder.MoveToFront(elem) + return elem.Value.(mapperCacheEntry).mappers, true +} + +func (s *Service) cacheMappers(server string, mappers []URLMapper) { + s.cacheLock.Lock() + defer s.cacheLock.Unlock() + + if elem, ok := s.mappersCache[server]; ok { + entry := elem.Value.(mapperCacheEntry) + entry.mappers = mappers + elem.Value = entry + s.cacheOrder.MoveToFront(elem) + return + } + elem := s.cacheOrder.PushFront(mapperCacheEntry{server: server, mappers: mappers}) + s.mappersCache[server] = elem + if s.cacheOrder.Len() <= mappersCacheCapacity { + return + } + oldest := s.cacheOrder.Back() + delete(s.mappersCache, oldest.Value.(mapperCacheEntry).server) + s.cacheOrder.Remove(oldest) +} + // ScheduleHealthCheck starts background loop with health-check func (s *Service) ScheduleHealthCheck(ctx context.Context, interval time.Duration) { log.Printf("health-check scheduled every %s", interval) diff --git a/app/discovery/discovery_test.go b/app/discovery/discovery_test.go index 67beb97d..517258ea 100644 --- a/app/discovery/discovery_test.go +++ b/app/discovery/discovery_test.go @@ -387,6 +387,41 @@ func TestService_MatchConcurrent(t *testing.T) { } } +func TestService_MappersCacheBoundedLRU(t *testing.T) { + svc := matchTestService(t, []URLMapper{{ + Server: "*.example.com", SrcMatch: *regexp.MustCompile("^/(.*)"), + Dst: "http://127.0.0.1:8080/$1", MatchType: MTProxy, + }}) + + for i := range mappersCacheCapacity { + host := fmt.Sprintf("host-%d.example.com", i) + require.Len(t, svc.Match(host, "/some").Routes, 1) + } + + require.Len(t, svc.Match("host-0.example.com", "/some").Routes, 1) + require.Len(t, svc.Match("overflow.example.com", "/some").Routes, 1) + + svc.cacheLock.RLock() + defer svc.cacheLock.RUnlock() + assert.Len(t, svc.mappersCache, mappersCacheCapacity) + assert.Contains(t, svc.mappersCache, "host-0.example.com") + assert.NotContains(t, svc.mappersCache, "host-1.example.com") +} + +func TestService_ServerRegexCompiledAtInstall(t *testing.T) { + svc := matchTestService(t, []URLMapper{ + {Server: "(.*)\\.example\\.com", SrcMatch: *regexp.MustCompile("^/"), Dst: "http://127.0.0.1"}, + {Server: "[", SrcMatch: *regexp.MustCompile("^/"), Dst: "http://127.0.0.2"}, + }) + + svc.lock.RLock() + defer svc.lock.RUnlock() + re, ok := svc.serverRegexps["(.*)\\.example\\.com"] + require.True(t, ok) + assert.True(t, re.MatchString("host.example.com")) + assert.NotContains(t, svc.serverRegexps, "[") +} + func TestService_MatchServerRegexInvalidateCache(t *testing.T) { res := make(chan ProviderID) serverRegex := "test-(.*)"