Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Investigation] Add lifecycle test #6677

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

titpetric
Copy link
Contributor

@titpetric titpetric commented Oct 28, 2024

This adds an aggresive test against the gateway test lifecycle. It concurrently starts and stops an unbounded amount of gateway test servers and attempts to exit the test after 5 seconds with a dumb time.Sleep. It starts about 20K of them on my local and triggers many data races / panics due to various reasons like global state.

Fixed a data race:

  • cli.Init is invoked in each test, as it holds global app state, overwrites needlessly
  • testutil: defaultTestConfig / reset config (incorrect test usage? see fixed test)

Questions:

  1. Should we remove cli.Init from Tests? It should not be triggered from test scope perhaps. At least once should be enough to maintain behaviour and pass race checks.
  2. Investigate additional issues reported and propose fixes

Test with:

cd tests/lifecycle
task services:up
go test -v -race ./...

Left in draft to consider wider effort. The ideal outcome is us being able to parallelize all tests.

Remaining races detected:

     41       /root/tyk/tyk/gateway/testutil.go:1046 +0x224
     40       /root/tyk/tyk/gateway/testutil.go:1029 +0x19c
     37       /root/tyk/tyk/gateway/testutil.go:1178 +0xe56
     33       /root/tyk/tyk/gateway/server.go:1423 +0x28b
      4       /root/tyk/tyk/gateway/testutil.go:1101 +0xc4
      2       /root/tyk/tyk/gateway/server.go:1421 +0x249
      2       /root/tyk/tyk/gateway/server.go:1420 +0x217
      
root@carbon:~/tyk/tyk/tests/lifecycle# sed -n 1046p /root/tyk/tyk/gateway/testutil.go
	gw := s.newGateway(genConf)
root@carbon:~/tyk/tyk/tests/lifecycle# sed -n 1029p /root/tyk/tyk/gateway/testutil.go
	t.Gw = t.start(genConf)
root@carbon:~/tyk/tyk/tests/lifecycle# sed -n 1178p /root/tyk/tyk/gateway/testutil.go
	gw.afterConfSetup()
root@carbon:~/tyk/tyk/tests/lifecycle# sed -n 1101p /root/tyk/tyk/gateway/testutil.go
	if err := config.WriteDefault("", &gwConfig); err != nil {
root@carbon:~/tyk/tyk/tests/lifecycle# sed -n 1423p /root/tyk/tyk/gateway/server.go
	regexp.ResetCache(time.Second*time.Duration(conf.RegexpCacheExpire), !conf.DisableRegexpCache)
root@carbon:~/tyk/tyk/tests/lifecycle# sed -n 1421p /root/tyk/tyk/gateway/server.go
	rpc.GlobalRPCCallTimeout = time.Second * time.Duration(conf.SlaveOptions.CallTimeout)
root@carbon:~/tyk/tyk/tests/lifecycle# sed -n 1420p /root/tyk/tyk/gateway/server.go
	rpc.GlobalRPCPingTimeout = time.Second * time.Duration(conf.SlaveOptions.PingTimeout)

@titpetric titpetric force-pushed the test/concurrency-stress-failures branch from aa5bb05 to 200bd10 Compare October 28, 2024 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant