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

sessionctx/variable: refine slow log output by not print useless info #12016

Merged
merged 12 commits into from
Sep 11, 2019

Conversation

crazycs520
Copy link
Contributor

What problem does this PR solve?

What is changed and how it works

No need to print cop info when there is no cop task.

eg:

# Time: 2019-09-03T22:29:15.928961+08:00
# Txn_start_ts: 0
# User: root@::1
# Conn_ID: 1
# Query_time: 4.55850739
# DB: test
# Is_internal: false
# Digest: ff607ecdf92ded60eae3d47bd165392258321403520610cf5354fd5cf7057c53
# Num_cop_tasks: 0
# Cop_proc_avg: 0 Cop_proc_p90: 0 Cop_proc_max: 0 Cop_proc_addr:
# Cop_wait_avg: 0 Cop_wait_p90: 0 Cop_wait_max: 0 Cop_wait_addr:
# Succ: true
split table t between (0) and (10000) regions 5;

As you can see, Num_cop_tasks is 0, then the cop log is redundant.

@sre-bot
Copy link
Contributor

sre-bot commented Sep 3, 2019

Thanks for your PR.
This PR will be closed by bot because you already had 2 opened PRs, close or merge them before submitting a new one.
#11702 , #11981

Copy link
Contributor

@winkyao winkyao left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov
Copy link

codecov bot commented Sep 3, 2019

Codecov Report

Merging #12016 into master will increase coverage by 0.1688%.
The diff coverage is n/a.

@@               Coverage Diff                @@
##             master     #12016        +/-   ##
================================================
+ Coverage   81.2963%   81.4652%   +0.1688%     
================================================
  Files           451        451                
  Lines         96671      97379       +708     
================================================
+ Hits          78590      79330       +740     
+ Misses        12445      12400        -45     
- Partials       5636       5649        +13

@shenli
Copy link
Member

shenli commented Sep 3, 2019

Could we keep the # Num_cop_tasks: 0?

@crazycs520 crazycs520 reopened this Sep 5, 2019
@crazycs520
Copy link
Contributor Author

@shenli, Of course, Done.

@crazycs520
Copy link
Contributor Author

@qw4990 PTAL

Copy link
Contributor

@lonng lonng left a comment

Choose a reason for hiding this comment

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

LGTM

lonng
lonng previously approved these changes Sep 9, 2019
SlowLogCopWaitP90, SlowLogSpaceMarkStr, logItems.CopTasks.P90WaitTime.Seconds(),
SlowLogCopWaitMax, SlowLogSpaceMarkStr, logItems.CopTasks.MaxWaitTime.Seconds(),
SlowLogCopWaitAddr, SlowLogSpaceMarkStr, logItems.CopTasks.MaxWaitAddress) + "\n")
if logItems.CopTasks.NumCopTasks > 0 {
Copy link
Member

Choose a reason for hiding this comment

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

if NumCopTasks == 1, we only need to print SlowLogCopProcAvg

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cop_wait_avg, cop_proc_addr also need?
done.

@zz-jason zz-jason added needs-cherry-pick-2.1 status/LGT2 Indicates that a PR has LGTM 2. type/enhancement The issue or PR belongs to an enhancement. labels Sep 9, 2019
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

@crazycs520 crazycs520 merged commit 864fbc2 into pingcap:master Sep 11, 2019
@crazycs520 crazycs520 added the status/can-merge Indicates a PR has been approved by a committer. label Sep 11, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Sep 11, 2019

Your auto merge job has been accepted, waiting for #12094

@sre-bot
Copy link
Contributor

sre-bot commented Sep 11, 2019

cherry pick to release-3.0 in PR #12144

@sre-bot
Copy link
Contributor

sre-bot commented Sep 11, 2019

cherry pick to release-2.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-2.1. Please comment '/run-cherry-picker' to try to trigger the cherry-picker if we did fail to cherry-pick this commit before. @crazycs520 PTAL.

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

Successfully merging this pull request may close these issues.

6 participants