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

Adds method to unset last response in Client #221

Merged
merged 18 commits into from
Nov 5, 2024

Conversation

acya-skulskaya
Copy link
Contributor

I have a kafka consumer, which works as a demon. Manticore client is initialized one time at start. This consumer adds documents to manticore in bulk, say 100-200Mb at a time. I have noticed a memory leak, it increases every time the consumer adds documents to manticore. Turns out manticore client doesn't clean the lastResponse property after I execute $index->addDocuments($documents); - it is understandable that one may need this response later, but it also increases memory usage by ~200Mb for each running demon in my case, which is important when you have 20 of these demons running at the same time. So i have found that unsetting the lastResponse property each time after I add documents solves this memory leak problem

@sanikolaev
Copy link
Collaborator

Thanks for the PR @acya-skulskaya
Can you please improve it, so it passes the tests?

@acya-skulskaya
Copy link
Contributor Author

Thanks for the PR @acya-skulskaya Can you please improve it, so it passes the tests?

Hi, sorry I didn't have time yesterday to finish fixing the tests.
But the issue with failing tests doesn't seem to be connected with my changes? First it was showing that InsertTest was failing because the response had key table instead of _index - my guess it is because tests were written for a different version of manticoresearch?
I have updated my fork with today's release, but the tests are still failing all the same.
Also have tried to specify a previous version of manticoresearch in CI, but it didn't help with the tests.
I just really don't understand how adding a small method to manually unset a property could cause all these tests to be failing? And they seem to be failing because they expect completely different data in responses?

@sanikolaev
Copy link
Collaborator

@Nick-S-2018 pls review the PR and help with the failing tests.

@Nick-S-2018 Nick-S-2018 merged commit ed28112 into manticoresoftware:master Nov 5, 2024
2 of 10 checks passed
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.

3 participants