Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change client log level #94

Merged
merged 1 commit into from
Sep 22, 2020
Merged

Conversation

wu-hanqing
Copy link
Contributor

@wu-hanqing wu-hanqing commented Sep 17, 2020

What problem does this PR solve?

Problem Summary: Change some client log level

Check List

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

@wu-hanqing wu-hanqing force-pushed the change-log-level branch 3 times, most recently from 2dd2436 to 44f6f62 Compare September 17, 2020 06:49
@wu-hanqing wu-hanqing closed this Sep 17, 2020
@wu-hanqing wu-hanqing reopened this Sep 17, 2020
@wu-hanqing wu-hanqing closed this Sep 18, 2020
@wu-hanqing wu-hanqing reopened this Sep 18, 2020
@wu-hanqing wu-hanqing closed this Sep 18, 2020
@wu-hanqing wu-hanqing reopened this Sep 18, 2020
@wu-hanqing wu-hanqing closed this Sep 18, 2020
@wu-hanqing wu-hanqing reopened this Sep 18, 2020
@wu-hanqing wu-hanqing closed this Sep 18, 2020
@wu-hanqing wu-hanqing reopened this Sep 18, 2020
src/client/lease_executor.cpp Show resolved Hide resolved
src/client/lease_executor.cpp Show resolved Hide resolved
src/client/lease_executor.cpp Show resolved Hide resolved
src/client/libcurve_file.cpp Show resolved Hide resolved
@@ -309,7 +309,7 @@ void ClientClosure::OnRpcFailed() {
MetricHelper::IncremTimeOutRPCCount(fileMetric_, reqCtx_->optype_);
}

LOG_EVERY_N(ERROR, 10) << OpTypeToString(reqCtx_->optype_)
LOG_EVERY_SECOND(WARNING) << OpTypeToString(reqCtx_->optype_)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里改成warning是为啥?是否client出现超时也不报警了? 还有如果是超时以外的错误是否需要ERROR报警?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里只是一个rpc异常,比如超时,socket断开。但是后续会对请求进行重试,所以没必要ERROR

@@ -392,6 +404,8 @@ int FileClient::StatFile(const std::string& filename,
int ret;
if (mdsClient_ != nullptr) {
ret = mdsClient_->GetFileInfo(filename, userinfo, &fi);
LOG_IF(ERROR, ret != LIBCURVE_ERROR::OK)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetFileInfo 失败这种情况是否包含 文件不存在的情况, 如果文件不存在那么不需要报警吧,查询一个不存在的文件应该是这个正常场景

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里提供的接口都是给上层的,比如qemu/nbd,挂在卷后,通过这个接口获取卷大小。所以这里正常不会出现文件不存在的情况。

@@ -420,6 +434,8 @@ int FileClient::Listdir(const std::string& dirpath,
LIBCURVE_ERROR ret;
if (mdsClient_ != nullptr) {
ret = mdsClient_->Listdir(dirpath, userinfo, filestatVec);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

同上,Listdir失败是否包含不存在的情况,不存在的情况下由于ERROR日志触发报警不合理

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -442,6 +460,8 @@ int FileClient::Rmdir(const std::string& dirpath, const UserInfo_t& userinfo) {
LIBCURVE_ERROR ret;
if (mdsClient_ != nullptr) {
ret = mdsClient_->DeleteFile(dirpath, userinfo);
LOG_IF(ERROR, ret != LIBCURVE_ERROR::OK)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete是否幂等,返回notexist之类的是否需要打error

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里就涉及到幂等的问题了。即使这里不打印error,还是会返回给上层用户文件/目录已经被删除了。

@@ -178,7 +178,7 @@ bool Splitor::AssignInternal(IOTracker* iotracker,
(off_t)chunkidx * fileinfo->chunksize,
fileinfo,
&segInfo);
if (re == LIBCURVE_ERROR::FAILED || re == LIBCURVE_ERROR::AUTHFAIL) {
if (re != LIBCURVE_ERROR::OK) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果是not exist 是否不需要报error而触发报警

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里的标志位true,所以不会返回not exist

@@ -1163,7 +1163,7 @@ LIBCURVE_ERROR MDSClient::ListChunkServerInServer(
}

int statusCode = response.statuscode();
LOG_IF(ERROR, statusCode != 0)
LOG_IF(WARNING, statusCode != 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个文件统统改成warning是因为啥?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mds client一侧只是与mds交互的逻辑,所以这里不需要对异常状态进行报错。
具体的报错应该由调用方根据返回的状态进行处理。

@ilixiaocui ilixiaocui merged commit 8a1c225 into opencurve:master Sep 22, 2020
@wu-hanqing wu-hanqing deleted the change-log-level branch September 22, 2020 06:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants