Skip to content

Commit

Permalink
stop circuit breaker Go routine before reloading api spec
Browse files Browse the repository at this point in the history
  • Loading branch information
dencoded committed May 2, 2019
1 parent 5b8aa16 commit 79447d0
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 4 deletions.
31 changes: 30 additions & 1 deletion api_definition.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ type TransformSpec struct {

type ExtendedCircuitBreakerMeta struct {
apidef.CircuitBreakerMeta
CB *circuit.Breaker
CB *circuit.Breaker `json:"-"`
}

// APISpec represents a path specification for an API, to avoid enumerating multiple nested lists, a single
Expand Down Expand Up @@ -179,6 +179,29 @@ type APISpec struct {
OrgHasNoSession bool

middlewareChain *ChainObject

shouldRelease bool
}

// Release re;leases all resources associated with API spec
func (s *APISpec) Release() {
// mark spec as to be released
s.shouldRelease = true

// release circuit breaker resources
for _, path := range s.RxPaths {
for _, urlSpec := range path {
if urlSpec.CircuitBreaker.CB != nil {
// this will force CB-event reading routing to exit
urlSpec.CircuitBreaker.CB.Reset()

// TODO: close all circuit breaker's event receivers
// which should let event reading Go-routines to exit (need to change circuitbreaker package)
}
}
}

// release all other resources associated with spec
}

// APIDefinitionLoader will load an Api definition from a storage
Expand Down Expand Up @@ -714,6 +737,12 @@ func (a APIDefinitionLoader) compileCircuitBreakerPathSpec(paths []apidef.Circui
})

case circuit.BreakerReset:
// check if this spec is set to release resources
if spec.shouldRelease {
// time to stop this Go-routine
return
}

spec.FireEvent(EventBreakerTriggered, EventCurcuitBreakerMeta{
EventMetaDefault: EventMetaDefault{Message: "Breaker Reset"},
CircuitEvent: e,
Expand Down
6 changes: 6 additions & 0 deletions api_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,12 @@ func loadApps(specs []*APISpec, muxer *mux.Router) {

// Swap in the new register
apisMu.Lock()

// release current specs resources before overwriting map
for _, curSpec := range apisByID {
curSpec.Release()
}

apisByID = tmpSpecRegister
apisMu.Unlock()

Expand Down
45 changes: 43 additions & 2 deletions gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -918,15 +918,17 @@ func TestReloadGoroutineLeakWithAsyncWrites(t *testing.T) {

globalConf := config.Global()
globalConf.UseAsyncSessionWrite = true
globalConf.EnableJSVM = false
config.SetGlobal(globalConf)
defer resetTestConfig()

buildAndLoadAPI(func(spec *APISpec) {
specs := buildAndLoadAPI(func(spec *APISpec) {
spec.Proxy.ListenPath = "/"
})

before := runtime.NumGoroutine()
doReload()

loadAPI(specs...) // just doing doReload() doesn't load anything as buildAndLoadAPI cleans up folder with API specs

time.Sleep(100 * time.Millisecond)

Expand All @@ -937,6 +939,45 @@ func TestReloadGoroutineLeakWithAsyncWrites(t *testing.T) {
}
}

func TestReloadGoroutineLeakWithCircuitBreaker(t *testing.T) {
ts := newTykTestServer()
defer ts.Close()

globalConf := config.Global()
globalConf.EnableJSVM = false
config.SetGlobal(globalConf)
defer resetTestConfig()

specs := buildAndLoadAPI(func(spec *APISpec) {
spec.Proxy.ListenPath = "/"
updateAPIVersion(spec, "v1", func(version *apidef.VersionInfo) {
version.ExtendedPaths = apidef.ExtendedPathsSet{
CircuitBreaker: []apidef.CircuitBreakerMeta{
{
Path: "/",
Method: http.MethodGet,
ThresholdPercent: 0.5,
Samples: 5,
ReturnToServiceAfter: 10,
},
},
}
})
})

before := runtime.NumGoroutine()

loadAPI(specs...) // just doing doReload() doesn't load anything as buildAndLoadAPI cleans up folder with API specs

time.Sleep(100 * time.Millisecond)

after := runtime.NumGoroutine()

if before != after-1 { // there is one will be running until we fix circuitbreaker Subscribe() method
t.Errorf("Goroutine leak, was: %d, after reload: %d", before, after)
}
}

func TestProxyUserAgent(t *testing.T) {
ts := newTykTestServer()
defer ts.Close()
Expand Down
5 changes: 4 additions & 1 deletion helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,10 @@ func loadAPI(specs ...*APISpec) (out []*APISpec) {
}()

for i, spec := range specs {
specBytes, _ := json.Marshal(spec)
specBytes, err := json.Marshal(spec)
if err != nil {
panic(err)
}
specFilePath := filepath.Join(config.Global().AppPath, spec.APIID+strconv.Itoa(i)+".json")
if err := ioutil.WriteFile(specFilePath, specBytes, 0644); err != nil {
panic(err)
Expand Down

0 comments on commit 79447d0

Please sign in to comment.