tyk
tyk copied to clipboard
Fix/jsvm memory usage
This is an alternative implementation of https://github.com/TykTechnologies/tyk/pull/4215/files
Description
Reloading APIs, which use JSVM, cause gateway memory growth, and eventual crash.
Initialise JSVM only when needed
First of all I found that JSVM get initialized when ANY plugin is used. So first fix is: Initialise JSVM only when VirtualEndpoint used, or plugin with JSVM engine is used. It was done by removing initialization code from here https://github.com/TykTechnologies/tyk/compare/fix/jsvm-memory-usage?expand=1#diff-0cf80174bbafb36f6d4f4308ebbd971b2833b76a936bad568220aa1a4ba0ee8bL337-L354 And putting it after we already know plugin type https://github.com/TykTechnologies/tyk/compare/fix/jsvm-memory-usage?expand=1#diff-cdf0b7f176c9d18e1a314b78ddefc2cb3a94b3de66f1f360174692c915734c68R233-R237
Releasing pointers to easy GC
JSVM code by itself, is "a bit of" circular pointer hell. Trying to fix it leads to huge rewrite (and some parts really non obvious on how to do without having links to spec/gateway), which I do not want do as part of this PR. So now it cleanups pointers inside JSVM object, before API gets reloaded, to make GC job easier https://github.com/TykTechnologies/tyk/compare/fix/jsvm-memory-usage?expand=1#diff-78cd278aba997558b7daa7897051a794ef860076d45c93be792791db39381ca0R374-R380
Reloading only APIs which actually changed
For each API we now calculate SHA256 checksum, and if during API reload event, API has not changed, re-use already loaded API and its resources instead. https://github.com/TykTechnologies/tyk/compare/fix/jsvm-memory-usage?expand=1#diff-cdf0b7f176c9d18e1a314b78ddefc2cb3a94b3de66f1f360174692c915734c68R886-R898
How This Has Been Tested
Screenshots (if appropriate)
Types of changes
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] Refactoring or add test (improvements in base code or adds test coverage to functionality)
Checklist
- [ ] Make sure you are requesting to pull a topic/feature/bugfix branch (right side). If pulling from your own
fork, don't request your
master! - [ ] Make sure you are making a pull request against the
masterbranch (left side). Also, you should start your branch off our latestmaster. - [ ] My change requires a change to the documentation.
- [ ] If you've changed APIs, describe what needs to be updated in the documentation.
- [ ] If new config option added, ensure that it can be set via ENV variable
- [ ] I have updated the documentation accordingly.
- [ ] Modules and vendor dependencies have been updated; run
go mod tidy && go mod vendor - [ ] When updating library version must provide reason/explanation for this update.
- [ ] I have added tests to cover my changes.
- [ ] All new and existing tests passed.
- [ ] Check your code additions will not fail linting checks:
- [ ]
go fmt -s - [ ]
go vet
- [ ]
API tests result: failure :no_entry_sign: Branch used: refs/pull/4249/merge Commit: ca14d90235e281c04fed7df030a4437fe22424ca Triggered by: pull_request (@jeffy-mathew) Execution page
API tests result: failure :no_entry_sign: Branch used: refs/pull/4249/merge Commit: 179baaf3bd311ba03d185d34d8295e09129da289 Triggered by: pull_request (@jeffy-mathew) Execution page
API tests result: failure :no_entry_sign: Branch used: refs/pull/4249/merge Commit: 8f29ec7a42067709ae3d94339c77df7ef6903e14 Triggered by: pull_request (@jeffy-mathew) Execution page
API tests result: failure :no_entry_sign: Branch used: refs/pull/4249/merge Commit: 26f5b0b046de0123ef6bc0ca919f71450c31d0f8 Triggered by: pull_request (@jeffy-mathew) Execution page
API tests result: failure :no_entry_sign: Branch used: refs/pull/4249/merge Commit: 6caaaededd5f8d6bd68b81596f0b717849abb42a Triggered by: pull_request (@jeffy-mathew) Execution page
API tests result: failure :no_entry_sign: Branch used: refs/pull/4249/merge Commit: b971f305da1031532ee27f1a671c868d2a34859e Triggered by: pull_request (@jeffy-mathew) Execution page
API tests result: failure :no_entry_sign: Branch used: refs/pull/4249/merge Commit: b971f305da1031532ee27f1a671c868d2a34859e Triggered by: pull_request (@jeffy-mathew) Execution page
API tests result: success :white_check_mark: Branch used: refs/pull/4249/merge Commit: d9ccbc9cd369c4c4656adabe3dcb89e8b5ac7c3c Triggered by: pull_request (@jeffy-mathew) Execution page
API tests result: failure :no_entry_sign: Branch used: refs/pull/4249/merge Commit: 9e650d7f567fcecb96dfdbc4e74ac58319550bd3 Triggered by: pull_request (@jeffy-mathew) Execution page
API tests result: failure :no_entry_sign: Branch used: refs/pull/4249/merge Commit: ead324e887668b671fe2293852ff29a0e7a1faec Triggered by: pull_request (@jeffy-mathew) Execution page
:boom: CI tests failed :see_no_evil:
CI test log
Building go plugin
go test -race -count=1 -failfast -v -timeout 15m -coverprofile=tyk.cov github.com/TykTechnologies/tyk
# github.com/TykTechnologies/tyk/gateway
gateway/api_definition.go:416:6: err redeclared in this block
previous declaration at gateway/api_definition.go:336:13
gofmt
all ok
goimports
all ok
gogenerate
all ok
If the above are ok, please look at the run or in the Checks tab.
API tests result: failure :no_entry_sign: Branch used: refs/pull/4249/merge Commit: ffabfb99198770a3ab127e14373ec4d6ea78c517 Triggered by: pull_request (@jeffy-mathew) Execution page
API tests result: cancelled :no_entry_sign: Branch used: refs/pull/4249/merge Commit: ffabfb99198770a3ab127e14373ec4d6ea78c517 Triggered by: pull_request (@jeffy-mathew) Execution page
API tests result: success :white_check_mark: Branch used: refs/pull/4249/merge Commit: ffabfb99198770a3ab127e14373ec4d6ea78c517 Triggered by: pull_request (@jeffy-mathew) Execution page
API tests result: success :white_check_mark: Branch used: refs/pull/4249/merge Commit: cc0f562f26c57a86118ce2212bbe101b57143657 Triggered by: pull_request (@jeffy-mathew) Execution page
API tests result: success :white_check_mark: Branch used: refs/pull/4249/merge Commit: b99e839aad145e93a50032881018ad81b3ed0283 Triggered by: pull_request (@jeffy-mathew) Execution page
API tests result: success :white_check_mark: Branch used: refs/pull/4249/merge Commit: f178f2c03ef3ae3394721fcdb7bd3660013fca00 Triggered by: pull_request (@jeffy-mathew) Execution page
API tests result: success :white_check_mark: Branch used: refs/pull/4249/merge Commit: 570be88fb69f46e9c956a517761caf11f17f4a1f Triggered by: pull_request (@jeffy-mathew) Execution page
API tests result: success :white_check_mark: Branch used: refs/pull/4249/merge Commit: 5d5591a5af2edaa4ca70b2227a11c3b40125e048 Triggered by: pull_request (@jeffy-mathew) Execution page
API tests result: success :white_check_mark: Branch used: refs/pull/4249/merge Commit: df31193728ab45581090902d7e310efb47d3ef82 Triggered by: pull_request (@jeffy-mathew) Execution page
API tests result: success :white_check_mark: Branch used: refs/pull/4249/merge Commit: 407aa8cb071362fba5cbad0619bf76b8f3b139d1 Triggered by: pull_request (@jeffy-mathew) Execution page
API tests result: success :white_check_mark: Branch used: refs/pull/4249/merge Commit: ebdae098bd487e0d8679e174c79321769b1f026f Triggered by: pull_request (@jeffy-mathew) Execution page
API tests result: success :white_check_mark: Branch used: refs/pull/4249/merge Commit: 8062225b195b4522eea4aa19f724a215dfde0737 Triggered by: pull_request (@jeffy-mathew) Execution page
Looks ace!
API tests result: success :white_check_mark: Branch used: refs/pull/4249/merge Commit: fae61d7a2fa3e4268f63d2022ea1a18f36fd4825 Triggered by: pull_request (@jeffy-mathew) Execution page
/release to release-4
/release to release-4-lts
Working on it! Note that it can take a few minutes.
Working on it! Note that it can take a few minutes.







