Skip to content
This repository has been archived by the owner on Jan 9, 2024. It is now read-only.

fix(mem): removing kinesis batcher as possible memory leaker #826

Merged
merged 4 commits into from
Nov 30, 2023
Merged

Conversation

bassrock
Copy link
Member

@bassrock bassrock commented Nov 30, 2023

Goal

The mem leak from #825 continues.

I believe that it is likely the batch processor optimization that we had for Kinesis because it relied on timers and was polling. And because when removed, the tests no longer needed a forceExit param cause something was left open.

We will try deploying without the batch optimization since we should be well below kinesis limits for a good while.

Screenshot 2023-11-29 at 5 32 04 PM

@bassrock bassrock requested a review from a team as a code owner November 30, 2023 01:33
@bassrock bassrock requested review from efixler and kschelonka and removed request for a team and efixler November 30, 2023 01:33
@bassrock
Copy link
Member Author

No idea why the linter wanted these changes all of a sudden..

@@ -123,6 +123,7 @@ describe('Delete/Undelete SavedItem: ', () => {

afterAll(async () => {
await writeClient().destroy();
await readClient().destroy();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test needed the readClient destroyed to not hang them. I think its cause we made 2 queuries, 1 thats a mutation, 1 thats not. And we have logic that switches the client based on mutations. So when the query runs, it inits the readClient in the execution context, but we never cleaned it up.

@bassrock bassrock changed the title Mem leak fix(mem): removing kinesis batcher as possible memory leaker Nov 30, 2023
kschelonka
kschelonka previously approved these changes Nov 30, 2023
@bassrock bassrock merged commit bf7d49e into main Nov 30, 2023
8 checks passed
@bassrock bassrock deleted the mem-leak branch November 30, 2023 19:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants