storage: a couple of redis cleanups

First, don't make all the methods use pointer receivers. None of them modify the struct, and the struct itself is tiny - just a string and two bools, which should total three ints. This should mean a slight memory and performance win, as we don't have to allocate as many objects and we don't have to jump around memory as much. Second, refactor GetRelevantClusterReference into a method with a shorter name, making the code more understandable and simpler. While replacing RedisCluster pointers with structs, I also noticed how RedisNotificationHandler could discard notifications if the store wasn't initialised. Instead, synchronously set up the store, so we know it has been set up. This is not a problem, given that Connect doesn't actually block connecting to all the hosts.
zhailuxu · Oct 10, 2017 · 8e7743a · 8e7743a
1 parent 747c405
commit 8e7743a
Show file tree

Hide file tree

Showing 12 changed files with 103 additions and 112 deletions.
diff --git a/api.go b/api.go
@@ -1341,7 +1341,7 @@ func invalidateCacheHandler(w http.ResponseWriter, r *http.Request) {
 
 	keyPrefix := "cache-" + apiID
 	matchPattern := keyPrefix + "*"
-	store := &storage.RedisCluster{KeyPrefix: keyPrefix, IsCache: true}
+	store := storage.RedisCluster{KeyPrefix: keyPrefix, IsCache: true}
 
 	if ok := store.DeleteScanMatch(matchPattern); !ok {
 		err := errors.New("scan/delete failed")

diff --git a/api_loader.go b/api_loader.go
@@ -28,16 +28,16 @@ type ChainObject struct {
 	Subrouter      *mux.Router
 }
 
-func prepareStorage() (*storage.RedisCluster, *storage.RedisCluster, *storage.RedisCluster, *RPCStorageHandler, *RPCStorageHandler) {
+func prepareStorage() (storage.RedisCluster, storage.RedisCluster, storage.RedisCluster, *RPCStorageHandler, *RPCStorageHandler) {
 	redisStore := storage.RedisCluster{KeyPrefix: "apikey-", HashKeys: config.Global.HashKeys}
 	redisOrgStore := storage.RedisCluster{KeyPrefix: "orgkey."}
-	healthStore := &storage.RedisCluster{KeyPrefix: "apihealth."}
+	healthStore := storage.RedisCluster{KeyPrefix: "apihealth."}
 	rpcAuthStore := RPCStorageHandler{KeyPrefix: "apikey-", HashKeys: config.Global.HashKeys, UserKey: config.Global.SlaveOptions.APIKey, Address: config.Global.SlaveOptions.ConnectionString}
 	rpcOrgStore := RPCStorageHandler{KeyPrefix: "orgkey.", UserKey: config.Global.SlaveOptions.APIKey, Address: config.Global.SlaveOptions.ConnectionString}
 
 	FallbackKeySesionManager.Init(&redisStore)
 
-	return &redisStore, &redisOrgStore, healthStore, &rpcAuthStore, &rpcOrgStore
+	return redisStore, redisOrgStore, healthStore, &rpcAuthStore, &rpcOrgStore
 }
 
 func skipSpecBecauseInvalid(spec *APISpec) bool {
@@ -261,7 +261,7 @@ func processSpec(spec *APISpec, apisByListen map[string]int,
 	}
 
 	keyPrefix := "cache-" + spec.APIID
-	cacheStore := &storage.RedisCluster{KeyPrefix: keyPrefix, IsCache: true}
+	cacheStore := storage.RedisCluster{KeyPrefix: keyPrefix, IsCache: true}
 	cacheStore.Connect()
 
 	var chain http.Handler

diff --git a/coprocess_api.go b/coprocess_api.go
@@ -34,7 +34,7 @@ func TykStoreData(CKey, CValue *C.char, CTTL C.int) {
 	value := C.GoString(CValue)
 	ttl := int64(CTTL)
 
-	store := &storage.RedisCluster{KeyPrefix: CoProcessDefaultKeyPrefix}
+	store := storage.RedisCluster{KeyPrefix: CoProcessDefaultKeyPrefix}
 	store.SetKey(key, value, ttl)
 }
 
@@ -43,7 +43,7 @@ func TykStoreData(CKey, CValue *C.char, CTTL C.int) {
 func TykGetData(CKey *C.char) *C.char {
 	key := C.GoString(CKey)
 
-	store := &storage.RedisCluster{KeyPrefix: CoProcessDefaultKeyPrefix}
+	store := storage.RedisCluster{KeyPrefix: CoProcessDefaultKeyPrefix}
 	// TODO: return error
 	val, _ := store.GetKey(key)
 	return C.CString(val)

diff --git a/event_handler_webhooks.go b/event_handler_webhooks.go
@@ -66,7 +66,7 @@ func (w *WebHookHandler) Init(handlerConf interface{}) error {
 		return err
 	}
 
-	w.store = &storage.RedisCluster{KeyPrefix: "webhook.cache."}
+	w.store = storage.RedisCluster{KeyPrefix: "webhook.cache."}
 	w.store.Connect()
 
 	// Pre-load template on init

diff --git a/gateway_test.go b/gateway_test.go
@@ -519,9 +519,9 @@ const extendedPathGatewaySetup = `{
 func createSpecTest(t *testing.T, def string) *APISpec {
 	spec := createDefinitionFromString(def)
 	tname := t.Name()
-	redisStore := &storage.RedisCluster{KeyPrefix: tname + "-apikey."}
-	healthStore := &storage.RedisCluster{KeyPrefix: tname + "-apihealth."}
-	orgStore := &storage.RedisCluster{KeyPrefix: tname + "-orgKey."}
+	redisStore := storage.RedisCluster{KeyPrefix: tname + "-apikey."}
+	healthStore := storage.RedisCluster{KeyPrefix: tname + "-apihealth."}
+	orgStore := storage.RedisCluster{KeyPrefix: tname + "-orgKey."}
 	spec.Init(redisStore, redisStore, healthStore, orgStore)
 	return spec
 }

diff --git a/host_checker_test.go b/host_checker_test.go
@@ -148,7 +148,7 @@ func TestHostChecker(t *testing.T) {
 		t.Error("Should set defaults", GlobalHostChecker.checker.checkTimeout)
 	}
 
-	redisStore := GlobalHostChecker.store.(*storage.RedisCluster)
+	redisStore := GlobalHostChecker.store.(storage.RedisCluster)
 	if ttl, _ := redisStore.GetKeyTTL(PoolerHostSentinelKeyPrefix + testHttpFailure); int(ttl) != GlobalHostChecker.checker.checkTimeout+1 {
 		t.Error("HostDown expiration key should be checkTimeout + 1", ttl)
 	}

diff --git a/le_helpers.go b/le_helpers.go
@@ -18,7 +18,7 @@ func StoreLEState(m *letsencrypt.Manager) {
 
 	log.Debug("[SSL] --> Connecting to DB")
 
-	store := &storage.RedisCluster{KeyPrefix: LEKeyPrefix}
+	store := storage.RedisCluster{KeyPrefix: LEKeyPrefix}
 	connected := store.Connect()
 
 	log.Debug("--> Connected to DB")
@@ -41,7 +41,7 @@ func StoreLEState(m *letsencrypt.Manager) {
 func GetLEState(m *letsencrypt.Manager) {
 	checkKey := "cache"
 
-	store := &storage.RedisCluster{KeyPrefix: LEKeyPrefix}
+	store := storage.RedisCluster{KeyPrefix: LEKeyPrefix}
 
 	connected := store.Connect()
 	log.Debug("[SSL] --> Connected to DB")

diff --git a/main.go b/main.go
@@ -102,7 +102,7 @@ func setupGlobals() {
 	}
 
 	// Initialise our Host Checker
-	healthCheckStore := &storage.RedisCluster{KeyPrefix: "host-checker:"}
+	healthCheckStore := storage.RedisCluster{KeyPrefix: "host-checker:"}
 	InitHostCheckManager(healthCheckStore)
 
 	if config.Global.EnableAnalytics && analytics.Store == nil {
@@ -145,7 +145,7 @@ func setupGlobals() {
 	}).Debug("Notifier will not work in hybrid mode")
 	mainNotifierStore := storage.RedisCluster{}
 	mainNotifierStore.Connect()
-	MainNotifier = RedisNotifier{&mainNotifierStore, RedisPubSubChannel}
+	MainNotifier = RedisNotifier{mainNotifierStore, RedisPubSubChannel}
 
 	if config.Global.Monitor.EnableTriggerMonitors {
 		h := &WebHookHandler{}
@@ -924,7 +924,7 @@ func getGlobalStorageHandler(keyPrefix string, hashKeys bool) storage.Handler {
 	if config.Global.SlaveOptions.UseRPC {
 		return &RPCStorageHandler{KeyPrefix: keyPrefix, HashKeys: hashKeys, UserKey: config.Global.SlaveOptions.APIKey, Address: config.Global.SlaveOptions.ConnectionString}
 	}
-	return &storage.RedisCluster{KeyPrefix: keyPrefix, HashKeys: hashKeys}
+	return storage.RedisCluster{KeyPrefix: keyPrefix, HashKeys: hashKeys}
 }
 
 func main() {
@@ -1048,7 +1048,7 @@ func start(arguments map[string]interface{}) {
 		log.WithFields(logrus.Fields{
 			"prefix": "main",
 		}).Debug("Initialising default org store")
-		//DefaultOrgStore.Init(&storage.RedisCluster{KeyPrefix: "orgkey."})
+		//DefaultOrgStore.Init(storage.RedisCluster{KeyPrefix: "orgkey."})
 		DefaultOrgStore.Init(getGlobalStorageHandler("orgkey.", false))
 		//DefaultQuotaStore.Init(getGlobalStorageHandler(CloudHandler, "orgkey.", false))
 		DefaultQuotaStore.Init(getGlobalStorageHandler("orgkey.", false))

diff --git a/redis_signal_outbound.go b/redis_signal_outbound.go
@@ -17,33 +17,29 @@ type InterfaceNotification struct {
 }
 
 type RedisNotificationHandler struct {
-	CacheStore *storage.RedisCluster
+	CacheStore storage.RedisCluster
 }
 
 const (
 	UIChanName = "dashboard.ui.messages"
 )
 
 func (u *RedisNotificationHandler) Start() {
-	go u.StartUIPubSubConn()
+	u.CacheStore = storage.RedisCluster{KeyPrefix: "gateway-notifications:"}
+	u.CacheStore.Connect()
+	go u.PubSubLoop()
 }
 
 func (u *RedisNotificationHandler) Notify(n InterfaceNotification) error {
 	jsonError, err := json.Marshal(n)
 	if err != nil {
 		return err
 	}
-
-	if u.CacheStore != nil {
-		u.CacheStore.Publish(UIChanName, string(jsonError))
-	}
-
+	u.CacheStore.Publish(UIChanName, string(jsonError))
 	return nil
 }
 
-func (u *RedisNotificationHandler) StartUIPubSubConn() {
-	u.CacheStore = &storage.RedisCluster{KeyPrefix: "gateway-notifications:"}
-	u.CacheStore.Connect()
+func (u *RedisNotificationHandler) PubSubLoop() {
 	// On message, synchronize
 	for {
 		err := u.CacheStore.StartPubSubHandler(UIChanName, u.HandleIncommingRedisEvent)

diff --git a/redis_signals.go b/redis_signals.go
@@ -175,7 +175,7 @@ func isPayloadSignatureValid(notification Notification) bool {
 
 // RedisNotifier will use redis pub/sub channels to send notifications
 type RedisNotifier struct {
-	store   *storage.RedisCluster
+	store   storage.RedisCluster
 	channel string
 }
 

diff --git a/rpc_backup_handlers.go b/rpc_backup_handlers.go
@@ -35,7 +35,7 @@ func saveRPCDefinitionsBackup(list string) {
 
 	log.Info("--> Connecting to DB")
 
-	store := &storage.RedisCluster{KeyPrefix: RPCKeyPrefix}
+	store := storage.RedisCluster{KeyPrefix: RPCKeyPrefix}
 	connected := store.Connect()
 
 	log.Info("--> Connected to DB")
@@ -57,7 +57,7 @@ func LoadDefinitionsFromRPCBackup() []*APISpec {
 	tagList := getTagListAsString()
 	checkKey := BackupKeyBase + tagList
 
-	store := &storage.RedisCluster{KeyPrefix: RPCKeyPrefix}
+	store := storage.RedisCluster{KeyPrefix: RPCKeyPrefix}
 
 	connected := store.Connect()
 	log.Info("[RPC] --> Connected to DB")