[GitHub] [incubator-kvrocks] mapleFU commented on pull request #768: Add `StatusOr` for error handling in modern C++ style

[GitBox <gi...@apache.org>]

mapleFU commented on PR #768:
URL: https://github.com/apache/incubator-kvrocks/pull/768#issuecomment-1212116068

   I wonder why kvrocks' `Status` uses `std::string`, it would be large, likely 24B on stack ( for example: https://github.com/llvm-mirror/libcxx/blob/master/include/string#L765-L784 )
   
   When I gothrough the code of project, I found that most cases would be like:
   
   ```
   Status s = Func(...);
   if (!s.ok()) {
      // print log for `s`
     return Status::Err(...);  // return a "bad" status with a new status
   }
   ...
   ```
   
   or:
   
   ```
   Status s = Func(...);
   if (!s.ok()) {
      return s;
   }
   ...
   ```
   
   Firstly, on bad path, `Status` would be like: `Status(Status::DBGetWALErr, "iterator not valid");`, sometimes it would be so large that it can't be optimized by sso, causing some temp-on-heap allocating.
   
   Secondly, sometimes when we want to find the root cause of the problem, status make it hard to debug. apache doris thinks using exception is a better method: https://github.com/apache/doris/discussions/8406
   
   


-- 
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: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org