Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Simplify SHOW THREADS columns #1394

Closed
sanikolaev opened this issue Aug 25, 2023 · 7 comments
Closed

Simplify SHOW THREADS columns #1394

sanikolaev opened this issue Aug 25, 2023 · 7 comments
Assignees
Labels
rel::6.3.0 Released in 6.3.0

Comments

@sanikolaev
Copy link
Collaborator

As discussed in Slack, let's simplify and make less confusing the SHOW THREADS column names:

  • Time/Work time => Thread uptime - thread uptime with no conditions.
  • Time/Work time => Current job duration - how long the current job has been running. When the thread is idling - -.
  • Last job took => Previous job duration - how long the previous job took. When there was no previous job - -.
  • In idle: No (working) -> Thread status: working, idling
@sanikolaev
Copy link
Collaborator Author

As discussed on a call with Alexey, the specifics is that once a thread starts a new job we do not have access to the information about the previous job. It's only available if the thread is idling after processing a job. The thread uptime is mostly useful for VIP threads, for others it should be equal to the instance uptime. Provided that, the updated proposal is:

  • Time => remove as it's mostly not needed
  • Work time => This/prev job time, s:
    • when the thread is busy - how long the current job has been running, in seconds
    • when the thread is idling - previous job duration + suffix (prev)
  • In idle: No (working) -> Thread status: working, idling

A few more simplification ideas:

  • Tid => just ID to be consistent with ConnID in terms of letter capitalization and provided the command is called show threads just ID makes sense.
  • Host => Connection from, so it's clear what "host" is meant. The longer column name makes sense, since it's mostly not longer than the values anyway

Examples

Now:

+----------+--------+-------+-------+-----------------+--------+----------+-----------+-----------+---------------+--------------+--------------+
| Tid      | Name   | Proto | State | Host            | ConnID | Time     | Work time | Jobs done | Last job took | In idle      | Info         |
+----------+--------+-------+-------+-----------------+--------+----------+-----------+-----------+---------------+--------------+--------------+
| 12074427 | work_5 | mysql | query | 127.0.0.1:60975 |   2114 | 0.000571 | 17s       |     83567 | 577us         | No (working) | show threads |
| 12074428 | work_6 |       | -     |                 |     -1 | 23.03297 | 16s       |     85689 | 148us         | 374ms ago    |              |                                            |
+----------+--------+-------+-------+-----------------+--------+----------+-----------+-----------+---------------+--------------+--------------+
1 row in set (0.04 sec)

Suggested:

+----------+--------+-------+-------+-----------------+--------+-----------------------+-----------+---------------+---------------+
| ID       | Name   | Proto | State | Connection from | ConnID | This/prev job time, s | Jobs done | Thread status | Info          |
+----------+--------+-------+-------+-----------------+--------+-----------------------+-----------+---------------+---------------+
| 12074427 | work_5 | mysql | query | 127.0.0.1:60975 |   2114 | 0.000571              |     83567 | working       | show threads  |
| 12074428 | work_6 |       | -     |                 |     -1 | 0.000148 (prev)       |     85689 | idling, 1s    |               |
+----------+--------+-------+-------+-----------------+--------+-----------------------+-----------+---------------+---------------+
2 rows in set (0.04 sec)

@githubmanticore
Copy link
Contributor

➤ Aleksey N. Vinogradov commented:

suffix '(prev)' will make this column string-typed.
That is no difference for human, but will make parsing of such column significantly more complex.

@githubmanticore
Copy link
Contributor

➤ Aleksey N. Vinogradov commented:

Also, 'Time' column is quite useful when also cpustat is in game. Then you will see also cpu time, and thread efficiency columns.

They were all on the screen, but was hidden as 'useless'. Now (without them) simple 'Time' also looks 'useless', but on removing, cpu time and thread efficiency will also became 'useless'.
'Tid' is namely thread id, it reflects the tid in process explorer, and may be used to manipulate a thread from global OS context. Simple 'ID' doesn't have such sense; we can just assign any values starting from 1.
For me, looking for 'tid' or 'threadID' is immediately clue to the ps output. And also, in logs and crash dumps the very same tid is in game. Just 'ID' is much harder. You need to KNOW that is not a random ID, but very same ThreadID, or tid.

@sanikolaev
Copy link
Collaborator Author

suffix '(prev)' will make this column string-typed.
That is no difference for human, but will make parsing of such column significantly more complex.

I think this is a reasonable tradeoff.

'Tid' is namely thread id, it reflects the tid in process explorer...

Good point! Let it be TID then like in man proc: "Each child task is represented by its TID."

Also, 'Time' column is quite useful when also cpustat is in game

Right. I forgot about it. I suggest to rename Thd efficiency to CPU activity which should suffice since the absolute CPU time is not so interesting. CPU activity also makes it clear that it's about CPU, not something else.

So the corrected list of suggested actions is:

  • Time => remove
  • Work time => This/prev job time, s:
    • when the thread is busy - how long the current job has been running, in seconds
    • when the thread is idling - previous job duration + suffix (prev)
  • In idle: No (working) -> Thread status: working, idling
  • Tid => TID
  • Host => Connection from
  • Work time CPU => remove
  • Thd efficiency => CPU activity

@sanikolaev
Copy link
Collaborator Author

@PavelShilin89 please test that the change has been made as specified and update the docs if required (make a PR).

@sanikolaev sanikolaev reopened this Oct 3, 2023
@sanikolaev sanikolaev assigned PavelShilin89 and unassigned klirichek Oct 3, 2023
sanikolaev added a commit that referenced this issue Oct 7, 2023
@PavelShilin89
Copy link
Contributor

CLT tests done in #1509

@sanikolaev
Copy link
Collaborator Author

All done.

@sanikolaev sanikolaev added rel::upcoming Upcoming release and removed est::TO_ESTIMATE labels Oct 31, 2023
@sanikolaev sanikolaev added rel::6.3.0 Released in 6.3.0 and removed rel::upcoming Upcoming release labels May 23, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
rel::6.3.0 Released in 6.3.0
Projects
None yet
Development

No branches or pull requests

4 participants