search

jprupp / rocksdb-haskell

RocksDB Haskell Bindings
BSD 3-Clause "New" or "Revised" License
5 stars 5 forks source link

Production usage #1

Closed shinzui closed 2 years ago

shinzui commented 3 years ago

Hi,

I'm curious if you're using this in production. It appears that the original code has an issue concerning a memory leak. Does your fork address that?

jprupp commented 3 years ago

Yes, it's used on a fleet of servers in production without issue. The library functions are called heavily in this environment.

The memory leak wasn't so. It is a design choice to have the context object generated once through an "unsafe" IO action and reused implicitly throughout the library. Users of the library do not need to concern themselves with this context object. The context data is thread-safe and can reused according to upstream documents.

Since the context object is created and its heap memory is never subsequently deallocated, automated tools decide that the library has a memory leak. I don't know if this can be improved while preserving the pure and simple interface that doesn't need explicit context passing, such that automated memory analysis tools can be happy again.

jprupp commented 3 years ago

I'm sorry. That comment was for another package I maintain. I mistook this library for that other one. Disregard it.

I am not sure where a memory leak might hide in this code. The library is used heavily in production, with a fleet of servers remaining online for weeks.

Occasionally I have seen some servers restarting due to running out of memory, but I doubt it is due to this library. I'll keep my eyes open and review the code again. Perhaps if you can take a look as well we may uncover something unexpected.

Greetings

shinzui commented 3 years ago

Thank you for answering. I didn't start using the library yet.

I noticed two concerning issues in the original repo while investigating. The memory leak and the thread-safety one, and I didn't see any specific fix in the git log of this repository which lead me to open this issue.

jprupp commented 3 years ago

I'll check, but I did rewrite most of the code quite deeply.

jprupp commented 2 years ago

I did a quick review of the code today, and it seems good. I have made significant changes to the library, sanitising the C binding code and providing wrapper functions that run your database actions in a closure that automatically releases allocated resources.

Linear types can help to validate resource allocation algorithms better. Something to keep in mind for the future.

shinzui commented 2 years ago

Thank you for taking the time and reviewing the code. I feel more confident to use it now.

It would be nice to update the Readme to indicate that you rewrote the library and are using it in production.