Ryan4253 opened a new issue, #3017: URL: https://github.com/apache/kvrocks/issues/3017
### Search before asking - [x] I had searched in the [issues](https://github.com/apache/kvrocks/issues) and found no similar issues. ### Version OS Version: Debian 11 Kvrocks version: unstable (https://github.com/apache/kvrocks/commit/065d4cfbfea7034d80df770c0c525e4e4abbb702) ### 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](https://github.com/facebook/rocksdb/blob/7d80ea45442e84c25669db61cb7376ba0cd10ba5/db/db_impl/db_impl.cc#L464), which is called in [Server::Stop](https://github.com/apache/kvrocks/blob/065d4cfbfea7034d80df770c0c525e4e4abbb702/src/server/server.cc#L259), [Storage::CloseDB](https://github.com/apache/kvrocks/blob/065d4cfbfea7034d80df770c0c525e4e4abbb702/src/storage/storage.cc#L105), and [rocksdb::DbImpl::~DbImpl](https://github.com/facebook/rocksdb/blob/7d80ea45442e84c25669db61cb7376ba0cd10ba5/db/db_impl/db_impl.cc#L526). When a termination signal happens, it calls [Server::Stop](https://github.com/apache/kvrocks/blob/065d4cfbfea7034d80df770c0c525e4e4abbb702/src/cli/main.cc#L54), which leads to main returning and calling [Storage::~Storage](https://github.com/apache/kvrocks/blob/065d4cfbfea7034d80df770c0c525e4e4abbb702/src/cli/main.cc#L199). This calls [Storage::Close DB](https://github.com/apache/kvrocks/blob/065d4cfbfea7034d80df770c0c525e4e4abbb702/src/storage/storage.cc#L89) and then call [rocksdb::DbImpl::~DbImpl](https://github.com/apache/kvrocks/blob/065d4cfbfea7034d80df770c0c525e4e4abbb702/src/storage/storage.h#L354). 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](https://github.com/apache/kvrocks/blob/065d4cfbfea7034d80df770c0c525e4e4abbb702/src/storage/storage.cc#L106C1-L107C1) and [cancelling the task runner](https://github.com/apache/kvrocks/blob/065d4cfbfea7034d80df770c0c525e4e4abbb702/src/server/server.cc#L260). Neither seems to depend on cancelling background work. Server::PrepareRestoreDB [cancels tasks](https://github.com/apache/kvrocks/blob/065d4cfbfea7034d80df770c0c525e4e4abbb702/src/server/server.cc#L1371) and this example from RocksDB [destroys column families](https://github.com/facebook/rocksdb/blob/7d80ea45442e84c25669db61cb7376ba0cd10ba5/examples/column_families_example.cc#L45) - both doing so without cancelling background tasks. I looked at the PRs ([here](https://github.com/apache/kvrocks/pull/278) and [here](https://github.com/apache/kvrocks/pull/112)) that added the calls and neither seems to have an explanation on why this is added. ### Are you willing to submit a PR? - [x] I'm willing to submit a PR! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
