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

log: support reloading tidb-query-max-log-len after modified #12491

Merged
merged 2 commits into from
Oct 12, 2019

Conversation

fzhedu
Copy link
Contributor

@fzhedu fzhedu commented Sep 30, 2019

What problem does this PR solve?

improve the usability for the tidb-query-max-log-len

What is changed and how it works?

change the default value of tidb-query-max-log-lenfrom 2048 to 4096, and support reloading tidb-query-max-log-len after modified in the config file.

Check List

Tests

  • Manual test

step 1: select the current value: select @@tidb_query_log_max_len;

step 2: change the value of tidb-query-max-log-len in the config file

step 3: reload the config curl -X GET http://127.0.0.1:10080/reload-config

step 4: re-select the value: select @@tidb_query_log_max_len;

Code changes

  • Update global variables

Release note

  • [function] Change the default value of tidb-query-max-log-lenfrom 2048 to 4096, and support reloading tidb-query-max-log-len after modified in the config file.

@fzhedu fzhedu requested a review from zz-jason September 30, 2019 03:03
@fzhedu fzhedu changed the title Prm 96 Prm 96: support reloading tidb-query-max-log-len after modified Sep 30, 2019
@zz-jason
Copy link
Member

@fzhedu Thanks for your contribution! Please follow the Commit Message and Pull Request Style guide to reformat the PR title. BTW, you can use the keywords described in the Closing issues using keywords guide to close the related issue when this PR is merged.

@zz-jason zz-jason changed the title Prm 96: support reloading tidb-query-max-log-len after modified support reloading tidb-query-max-log-len after modified Sep 30, 2019
@fzhedu fzhedu changed the title support reloading tidb-query-max-log-len after modified log: support reloading tidb-query-max-log-len after modified Sep 30, 2019
@zz-jason zz-jason removed their request for review September 30, 2019 05:48
@zz-jason
Copy link
Member

@fzhedu please remove all the irrelevant changes.

…port tidb_query_log_max_len reload after modified
@fzhedu fzhedu requested review from SunRunAway, AndreMouche and jackysp and removed request for AndreMouche October 10, 2019 03:41
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 10, 2019
@SunRunAway
Copy link
Contributor

Should we add it into the release note?

@fzhedu
Copy link
Contributor Author

fzhedu commented Oct 10, 2019

Should we add it into the release note?

@zz-jason ?

Copy link
Member

@jackysp jackysp 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 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 12, 2019
@codecov
Copy link

codecov bot commented Oct 12, 2019

Codecov Report

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

@@             Coverage Diff             @@
##             master     #12491   +/-   ##
===========================================
  Coverage   79.8869%   79.8869%           
===========================================
  Files           461        461           
  Lines        103803     103803           
===========================================
  Hits          82925      82925           
  Misses        14791      14791           
  Partials       6087       6087

@sre-bot
Copy link
Contributor

sre-bot commented Oct 12, 2019

/run-all-tests

@fzhedu
Copy link
Contributor Author

fzhedu commented Oct 12, 2019

/merge

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/usability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants