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

msg/async: fix the bug of inaccurate calculation of l_msgr_send_bytes #16526

Merged
merged 2 commits into from Aug 1, 2017

Conversation

Projects
None yet
5 participants
@TsaiJin
Contributor

TsaiJin commented Jul 24, 2017

  • Fix the typo error of l_msgr_send_bytes.
  • Set the value of l_msgr_send_bytes to how much data has been sent successfully.
msg/async: fix the typo error of l_msgr_send_bytes.
Signed-off-by: Jin Cai <caijin.caij@alibaba-inc.com>
@TsaiJin

This comment has been minimized.

Contributor

TsaiJin commented Jul 24, 2017

retest this please

@TsaiJin

This comment has been minimized.

Contributor

TsaiJin commented Jul 24, 2017

@yuyuyu101 @joscollin please help take a look, thanks.

@TsaiJin TsaiJin changed the title from msg/async: set the value of l_msgr_send_bytes to how much data has been sent successfully to msg/async: fix the bug of inaccurate calculation of l_msgr_send_bytes Jul 24, 2017

@joscollin joscollin added the bug fix label Jul 24, 2017

ssize_t rc = _try_send(more);
if (rc < 0) {
ldout(async_msgr->cct, 1) << __func__ << " error sending " << m << ", "
<< cpp_strerror(rc) << dendl;
} else if (rc == 0) {
logger->inc(l_msgr_send_bytes, outcoming_bl.length() - original_bl_len);

This comment has been minimized.

@yuyuyu101

yuyuyu101 Jul 24, 2017

Member

total_send_size

This comment has been minimized.

@TsaiJin

TsaiJin Jul 25, 2017

Contributor

Good catch

msg/async/AsyncConnection: set the value of l_msgr_send_bytes to how …
…much data has been sent successfully

Signed-off-by: Jin Cai <caijin.caij@alibaba-inc.com>
@TsaiJin

This comment has been minimized.

Contributor

TsaiJin commented Jul 25, 2017

@yuyuyu101 I modify the code with your advice.

ldout(async_msgr->cct, 10) << __func__ << " sending " << m << " done." << dendl;
} else {
logger->inc(l_msgr_send_bytes, total_send_size - outcoming_bl.length());

This comment has been minimized.

@liupan1111

liupan1111 Jul 25, 2017

Contributor

why use "- outcoming_bl.length"? Is it equal to total_send_size?

This comment has been minimized.

@TsaiJin

TsaiJin Jul 25, 2017

Contributor

Because after the call of function _try_send, the outcoming_bl will just contain left data that needs to be sent next time.
So total_send_size - outcoming_bl.length() here means how much data has been sent successfully this time.

This comment has been minimized.

@joscollin

joscollin Jul 25, 2017

Member

@liupan1111
I was also checking about the same. It looks like it evaluates to 0. In this case, we need to verify what _try_send() does to outcoming_bl. Is _try_send() modifies outcoming_bl ? After a quick check, I think it does. But please verify again, @TsaiJin.

This comment has been minimized.

@TsaiJin

TsaiJin Jul 25, 2017

Contributor

@joscollin Yes, you are right.
As what I replied to @liupan1111, _try_send() modifies outcoming_bl.

This comment has been minimized.

@joscollin

joscollin Jul 25, 2017

Member

@TsaiJin @liupan1111
I guess, we all replied at the same time :-D.
LGTM.

@yuyuyu101 yuyuyu101 added the needs-qa label Jul 25, 2017

@TsaiJin

This comment has been minimized.

Contributor

TsaiJin commented Jul 26, 2017

@joscollin can you help execute a QA test for this PR, thanks.

@tchaikov tchaikov merged commit 69607f8 into ceph:master Aug 1, 2017

4 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
make check (arm64) make check succeeded
Details

@TsaiJin TsaiJin deleted the TsaiJin:wip-fix-msg-typo branch Aug 1, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment