29579 – Show saved SQL report ID in database query

Bug 29579 - Show saved SQL report ID in database query

Summary: Show saved SQL report ID in database query

Status:	CLOSED FIXED

Alias:	None

Product:	Koha
Classification:	Unclassified
Component:	Reports (show other bugs)
Version:	unspecified
Hardware:	All All

Importance:	P5 - low enhancement
Assignee:	Kyle M Hall
QA Contact:	Testopia

URL:
Keywords:

Depends on:
Blocks:

Reported:	2021-11-26 11:24 UTC by paxed
Modified:	2023-06-08 22:26 UTC (History)
CC List:	5 users (show)

See Also:
Change sponsored?:	---
Patch complexity:	Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:	This enhancement shows a saved SQL report's ID in the database process list (from example, "/* saved_sql.id: 123 */). This can help when troubleshooting reports that are causing issues, such as taking too long to run or taking up too many system resources.
Version(s) released in:	22.11.00
Circulation function:

Attachments
Bug 29579: Show saved SQL report ID in database query (1.28 KB, patch) 2021-11-26 11:27 UTC, paxed	Details \| Diff \| Splinter Review
Bug 29579: Show saved SQL report ID in database query (1.33 KB, patch) 2022-02-11 13:46 UTC, Owen Leonard	Details \| Diff \| Splinter Review
Bug 29579: Show saved SQL report ID in database query (1.39 KB, patch) 2022-02-13 14:14 UTC, Katrin Fischer	Details \| Diff \| Splinter Review
Bug 29579: Show saved SQL report ID in database query (1.21 KB, patch) 2022-07-21 17:23 UTC, Kyle M Hall	Details \| Diff \| Splinter Review
Bug 29579: Show saved SQL report ID in database query (1.21 KB, patch) 2022-07-21 17:26 UTC, Kyle M Hall	Details \| Diff \| Splinter Review
Bug 29579: Show saved SQL report ID in database query (1.26 KB, patch) 2022-07-25 01:30 UTC, David Nind	Details \| Diff \| Splinter Review
Bug 29579: (QA follow-up) Fix up t/db_dependent/Koha/Reports.t (2.20 KB, patch) 2022-08-01 12:29 UTC, Kyle M Hall	Details \| Diff \| Splinter Review
Bug 29579: Show saved SQL report ID in database query (1.33 KB, patch) 2022-08-01 12:49 UTC, Jonathan Druart	Details \| Diff \| Splinter Review
Bug 29579: (QA follow-up) Fix up t/db_dependent/Koha/Reports.t (2.27 KB, patch) 2022-08-01 12:49 UTC, Jonathan Druart	Details \| Diff \| Splinter Review
Show Obsolete (7) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description paxed 2021-11-26 11:24:18 UTC

When users run saved SQL reports, some of them might cause problems on the database end, for example taking too long or requiring too much resources.

It would be useful if it was possible to see in the database server end which saved SQL report was the culprit.

Comment 1 paxed 2021-11-26 11:27:07 UTC

Created attachment 128037 [details] [review]
Bug 29579: Show saved SQL report ID in database query

Bug 29579: Show saved SQL report ID in database query
    
When trying to figure out which saved SQL report caused too much load,
it's useful to have the report id show in the mysql process list.
    
This patch adds the saved SQL ID number as a comment line in front
of the SQL before passing it to the database.
    
To test:
1) Run a saved report that takes long enough time, so you can:
2) Connect to the database with your preferred client, and use
   "show processlist;" to list all the running mysql processes.
3) The running saved SQL report should show up with
   "-- saved_sql.id=123" in the process info field.

Comment 2 David Nind 2022-02-05 22:59:34 UTC

I tried testing this using koha-testing-koha. However, I couldn't create a report that would run long enough as there are only 400-500 records in the sample database.

Do you have a complicated report that takes a while to run, or do I just need to import a large number of MARC records?

Comment 3 Owen Leonard 2022-02-11 13:46:07 UTC

Created attachment 130496 [details] [review]
Bug 29579: Show saved SQL report ID in database query

When trying to figure out which saved SQL report caused too much load,
it's useful to have the report id show in the mysql process list.

This patch adds the saved SQL ID number as a comment line in front
of the SQL before passing it to the database.

To test:
1) Run a saved report that takes long enough time, so you can:
2) Connect to the database with your preferred client, and use
   "show processlist;" to list all the running mysql processes.
3) The running saved SQL report should show up with
   "-- saved_sql.id=123" in the process info field.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Comment 4 Owen Leonard 2022-02-11 13:46:35 UTC

Tested using the query "select sleep(10)"

Comment 5 Katrin Fischer 2022-02-13 14:14:34 UTC

Created attachment 130538 [details] [review]
Bug 29579: Show saved SQL report ID in database query

When trying to figure out which saved SQL report caused too much load,
it's useful to have the report id show in the mysql process list.

This patch adds the saved SQL ID number as a comment line in front
of the SQL before passing it to the database.

To test:
1) Run a saved report that takes long enough time, so you can:
2) Connect to the database with your preferred client, and use
   "show processlist;" to list all the running mysql processes.
3) The running saved SQL report should show up with
   "-- saved_sql.id=123" in the process info field.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Comment 6 David Nind 2022-02-14 16:43:37 UTC

