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

Add USEMYSQLFULLPROCESSLIST option to mysql plugin #233

Merged
merged 2 commits into from
Aug 14, 2022

Conversation

tonyskapunk
Copy link
Contributor

Fixes #231

  • Modifies the USEMYSQLPROCESSLIST to be truly "show processlist"
  • Adds a new option USEMYSQLFULLPROCESSLIST, this option perfoms a "show full processlist", this was the original functionality of USEMYSQLPROCESSLIST

Ran a test that shows the two different outputs ("show processlist" and "show full processlist") in this gist

@mikegriffin
Copy link

Hello, with recent activity in another bug I was wondering if you would still consider merging this PR

@tonyskapunk
Copy link
Contributor Author

Hi @mikegriffin, It Is good to go, but I really would like to have someone else review it before merging, but it has been out for longer than a year; I'm not sure if anyone would have some free cycles to look at it 😿

@mikegriffin
Copy link

Reviewing the patch, it appears to work on my system, with some minor problems:

  • Code comment in src/core/mysql should probably say "mysql [full] processlist"

  • When all MySQL options are set to yes, you get both processlist and full processlist output. I suspect we want something like the following in /usr/sbin/recap:

375,378d374
<     if [[ "${USEMYSQLPROCESSLIST,,}" == "yes" ]]; then
<       print_blankline "${ITEM_FILE}"
<       print_mysql_procs "${ITEM_FILE}" "${MYCNF}"
<     fi
381a378,382
>       else
>               if [[ "${USEMYSQLPROCESSLIST,,}" == "yes" ]]; then
>               print_blankline "${ITEM_FILE}"
>               print_mysql_procs "${ITEM_FILE}" "${MYCNF}"
>               fi

@tonyskapunk
Copy link
Contributor Author

👋 @mikegriffin sorry for the lack of response here and thanks for the review.

Code comment in src/core/mysql should probably say "mysql [full] processlist"

This is fixed in my next commit

When all MySQL options are set to yes, you get both processlist and full processlist output

In that case, I'm making full processlist to get priority over processlist. If both settings are enabled.

Please give it a look, and if everything looks good I'll merge it

Copy link

@mikegriffin mikegriffin left a comment

Choose a reason for hiding this comment

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

LGTM

@tonyskapunk tonyskapunk merged commit 49910e2 into rackerlabs:development Aug 14, 2022
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.

2 participants