Use of std thread primitives bypass rocksdb::port and rocksdb::Env
- Dominant language
- C++
- Stars
- 32.1k
- Forks
- 6.9k
- Avg merge
- 32m
- Merged PRs (30d)
- 1
Description
There are several instances now where stdlib thread interfaces are used directly without being wrapped by `rocksdb::port` or `rocksdb::Env` as they ought to be.
Notable examples:
- https://github.com/facebook/rocksdb/blob/5.16.fb/memtable/write_buffer_manager.cc#L25
`std::mutex` is used here rather than `rocksdb::port::Mutex`. When the embedding adapts the `rocksdb::port` for alternative threading, contending for this `std::mutex` will deadlock the program.
- https://github.com/facebook/rocksdb/blob/5.16.fb/port/port_posix.h#L167
`rocsdb::port` itself should not typedef `Thread` to `std::thread` because that bypasses the `rocksdb::Env` interface for thread-related callbacks. In this case I would advise RocksDB to create their own internal `Thread` object which leads to `rocksdb::Env::StartThread()` et al. An example of where this is a problem is the `rocksdb::util::SstFileManager` where each instance spawns a `port::Thread` thus unconditionally spawning an `std::thread` for an embedding implementing the env callback instead (which is not called).
- Various instances of `std::lock_guard` can be found with a `grep`. The `std::lock_guard` template can be used with any lockable-concept class; in other words it should be `std::lock_guard` throughout RocksDB's code.
Adapting to and optimizing for different environments and platforms has always been a major asset of LevelDB/RocksDB: that is why the `rocksdb::Env` and `rocksdb::port` interfaces are so comprehensive. I hope use of stdlib thread interfaces will remain fully wrapped going forward.
Contributor guide
Research direction
Start by reviewing memtable/write_buffer_manager.cc and port/port_posix.h, then grep for std::lock_guard and other direct standard-thread usage. Check rocksdb::Env::StartThread and the util::SstFileManager entry point to understand the affected interfaces. Done means thread primitives consistently use rocksdb::port or rocksdb::Env without bypassing embedding callbacks.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- databases
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100