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

*: fix the Host column of 'show processlist' #11069

Closed
wants to merge 2 commits into from

Conversation

wshwsh12
Copy link
Contributor

@wshwsh12 wshwsh12 commented Jul 4, 2019

What problem does this PR solve?

#10903
The Host column should be host_name:client_port format.

What is changed and how it works?

open 2 mysql client and run show processlist; both.
Before this pr:

mysql> show processlist;
+------+------+-----------+------+---------+------+-------+------------------+
| Id   | User | Host      | db   | Command | Time | State | Info             |
+------+------+-----------+------+---------+------+-------+------------------+
|    1 | root | 127.0.0.1 | NULL | Sleep   |    6 | 2     | NULL             |
|    2 | root | 127.0.0.1 | NULL | Query   |    0 | 2     | show processlist |
+------+------+-----------+------+---------+------+-------+------------------+
2 rows in set (0.00 sec)

This pr:

mysql> show processlist;
+------+------+-----------------+------+---------+------+-------+------------------+
| Id   | User | Host            | db   | Command | Time | State | Info             |
+------+------+-----------------+------+---------+------+-------+------------------+
|    1 | root | 127.0.0.1:49032 | NULL | Sleep   |   10 | 2     | NULL             |
|    2 | root | 127.0.0.1:49034 | NULL | Query   |    0 | 2     | show processlist |
+------+------+-----------------+------+---------+------+-------+------------------+
2 rows in set (0.00 sec)


Check List

Tests

  • Unit test
  • Manual test (add detailed scripts or steps below)

Code changes

  • Has exported function/method change

Side effects

  • Increased code complexity

Related changes

@wshwsh12
Copy link
Contributor Author

wshwsh12 commented Jul 4, 2019

/run-all-tests

@codecov
Copy link

codecov bot commented Jul 4, 2019

Codecov Report

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

@@             Coverage Diff             @@
##             master     #11069   +/-   ##
===========================================
  Coverage   81.1622%   81.1622%           
===========================================
  Files           419        419           
  Lines         90053      90053           
===========================================
  Hits          73089      73089           
  Misses        11700      11700           
  Partials       5264       5264

@wshwsh12
Copy link
Contributor Author

wshwsh12 commented Jul 4, 2019

/run-all-tests

@wshwsh12
Copy link
Contributor Author

wshwsh12 commented Jul 4, 2019

/run-integration-common-test

@@ -990,6 +990,9 @@ func (s *session) SetProcessInfo(sql string, t time.Time, command byte, maxExecu
if s.sessionVars.User != nil {
pi.User = s.sessionVars.User.Username
pi.Host = s.sessionVars.User.Hostname
if s.sessionVars.ConnectionInfo != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we put this check outside of if s.sessionVars.User != nil {?

@lysu
Copy link
Contributor

lysu commented Jul 5, 2019

hi @wshwsh12 , ConnectionInfo should better not use in Non-plugin function... that structure maybe contain more info fields and be more heavy in future.

how about add a new ClientConn interface and make server.clientConn impl that interface then save it in SessionVars? current question is we can not get clientConn from session?

@wshwsh12 wshwsh12 closed this Jul 18, 2019
@wshwsh12 wshwsh12 deleted the fix_port branch July 25, 2019 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/execution SIG execution type/bugfix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants