prevent read to Redis from OrganizationMonitor MW (TykTechnologies#1787)

While experimenting with changes to eliminate extra I/O to redis I saw some good improvements so decided to make PR for that. These changes include: - in case org limiter was enabled for spec but org session wasn't created - don't read from Redis within every single API request - if we know that spec has org session - use in-app cache for org sessions 1st then fall back to Redis read - when org session is created - set spec's flag to identify that org rate limiter should start using it - when org session is deleted - set spec's flag to identify that org rate limiter should stop using it I think 1st point covers some special/corner case, however it still needs to be addressed imho. benchcmp vs. current master: ``` benchmark old ns/op new ns/op delta BenchmarkURLReplacer-8 1884 1892 +0.42% BenchmarkTagHeaders-8 944 949 +0.53% BenchmarkDefaultVersion-8 1029116 1019975 -0.89% BenchmarkGetVersionFromRequest/Header_location-8 526065 495921 -5.73% BenchmarkGetVersionFromRequest/URL_param_location-8 493047 535067 +8.52% BenchmarkGetVersionFromRequest/URL_location-8 459850 473349 +2.94% BenchmarkApiReload-8 3571882 3483883 -2.46% BenchmarkValueExtractor/HeaderSource-8 121697 120546 -0.95% BenchmarkValueExtractor/FormSource-8 137251 131319 -4.32% BenchmarkRegexExtractor/HeaderSource-8 124470 123174 -1.04% BenchmarkRegexExtractor/BodySource-8 150863 124594 -17.41% BenchmarkRegexExtractor/FormSource-8 140622 135016 -3.99% BenchmarkXPathExtractor/HeaderSource-8 134985 134931 -0.04% BenchmarkXPathExtractor/BodySource-8 139853 140675 +0.59% BenchmarkXPathExtractor/FormSource-8 148543 149929 +0.93% BenchmarkInitGenericEventHandlers-8 744 760 +2.15% BenchmarkMultiSession_BA_Standard_OK-8 2175804 2154980 -0.96% BenchmarkBearerTokenAuthKeySession-8 1886871 1851900 -1.85% BenchmarkMultiAuthBackwardsCompatibleSession-8 1914219 1829508 -4.43% BenchmarkBasicAuth-8 172803046 157311116 -8.97% BenchmarkContextVarsMiddleware-8 1361636 1285044 -5.62% BenchmarkContextVarsMiddlewareProcessRequest/GET_with_query_string-8 7882 7907 +0.32% BenchmarkContextVarsMiddlewareProcessRequest/POST_with_query_string_and_encoded_form_data-8 14244 11794 -17.20% BenchmarkContextVarsMiddlewareProcessRequest/POST_with_query_string_and_encoded_form_data_and_cookies-8 18682 14157 -24.22% BenchmarkHMACAuthSessionPass-8 1115659 1167463 +4.64% BenchmarkHMACAuthSessionPassWithHeaderField-8 1165189 1182420 +1.48% BenchmarkIPBlacklistMiddleware-8 27675 27189 -1.76% BenchmarkIPMiddlewarePass-8 17895 17531 -2.03% BenchmarkJWTSessionHMAC-8 354297 352147 -0.61% BenchmarkJWTSessionRSA-8 472412 461560 -2.30% BenchmarkJWTSessionRSABearer-8 514141 459284 -10.67% BenchmarkJWTSessionRSAWithRawSourceOnWithClientID-8 522359 469978 -10.03% BenchmarkJWTSessionRSAWithRawSource-8 519566 469399 -9.66% BenchmarkJWTSessionRSAWithJWK-8 515211 467952 -9.17% BenchmarkJWTSessionRSAWithEncodedJWK-8 555323 470708 -15.24% BenchmarkProcessRequestLiveQuotaLimit-8 627191 455293 -27.41% BenchmarkProcessRequestOffThreadQuotaLimit-8 582337 319647 -45.11% BenchmarkProcessRequestLiveRedisRollingLimiter-8 1051079 862438 -17.95% BenchmarkStripAuth_stripFromParams-8 40449 39482 -2.39% BenchmarkTransformNonAscii-8 15543 15329 -1.38% BenchmarkTransformJSONMarshal-8 16803 16289 -3.06% BenchmarkRewriter-8 33042 32356 -2.08% BenchmarkValidateJSONSchema-8 1768715 1348011 -23.79% BenchmarkVersioning-8 2098299 1640681 -21.81% BenchmarkVirtualEndpoint-8 345916 271487 -21.52% BenchmarkApplyPolicies-8 20984 21138 +0.73% BenchmarkResponseHeaderInjection-8 2050389 1584781 -22.71% BenchmarkRequestIPHops-8 416 409 -1.68% BenchmarkWrappedServeHTTP-8 141251 140336 -0.65% BenchmarkCopyRequestResponse-8 14666 14124 -3.70% benchmark old allocs new allocs delta BenchmarkURLReplacer-8 12 12 +0.00% BenchmarkTagHeaders-8 15 15 +0.00% BenchmarkDefaultVersion-8 1339 1340 +0.07% BenchmarkGetVersionFromRequest/Header_location-8 565 545 -3.54% BenchmarkGetVersionFromRequest/URL_param_location-8 529 529 +0.00% BenchmarkGetVersionFromRequest/URL_location-8 510 510 +0.00% BenchmarkApiReload-8 23903 23903 +0.00% BenchmarkValueExtractor/HeaderSource-8 50 50 +0.00% BenchmarkValueExtractor/FormSource-8 77 77 +0.00% BenchmarkRegexExtractor/HeaderSource-8 52 52 +0.00% BenchmarkRegexExtractor/BodySource-8 62 62 +0.00% BenchmarkRegexExtractor/FormSource-8 79 79 +0.00% BenchmarkXPathExtractor/HeaderSource-8 108 108 +0.00% BenchmarkXPathExtractor/BodySource-8 116 116 +0.00% BenchmarkXPathExtractor/FormSource-8 140 140 +0.00% BenchmarkInitGenericEventHandlers-8 8 8 +0.00% BenchmarkMultiSession_BA_Standard_OK-8 273 273 +0.00% BenchmarkBearerTokenAuthKeySession-8 241 241 +0.00% BenchmarkMultiAuthBackwardsCompatibleSession-8 246 246 +0.00% BenchmarkBasicAuth-8 19895 19872 -0.12% BenchmarkContextVarsMiddleware-8 1691 1691 +0.00% BenchmarkContextVarsMiddlewareProcessRequest/GET_with_query_string-8 40 40 +0.00% BenchmarkContextVarsMiddlewareProcessRequest/POST_with_query_string_and_encoded_form_data-8 64 64 +0.00% BenchmarkContextVarsMiddlewareProcessRequest/POST_with_query_string_and_encoded_form_data_and_cookies-8 80 80 +0.00% BenchmarkHMACAuthSessionPass-8 227 232 +2.20% BenchmarkHMACAuthSessionPassWithHeaderField-8 254 256 +0.79% BenchmarkIPBlacklistMiddleware-8 152 152 +0.00% BenchmarkIPMiddlewarePass-8 95 95 +0.00% BenchmarkJWTSessionHMAC-8 477 477 +0.00% BenchmarkJWTSessionRSA-8 576 576 +0.00% BenchmarkJWTSessionRSABearer-8 578 578 +0.00% BenchmarkJWTSessionRSAWithRawSourceOnWithClientID-8 630 600 -4.76% BenchmarkJWTSessionRSAWithRawSource-8 628 598 -4.78% BenchmarkJWTSessionRSAWithJWK-8 631 601 -4.75% BenchmarkJWTSessionRSAWithEncodedJWK-8 636 606 -4.72% BenchmarkProcessRequestLiveQuotaLimit-8 466 402 -13.73% BenchmarkProcessRequestOffThreadQuotaLimit-8 439 382 -12.98% BenchmarkProcessRequestLiveRedisRollingLimiter-8 1909 2048 +7.28% BenchmarkStripAuth_stripFromParams-8 220 220 +0.00% BenchmarkTransformNonAscii-8 98 98 +0.00% BenchmarkTransformJSONMarshal-8 101 101 +0.00% BenchmarkRewriter-8 238 238 +0.00% BenchmarkValidateJSONSchema-8 2261 2190 -3.14% BenchmarkVersioning-8 2853 2768 -2.98% BenchmarkVirtualEndpoint-8 319 305 -4.39% BenchmarkApplyPolicies-8 86 86 +0.00% BenchmarkResponseHeaderInjection-8 2258 1988 -11.96% BenchmarkRequestIPHops-8 4 4 +0.00% BenchmarkWrappedServeHTTP-8 144 144 +0.00% BenchmarkCopyRequestResponse-8 76 76 +0.00% benchmark old bytes new bytes delta BenchmarkURLReplacer-8 1088 1088 +0.00% BenchmarkTagHeaders-8 384 384 +0.00% BenchmarkDefaultVersion-8 191556 191708 +0.08% BenchmarkGetVersionFromRequest/Header_location-8 86727 85999 -0.84% BenchmarkGetVersionFromRequest/URL_param_location-8 85678 85677 -0.00% BenchmarkGetVersionFromRequest/URL_location-8 82902 82883 -0.02% BenchmarkApiReload-8 1788972 1788973 +0.00% BenchmarkValueExtractor/HeaderSource-8 4239 4239 +0.00% BenchmarkValueExtractor/FormSource-8 10337 10337 +0.00% BenchmarkRegexExtractor/HeaderSource-8 4274 4275 +0.02% BenchmarkRegexExtractor/BodySource-8 8731 8727 -0.05% BenchmarkRegexExtractor/FormSource-8 10378 10378 +0.00% BenchmarkXPathExtractor/HeaderSource-8 9047 9045 -0.02% BenchmarkXPathExtractor/BodySource-8 13465 13465 +0.00% BenchmarkXPathExtractor/FormSource-8 15659 15659 +0.00% BenchmarkInitGenericEventHandlers-8 528 528 +0.00% BenchmarkMultiSession_BA_Standard_OK-8 59058 59058 +0.00% BenchmarkBearerTokenAuthKeySession-8 56594 56591 -0.01% BenchmarkMultiAuthBackwardsCompatibleSession-8 58335 58338 +0.01% BenchmarkBasicAuth-8 1039184 1037416 -0.17% BenchmarkContextVarsMiddleware-8 279373 279372 -0.00% BenchmarkContextVarsMiddlewareProcessRequest/GET_with_query_string-8 7112 7112 +0.00% BenchmarkContextVarsMiddlewareProcessRequest/POST_with_query_string_and_encoded_form_data-8 10501 10501 +0.00% BenchmarkContextVarsMiddlewareProcessRequest/POST_with_query_string_and_encoded_form_data_and_cookies-8 12185 12187 +0.02% BenchmarkHMACAuthSessionPass-8 44427 46466 +4.59% BenchmarkHMACAuthSessionPassWithHeaderField-8 47557 48139 +1.22% BenchmarkIPBlacklistMiddleware-8 32341 32342 +0.00% BenchmarkIPMiddlewarePass-8 26733 26730 -0.01% BenchmarkJWTSessionHMAC-8 74235 74229 -0.01% BenchmarkJWTSessionRSA-8 94670 94679 +0.01% BenchmarkJWTSessionRSABearer-8 95865 95853 -0.01% BenchmarkJWTSessionRSAWithRawSourceOnWithClientID-8 99674 98320 -1.36% BenchmarkJWTSessionRSAWithRawSource-8 99547 98184 -1.37% BenchmarkJWTSessionRSAWithJWK-8 98547 97179 -1.39% BenchmarkJWTSessionRSAWithEncodedJWK-8 98781 97379 -1.42% BenchmarkProcessRequestLiveQuotaLimit-8 75919 71510 -5.81% BenchmarkProcessRequestOffThreadQuotaLimit-8 74972 70797 -5.57% BenchmarkProcessRequestLiveRedisRollingLimiter-8 134140 137887 +2.79% BenchmarkStripAuth_stripFromParams-8 14163 14198 +0.25% BenchmarkTransformNonAscii-8 11061 11060 -0.01% BenchmarkTransformJSONMarshal-8 11509 11508 -0.01% BenchmarkRewriter-8 45600 45600 +0.00% BenchmarkValidateJSONSchema-8 263473 255973 -2.85% BenchmarkVersioning-8 273570 264589 -3.28% BenchmarkVirtualEndpoint-8 65425 63949 -2.26% BenchmarkApplyPolicies-8 5925 5925 +0.00% BenchmarkResponseHeaderInjection-8 376155 360151 -4.25% BenchmarkRequestIPHops-8 432 432 +0.00% BenchmarkWrappedServeHTTP-8 48501 48486 -0.03% BenchmarkCopyRequestResponse-8 35585 35585 +0.00% ``` Also, I saw some good boost while testing with hey: old: ``` Total: 0.9978 secs Slowest: 0.0152 secs Fastest: 0.0004 secs Average: 0.0020 secs Requests/sec: 10022.3216 ``` new: ``` Total: 0.7754 secs Slowest: 0.0195 secs Fastest: 0.0003 secs Average: 0.0015 secs Requests/sec: 12897.0451 ```
tbuchaillot · Aug 19, 2018 · f656fec · f656fec
1 parent 0bf374d
commit f656fec
Show file tree

Hide file tree

Showing 3 changed files with 93 additions and 30 deletions.
diff --git a/api.go b/api.go
@@ -827,6 +827,13 @@ func handleOrgAddOrUpdate(keyName string, r *http.Request) (interface{}, int) {
 		return apiError("Error writing to key store " + err.Error()), http.StatusInternalServerError
 	}
 
+	// identify that spec has org session
+	if spec != nil {
+		spec.Lock()
+		spec.OrgHasNoSession = false
+		spec.Unlock()
+	}
+
 	log.WithFields(logrus.Fields{
 		"prefix": "api",
 		"org":    keyName,
@@ -908,6 +915,13 @@ func handleDeleteOrgKey(orgID string) (interface{}, int) {
 		"status": "ok",
 	}).Info("Org key deleted.")
 
+	// identify that spec has no org session
+	if spec != nil {
+		spec.Lock()
+		spec.OrgHasNoSession = true
+		spec.Unlock()
+	}
+
 	statusObj := apiModifyKeySuccess{
 		Key:    orgID,
 		Status: "ok",

diff --git a/api_definition.go b/api_definition.go
@@ -128,7 +128,7 @@ type ExtendedCircuitBreakerMeta struct {
 // flattened URL list is checked for matching paths and then it's status evaluated if found.
 type APISpec struct {
 	*apidef.APIDefinition
-	sync.Mutex
+	sync.RWMutex
 
 	RxPaths                  map[string][]URLSpec
 	WhiteListEnabled         map[string]bool
@@ -153,6 +153,7 @@ type APISpec struct {
 	WSTransport              http.RoundTripper
 	WSTransportCreated       time.Time
 	GlobalConfig             config.Config
+	OrgHasNoSession          bool
 
 	middlewareChain *ChainObject
 }

diff --git a/mw_organisation_activity.go b/mw_organisation_activity.go
@@ -6,6 +6,8 @@ import (
 
 	"errors"
 
+	"time"
+
 	"github.com/TykTechnologies/tyk/request"
 	"github.com/TykTechnologies/tyk/user"
 )
@@ -36,27 +38,60 @@ func (k *OrganizationMonitor) EnabledForSpec() bool {
 	return k.Spec.GlobalConfig.EnforceOrgQuotas
 }
 
+func (k *OrganizationMonitor) getOrgHasNoSession() bool {
+	k.Spec.RLock()
+	defer k.Spec.RUnlock()
+	return k.Spec.OrgHasNoSession
+}
+
+func (k *OrganizationMonitor) setOrgHasNoSession(val bool) {
+	k.Spec.Lock()
+	defer k.Spec.Unlock()
+	k.Spec.OrgHasNoSession = val
+}
+
 func (k *OrganizationMonitor) ProcessRequest(w http.ResponseWriter, r *http.Request, conf interface{}) (error, int) {
+	// short path for specs which have organization limiter enabled but organization has no session
+	if k.getOrgHasNoSession() {
+		return nil, http.StatusOK
+	}
+
+	var orgSession user.SessionState
+	var found bool
+
+	// try to check in in-app cache 1st
+	if !k.Spec.GlobalConfig.LocalSessionCache.DisableCacheSessionState {
+		var cachedSession interface{}
+		if cachedSession, found = SessionCache.Get(k.Spec.OrgID); found {
+			orgSession = cachedSession.(user.SessionState)
+		}
+	}
+
+	// try to get from Redis
+	if !found {
+		// not found in in-app cache, let's read from Redis
+		orgSession, found = k.OrgSession(k.Spec.OrgID)
+		if !found {
+			// prevent reads from in-app cache and from Redis for next runs
+			k.setOrgHasNoSession(true)
+			// No organisation session has not been created, should not be a pre-requisite in site setups, so we pass the request on
+			return nil, http.StatusOK
+		}
+	}
+
 	if k.Spec.GlobalConfig.ExperimentalProcessOrgOffThread {
 		// Make a copy of request before before sending to goroutine
 		r2 := r.WithContext(r.Context())
-		return k.ProcessRequestOffThread(r2)
+		return k.ProcessRequestOffThread(r2, orgSession)
 	}
-	return k.ProcessRequestLive(r)
+	return k.ProcessRequestLive(r, orgSession)
 }
 
 // ProcessRequest will run any checks on the request on the way through the system, return an error to have the chain fail
-func (k *OrganizationMonitor) ProcessRequestLive(r *http.Request) (error, int) {
-
-	session, found := k.OrgSession(k.Spec.OrgID)
-	if !found {
-		// No organisation session has been created, should not be a pre-requisite in site setups, so we pass the request on
-		return nil, http.StatusOK
-	}
-
+func (k *OrganizationMonitor) ProcessRequestLive(r *http.Request, orgSession user.SessionState) (error, int) {
 	// Is it active?
 	logEntry := getLogEntryForRequest(r, k.Spec.OrgID, nil)
-	if session.IsInactive {
+	if orgSession.IsInactive {
 		logEntry.Warning("Organisation access is disabled.")
 
 		return errors.New("this organisation access has been disabled, please contact your API administrator"), http.StatusForbidden
@@ -65,15 +100,24 @@ func (k *OrganizationMonitor) ProcessRequestLive(r *http.Request) (error, int) {
 	// We found a session, apply the quota and rate limiter
 	reason := k.sessionlimiter.ForwardMessage(
 		r,
-		&session,
+		&orgSession,
 		k.Spec.OrgID,
 		k.Spec.OrgSessionManager.Store(),
-		session.Per > 0 && session.Rate > 0,
+		orgSession.Per > 0 && orgSession.Rate > 0,
 		true,
 		&k.Spec.GlobalConfig,
 	)
 
-	k.Spec.OrgSessionManager.UpdateSession(k.Spec.OrgID, &session, session.Lifetime(k.Spec.SessionLifetime), false)
+	sessionLifeTime := orgSession.Lifetime(k.Spec.SessionLifetime)
+
+	if err := k.Spec.OrgSessionManager.UpdateSession(k.Spec.OrgID, &orgSession, sessionLifeTime, false); err == nil {
+		// update in-app cache if needed
+		if !k.Spec.GlobalConfig.LocalSessionCache.DisableCacheSessionState {
+			SessionCache.Set(k.Spec.OrgID, orgSession, time.Second*time.Duration(sessionLifeTime))
+		}
+	} else {
+		log.WithError(err).WithField("orgID", k.Spec.OrgID).Error("Could not update org session")
+	}
 
 	switch reason {
 	case sessionFailNone:
@@ -116,11 +160,11 @@ func (k *OrganizationMonitor) ProcessRequestLive(r *http.Request) (error, int) {
 
 	if k.Spec.GlobalConfig.Monitor.MonitorOrgKeys {
 		// Run the trigger monitor
-		k.mon.Check(&session, "")
+		k.mon.Check(&orgSession, "")
 	}
 
 	// Lets keep a reference of the org
-	setCtxValue(r, OrgSessionContext, session)
+	setCtxValue(r, OrgSessionContext, orgSession)
 
 	// Request is valid, carry on
 	return nil, http.StatusOK
@@ -133,13 +177,7 @@ func (k *OrganizationMonitor) SetOrgSentinel(orgChan chan bool, orgId string) {
 	}
 }
 
-func (k *OrganizationMonitor) ProcessRequestOffThread(r *http.Request) (error, int) {
-	session, found := k.OrgSession(k.Spec.OrgID)
-	if !found {
-		// No organisation session has been created, should not be a pre-requisite in site setups, so we pass the request on
-		return nil, http.StatusOK
-	}
-
+func (k *OrganizationMonitor) ProcessRequestOffThread(r *http.Request, orgSession user.SessionState) (error, int) {
 	orgChanMap.Lock()
 	orgChan, ok := orgChanMap.channels[k.Spec.OrgID]
 	if !ok {
@@ -150,12 +188,13 @@ func (k *OrganizationMonitor) ProcessRequestOffThread(r *http.Request) (error, i
 	orgChanMap.Unlock()
 	active, found := orgActiveMap.Load(k.Spec.OrgID)
 
+	orgSessionCopy := orgSession
 	go k.AllowAccessNext(
 		orgChan,
 		r.URL.Path,
 		request.RealIP(r),
 		r,
-		session,
+		&orgSessionCopy,
 	)
 
 	if found && !active.(bool) {
@@ -166,7 +205,7 @@ func (k *OrganizationMonitor) ProcessRequestOffThread(r *http.Request) (error, i
 	// Lets keep a reference of the org
 	// session might be updated by go-routine AllowAccessNext and we loose those changes here
 	// but it is OK as we need it in context for detailed org logging
-	setCtxValue(r, OrgSessionContext, session)
+	setCtxValue(r, OrgSessionContext, orgSession)
 
 	// Request is valid, carry on
 	return nil, http.StatusOK
@@ -177,7 +216,7 @@ func (k *OrganizationMonitor) AllowAccessNext(
 	path string,
 	IP string,
 	r *http.Request,
-	session user.SessionState) {
+	session *user.SessionState) {
 
 	// Is it active?
 	logEntry := getExplicitLogEntryForRequest(path, IP, k.Spec.OrgID, nil)
@@ -190,15 +229,24 @@ func (k *OrganizationMonitor) AllowAccessNext(
 	// We found a session, apply the quota and rate limiter
 	reason := k.sessionlimiter.ForwardMessage(
 		r,
-		&session,
+		session,
 		k.Spec.OrgID,
 		k.Spec.OrgSessionManager.Store(),
 		session.Per > 0 && session.Rate > 0,
 		true,
 		&k.Spec.GlobalConfig,
 	)
 
-	k.Spec.OrgSessionManager.UpdateSession(k.Spec.OrgID, &session, session.Lifetime(k.Spec.SessionLifetime), false)
+	sessionLifeTime := session.Lifetime(k.Spec.SessionLifetime)
+
+	if err := k.Spec.OrgSessionManager.UpdateSession(k.Spec.OrgID, session, sessionLifeTime, false); err == nil {
+		// update in-app cache if needed
+		if !k.Spec.GlobalConfig.LocalSessionCache.DisableCacheSessionState {
+			SessionCache.Set(k.Spec.OrgID, *session, time.Second*time.Duration(sessionLifeTime))
+		}
+	} else {
+		log.WithError(err).WithField("orgID", k.Spec.OrgID).Error("Could not update org session")
+	}
 
 	isExceeded := false
 	switch reason {
@@ -242,7 +290,7 @@ func (k *OrganizationMonitor) AllowAccessNext(
 
 	if k.Spec.GlobalConfig.Monitor.MonitorOrgKeys {
 		// Run the trigger monitor
-		k.mon.Check(&session, "")
+		k.mon.Check(session, "")
 	}
 
 	if isExceeded {