Merge pull request #1054 from nats-io/sublist_stats

Protect stats when no cache is present

server/sublist.go

@@ -691,35 +691,41 @@ type SublistStats struct {
 
 // Stats will return a stats structure for the current state.
 func (s *Sublist) Stats() *SublistStats {
-	s.Lock()
-	defer s.Unlock()
-
 	st := &SublistStats{}
+
+	s.RLock()
+	cache := s.cache
 	st.NumSubs = s.count
-	st.NumCache = uint32(atomic.LoadInt32(&s.cacheNum))
 	st.NumInserts = s.inserts
 	st.NumRemoves = s.removes
+	s.RUnlock()
+
+	if cn := atomic.LoadInt32(&s.cacheNum); cn > 0 {
+		st.NumCache = uint32(cn)
+	}
 	st.NumMatches = atomic.LoadUint64(&s.matches)
 	if st.NumMatches > 0 {
 		st.CacheHitRate = float64(atomic.LoadUint64(&s.cacheHits)) / float64(st.NumMatches)
 	}
 
 	// whip through cache for fanout stats, this can be off if cache is full and doing evictions.
-	tot, max := 0, 0
-	clen := 0
-	s.cache.Range(func(k, v interface{}) bool {
-		clen++
-		r := v.(*SublistResult)
-		l := len(r.psubs) + len(r.qsubs)
-		tot += l
-		if l > max {
-			max = l
+	// If this is called frequently, which it should not be, this could hurt performance.
+	if cache != nil {
+		tot, max, clen := 0, 0, 0
+		s.cache.Range(func(k, v interface{}) bool {
+			clen++
+			r := v.(*SublistResult)
+			l := len(r.psubs) + len(r.qsubs)
+			tot += l
+			if l > max {
+				max = l
+			}
+			return true
+		})
+		st.MaxFanout = uint32(max)
+		if tot > 0 {
+			st.AvgFanout = float64(tot) / float64(clen)
 		}
-		return true
-	})
-	st.MaxFanout = uint32(max)
-	if tot > 0 {
-		st.AvgFanout = float64(tot) / float64(clen)
 	}
 	return st
 }

server/sublist_test.go

@@ -1,4 +1,4 @@
-// Copyright 2016-2018 The NATS Authors
+// Copyright 2016-2019 The NATS Authors
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
@@ -1007,6 +1007,20 @@ func TestSublistSharedEmptyResult(t *testing.T) {
 	}
 }
 
+func TestSublistNoCacheStats(t *testing.T) {
+	s := NewSublistNoCache()
+	s.Insert(newSub("foo"))
+	s.Insert(newSub("bar"))
+	s.Insert(newSub("baz"))
+	s.Insert(newSub("foo.bar.baz"))
+	s.Match("a.b.c")
+	s.Match("bar")
+	stats := s.Stats()
+	if stats.NumCache != 0 {
+		t.Fatalf("Expected 0 for NumCache stat, got %d", stats.NumCache)
+	}
+}
+
 // -- Benchmarks Setup --
 
 var benchSublistSubs []*subscription