github-actions[bot] commented on code in PR #63004:
URL: https://github.com/apache/doris/pull/63004#discussion_r3224341738
##########
be/src/load/stream_load/stream_load_recorder.cpp:
##########
@@ -39,15 +40,19 @@ StreamLoadRecorder::StreamLoadRecorder(std::string
root_path)
StreamLoadRecorder::~StreamLoadRecorder() {
if (_db != nullptr) {
- for (auto* handle : _handles) {
- _db->DestroyColumnFamilyHandle(handle);
- handle = nullptr;
- }
rocksdb::Status s = _db->SyncWAL();
if (!s.ok()) {
LOG(WARNING) << "rocksdb sync wal failed: " << s.ToString();
}
- // no need to Close(), will be called in destruction
+ rocksdb::CancelAllBackgroundWork(_db.get(), true);
+ for (auto* handle : _handles) {
+ s = _db->DestroyColumnFamilyHandle(handle);
+ if (!s.ok()) {
+ LOG(WARNING) << "rocksdb destroy column family handle failed:
" << s.ToString();
+ }
+ }
Review Comment:
This still relies on the `DBWithTTL` destructor to close RocksDB because
`_db.reset()` is called without an explicit `_db->Close()`. The PR description
says the crash happens in the implicit destructor path
(`DBImpl::CloseHelper()`), and the comparable `OlapMeta` cleanup calls
`db->Close()` before deleting the DB. Please destroy the column family handles,
call `_db->Close()` and check/log its status, then reset the pointer; otherwise
this change does not actually guarantee that shutdown avoids the implicit close
path it is intended to fix.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
