From 4f12095a926cad0d23de18e580031e3c30a86bed Mon Sep 17 00:00:00 2001 From: Vee Zhang Date: Mon, 26 Dec 2022 18:40:40 +0800 Subject: [PATCH] revert src/kvstore --- src/kvstore/DiskManager.cpp | 6 ++--- src/kvstore/NebulaStore.cpp | 4 +-- src/kvstore/RocksEngine.cpp | 2 -- src/kvstore/RocksEngineConfig.cpp | 3 +-- src/kvstore/listener/Listener.cpp | 5 +--- src/kvstore/listener/Listener.h | 10 +++---- .../listener/elasticsearch/ESListener.cpp | 27 +++++++------------ .../listener/elasticsearch/ESListener.h | 2 +- .../listener/test/NebulaListenerTest.cpp | 4 +-- src/kvstore/plugins/hbase/HBaseStore.cpp | 9 +++---- src/kvstore/plugins/hbase/HBaseStore.h | 4 +-- src/kvstore/raftex/RaftPart.cpp | 7 +++-- src/kvstore/wal/FileBasedWal.cpp | 5 ++-- 13 files changed, 33 insertions(+), 55 deletions(-) diff --git a/src/kvstore/DiskManager.cpp b/src/kvstore/DiskManager.cpp index efd7cc694a1..30e58c5bca6 100644 --- a/src/kvstore/DiskManager.cpp +++ b/src/kvstore/DiskManager.cpp @@ -20,8 +20,6 @@ DiskManager::DiskManager(const std::vector& dataPaths, Paths* paths = new Paths(); paths_.store(paths); size_t index = 0; - - // TODO(vee): Add initialize function to avoid using LOG(FATAL) in constructor. for (const auto& path : dataPaths) { auto absolute = boost::filesystem::absolute(path); if (!boost::filesystem::exists(absolute)) { @@ -95,7 +93,7 @@ void DiskManager::addPartToPath(GraphSpaceID spaceId, PartitionID partId, const paths_.store(newPaths, std::memory_order_release); folly::rcu_retire(oldPaths, std::default_delete()); } catch (boost::filesystem::filesystem_error& e) { - LOG(DFATAL) << "Invalid path: " << e.what(); + LOG(FATAL) << "Invalid path: " << e.what(); } } @@ -116,7 +114,7 @@ void DiskManager::removePartFromPath(GraphSpaceID spaceId, paths_.store(newPaths, std::memory_order_release); folly::rcu_retire(oldPaths, std::default_delete()); } catch (boost::filesystem::filesystem_error& e) { - LOG(DFATAL) << "Invalid path: " << e.what(); + LOG(FATAL) << "Invalid path: " << e.what(); } } diff --git a/src/kvstore/NebulaStore.cpp b/src/kvstore/NebulaStore.cpp index 67859ca1997..6eaa674a518 100644 --- a/src/kvstore/NebulaStore.cpp +++ b/src/kvstore/NebulaStore.cpp @@ -343,7 +343,7 @@ std::unique_ptr NebulaStore::newEngine(GraphSpaceID spaceId, return std::make_unique( spaceId, vIdLen, dataPath, walPath, options_.mergeOp_, cfFactory); } else { - LOG(DFATAL) << "Unknown engine type " << FLAGS_engine_type; + LOG(FATAL) << "Unknown engine type " << FLAGS_engine_type; return nullptr; } } @@ -627,7 +627,7 @@ std::shared_ptr NebulaStore::newListener(GraphSpaceID spaceId, listener = std::make_shared( spaceId, partId, raftAddr_, walPath, ioPool_, bgWorkers_, workers_, options_.schemaMan_); } else { - LOG(DFATAL) << "Should not reach here"; + LOG(FATAL) << "Should not reach here"; return nullptr; } raftService_->addPartition(listener); diff --git a/src/kvstore/RocksEngine.cpp b/src/kvstore/RocksEngine.cpp index daf9ba1ff93..eadacf20092 100644 --- a/src/kvstore/RocksEngine.cpp +++ b/src/kvstore/RocksEngine.cpp @@ -47,8 +47,6 @@ RocksEngine::RocksEngine(GraphSpaceID spaceId, walPath_ = folly::stringPrintf("%s/nebula/%d", walPath.c_str(), spaceId); } auto path = folly::stringPrintf("%s/data", dataPath_.c_str()); - - // TODO(vee): Add initialize function to avoid using LOG(FATAL) in constructor. if (FileUtils::fileType(path.c_str()) == FileType::NOTEXIST) { if (readonly) { LOG(FATAL) << "Path " << path << " not exist"; diff --git a/src/kvstore/RocksEngineConfig.cpp b/src/kvstore/RocksEngineConfig.cpp index 4497b40ea55..50e3090b5cb 100644 --- a/src/kvstore/RocksEngineConfig.cpp +++ b/src/kvstore/RocksEngineConfig.cpp @@ -248,8 +248,7 @@ rocksdb::Status initRocksdbOptions(rocksdb::Options& baseOpts, auto walDir = folly::stringPrintf("%s/rocksdb_wal/%d", FLAGS_rocksdb_wal_dir.c_str(), spaceId); if (fs::FileUtils::fileType(walDir.c_str()) == fs::FileType::NOTEXIST) { if (!fs::FileUtils::makeDir(walDir)) { - LOG(DFATAL) << "makeDir " << walDir << " failed"; - return rocksdb::Status::InvalidArgument(); + LOG(FATAL) << "makeDir " << walDir << " failed"; } } LOG(INFO) << "set rocksdb wal of space " << spaceId << " to " << walDir; diff --git a/src/kvstore/listener/Listener.cpp b/src/kvstore/listener/Listener.cpp index cea5602b582..f50d5ee11c8 100644 --- a/src/kvstore/listener/Listener.cpp +++ b/src/kvstore/listener/Listener.cpp @@ -39,10 +39,7 @@ Listener::Listener(GraphSpaceID spaceId, void Listener::start(std::vector&& peers, bool) { std::lock_guard g(raftLock_); - if (!init()) { - // TODO(vee): return bool to avoid using LOG(FATAL) - LOG(FATAL) << "Listener init failed"; - } + init(); lastLogId_ = wal_->lastLogId(); lastLogTerm_ = wal_->lastLogTerm(); diff --git a/src/kvstore/listener/Listener.h b/src/kvstore/listener/Listener.h index 4354d97c38f..fbb53a54425 100644 --- a/src/kvstore/listener/Listener.h +++ b/src/kvstore/listener/Listener.h @@ -75,7 +75,7 @@ using RaftClient = thrift::ThriftClientManagergetSpaceVidLen(spaceId_); if (!vRet.ok()) { - LOG(DFATAL) << "vid length error"; - return false; + LOG(FATAL) << "vid length error"; } vIdLen_ = vRet.value(); auto vidTypeRet = schemaMan_->getSpaceVidType(spaceId_); @@ -29,8 +28,7 @@ bool ESListener::init() { auto cRet = schemaMan_->getServiceClients(meta::cpp2::ExternalServiceType::ELASTICSEARCH); if (!cRet.ok() || cRet.value().empty()) { - LOG(DFATAL) << "elasticsearch clients error"; - return false; + LOG(FATAL) << "elasticsearch clients error"; } std::vector esClients; for (const auto& c : cRet.value()) { @@ -46,11 +44,9 @@ bool ESListener::init() { esAdapter_.setClients(std::move(esClients)); auto sRet = schemaMan_->toGraphSpaceName(spaceId_); if (!sRet.ok()) { - LOG(DFATAL) << "space name error"; - return false; + LOG(FATAL) << "space name error"; } spaceName_ = std::make_unique(sRet.value()); - return true; } bool ESListener::apply(const BatchHolder& batch) { @@ -67,7 +63,7 @@ bool ESListener::apply(const BatchHolder& batch) { } else if (type == BatchLogType::OP_BATCH_REMOVE) { bulk.delete_(index, vid, src, dst, rank); } else { - LOG(DFATAL) << "Unexpect"; + LOG(FATAL) << "Unexpect"; } }; for (const auto& log : batch.getBatch()) { @@ -156,8 +152,7 @@ void ESListener::pickTagAndEdgeData(BatchLogType type, bool ESListener::persist(LogID lastId, TermID lastTerm, LogID lastApplyLogId) { if (!writeAppliedId(lastId, lastTerm, lastApplyLogId)) { - LOG(DFATAL) << "last apply ids write failed"; - return false; + LOG(FATAL) << "last apply ids write failed"; } return true; } @@ -169,9 +164,8 @@ std::pair ESListener::lastCommittedLogId() { } int32_t fd = open(lastApplyLogFile_->c_str(), O_RDONLY); if (fd < 0) { - LOG(DFATAL) << "Failed to open the file \"" << lastApplyLogFile_->c_str() << "\" (" << errno - << "): " << strerror(errno); - return {0, 0}; + LOG(FATAL) << "Failed to open the file \"" << lastApplyLogFile_->c_str() << "\" (" << errno + << "): " << strerror(errno); } // read last logId from listener wal file. LogID logId; @@ -193,9 +187,8 @@ LogID ESListener::lastApplyLogId() { } int32_t fd = open(lastApplyLogFile_->c_str(), O_RDONLY); if (fd < 0) { - LOG(DFATAL) << "Failed to open the file \"" << lastApplyLogFile_->c_str() << "\" (" << errno - << "): " << strerror(errno); - return 0; + LOG(FATAL) << "Failed to open the file \"" << lastApplyLogFile_->c_str() << "\" (" << errno + << "): " << strerror(errno); } // read last applied logId from listener wal file. LogID logId; diff --git a/src/kvstore/listener/elasticsearch/ESListener.h b/src/kvstore/listener/elasticsearch/ESListener.h index daa37b9a2ee..33073f4eb96 100644 --- a/src/kvstore/listener/elasticsearch/ESListener.h +++ b/src/kvstore/listener/elasticsearch/ESListener.h @@ -46,7 +46,7 @@ class ESListener : public Listener { /** * @brief Init work: get vid length, get es client */ - bool init() override; + void init() override; /** * @brief Send data by es client diff --git a/src/kvstore/listener/test/NebulaListenerTest.cpp b/src/kvstore/listener/test/NebulaListenerTest.cpp index 650b39aa9d1..4090f8051e6 100644 --- a/src/kvstore/listener/test/NebulaListenerTest.cpp +++ b/src/kvstore/listener/test/NebulaListenerTest.cpp @@ -211,9 +211,7 @@ class DummyListener : public Listener { } protected: - bool init() override { - return true; - } + void init() override {} bool apply(const BatchHolder& batch) { for (auto& log : batch.getBatch()) { diff --git a/src/kvstore/plugins/hbase/HBaseStore.cpp b/src/kvstore/plugins/hbase/HBaseStore.cpp index f329c535ef5..65f3e1a8254 100644 --- a/src/kvstore/plugins/hbase/HBaseStore.cpp +++ b/src/kvstore/plugins/hbase/HBaseStore.cpp @@ -212,8 +212,7 @@ ResultCode HBaseStore::rangeWithPrefix(GraphSpaceID spaceId, ResultCode HBaseStore::sync(GraphSpaceID spaceId, PartitionID partId) { UNUSED(spaceId); UNUSED(partId); - LOG(DFATAL) << "Unimplement"; - return ResultCode::ERR_UNSUPPORTED; + LOG(FATAL) << "Unimplement"; } ResultCode HBaseStore::multiRemove(GraphSpaceID spaceId, std::vector& keys) { @@ -404,13 +403,11 @@ void HBaseStore::asyncRemovePrefix(GraphSpaceID spaceId, } ResultCode HBaseStore::ingest(GraphSpaceID) { - LOG(DFATAL) << "Unimplement"; - return ResultCode::ERR_UNSUPPORTED; + LOG(FATAL) << "Unimplement"; } int32_t HBaseStore::allLeader(std::unordered_map>&) { - LOG(DFATAL) << "Unimplement"; - return 0; + LOG(FATAL) << "Unimplement"; } } // namespace kvstore diff --git a/src/kvstore/plugins/hbase/HBaseStore.h b/src/kvstore/plugins/hbase/HBaseStore.h index 4f165c896c8..7b085be7fb0 100644 --- a/src/kvstore/plugins/hbase/HBaseStore.h +++ b/src/kvstore/plugins/hbase/HBaseStore.h @@ -181,11 +181,11 @@ class HBaseStore : public KVStore { KVCallback cb); void asyncAtomicOp(GraphSpaceID, PartitionID, raftex::AtomicOp, KVCallback) override { - LOG(DFATAL) << "Not supported yet!"; + LOG(FATAL) << "Not supported yet!"; } void asyncAtomicOp(GraphSpaceID, PartitionID, std::string&& multiValues, KVCallback) override { - LOG(DFATAL) << "Not supported yet!"; + LOG(FATAL) << "Not supported yet!"; } ResultCode ingest(GraphSpaceID spaceId) override; diff --git a/src/kvstore/raftex/RaftPart.cpp b/src/kvstore/raftex/RaftPart.cpp index 49b4bb792ef..ff3909b3970 100644 --- a/src/kvstore/raftex/RaftPart.cpp +++ b/src/kvstore/raftex/RaftPart.cpp @@ -95,7 +95,7 @@ class AppendLogsIterator final : public LogIterator { } } if (notFulfilledPromise > 0) { - LOG(DFATAL) << "notFulfilledPromise == " << notFulfilledPromise; + LOG(FATAL) << "notFulfilledPromise == " << notFulfilledPromise; } } } @@ -394,7 +394,7 @@ const char* RaftPart::roleStr(Role role) const { case Role::LEARNER: return "Learner"; default: - LOG(DFATAL) << idStr_ << "Invalid role"; + LOG(FATAL) << idStr_ << "Invalid role"; } return nullptr; } @@ -1091,8 +1091,7 @@ void RaftPart::processAppendLogResponses(const AppendLogResponses& resps, [self = shared_from_this(), term = term_] { self->onLeaderReady(term); }); } } else { - LOG(DFATAL) << idStr_ << "Failed to commit logs"; - return; + LOG(FATAL) << idStr_ << "Failed to commit logs"; } VLOG(4) << idStr_ << "Leader succeeded in committing the logs " << committedId + 1 << " to " << lastLogId; diff --git a/src/kvstore/wal/FileBasedWal.cpp b/src/kvstore/wal/FileBasedWal.cpp index 14b36a0c9df..91435b4955c 100644 --- a/src/kvstore/wal/FileBasedWal.cpp +++ b/src/kvstore/wal/FileBasedWal.cpp @@ -479,9 +479,8 @@ bool FileBasedWal::appendLogInternal(LogID id, ssize_t bytesWritten = write(currFd_, strBuf.data(), strBuf.size()); if (bytesWritten != (ssize_t)strBuf.size()) { - LOG(DFATAL) << idStr_ << "bytesWritten:" << bytesWritten << ", expected:" << strBuf.size() - << ", error:" << strerror(errno); - return false; + LOG(FATAL) << idStr_ << "bytesWritten:" << bytesWritten << ", expected:" << strBuf.size() + << ", error:" << strerror(errno); } if (policy_.sync && ::fsync(currFd_) == -1) {