(In reply to Owen Leonard from comment #4)
> Tested using the query "select sleep(10)"

Nice tip - thanks Owen!

Comment 7 Fridolin Somers 2022-02-15 07:42:46 UTC

Pushed to master for 22.05, thanks to everybody involved ﷐[U+1F984]﷑

Comment 8 Jonathan Druart 2022-02-15 08:48:27 UTC

t/db_dependent/Reports/Guided.t is failing because of this patch.

Comment 9 Jonathan Druart 2022-02-15 08:48:48 UTC

(In reply to Jonathan Druart from comment #8)
> t/db_dependent/Reports/Guided.t is failing because of this patch.

    #   Failed test 'Wrong SQL syntax raises warning' 
    #   at t/db_dependent/Reports/Guided.t line 264.
    # found warning: Use of uninitialized value $report_id in concatenation (.) or string at /kohadevbox/k
oha/C4/Reports/Guided.pm line 596.

Comment 10 Fridolin Somers 2022-02-16 09:16:56 UTC

I revert from master

Comment 11 Fridolin Somers 2022-02-16 09:18:42 UTC

Also fails :
t_db_dependent_Reports_Guided_t.Email report test

See https://jenkins.koha-community.org/view/master/job/Koha_Master_D10/529/testReport/

Comment 12 Kyle M Hall 2022-07-21 17:23:49 UTC

Created attachment 137989 [details] [review]
Bug 29579: Show saved SQL report ID in database query

When trying to figure out which saved SQL report caused too much load, it's useful to have the report id show in the mysql process list.

This patch adds the saved SQL ID number as a comment line in front
of the SQL before passing it to the database.

Test Plan:
1) Apply this patch
2) Restart all the things!
3) Create a long running query like:
   SELECT * FROM borrowers a, borrowers b, borrowers c, borrowers d
4) Connect to the database using koha-mysql
5) Execute "show processlist;"
6) Note the Info column looks something like:
   SELECT * FROM borrowers a, borrowers b, borrowers c, borrowers d /*saved_sql.id: 1*/

Comment 13 Kyle M Hall 2022-07-21 17:24:52 UTC

I was working on this very same feature when I found this bug report. My version does pass unit tests, probably due to the different commenting style I chose. I changed up my string to more closely match what paxed did.

Comment 14 Kyle M Hall 2022-07-21 17:26:01 UTC

Created attachment 137990 [details] [review]
Bug 29579: Show saved SQL report ID in database query

When trying to figure out which saved SQL report caused too much load, it's useful to have the report id show in the mysql process list.

This patch adds the saved SQL ID number as a comment line in front
of the SQL before passing it to the database.

Test Plan:
1) Apply this patch
2) Restart all the things!
3) Create a long running query like:
   SELECT * FROM borrowers a, borrowers b, borrowers c, borrowers d
4) Connect to the database using koha-mysql
5) Execute "show processlist;"
6) Note the Info column looks something like:
   SELECT * FROM borrowers a, borrowers b, borrowers c, borrowers d /* saved_sql.id: 1 */

Comment 15 David Nind 2022-07-25 01:30:48 UTC

Created attachment 138068 [details] [review]
Bug 29579: Show saved SQL report ID in database query

When trying to figure out which saved SQL report caused too much load, it's useful to have the report id show in the mysql process list.

This patch adds the saved SQL ID number as a comment line in front
of the SQL before passing it to the database.

Test Plan:
1) Apply this patch
2) Restart all the things!
3) Create a long running query like:
   SELECT * FROM borrowers a, borrowers b, borrowers c, borrowers d
4) Connect to the database using koha-mysql
5) Execute "show processlist;"
6) Note the Info column looks something like:
   SELECT * FROM borrowers a, borrowers b, borrowers c, borrowers d /* saved_sql.id: 1 */

Signed-off-by: David Nind <david@davidnind.com>

Comment 16 Jonathan Druart 2022-08-01 10:13:24 UTC

You need to adjust t/db_dependent/Koha/Reports.t

Comment 17 Kyle M Hall 2022-08-01 12:29:09 UTC

Created attachment 138460 [details] [review]
Bug 29579: (QA follow-up) Fix up t/db_dependent/Koha/Reports.t

Comment 18 Jonathan Druart 2022-08-01 12:49:22 UTC

Created attachment 138466 [details] [review]
Bug 29579: Show saved SQL report ID in database query

When trying to figure out which saved SQL report caused too much load, it's useful to have the report id show in the mysql process list.

This patch adds the saved SQL ID number as a comment line in front
of the SQL before passing it to the database.

Test Plan:
1) Apply this patch
2) Restart all the things!
3) Create a long running query like:
   SELECT * FROM borrowers a, borrowers b, borrowers c, borrowers d
4) Connect to the database using koha-mysql
5) Execute "show processlist;"
6) Note the Info column looks something like:
   SELECT * FROM borrowers a, borrowers b, borrowers c, borrowers d /* saved_sql.id: 1 */

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Comment 19 Jonathan Druart 2022-08-01 12:49:27 UTC

Created attachment 138467 [details] [review]
Bug 29579: (QA follow-up) Fix up t/db_dependent/Koha/Reports.t

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Comment 20 Tomás Cohen Arazi 2022-08-01 17:38:06 UTC

Pushed to master for 22.11.

Nice work everyone, thanks!

Comment 21 Lucas Gass 2022-09-29 15:57:13 UTC

Enhancement will not be backported to 22.05.x series