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
Implement a rough memory Limit for the IdTables #344
Conversation
a8dd5d9
to
769ea70
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this valuable PR which saved us from hundreds of avoidable QLever crashes already. We already discussed much of it via ZOOM. Most of my comments are about better names, comments, or logging. Note that several of my comments have the property that they have consequences for the names in a lot of places, but I only made the comment once.
Let me know if you don't agree with a name change or if it seems that I misunderstood something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for the changes, a few minor things and we are done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two minor comments
- It keeps track of the number of maximally allowed bytes, using a synchronized, shared state, checks if an allocation is OK. - The actual allocation is still done by a std::allocator. - The IdTable now also internally uses an allocator template insteead of malloc and free. - Integrated the memory Limit into the complete project. - Required minor adaptations for a lot of Unit tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are done, thanks a lot :-)
Allows limiting the size of all IdTables that are used at the same time by using a stateful allocator.
Exceeding the limit will cause an exception.
We currently have no automatic cleanup mechanism etc. , clearing the cache
is the only possibility to get out of the OOM state under heavy load.
However, this is very useful nonetheless because it prevents most
std::bad_alloc
(or even worse OOM or silent)crashes.
TODO: