refactor: Replace std::optional with CraneExpect

Signed-off-by: Li Junlin <[email protected]>
PKUHPC · Nov 21, 2024 · c8aef71 · c8aef71
1 parent 57cead2
commit c8aef71
Show file tree

Hide file tree

Showing 6 changed files with 21 additions and 28 deletions.
diff --git a/src/Craned/CranedPublicDefs.h b/src/Craned/CranedPublicDefs.h
@@ -29,7 +29,7 @@ namespace Craned {
 
 inline const uint64_t kEvSigChldResendMs = 500'000;
 
-using EnvPair = std::pair<std::string, std::string>;
+using EnvMap = std::unordered_map<std::string, std::string>;
 
 struct TaskStatusChange {
   task_id_t task_id{};

diff --git a/src/Craned/CranedServer.cpp b/src/Craned/CranedServer.cpp
@@ -137,12 +137,11 @@ grpc::Status CranedServiceImpl::QueryTaskIdFromPort(
 
   // 3. pid2jobid
   do {
-    std::optional<uint32_t> task_id_opt =
-        g_task_mgr->QueryTaskIdFromPidAsync(pid_i);
-    if (task_id_opt.has_value()) {
-      CRANE_TRACE("Task id for pid {} is #{}", pid_i, task_id_opt.value());
+    auto task_id_expt = g_task_mgr->QueryTaskIdFromPidAsync(pid_i);
+    if (task_id_expt.has_value()) {
+      CRANE_TRACE("Task id for pid {} is #{}", pid_i, task_id_expt.value());
       response->set_ok(true);
-      response->set_task_id(task_id_opt.value());
+      response->set_task_id(task_id_expt.value());
       return Status::OK;
     } else {
       std::string proc_dir = fmt::format("/proc/{}/status", pid_i);

diff --git a/src/Craned/DeviceManager.cpp b/src/Craned/DeviceManager.cpp
@@ -54,15 +54,8 @@ BasicDevice::BasicDevice(const std::string& device_name,
 
 bool BasicDevice::Init() {
   for (auto& device_file_meta : device_file_metas) {
-    const auto& device_major_minor_optype_option =
-        DeviceManager::GetDeviceFileMajorMinorOpType(device_file_meta.path);
-    if (!device_major_minor_optype_option.has_value()) return false;
-    const auto& device_major_minor_optype =
-        device_major_minor_optype_option.value();
-
-    device_file_meta.major = std::get<0>(device_major_minor_optype);
-    device_file_meta.minor = std::get<1>(device_major_minor_optype);
-    device_file_meta.op_type = std::get<2>(device_major_minor_optype);
+    auto err = DeviceManager::GetDeviceFileMajorMinorOpType(&device_file_meta);
+    if (err != CraneErr::kOk) return false;
   }
   return true;
 }
@@ -76,20 +69,24 @@ BasicDevice::operator std::string() const {
                      util::HostNameListToStr(device_files));
 }
 
-std::optional<std::tuple<unsigned int, unsigned int, char>>
-DeviceManager::GetDeviceFileMajorMinorOpType(const std::string& path) {
+CraneErr DeviceManager::GetDeviceFileMajorMinorOpType(
+    DeviceFileMeta* device_file_meta) {
   struct stat device_file_info {};
-  if (stat(path.c_str(), &device_file_info) == 0) {
+  if (stat(device_file_meta->path.c_str(), &device_file_info) == 0) {
     char op_type = 'a';
     if (S_ISBLK(device_file_info.st_mode)) {
       op_type = 'b';
     } else if (S_ISCHR(device_file_info.st_mode)) {
       op_type = 'c';
     }
-    return std::make_tuple(major(device_file_info.st_rdev),
-                           minor(device_file_info.st_rdev), op_type);
+    device_file_meta->major = major(device_file_info.st_rdev);
+    device_file_meta->minor = minor(device_file_info.st_rdev);
+    device_file_meta->op_type = op_type;
+    return CraneErr::kOk;
   } else {
-    return std::nullopt;
+    CRANE_ERROR("Failed to stat device file {} err:{}", device_file_meta->path,
+                std::strerror(errno));
+    return CraneErr::kSystemErr;
   }
 }
 

diff --git a/src/Craned/DeviceManager.h b/src/Craned/DeviceManager.h
@@ -101,8 +101,8 @@ class DeviceManager {
       const std::vector<std::string>& device_path,
       DeviceEnvInjector env_injector);
 
-  static std::optional<std::tuple<unsigned int, unsigned int, char>>
-  GetDeviceFileMajorMinorOpType(const std::string& path);
+  static CraneErr GetDeviceFileMajorMinorOpType(
+      DeviceFileMeta* device_file_meta);
 
   static std::unordered_map<std::string, std::string> GetDevEnvListByResInNode(
       const crane::grpc::DedicatedResourceInNode& res_in_node);

diff --git a/src/Craned/TaskManager.cpp b/src/Craned/TaskManager.cpp
@@ -632,8 +632,7 @@ CraneErr TaskManager::SpawnProcessInInstance_(TaskInstance* instance,
   // the child proc will block forever.
   // That's why we should copy it here and the child proc should not hold any
   // lock.
-  std::optional<crane::grpc::ResourceInNode> res_in_node =
-      g_cg_mgr->GetTaskResourceInNode(instance->task.task_id());
+  auto res_in_node = g_cg_mgr->GetTaskResourceInNode(instance->task.task_id());
   if (!res_in_node.has_value()) {
     CRANE_ERROR("Failed to get resource info for task #{}",
                 instance->task.task_id());

diff --git a/src/Utilities/PublicHeader/include/crane/PublicHeader.h b/src/Utilities/PublicHeader/include/crane/PublicHeader.h
@@ -412,6 +412,4 @@ struct CgroupSpec {
   task_id_t task_id;
   crane::grpc::ResourceInNode res_in_node;
   std::string execution_node;
-};
-
-using EnvMap = std::unordered_map<std::string, std::string>;
+};