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

executor: update the memory usage format in expensive logs #12809

Merged
merged 9 commits into from
Oct 21, 2019

Conversation

qw4990
Copy link
Contributor

@qw4990 qw4990 commented Oct 18, 2019

What problem does this PR solve?

Update the memory usage format in expensive logs to let it can be analyzed by tools easily.

What is changed and how it works?

Before this PR: mem_max: 1 MB.
After this PR: mem_max: 1048576 Bytes (1 MB).

Check List

Tests

  • Unit test

Copy link
Contributor

@SunRunAway SunRunAway left a comment

Choose a reason for hiding this comment

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

LGTM

@SunRunAway SunRunAway added the status/LGT1 Indicates that a PR has LGTM 1. label Oct 18, 2019
Copy link
Contributor

@XuHuaiyu XuHuaiyu left a comment

Choose a reason for hiding this comment

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

LGTM

@XuHuaiyu
Copy link
Contributor

Should we cherry-pick this commit to release-3.0 and release-3.1

@XuHuaiyu XuHuaiyu added status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Oct 18, 2019
@qw4990
Copy link
Contributor Author

qw4990 commented Oct 18, 2019

Should we cherry-pick this commit to release-3.0 and release-3.1

I will pick this PR later.

@sre-bot
Copy link
Contributor

sre-bot commented Oct 18, 2019

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Oct 18, 2019

@qw4990 merge failed.

@codecov
Copy link

codecov bot commented Oct 18, 2019

Codecov Report

Merging #12809 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #12809   +/-   ##
===========================================
  Coverage   80.1539%   80.1539%           
===========================================
  Files           465        465           
  Lines        107200     107200           
===========================================
  Hits          85925      85925           
  Misses        14895      14895           
  Partials       6380       6380

Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@qw4990 qw4990 merged commit 44c95fb into pingcap:master Oct 21, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Oct 21, 2019

cherry pick to release-3.0 failed

@sre-bot
Copy link
Contributor

sre-bot commented Oct 21, 2019

cherry pick to release-3.1 failed

@sre-bot
Copy link
Contributor

sre-bot commented Apr 7, 2020

It seems that, not for sure, we failed to cherry-pick this commit to release-3.0. Please comment '/run-cherry-picker' to try to trigger the cherry-picker if we did fail to cherry-pick this commit before. @qw4990 PTAL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/execution SIG execution status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/usability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants