print error code with more meaningful name #2286

gueFDF · 2023-03-03T11:34:48Z

What problem does this PR solve?

Issue Number: #2185

Problem Summary:

What is changed and how it works?

What's Changed:
Added some log information, print error code with more meaningful name.

How it Works:
Call the LibCurveErrorName(LIBCURVE_ERROR ret) function to return the specific error information corresponding to the error number.

Side effects(Breaking backward compatibility? Performance regression?):
Basically no side effects.

Check List

Relevant documentation/comments is changed or added
I acknowledge that all my contributions will be made under the project's license

cr-gpt · 2023-03-03T11:34:58Z

include/client/libcurve_define.h

    LibCurveAioCallBack cb;
-    void*               buf;
+    void *buf;
 } CurveAioContext;

 #endif  // INCLUDE_CLIENT_LIBCURVE_DEFINE_H_


a brief code review.

The enum LIBCURVE_ERROR and LIBCURVE_OP have been defined to provide clarity and consistency when writing code.

The function signature for LibCurveAioCallBack has been changed to better reflect its purpose.

off_t, size_t, and int have been used consistently in CurveAioContext, ensuring that all types are properly aligned.

A comment has been added to the Client_NOT_SUPPORT_SNAPSHOT line to explain why it is not supported.

The #endif comment has been added to the end of the file.

cr-gpt · 2023-03-03T11:35:02Z

src/client/chunkserver_broadcaster.cpp

-}   // namespace client
-}   // namespace curve
+}  // namespace client
+}  // namespace curve


code review

The logic of the code is clear and easy to understand.

About the coding style, it meets the standard.

There is no bug risk.

For improvement suggestion, it would be better to add some comments to make the code more readable.

cr-gpt · 2023-03-03T11:35:08Z

src/client/discard_task.cpp

            currentTasks.emplace(kv.first);
        }
    }

-    for (const auto& timerId : currentTasks) {
+    for (const auto &timerId : currentTasks) {
        int ret = bthread_timer_del(timerId);

        if (ret == 0) {


with the code review.

Proper indentation: The code is formatted properly, which helps make it easier to read.

Variable naming: Variable names are descriptive and easy to understand.

Type safety: The code uses the correct types for variables, such as uint64_t for offset and FileSegment* for fileSegment.

Error handling: The code checks for errors and logs them if they occur.

Overall, the code looks well written and follows best practices.

cr-gpt · 2023-03-03T11:35:17Z

src/client/lease_executor.cpp

-}   // namespace client
-}   // namespace curve
+}  // namespace client
+}  // namespace curve


with the code review:

The code is generally well written and organized, but there are some small changes that could be made for better readability and maintainability. For example, the line containing "// namespace client" and "// namespace curve" should be indented to match the indentation of the corresponding code blocks.

The variable names used in the code seem to be descriptive, which is a good practice.

In LeaseExecutor::Start(), the parameter fi is not used in the method. This should either be removed or used in the code.

In LeaseExecutor::RefreshLease(), the “if” statement should include an “else” clause to handle the case when the refresh was successful.

In LeaseExecutor::CheckNeedUpdateFileInfo(), the variable newSn is not used in the method. This should either be removed or used in the code.

Overall, the code looks good and there are no major risk areas.

cr-gpt · 2023-03-03T11:35:27Z

src/client/libcbd_libcurve.cpp

@@ -115,4 +115,5 @@ int cbd_libcurve_increase_epoch(const char* filename) {
    return IncreaseEpoch(filename);
 }

+
 }  // extern "C"


the code review

This code appears to be a C++ function named cbd_libcurve_increase_epoch() that takes a single argument, a const char* filename. The intention of the function is to increase the epoch value of the file indicated by the filename argument. The function ends with an extern "C" block, which indicates that this function is meant to be used from a C program.

The code looks correct, however there are a few potential issues that should be considered:

The function does not check the filename argument for validity - it is possible to pass a null pointer or invalid filename to the function.

The function does not check the return value of IncreaseEpoch() to ensure that it was successful.

The code does not indicate what type of error handling will be done if IncreaseEpoch() fails.

These potential issues should be addressed to ensure that the function is robust and reliable.

