From 91ad4adb4682aa8bb502540f344e3dbcab870c27 Mon Sep 17 00:00:00 2001 From: NopeDl Date: Sat, 14 Oct 2023 00:41:45 +0800 Subject: [PATCH 1/3] [refactor]mds: print the error code Signed-off-by: NopeDl --- src/mds/common/mds_define.h | 21 ++++++++++++ src/mds/nameserver2/clean_core.cpp | 24 ++++++++------ src/mds/nameserver2/clean_manager.cpp | 6 ++-- src/mds/nameserver2/curvefs.cpp | 44 ++++++++++++++----------- src/mds/nameserver2/namespace_storage.h | 12 +++++++ 5 files changed, 76 insertions(+), 31 deletions(-) diff --git a/src/mds/common/mds_define.h b/src/mds/common/mds_define.h index 8dd0655476..c61cb95e4a 100644 --- a/src/mds/common/mds_define.h +++ b/src/mds/common/mds_define.h @@ -47,6 +47,27 @@ const int kCsClientReturnFail = -5; // error code: chunkserver offline const int kCsClientCSOffline = -6; +inline const char* PrintMdsDescByErrorCode(int code) { + switch (code) { + case kMdsSuccess: + return "MDS execution succeeded"; + case kMdsFail: + return "MDS execution failed"; + case kCsClientInternalError: + return "chunkserverclient internal error"; + case kCsClientNotLeader: + return "chunkserverclient request is not from the leader"; + case kRpcChannelInitFail: + return "brpc channel init fail"; + case kRpcFail: + return "RPC fail or Chunkserverclient request return fail"; + case kCsClientCSOffline: + return "chunkserver offline"; + default: + return "undefied code"; + } +} + // kStaledRequestTimeIntervalUs indicates the expiration time of the request // to prevent the request from being intercepted and played back const uint64_t kStaledRequestTimeIntervalUs = 15 * 1000 * 1000u; diff --git a/src/mds/nameserver2/clean_core.cpp b/src/mds/nameserver2/clean_core.cpp index 54f743c300..eb0169cf3c 100644 --- a/src/mds/nameserver2/clean_core.cpp +++ b/src/mds/nameserver2/clean_core.cpp @@ -65,11 +65,11 @@ StatusCode CleanCore::CleanSnapShotFile(const FileInfo & fileInfo, correctSn); if (ret != 0) { LOG(ERROR) << "CleanSnapShotFile Error: " - << "DeleteChunkSnapshotOrCorrectSn Error" - << ", ret = " << ret - << ", inodeid = " << fileInfo.id() - << ", filename = " << fileInfo.filename() - << ", correctSn = " << correctSn; + << "DeleteChunkSnapshotOrCorrectSn Error" + << ", ret = " << PrintMdsDescByErrorCode(ret) + << ", inodeid = " << fileInfo.id() + << ", filename = " << fileInfo.filename() + << ", correctSn = " << correctSn; progress->SetStatus(TaskStatus::FAILED); return StatusCode::kSnapshotFileDeleteError; } @@ -81,7 +81,8 @@ StatusCode CleanCore::CleanSnapShotFile(const FileInfo & fileInfo, StoreStatus ret = storage_->DeleteSnapshotFile(fileInfo.parentid(), fileInfo.filename()); if (ret != StoreStatus::OK) { - LOG(INFO) << "delete snapshotfile error, retCode = " << ret; + LOG(INFO) << "delete snapshotfile error, retCode = " + << PrintStoreStatusByErrorCode(ret); progress->SetStatus(TaskStatus::FAILED); return StatusCode::kSnapshotFileDeleteError; } else { @@ -123,7 +124,7 @@ StatusCode CleanCore::CleanFile(const FileInfo & commonFile, int ret = DeleteChunksInSegment(segment, commonFile.seqnum()); if (ret != 0) { LOG(ERROR) << "Clean common File Error: " - << ", ret = " << ret + << ", ret = " << PrintMdsDescByErrorCode(ret) << ", inodeid = " << commonFile.id() << ", filename = " << commonFile.filename() << ", sequenceNum = " << commonFile.seqnum(); @@ -153,7 +154,8 @@ StatusCode CleanCore::CleanFile(const FileInfo & commonFile, StoreStatus ret = storage_->DeleteFile(commonFile.parentid(), commonFile.filename()); if (ret != StoreStatus::OK) { - LOG(INFO) << "delete common file error, retCode = " << ret; + LOG(INFO) << "delete common file error, retDesc = " + << PrintStoreStatusByErrorCode(ret); progress->SetStatus(TaskStatus::FAILED); return StatusCode::kCommonFileDeleteError; } else { @@ -185,7 +187,8 @@ StatusCode CleanCore::CleanDiscardSegment( int ret = DeleteChunksInSegment(segment, seq); if (ret != 0) { LOG(ERROR) << "CleanDiscardSegment failed, DeleteChunk Error, ret = " - << ret << ", filename = " << fileInfo.filename() + << PrintMdsDescByErrorCode(ret) + << ", filename = " << fileInfo.filename() << ", inodeid = " << fileInfo.id() << ", segment offset = " << segment.startoffset(); progress->SetStatus(TaskStatus::FAILED); @@ -229,7 +232,8 @@ int CleanCore::DeleteChunksInSegment(const PageFileSegment& segment, seq); if (ret != 0) { - LOG(ERROR) << "DeleteChunk failed, ret = " << ret + LOG(ERROR) << "DeleteChunk failed, ret = " + << PrintMdsDescByErrorCode(ret) << ", logicalpoolid = " << logicalPoolId << ", copysetid = " << segment.chunks()[i].copysetid() << ", chunkid = " << segment.chunks()[i].chunkid() diff --git a/src/mds/nameserver2/clean_manager.cpp b/src/mds/nameserver2/clean_manager.cpp index f0916b8003..41bb349759 100644 --- a/src/mds/nameserver2/clean_manager.cpp +++ b/src/mds/nameserver2/clean_manager.cpp @@ -89,7 +89,8 @@ bool CleanManager::RecoverCleanTasks(void) { std::vector snapShotFiles; StoreStatus ret = storage_->LoadSnapShotFile(&snapShotFiles); if (ret != StoreStatus::OK) { - LOG(ERROR) << "Load SnapShotFile error, ret = " << ret; + LOG(ERROR) << "Load SnapShotFile error, ret = " + << PrintStoreStatusByErrorCode(ret); return false; } @@ -104,7 +105,8 @@ bool CleanManager::RecoverCleanTasks(void) { StoreStatus ret1 = storage_->ListFile(RECYCLEBININODEID, RECYCLEBININODEID + 1, &commonFiles); if (ret1 != StoreStatus::OK) { - LOG(ERROR) << "Load recylce bin file error, ret = " << ret1; + LOG(ERROR) << "Load recylce bin file error, ret = " + << PrintStoreStatusByErrorCode(ret1); return false; } diff --git a/src/mds/nameserver2/curvefs.cpp b/src/mds/nameserver2/curvefs.cpp index 5d5af4d75f..d931d431f2 100644 --- a/src/mds/nameserver2/curvefs.cpp +++ b/src/mds/nameserver2/curvefs.cpp @@ -115,7 +115,8 @@ bool CurveFS::InitRecycleBinDir() { return true; } else { // internal error - LOG(INFO) << "InitRecycleBinDir error ,ret = " << ret; + LOG(INFO) << "InitRecycleBinDir error ,ret = " + << PrintStoreStatusByErrorCode(ret); return false; } } @@ -206,7 +207,8 @@ StatusCode CurveFS::WalkPath(const std::string &fileName, } else if (ret == StoreStatus::KeyNotExist) { return StatusCode::kFileNotExists; } else { - LOG(ERROR) << "GetFile error, errcode = " << ret; + LOG(ERROR) << "GetFile error, errcode = " + << PrintStoreStatusByErrorCode(ret); return StatusCode::kStorageError; } // assert(fileInfo->parentid() != parentID); @@ -680,7 +682,7 @@ StatusCode CurveFS::DeleteFile(const std::string & filename, uint64_t fileId, if (ret != StoreStatus::OK) { LOG(ERROR) << "delete file, file is directory and delete fail" << ", filename = " << filename - << ", ret = " << ret; + << ", ret = " << PrintStoreStatusByErrorCode(ret); return StatusCode::kStorageError; } @@ -728,8 +730,8 @@ StatusCode CurveFS::DeleteFile(const std::string & filename, uint64_t fileId, storage_->MoveFileToRecycle(fileInfo, recycleFileInfo); if (ret1 != StoreStatus::OK) { LOG(ERROR) << "delete file, move file to recycle fail" - << ", filename = " << filename - << ", ret = " << ret1; + << ", filename = " << filename + << ", ret = " << PrintStoreStatusByErrorCode(ret1); return StatusCode::kStorageError; } LOG(INFO) << "file delete to recyclebin, fileName = " << filename @@ -877,7 +879,8 @@ StatusCode CurveFS::RecoverFile(const std::string & originFileName, auto ret1 = storage_->RenameFile(recycleFileInfo, recoverFileInfo); if ( ret1 != StoreStatus::OK ) { - LOG(ERROR) << "storage_ recoverfile error, error = " << ret1; + LOG(ERROR) << "storage_ recoverfile error, error = " + << PrintStoreStatusByErrorCode(ret1); return StatusCode::kStorageError; } return StatusCode::kOK; @@ -1122,9 +1125,9 @@ StatusCode CurveFS::RenameFile(const std::string & sourceFileName, recycleFileInfo); if (ret1 != StoreStatus::OK) { LOG(ERROR) << "storage_ ReplaceFileAndRecycleOldFile error" - << ", sourceFileName = " << sourceFileName - << ", destFileName = " << destFileName - << ", ret = " << ret1; + << ", sourceFileName = " << sourceFileName + << ", destFileName = " << destFileName + << ", ret = " << PrintStoreStatusByErrorCode(ret1); return StatusCode::kStorageError; } @@ -1138,7 +1141,8 @@ StatusCode CurveFS::RenameFile(const std::string & sourceFileName, auto ret = storage_->RenameFile(sourceFileInfo, destFileInfo); if ( ret != StoreStatus::OK ) { - LOG(ERROR) << "storage_ renamefile error, error = " << ret; + LOG(ERROR) << "storage_ renamefile error, error = " + << PrintStoreStatusByErrorCode(ret); return StatusCode::kStorageError; } return StatusCode::kOK; @@ -1385,8 +1389,8 @@ StatusCode CurveFS::DeAllocateSegment(const std::string& fileName, storeRet = storage_->DiscardSegment(fileInfo, segment); if (storeRet != StoreStatus::OK) { LOG(WARNING) << "Storage CleanSegment return error, filename = " - << fileName << ", offset = " << offset - << ", error = " << storeRet; + << fileName << ", offset = " << offset + << ", error = " << PrintStoreStatusByErrorCode(storeRet); return StatusCode::kStorageError; } @@ -1508,7 +1512,8 @@ StatusCode CurveFS::ListSnapShotFile(const std::string & fileName, storeStatus == StoreStatus::OK) { return StatusCode::kOK; } else { - LOG(ERROR) << fileName << ", storage ListFile return = " << storeStatus; + LOG(ERROR) << fileName << ", storage ListFile return = " + << PrintStoreStatusByErrorCode(storeStatus); return StatusCode::kStorageError; } } @@ -1702,10 +1707,10 @@ StatusCode CurveFS::GetSnapShotFileSegment( << ", offset = " << offset; return StatusCode::kSegmentNotAllocated; } else { - LOG(ERROR) << "get segment fail, KInternalError, ret = " << storeRet - << ", fileInfo.id() = " - << fileInfo.id() - << ", offset = " << offset; + LOG(ERROR) << "get segment fail, KInternalError, ret = " + << PrintStoreStatusByErrorCode(storeRet) + << ", fileInfo.id() = " << fileInfo.id() + << ", offset = " << offset; return StatusCode::KInternalError; } } @@ -2038,7 +2043,8 @@ StatusCode CurveFS::CheckPathOwnerInternal(const std::string &filename, LOG(WARNING) << paths[i] << " not exist"; return StatusCode::kFileNotExists; } else { - LOG(ERROR) << "GetFile " << paths[i] << " error, errcode = " << ret; + LOG(ERROR) << "GetFile " << paths[i] << " error, errcode = " + << PrintStoreStatusByErrorCode(ret); return StatusCode::kStorageError; } tempParentID = fileInfo.id(); @@ -2393,7 +2399,7 @@ StatusCode CurveFS::ListCloneSourceFileSegments( "filename = " << fileInfo->filename() << ", source file name = " << fileInfo->clonesource() - << ", ret = " << status; + << ", ret = " << PrintStoreStatusByErrorCode(status); return StatusCode::kStorageError; } diff --git a/src/mds/nameserver2/namespace_storage.h b/src/mds/nameserver2/namespace_storage.h index df4e1fe0dd..900a8b451d 100644 --- a/src/mds/nameserver2/namespace_storage.h +++ b/src/mds/nameserver2/namespace_storage.h @@ -51,6 +51,18 @@ enum class StoreStatus { }; std::ostream& operator << (std::ostream & os, StoreStatus &s); +inline const char* PrintStoreStatusByErrorCode(StoreStatus status) { + switch (status) { + case StoreStatus::OK: + return "OK"; + case StoreStatus::KeyNotExist: + return "KeyNotExist"; + case StoreStatus::InternalError: + return "InternalError"; + default: + return "unknown status"; + } +} // TODO(hzsunjianliang): may be storage need high level abstraction // put the encoding internal, not external From 3f71912626a5ead46ecded3b179c6c03dcc46b83 Mon Sep 17 00:00:00 2001 From: NopeDl Date: Wed, 25 Oct 2023 01:57:31 +0800 Subject: [PATCH 2/3] [refactor]mds: print the error code add topology support and remove storage status Signed-off-by: NopeDl --- src/mds/common/mds_define.h | 55 ++++++++++++++++++- .../heartbeat/chunkserver_healthy_checker.cpp | 5 +- src/mds/heartbeat/copyset_conf_generator.cpp | 4 +- src/mds/heartbeat/heartbeat_manager.cpp | 2 +- src/mds/heartbeat/topo_updater.cpp | 5 +- src/mds/nameserver2/clean_core.cpp | 6 +- src/mds/nameserver2/clean_manager.cpp | 6 +- src/mds/nameserver2/curvefs.cpp | 45 ++++++--------- src/mds/nameserver2/namespace_storage.h | 16 +----- 9 files changed, 88 insertions(+), 56 deletions(-) diff --git a/src/mds/common/mds_define.h b/src/mds/common/mds_define.h index c61cb95e4a..2e4479cffc 100644 --- a/src/mds/common/mds_define.h +++ b/src/mds/common/mds_define.h @@ -47,7 +47,7 @@ const int kCsClientReturnFail = -5; // error code: chunkserver offline const int kCsClientCSOffline = -6; -inline const char* PrintMdsDescByErrorCode(int code) { +const char* PrintMdsDescByErrorCode(int code) { switch (code) { case kMdsSuccess: return "MDS execution succeeded"; @@ -125,6 +125,59 @@ const int kTopoErrCodePoolsetNotFound = -20; const int kTopoErrCodeCannotDeleteDefaultPoolset = -21; const int kTopoErrCodeConflictBlockSizeAndChunkSize = -22; +const char* PrintTopoErrCodeDescription(int code) { + switch (code) { + case kTopoErrCodeSuccess: + return "kTopoErrCodeSuccess"; + case kTopoErrCodeInternalError: + return "kTopoErrCodeInternalError"; + case kTopoErrCodeInvalidParam: + return "kTopoErrCodeInvalidParam"; + case kTopoErrCodeInitFail: + return "kTopoErrCodeInitFail"; + case kTopoErrCodeStorgeFail: + return "kTopoErrCodeStorgeFail"; + case kTopoErrCodeIdDuplicated: + return "kTopoErrCodeIdDuplicated"; + case kTopoErrCodeChunkServerNotFound: + return "kTopoErrCodeChunkServerNotFound"; + case kTopoErrCodeServerNotFound: + return "kTopoErrCodeServerNotFound"; + case kTopoErrCodeZoneNotFound: + return "kTopoErrCodeZoneNotFound"; + case kTopoErrCodePhysicalPoolNotFound: + return "kTopoErrCodePhysicalPoolNotFound"; + case kTopoErrCodeLogicalPoolNotFound: + return "kTopoErrCodeLogicalPoolNotFound"; + case kTopoErrCodeCopySetNotFound: + return "kTopoErrCodeCopySetNotFound"; + case kTopoErrCodeGenCopysetErr: + return "kTopoErrCodeGenCopysetErr"; + case kTopoErrCodeAllocateIdFail: + return "kTopoErrCodeAllocateIdFail"; + case kTopoErrCodeCannotRemoveWhenNotEmpty: + return "kTopoErrCodeCannotRemoveWhenNotEmpty"; + case kTopoErrCodeIpPortDuplicated: + return "kTopoErrCodeIpPortDuplicated"; + case kTopoErrCodeNameDuplicated: + return "kTopoErrCodeNameDuplicated"; + case kTopoErrCodeCreateCopysetNodeOnChunkServerFail: + return "kTopoErrCodeCreateCopysetNodeOnChunkServerFail"; + case kTopoErrCodeCannotRemoveNotRetired: + return "kTopoErrCodeCannotRemoveNotRetired"; + case kTopoErrCodeLogicalPoolExist: + return "kTopoErrCodeLogicalPoolExist"; + case kTopoErrCodePoolsetNotFound: + return "kTopoErrCodePoolsetNotFound"; + case kTopoErrCodeCannotDeleteDefaultPoolset: + return "kTopoErrCodeCannotDeleteDefaultPoolset"; + case kTopoErrCodeConflictBlockSizeAndChunkSize: + return "kTopoErrCodeConflictBlockSizeAndChunkSize"; + default: + return "undefined status"; + } +} + } // namespace topology } // namespace mds } // namespace curve diff --git a/src/mds/heartbeat/chunkserver_healthy_checker.cpp b/src/mds/heartbeat/chunkserver_healthy_checker.cpp index ce4225bd1d..26e7d88fef 100644 --- a/src/mds/heartbeat/chunkserver_healthy_checker.cpp +++ b/src/mds/heartbeat/chunkserver_healthy_checker.cpp @@ -29,6 +29,7 @@ using ::curve::mds::topology::ChunkServerState; using ::curve::mds::topology::ChunkServerStatus; using ::curve::mds::topology::ChunkServer; using ::curve::mds::topology::kTopoErrCodeSuccess; +using ::curve::mds::topology::PrintTopoErrCodeDescription; using std::chrono::milliseconds; @@ -127,7 +128,7 @@ void ChunkserverHealthyChecker::UpdateChunkServerOnlineState( if (kTopoErrCodeSuccess != errCode) { LOG(WARNING) << "heartbeatManager update chunkserver get error code: " - << errCode; + << PrintTopoErrCodeDescription(errCode); } } @@ -168,7 +169,7 @@ bool ChunkserverHealthyChecker::TrySetChunkServerRetiredIfNeed( ChunkServerStatus::RETIRED, info.csId); if (kTopoErrCodeSuccess != updateErrCode) { LOG(WARNING) << "heartbeatManager update chunkserver get error code: " - << updateErrCode; + << PrintTopoErrCodeDescription(updateErrCode); return false; } diff --git a/src/mds/heartbeat/copyset_conf_generator.cpp b/src/mds/heartbeat/copyset_conf_generator.cpp index 9139275e4e..2f67e64d98 100644 --- a/src/mds/heartbeat/copyset_conf_generator.cpp +++ b/src/mds/heartbeat/copyset_conf_generator.cpp @@ -27,6 +27,7 @@ using std::chrono::milliseconds; using ::curve::mds::heartbeat::ConfigChangeInfo; +using ::curve::mds::topology::PrintTopoErrCodeDescription; namespace curve { namespace mds { @@ -65,7 +66,8 @@ bool CopysetConfGenerator::GenCopysetConf( LOG(WARNING) << "topoUpdater update copyset(" << reportCopySetInfo.GetLogicalPoolId() << "," << reportCopySetInfo.GetId() - << ") got error code: " << updateCode; + << ") got error code: " + << PrintTopoErrCodeDescription(updateCode); return false; } else { // update to memory successfully diff --git a/src/mds/heartbeat/heartbeat_manager.cpp b/src/mds/heartbeat/heartbeat_manager.cpp index b5100eaa73..bf31c972ba 100644 --- a/src/mds/heartbeat/heartbeat_manager.cpp +++ b/src/mds/heartbeat/heartbeat_manager.cpp @@ -115,7 +115,7 @@ void HeartbeatManager::UpdateChunkServerDiskStatus( request.chunkserverid()); if (ret != curve::mds::topology::kTopoErrCodeSuccess) { LOG(ERROR) << "heartbeat UpdateDiskStatus get an error, ret =" - << ret; + << curve::mds::topology::PrintTopoErrCodeDescription(ret); } } diff --git a/src/mds/heartbeat/topo_updater.cpp b/src/mds/heartbeat/topo_updater.cpp index 154bbcb67e..9db83ea4d1 100644 --- a/src/mds/heartbeat/topo_updater.cpp +++ b/src/mds/heartbeat/topo_updater.cpp @@ -23,6 +23,8 @@ #include #include "src/mds/heartbeat/topo_updater.h" +using curve::mds::topology::PrintTopoErrCodeDescription; + namespace curve { namespace mds { namespace heartbeat { @@ -178,7 +180,8 @@ void TopoUpdater::UpdateTopo(const CopySetInfo &reportCopySetInfo) { LOG(ERROR) << "topoUpdater update copyset(" << reportCopySetInfo.GetLogicalPoolId() << "," << reportCopySetInfo.GetId() - << ") got error code: " << updateCode; + << ") got error code: " + << PrintTopoErrCodeDescription(updateCode); return; } } diff --git a/src/mds/nameserver2/clean_core.cpp b/src/mds/nameserver2/clean_core.cpp index eb0169cf3c..7370e9ef26 100644 --- a/src/mds/nameserver2/clean_core.cpp +++ b/src/mds/nameserver2/clean_core.cpp @@ -81,8 +81,7 @@ StatusCode CleanCore::CleanSnapShotFile(const FileInfo & fileInfo, StoreStatus ret = storage_->DeleteSnapshotFile(fileInfo.parentid(), fileInfo.filename()); if (ret != StoreStatus::OK) { - LOG(INFO) << "delete snapshotfile error, retCode = " - << PrintStoreStatusByErrorCode(ret); + LOG(INFO) << "delete snapshotfile error, retCode = " << ret; progress->SetStatus(TaskStatus::FAILED); return StatusCode::kSnapshotFileDeleteError; } else { @@ -154,8 +153,7 @@ StatusCode CleanCore::CleanFile(const FileInfo & commonFile, StoreStatus ret = storage_->DeleteFile(commonFile.parentid(), commonFile.filename()); if (ret != StoreStatus::OK) { - LOG(INFO) << "delete common file error, retDesc = " - << PrintStoreStatusByErrorCode(ret); + LOG(INFO) << "delete common file error, retDesc = " << ret; progress->SetStatus(TaskStatus::FAILED); return StatusCode::kCommonFileDeleteError; } else { diff --git a/src/mds/nameserver2/clean_manager.cpp b/src/mds/nameserver2/clean_manager.cpp index 41bb349759..f0916b8003 100644 --- a/src/mds/nameserver2/clean_manager.cpp +++ b/src/mds/nameserver2/clean_manager.cpp @@ -89,8 +89,7 @@ bool CleanManager::RecoverCleanTasks(void) { std::vector snapShotFiles; StoreStatus ret = storage_->LoadSnapShotFile(&snapShotFiles); if (ret != StoreStatus::OK) { - LOG(ERROR) << "Load SnapShotFile error, ret = " - << PrintStoreStatusByErrorCode(ret); + LOG(ERROR) << "Load SnapShotFile error, ret = " << ret; return false; } @@ -105,8 +104,7 @@ bool CleanManager::RecoverCleanTasks(void) { StoreStatus ret1 = storage_->ListFile(RECYCLEBININODEID, RECYCLEBININODEID + 1, &commonFiles); if (ret1 != StoreStatus::OK) { - LOG(ERROR) << "Load recylce bin file error, ret = " - << PrintStoreStatusByErrorCode(ret1); + LOG(ERROR) << "Load recylce bin file error, ret = " << ret1; return false; } diff --git a/src/mds/nameserver2/curvefs.cpp b/src/mds/nameserver2/curvefs.cpp index d931d431f2..d4243841f0 100644 --- a/src/mds/nameserver2/curvefs.cpp +++ b/src/mds/nameserver2/curvefs.cpp @@ -115,8 +115,7 @@ bool CurveFS::InitRecycleBinDir() { return true; } else { // internal error - LOG(INFO) << "InitRecycleBinDir error ,ret = " - << PrintStoreStatusByErrorCode(ret); + LOG(INFO) << "InitRecycleBinDir error ,ret = " << ret; return false; } } @@ -207,8 +206,7 @@ StatusCode CurveFS::WalkPath(const std::string &fileName, } else if (ret == StoreStatus::KeyNotExist) { return StatusCode::kFileNotExists; } else { - LOG(ERROR) << "GetFile error, errcode = " - << PrintStoreStatusByErrorCode(ret); + LOG(ERROR) << "GetFile error, errcode = " << ret; return StatusCode::kStorageError; } // assert(fileInfo->parentid() != parentID); @@ -681,8 +679,7 @@ StatusCode CurveFS::DeleteFile(const std::string & filename, uint64_t fileId, fileInfo.filename()); if (ret != StoreStatus::OK) { LOG(ERROR) << "delete file, file is directory and delete fail" - << ", filename = " << filename - << ", ret = " << PrintStoreStatusByErrorCode(ret); + << ", filename = " << filename << ", ret = " << ret; return StatusCode::kStorageError; } @@ -730,8 +727,7 @@ StatusCode CurveFS::DeleteFile(const std::string & filename, uint64_t fileId, storage_->MoveFileToRecycle(fileInfo, recycleFileInfo); if (ret1 != StoreStatus::OK) { LOG(ERROR) << "delete file, move file to recycle fail" - << ", filename = " << filename - << ", ret = " << PrintStoreStatusByErrorCode(ret1); + << ", filename = " << filename << ", ret = " << ret1; return StatusCode::kStorageError; } LOG(INFO) << "file delete to recyclebin, fileName = " << filename @@ -879,8 +875,7 @@ StatusCode CurveFS::RecoverFile(const std::string & originFileName, auto ret1 = storage_->RenameFile(recycleFileInfo, recoverFileInfo); if ( ret1 != StoreStatus::OK ) { - LOG(ERROR) << "storage_ recoverfile error, error = " - << PrintStoreStatusByErrorCode(ret1); + LOG(ERROR) << "storage_ recoverfile error, error = " << ret; return StatusCode::kStorageError; } return StatusCode::kOK; @@ -1127,7 +1122,7 @@ StatusCode CurveFS::RenameFile(const std::string & sourceFileName, LOG(ERROR) << "storage_ ReplaceFileAndRecycleOldFile error" << ", sourceFileName = " << sourceFileName << ", destFileName = " << destFileName - << ", ret = " << PrintStoreStatusByErrorCode(ret1); + << ", ret = " << ret1; return StatusCode::kStorageError; } @@ -1141,8 +1136,7 @@ StatusCode CurveFS::RenameFile(const std::string & sourceFileName, auto ret = storage_->RenameFile(sourceFileInfo, destFileInfo); if ( ret != StoreStatus::OK ) { - LOG(ERROR) << "storage_ renamefile error, error = " - << PrintStoreStatusByErrorCode(ret); + LOG(ERROR) << "storage_ renamefile error, error = " << ret; return StatusCode::kStorageError; } return StatusCode::kOK; @@ -1390,7 +1384,7 @@ StatusCode CurveFS::DeAllocateSegment(const std::string& fileName, if (storeRet != StoreStatus::OK) { LOG(WARNING) << "Storage CleanSegment return error, filename = " << fileName << ", offset = " << offset - << ", error = " << PrintStoreStatusByErrorCode(storeRet); + << ", error = " << storeRet; return StatusCode::kStorageError; } @@ -1512,14 +1506,14 @@ StatusCode CurveFS::ListSnapShotFile(const std::string & fileName, storeStatus == StoreStatus::OK) { return StatusCode::kOK; } else { - LOG(ERROR) << fileName << ", storage ListFile return = " - << PrintStoreStatusByErrorCode(storeStatus); + LOG(ERROR) << fileName << ", storage ListFile return = " << storeStatus; return StatusCode::kStorageError; } } -StatusCode CurveFS::GetSnapShotFileInfo(const std::string &fileName, - FileSeqType seq, FileInfo *snapshotFileInfo) const { +StatusCode CurveFS::GetSnapShotFileInfo(const std::string& fileName, + FileSeqType seq, + FileInfo* snapshotFileInfo) const { std::vector snapShotFileInfos; StatusCode ret = ListSnapShotFile(fileName, &snapShotFileInfos); if (ret != StatusCode::kOK) { @@ -1707,18 +1701,16 @@ StatusCode CurveFS::GetSnapShotFileSegment( << ", offset = " << offset; return StatusCode::kSegmentNotAllocated; } else { - LOG(ERROR) << "get segment fail, KInternalError, ret = " - << PrintStoreStatusByErrorCode(storeRet) + LOG(ERROR) << "get segment fail, KInternalError, ret = " << storeRet << ", fileInfo.id() = " << fileInfo.id() << ", offset = " << offset; return StatusCode::KInternalError; } } -StatusCode CurveFS::OpenFile(const std::string &fileName, - const std::string &clientIP, - ProtoSession *protoSession, - FileInfo *fileInfo, +StatusCode CurveFS::OpenFile(const std::string& fileName, + const std::string& clientIP, + ProtoSession* protoSession, FileInfo* fileInfo, CloneSourceSegment* cloneSourceSegment) { // check the existence of the file StatusCode ret; @@ -2043,8 +2035,7 @@ StatusCode CurveFS::CheckPathOwnerInternal(const std::string &filename, LOG(WARNING) << paths[i] << " not exist"; return StatusCode::kFileNotExists; } else { - LOG(ERROR) << "GetFile " << paths[i] << " error, errcode = " - << PrintStoreStatusByErrorCode(ret); + LOG(ERROR) << "GetFile " << paths[i] << " error, errcode = " << ret; return StatusCode::kStorageError; } tempParentID = fileInfo.id(); @@ -2399,7 +2390,7 @@ StatusCode CurveFS::ListCloneSourceFileSegments( "filename = " << fileInfo->filename() << ", source file name = " << fileInfo->clonesource() - << ", ret = " << PrintStoreStatusByErrorCode(status); + << ", ret = " << status; return StatusCode::kStorageError; } diff --git a/src/mds/nameserver2/namespace_storage.h b/src/mds/nameserver2/namespace_storage.h index 900a8b451d..52a94d1f02 100644 --- a/src/mds/nameserver2/namespace_storage.h +++ b/src/mds/nameserver2/namespace_storage.h @@ -50,27 +50,13 @@ enum class StoreStatus { InternalError, }; std::ostream& operator << (std::ostream & os, StoreStatus &s); - -inline const char* PrintStoreStatusByErrorCode(StoreStatus status) { - switch (status) { - case StoreStatus::OK: - return "OK"; - case StoreStatus::KeyNotExist: - return "KeyNotExist"; - case StoreStatus::InternalError: - return "InternalError"; - default: - return "unknown status"; - } -} // TODO(hzsunjianliang): may be storage need high level abstraction // put the encoding internal, not external - // kv value storage for namespace and segment class NameServerStorage { public: - virtual ~NameServerStorage(void) {} + virtual ~NameServerStorage(void) {} /** * @brief PutFile Store fileInfo From 43481f7cfa1049b9cb290b1f3062484b7e1e5cb2 Mon Sep 17 00:00:00 2001 From: NopeDl Date: Wed, 8 Nov 2023 23:17:09 +0800 Subject: [PATCH 3/3] mds: code review Signed-off-by: NopeDl --- src/mds/common/mds_define.h | 4 ++-- src/mds/heartbeat/chunkserver_healthy_checker.cpp | 6 +++--- src/mds/heartbeat/copyset_conf_generator.cpp | 4 ++-- src/mds/heartbeat/heartbeat_manager.cpp | 2 +- src/mds/heartbeat/topo_updater.cpp | 4 ++-- src/mds/nameserver2/clean_core.cpp | 8 ++++---- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/mds/common/mds_define.h b/src/mds/common/mds_define.h index 2e4479cffc..7c000a5276 100644 --- a/src/mds/common/mds_define.h +++ b/src/mds/common/mds_define.h @@ -47,7 +47,7 @@ const int kCsClientReturnFail = -5; // error code: chunkserver offline const int kCsClientCSOffline = -6; -const char* PrintMdsDescByErrorCode(int code) { +inline char* MdsErrCodeToName(int code) { switch (code) { case kMdsSuccess: return "MDS execution succeeded"; @@ -125,7 +125,7 @@ const int kTopoErrCodePoolsetNotFound = -20; const int kTopoErrCodeCannotDeleteDefaultPoolset = -21; const int kTopoErrCodeConflictBlockSizeAndChunkSize = -22; -const char* PrintTopoErrCodeDescription(int code) { +inline const char* TopoErrCodeToName(int code) { switch (code) { case kTopoErrCodeSuccess: return "kTopoErrCodeSuccess"; diff --git a/src/mds/heartbeat/chunkserver_healthy_checker.cpp b/src/mds/heartbeat/chunkserver_healthy_checker.cpp index 26e7d88fef..aea9bf02c8 100644 --- a/src/mds/heartbeat/chunkserver_healthy_checker.cpp +++ b/src/mds/heartbeat/chunkserver_healthy_checker.cpp @@ -29,7 +29,7 @@ using ::curve::mds::topology::ChunkServerState; using ::curve::mds::topology::ChunkServerStatus; using ::curve::mds::topology::ChunkServer; using ::curve::mds::topology::kTopoErrCodeSuccess; -using ::curve::mds::topology::PrintTopoErrCodeDescription; +using ::curve::mds::topology::TopoErrCodeToName; using std::chrono::milliseconds; @@ -128,7 +128,7 @@ void ChunkserverHealthyChecker::UpdateChunkServerOnlineState( if (kTopoErrCodeSuccess != errCode) { LOG(WARNING) << "heartbeatManager update chunkserver get error code: " - << PrintTopoErrCodeDescription(errCode); + << TopoErrCodeToName(errCode); } } @@ -169,7 +169,7 @@ bool ChunkserverHealthyChecker::TrySetChunkServerRetiredIfNeed( ChunkServerStatus::RETIRED, info.csId); if (kTopoErrCodeSuccess != updateErrCode) { LOG(WARNING) << "heartbeatManager update chunkserver get error code: " - << PrintTopoErrCodeDescription(updateErrCode); + << TopoErrCodeToName(updateErrCode); return false; } diff --git a/src/mds/heartbeat/copyset_conf_generator.cpp b/src/mds/heartbeat/copyset_conf_generator.cpp index 2f67e64d98..36431a062a 100644 --- a/src/mds/heartbeat/copyset_conf_generator.cpp +++ b/src/mds/heartbeat/copyset_conf_generator.cpp @@ -27,7 +27,7 @@ using std::chrono::milliseconds; using ::curve::mds::heartbeat::ConfigChangeInfo; -using ::curve::mds::topology::PrintTopoErrCodeDescription; +using ::curve::mds::topology::TopoErrCodeToName; namespace curve { namespace mds { @@ -67,7 +67,7 @@ bool CopysetConfGenerator::GenCopysetConf( << reportCopySetInfo.GetLogicalPoolId() << "," << reportCopySetInfo.GetId() << ") got error code: " - << PrintTopoErrCodeDescription(updateCode); + << TopoErrCodeToName(updateCode); return false; } else { // update to memory successfully diff --git a/src/mds/heartbeat/heartbeat_manager.cpp b/src/mds/heartbeat/heartbeat_manager.cpp index 024d136911..bac8fea22d 100644 --- a/src/mds/heartbeat/heartbeat_manager.cpp +++ b/src/mds/heartbeat/heartbeat_manager.cpp @@ -115,7 +115,7 @@ void HeartbeatManager::UpdateChunkServerDiskStatus( request.chunkserverid()); if (ret != curve::mds::topology::kTopoErrCodeSuccess) { LOG(ERROR) << "heartbeat UpdateDiskStatus get an error, ret =" - << curve::mds::topology::PrintTopoErrCodeDescription(ret); + << curve::mds::topology::TopoErrCodeToName(ret); } } diff --git a/src/mds/heartbeat/topo_updater.cpp b/src/mds/heartbeat/topo_updater.cpp index 9db83ea4d1..eee42fd871 100644 --- a/src/mds/heartbeat/topo_updater.cpp +++ b/src/mds/heartbeat/topo_updater.cpp @@ -23,7 +23,7 @@ #include #include "src/mds/heartbeat/topo_updater.h" -using curve::mds::topology::PrintTopoErrCodeDescription; +using curve::mds::topology::TopoErrCodeToName; namespace curve { namespace mds { @@ -181,7 +181,7 @@ void TopoUpdater::UpdateTopo(const CopySetInfo &reportCopySetInfo) { << reportCopySetInfo.GetLogicalPoolId() << "," << reportCopySetInfo.GetId() << ") got error code: " - << PrintTopoErrCodeDescription(updateCode); + << TopoErrCodeToName(updateCode); return; } } diff --git a/src/mds/nameserver2/clean_core.cpp b/src/mds/nameserver2/clean_core.cpp index 7370e9ef26..20a4ed1ea1 100644 --- a/src/mds/nameserver2/clean_core.cpp +++ b/src/mds/nameserver2/clean_core.cpp @@ -66,7 +66,7 @@ StatusCode CleanCore::CleanSnapShotFile(const FileInfo & fileInfo, if (ret != 0) { LOG(ERROR) << "CleanSnapShotFile Error: " << "DeleteChunkSnapshotOrCorrectSn Error" - << ", ret = " << PrintMdsDescByErrorCode(ret) + << ", ret = " << MdsErrCodeToName(ret) << ", inodeid = " << fileInfo.id() << ", filename = " << fileInfo.filename() << ", correctSn = " << correctSn; @@ -123,7 +123,7 @@ StatusCode CleanCore::CleanFile(const FileInfo & commonFile, int ret = DeleteChunksInSegment(segment, commonFile.seqnum()); if (ret != 0) { LOG(ERROR) << "Clean common File Error: " - << ", ret = " << PrintMdsDescByErrorCode(ret) + << ", ret = " << MdsErrCodeToName(ret) << ", inodeid = " << commonFile.id() << ", filename = " << commonFile.filename() << ", sequenceNum = " << commonFile.seqnum(); @@ -185,7 +185,7 @@ StatusCode CleanCore::CleanDiscardSegment( int ret = DeleteChunksInSegment(segment, seq); if (ret != 0) { LOG(ERROR) << "CleanDiscardSegment failed, DeleteChunk Error, ret = " - << PrintMdsDescByErrorCode(ret) + << MdsErrCodeToName(ret) << ", filename = " << fileInfo.filename() << ", inodeid = " << fileInfo.id() << ", segment offset = " << segment.startoffset(); @@ -231,7 +231,7 @@ int CleanCore::DeleteChunksInSegment(const PageFileSegment& segment, if (ret != 0) { LOG(ERROR) << "DeleteChunk failed, ret = " - << PrintMdsDescByErrorCode(ret) + << MdsErrCodeToName(ret) << ", logicalpoolid = " << logicalPoolId << ", copysetid = " << segment.chunks()[i].copysetid() << ", chunkid = " << segment.chunks()[i].chunkid()