From 3e0bfbb0f5857e77cae4eeece1791b5485129633 Mon Sep 17 00:00:00 2001 From: Benedikt Rudolph Date: Thu, 28 Feb 2019 11:32:40 +0100 Subject: [PATCH 1/3] Add feature cache backends Add support for various cache backends in anticipation of the merge with master that has an additional redis backend. The current memory based cache backend is refactored to implement the new interface. --- bird/bird.go | 108 +++++++++++++++++++++++-------------------- bird/memory_cache.go | 61 ++++++++++++++++++++++++ birdwatcher.go | 8 ++++ 3 files changed, 127 insertions(+), 50 deletions(-) create mode 100644 bird/memory_cache.go diff --git a/bird/bird.go b/bird/bird.go index 181fbc3..0d1d32e 100644 --- a/bird/bird.go +++ b/bird/bird.go @@ -3,6 +3,7 @@ package bird import ( "bytes" "io" + "log" "reflect" "strconv" "strings" @@ -12,31 +13,71 @@ import ( "os/exec" ) +type Cache interface { + Set(key string, val Parsed, ttl int) error + Get(key string) (Parsed, error) +} + var ClientConf BirdConfig var StatusConf StatusConfig var IPVersion = "4" +var cache Cache // stores parsed birdc output var RateLimitConf struct { sync.RWMutex Conf RateLimitConfig } +var RunQueue sync.Map // queue birdc commands before execution -type Cache struct { - sync.RWMutex - m map[string]Parsed -} - -var ParsedCache = Cache{m: make(map[string]Parsed)} -var MetaCache = Cache{m: make(map[string]Parsed)} - -var NilParse Parsed = (Parsed)(nil) +var NilParse Parsed = (Parsed)(nil) // special Parsed values var BirdError Parsed = Parsed{"error": "bird unreachable"} -var RunQueue sync.Map - -func IsSpecial(ret Parsed) bool { +func IsSpecial(ret Parsed) bool { // test for special Parsed values return reflect.DeepEqual(ret, NilParse) || reflect.DeepEqual(ret, BirdError) } +// intitialize the Cache once during setup with either a MemoryCache or +// RedisCache implementation. +// TODO implement singleton pattern +func InitializeCache(c Cache) { + cache = c +} + +/* Convenience method to make new entries in the cache. + * Abstracts over the specific caching implementation and the ability to set + * individual TTL values for entries. Always use the default TTL value from the + * config. + */ +func toCache(key string, val Parsed) bool { + var ttl int + if ClientConf.CacheTtl > 0 { + ttl = ClientConf.CacheTtl + } else { + ttl = 5 // five minutes + } + + if err := cache.Set(key, val, ttl); err == nil { + return true + } else { + log.Println(err) + return false + } +} + +/* Convenience method to retrieve entries from the cache. + * Abstracts over the specific caching implementations. + */ +func fromCache(key string) (Parsed, bool) { + val, err := cache.Get(key) + if err != nil { + log.Println(err) + return val, false + } else if IsSpecial(val) { // cache may return NilParse e.g. if ttl is expired + return val, false + } else { + return val, true + } +} + // Determines the key in the cache, where the result of specific functions are stored. // Eliminates the need to know what command was executed by that function. func GetCacheKey(fname string, fargs ...interface{}) string { @@ -52,39 +93,6 @@ func GetCacheKey(fname string, fargs ...interface{}) string { return key } -func (c *Cache) Store(key string, val Parsed) { - var ttl int = 5 - if ClientConf.CacheTtl > 0 { - ttl = ClientConf.CacheTtl - } - cachedAt := time.Now().UTC() - cacheTtl := cachedAt.Add(time.Duration(ttl) * time.Minute) - - c.Lock() - // This is not a really ... clean way of doing this. - val["ttl"] = cacheTtl - val["cached_at"] = cachedAt - - c.m[key] = val - c.Unlock() -} - -func (c *Cache) Get(key string) (Parsed, bool) { - c.RLock() - val, ok := c.m[key] - c.RUnlock() - if !ok { - return NilParse, false - } - - ttl, correct := val["ttl"].(time.Time) - if !correct || ttl.Before(time.Now()) { - return NilParse, false - } - - return val, ok -} - func Run(args string) (io.Reader, error) { args = "-r " + "show " + args // enforce birdc in restricted mode with "-r" argument argsList := strings.Split(args, " ") @@ -132,7 +140,7 @@ func checkRateLimit() bool { } func RunAndParse(key string, cmd string, parser func(io.Reader) Parsed, updateCache func(*Parsed)) (Parsed, bool) { - if val, ok := ParsedCache.Get(cmd); ok { + if val, ok := fromCache(cmd); ok { return val, true } @@ -141,7 +149,7 @@ func RunAndParse(key string, cmd string, parser func(io.Reader) Parsed, updateCa if queueGroup, queueLoaded := RunQueue.LoadOrStore(cmd, &wg); queueLoaded { (*queueGroup.(*sync.WaitGroup)).Wait() - if val, ok := ParsedCache.Get(cmd); ok { + if val, ok := fromCache(cmd); ok { return val, true } else { // TODO BirdError should also be signaled somehow @@ -169,7 +177,7 @@ func RunAndParse(key string, cmd string, parser func(io.Reader) Parsed, updateCa updateCache(&parsed) } - ParsedCache.Store(cmd, parsed) + toCache(cmd, parsed) wg.Done() @@ -228,7 +236,7 @@ func Protocols() (Parsed, bool) { metaProtocol["protocols"].(Parsed)["bird_protocol"].(Parsed)[birdProtocol].(Parsed)[protocol] = &parsed } - MetaCache.Store(GetCacheKey("Protocols"), metaProtocol) + toCache(GetCacheKey("Protocols"), metaProtocol) } res, from_cache := RunAndParse(GetCacheKey("Protocols"), "protocols all", parseProtocols, createMetaCache) @@ -241,7 +249,7 @@ func ProtocolsBgp() (Parsed, bool) { return protocols, from_cache } - protocolsMeta, _ := MetaCache.Get(GetCacheKey("Protocols")) + protocolsMeta, _ := fromCache(GetCacheKey("Protocols")) //TODO geht das einfach so? metaProtocol := protocolsMeta["protocols"].(Parsed) bgpProtocols := Parsed{} diff --git a/bird/memory_cache.go b/bird/memory_cache.go new file mode 100644 index 0000000..8213336 --- /dev/null +++ b/bird/memory_cache.go @@ -0,0 +1,61 @@ +package bird + +import ( + "errors" + "sync" + "time" +) + +// Implementation of the MemoryCache backend. + +type MemoryCache struct { + sync.RWMutex + m map[string]Parsed +} + +func NewMemoryCache() (*MemoryCache, error) { + var cache *MemoryCache + cache = &MemoryCache{m: make(map[string]Parsed)} + return cache, nil +} + +func (c *MemoryCache) Get(key string) (Parsed, error) { + c.RLock() + val, ok := c.m[key] + c.RUnlock() + if !ok { + return NilParse, errors.New("Could not retrive key" + key + "from MemoryCache.") + } + + ttl, correct := val["ttl"].(time.Time) + if !correct { + return NilParse, errors.New("Invalid TTL value for key" + key) + } + + if ttl.Before(time.Now()) { + return NilParse, nil // TTL expired + } else { + return val, nil // cache hit + } +} + +func (c *MemoryCache) Set(key string, val Parsed, ttl int) error { + switch { + case ttl == 0: + return nil // do not cache + case ttl > 0: + cachedAt := time.Now().UTC() + cacheTtl := cachedAt.Add(time.Duration(ttl) * time.Minute) + + c.Lock() + // This is not a really ... clean way of doing this. + val["ttl"] = cacheTtl + val["cached_at"] = cachedAt + + c.m[key] = val + c.Unlock() + return nil + default: // ttl negative - invalid + return errors.New("Negative TTL value for key" + key) + } +} diff --git a/birdwatcher.go b/birdwatcher.go index 208a9f9..3ad6a08 100644 --- a/birdwatcher.go +++ b/birdwatcher.go @@ -163,6 +163,14 @@ func main() { bird.RateLimitConf.Conf = conf.Ratelimit bird.RateLimitConf.Unlock() bird.ParserConf = conf.Parser + + var cache bird.Cache + cache, err = bird.NewMemoryCache() // initialze the MemoryCache + if err != nil { + log.Fatal("Could not initialize MemoryCache:", err) + } + bird.InitializeCache(cache) + endpoints.Conf = conf.Server // Make server From 75380807de54035c48663d1e32b41330d91fc193 Mon Sep 17 00:00:00 2001 From: Benedikt Rudolph Date: Thu, 28 Feb 2019 13:02:11 +0100 Subject: [PATCH 2/3] Add test for memory cache backend Improve error handling in case value can not be retrieved. Either return the value and nil, or a value and an error. --- bird/bird.go | 11 +++--- bird/memory_cache.go | 8 ++--- bird/memory_cache_test.go | 74 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 10 deletions(-) create mode 100644 bird/memory_cache_test.go diff --git a/bird/bird.go b/bird/bird.go index 0d1d32e..953db74 100644 --- a/bird/bird.go +++ b/bird/bird.go @@ -68,14 +68,13 @@ func toCache(key string, val Parsed) bool { */ func fromCache(key string) (Parsed, bool) { val, err := cache.Get(key) - if err != nil { - log.Println(err) - return val, false - } else if IsSpecial(val) { // cache may return NilParse e.g. if ttl is expired - return val, false - } else { + if err == nil { return val, true + } else { + return val, false } + //DEBUG log.Println(err) + } // Determines the key in the cache, where the result of specific functions are stored. diff --git a/bird/memory_cache.go b/bird/memory_cache.go index 8213336..89384b4 100644 --- a/bird/memory_cache.go +++ b/bird/memory_cache.go @@ -23,17 +23,17 @@ func (c *MemoryCache) Get(key string) (Parsed, error) { c.RLock() val, ok := c.m[key] c.RUnlock() - if !ok { - return NilParse, errors.New("Could not retrive key" + key + "from MemoryCache.") + if !ok { // cache miss + return NilParse, errors.New("Failed to retrive key '" + key + "' from MemoryCache.") } ttl, correct := val["ttl"].(time.Time) if !correct { - return NilParse, errors.New("Invalid TTL value for key" + key) + return NilParse, errors.New("Invalid TTL value for key '" + key + "'") } if ttl.Before(time.Now()) { - return NilParse, nil // TTL expired + return val, errors.New("TTL expired for key '" + key + "'") // TTL expired } else { return val, nil // cache hit } diff --git a/bird/memory_cache_test.go b/bird/memory_cache_test.go new file mode 100644 index 0000000..b4a6a78 --- /dev/null +++ b/bird/memory_cache_test.go @@ -0,0 +1,74 @@ +package bird + +import ( + "testing" +) + +func Test_MemoryCacheAccess(t *testing.T) { + + cache, err := NewMemoryCache() + + parsed := Parsed{ + "foo": 23, + "bar": 42, + "baz": true, + } + + t.Log("Setting memory cache...") + err = cache.Set("testkey", parsed, 5) + if err != nil { + t.Error(err) + } + + t.Log("Fetching from memory cache...") + parsed, err = cache.Get("testkey") + if err != nil { + t.Error(err) + } + + t.Log(parsed) +} + +func Test_MemoryCacheAccessKeyMissing(t *testing.T) { + + cache, err := NewMemoryCache() + + parsed, err := cache.Get("test_missing_key") + if !IsSpecial(parsed) { + t.Error(err) + } + t.Log("Cache error:", err) + t.Log(parsed) +} + +func Test_MemoryCacheRoutes(t *testing.T) { + f, err := openFile("routes_bird1_ipv4.sample") + if err != nil { + t.Error(err) + } + defer f.Close() + + parsed := parseRoutes(f) + _, ok := parsed["routes"].([]Parsed) + if !ok { + t.Fatal("Error getting routes") + } + + cache, err := NewMemoryCache() + + err = cache.Set("routes_protocol_test", parsed, 5) + if err != nil { + t.Error(err) + } + + parsed, err = cache.Get("routes_protocol_test") + if err != nil { + t.Error(err) + return + } + routes, ok := parsed["routes"].([]Parsed) + if !ok { + t.Error("Error getting routes") + } + t.Log("Retrieved routes:", len(routes)) +} From bab2be095744b29831292d0a56b306de7b832d94 Mon Sep 17 00:00:00 2001 From: Benedikt Rudolph Date: Thu, 28 Feb 2019 13:25:47 +0100 Subject: [PATCH 3/3] Avoid overwriting existing cache entry also fix RoutesNoExport. --- bird/bird.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bird/bird.go b/bird/bird.go index 953db74..3fe2478 100644 --- a/bird/bird.go +++ b/bird/bird.go @@ -235,10 +235,10 @@ func Protocols() (Parsed, bool) { metaProtocol["protocols"].(Parsed)["bird_protocol"].(Parsed)[birdProtocol].(Parsed)[protocol] = &parsed } - toCache(GetCacheKey("Protocols"), metaProtocol) + toCache(GetCacheKey("metaProtocol"), metaProtocol) } - res, from_cache := RunAndParse(GetCacheKey("Protocols"), "protocols all", parseProtocols, createMetaCache) + res, from_cache := RunAndParse(GetCacheKey("metaProtocol"), "protocols all", parseProtocols, createMetaCache) return res, from_cache } @@ -248,7 +248,7 @@ func ProtocolsBgp() (Parsed, bool) { return protocols, from_cache } - protocolsMeta, _ := fromCache(GetCacheKey("Protocols")) //TODO geht das einfach so? + protocolsMeta, _ := fromCache(GetCacheKey("metaProtocol")) metaProtocol := protocolsMeta["protocols"].(Parsed) bgpProtocols := Parsed{}