Kvrocks logs extraneous shutdowns in RocksDB #3017
Description
Search before asking
- I had searched in the issues and found no similar issues.
Version
OS Version: Debian 11
Kvrocks version: unstable (065d4cf)
Minimal reproduce step
Start client, send sigterm, read [path]/kvrocks/db/LOG
What did you expect to see?
2025/06/05-14:14:15.002071 42139 [db/db_impl/db_impl.cc:463] Shutdown: canceling all background work
What did you see instead?
2025/06/05-14:48:25.726530 66707 [db/db_impl/db_impl.cc:463] Shutdown: canceling all background work
2025/06/05-14:48:25.819216 66707 [db/db_impl/db_impl.cc:463] Shutdown: canceling all background work
2025/06/05-14:48:25.819338 66707 [db/db_impl/db_impl.cc:463] Shutdown: canceling all background work
Anything Else?
I traced the code for a bit. "Shutdown: canceling all background work" is logged in rocksdb::CancelAllBackgroundWork, which is called in Server::Stop, Storage::CloseDB, and rocksdb::DbImpl::~DbImpl. When a termination signal happens, it calls Server::Stop, which leads to main returning and calling Storage::~Storage. This calls Storage::CloseDB and then call rocksdb::DbImpl::~DbImpl.
From a control flow perspective, rocksdb::DbImpl::~DbImpl is always called whenever the other two functions are called, so cancelling background tasks in the other two functions are redundant.
The only significant thing done from between Server::Stop and rocksdb::DbImpl::~DbImpl is freeing all the handles and cancelling the task runner. Neither seems to depend on having no background work. Server::PrepareRestoreDB cancels tasks and this example from RocksDB destroys column families - both doing so without depending on having no background tasks.
I looked at the PRs (here and here) that added the calls and neither seems to have an explanation on why this is added, so I am not sure if there is any other reason that these calls exist.
Are you willing to submit a PR?
- I'm willing to submit a PR!