cr-gpt · 2023-03-03T11:35:35Z

src/client/libcurve_snapshot.cpp


    if (ret != LIBCURVE_ERROR::OK) {
-        LOG(INFO) << "GetSnapshotSegmentInfo failed, ret = " << ret;
+        LOG(INFO) << "GetSnapshotSegmentInfo failed, ret = " << ret
+                  << LibCurveErrorName((LIBCURVE_ERROR)ret);
        return -ret;
    }



code review

In the GetSnapshotSegmentInfo function, it is suggested to add log information and print the name of the error code in addition to the return value.

In the CreateCloneFile function, it is recommended to add a log output and consider adding an exception handling.

In the GetFileInfo function, it is recommended to print the name of the error code in addition to the return value when calling the GetFileInfo function in the mdsclient_.

In the GetOrAllocateSegmentInfo function, it is recommended to add log information and print the name of the error code in addition to the return value.

cr-gpt · 2023-03-03T11:35:39Z

src/client/source_reader.cpp

@@ -175,7 +175,8 @@ int SourceReader::Read(const std::vector<RequestContext*>& reqCtxVec,
                                          UserDataType::IOBuffer);
        if (ret != LIBCURVE_ERROR::OK) {
            LOG(ERROR) << "Read curve failed failed, filename = " << fileName
-                       << ", error = " << ret;
+                       << ", error = " << ret
+                       << LibCurveErrorName((LIBCURVE_ERROR)ret);
            delete curveCombineCtx;
            return -1;
        } else {


the code review:

The code patch looks fine. It adds more information to an error log message in case of a failed read operation from libCurve. This should be helpful for debugging purposes. There are no evident bug risks and no further improvement suggestions.

cr-gpt · 2023-03-03T11:35:44Z

test/integration/snapshotcloneserver/snapshotcloneserver_recover_test.cpp

-                       << ", ret = " << ret;
+                       << ", ret = " << ret
+                       << LibCurveErrorName((LIBCURVE_ERROR)ret);
+
            return ret;
        }
        return LIBCURVE_ERROR::OK;


with a brief code review

The code patch looks good and appears to be well written. There doesn't seem to be any immediate bug risks, but it would be beneficial to add more logging so that errors can be more easily identified and tracked in the future. Additionally, it may be beneficial to add more comments in the code to make it easier to understand what is happening in each section of the code.

wu-hanqing · 2023-03-03T13:37:51Z

cicheck

cr-gpt bot reviewed Mar 3, 2023

View reviewed changes

gueFDF changed the title ~~Signed-off-by: gueFDF <[email protected]>~~ print error code with more meaningful name Mar 3, 2023

gueFDF mentioned this pull request Mar 3, 2023

print error code with more meaningful name #2185

Open

gueFDF closed this Mar 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

print error code with more meaningful name #2286

print error code with more meaningful name #2286

gueFDF commented Mar 3, 2023

cr-gpt bot Mar 3, 2023

cr-gpt bot Mar 3, 2023

cr-gpt bot Mar 3, 2023

cr-gpt bot Mar 3, 2023

cr-gpt bot Mar 3, 2023

cr-gpt bot Mar 3, 2023

cr-gpt bot Mar 3, 2023

cr-gpt bot Mar 3, 2023

wu-hanqing commented Mar 3, 2023

print error code with more meaningful name #2286

print error code with more meaningful name #2286

Conversation

gueFDF commented Mar 3, 2023

What problem does this PR solve?

What is changed and how it works?

Check List

cr-gpt bot Mar 3, 2023

Choose a reason for hiding this comment

cr-gpt bot Mar 3, 2023

Choose a reason for hiding this comment

cr-gpt bot Mar 3, 2023

Choose a reason for hiding this comment

cr-gpt bot Mar 3, 2023

Choose a reason for hiding this comment

cr-gpt bot Mar 3, 2023

Choose a reason for hiding this comment

cr-gpt bot Mar 3, 2023

Choose a reason for hiding this comment

cr-gpt bot Mar 3, 2023

Choose a reason for hiding this comment

cr-gpt bot Mar 3, 2023

Choose a reason for hiding this comment

wu-hanqing commented Mar 3, 2023