Bug 28534 - pending_offline_circulations table uses MyISAM engine
Summary: pending_offline_circulations table uses MyISAM engine
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Database (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Nick Clemens
QA Contact: Kyle M Hall
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-06-09 13:44 UTC by Nick Clemens
Modified: 2022-06-06 20:25 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This updates the database structure for the pending_offline_operations table so that it uses the InnoDB engine instead of the MyISAM engine.
Version(s) released in:
21.11.00


Attachments
Bug 28534: Fix engine for pending_offline_circulations table (2.26 KB, patch)
2021-06-09 18:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28534: Atomic update to fix existing installs (1.73 KB, patch)
2021-06-14 12:44 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28534: Fix engine for pending_offline_circulations table (2.30 KB, patch)
2021-06-14 16:50 UTC, David Nind
Details | Diff | Splinter Review
Bug 28534: Atomic update to fix existing installs (1.77 KB, patch)
2021-06-14 16:50 UTC, David Nind
Details | Diff | Splinter Review
Bug 28534: Fix engine for pending_offline_circulations table (2.38 KB, patch)
2021-07-23 13:36 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 28534: Atomic update to fix existing installs (1.84 KB, patch)
2021-07-23 13:36 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 28534: (follow-up) Don't adjust original DB update (1.59 KB, patch)
2021-09-14 13:06 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28534: Fix engine for pending_offline_circulations table (2.43 KB, patch)
2021-09-24 15:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28534: Atomic update to fix existing installs (1.89 KB, patch)
2021-09-24 15:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28534: (follow-up) Don't adjust original DB update (1.66 KB, patch)
2021-09-24 15:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28534: Fix engine for pending_offline_circulations table (1.23 KB, patch)
2021-09-28 10:32 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2021-06-09 13:44:30 UTC
This appears to be a holdover that was never updated
Comment 1 Nick Clemens 2021-06-09 18:12:48 UTC
Created attachment 121746 [details] [review]
Bug 28534: Fix engine for pending_offline_circulations table
Comment 2 David Nind 2021-06-10 19:44:37 UTC
I wasn't sure how to test this (using koha-testing-docker), but thought that this would work:

1. Check table structure in the database:
   . koha-mysql kohadev
   . show create table pending_offline_operations;
   . should show as "..ENGINE=MyISAM..."
2. Apply patch
3. Update the database (updatedatabase)
4. Check the table structure as per step 1 - should show as "..ENGINE=INNODB.."

It does update if I do a reset_all, but for the existing installations I seem to be missing something... (restarting the container didn't work for me - docker restart koha_db_1)?
Comment 3 Nick Clemens 2021-06-14 12:44:10 UTC
Created attachment 121908 [details] [review]
Bug 28534: Atomic update to fix existing installs
Comment 4 Nick Clemens 2021-06-14 12:47:00 UTC
(In reply to David Nind from comment #2)
> I wasn't sure how to test this (using koha-testing-docker), but thought that
> this would work:
> 
> 1. Check table structure in the database:
>    . koha-mysql kohadev
>    . show create table pending_offline_operations;
>    . should show as "..ENGINE=MyISAM..."
> 2. Apply patch
> 3. Update the database (updatedatabase)
> 4. Check the table structure as per step 1 - should show as
> "..ENGINE=INNODB.."
> 
> It does update if I do a reset_all, but for the existing installations I
> seem to be missing something... (restarting the container didn't work for me
> - docker restart koha_db_1)?

I added an atomic update to fix existing installs. I added a create table as well, one of our sites did not have MyISAM enabled, so the table was not created. To test:
1 - Check current table structure
2 - Apply patch
3 - Update database
4 - Confirm table structure is correct
5 - Drop the table 'DROP TABLE pending_offline_operations'
6 - Update database
7 - Confirm table is created
8 - reset_all
9 - Check the structure and confirm table is created as InnoDB
Comment 5 David Nind 2021-06-14 16:50:25 UTC
Created attachment 121917 [details] [review]
Bug 28534: Fix engine for pending_offline_circulations table

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 David Nind 2021-06-14 16:50:29 UTC
Created attachment 121918 [details] [review]
Bug 28534: Atomic update to fix existing installs

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 Kyle M Hall 2021-07-23 13:36:19 UTC
Created attachment 123112 [details] [review]
Bug 28534: Fix engine for pending_offline_circulations table

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 8 Kyle M Hall 2021-07-23 13:36:30 UTC
Created attachment 123113 [details] [review]
Bug 28534: Atomic update to fix existing installs

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 9 Jonathan Druart 2021-07-26 07:55:03 UTC
1. I would not modify the old DB rev

2.
There was a comment about that prior to
  commit 8c5845ef9ecd92584723cd60a221ecd4710a2516
  Bug 26947: Update kohastructure.sql


-- this table is MyISAM, InnoDB tables are growing only and this table is filled/emptied/filled/emptied...
-- so MyISAM is better in this case

So it seems that it was expected.

I don't know how it's still relevant however.
Comment 10 Nick Clemens 2021-09-14 13:06:16 UTC
Created attachment 124865 [details] [review]
Bug 28534: (follow-up) Don't adjust original DB update
Comment 11 Kyle M Hall 2021-09-14 13:24:45 UTC
> -- this table is MyISAM, InnoDB tables are growing only and this table is
> filled/emptied/filled/emptied...
> -- so MyISAM is better in this case
> 
> So it seems that it was expected.
> 
> I don't know how it's still relevant however.

This appears to be incorrect for the most part. It is only true if innodb_file_per_table is disabled, but innodb_file_per_table is enabled by default. It lists this behavior specifically as a reason not to disable it.

https://dev.mysql.com/doc/refman/8.0/en/innodb-file-per-table-tablespaces.html
Comment 12 Nick Clemens 2021-09-16 09:53:44 UTC
(In reply to Jonathan Druart from comment #9)
> 1. I would not modify the old DB rev
> 
> 2.
> There was a comment about that prior to
>   commit 8c5845ef9ecd92584723cd60a221ecd4710a2516
>   Bug 26947: Update kohastructure.sql
> 
> 
> -- this table is MyISAM, InnoDB tables are growing only and this table is
> filled/emptied/filled/emptied...
> -- so MyISAM is better in this case

tmp_holdsqueue is constantly filled/emptied and is InnoDB and has no issues

It seems the other problem of InnoDB is the resetting of auto_increment on a restart, which we have handled and should not affect here
Comment 13 Joonas Kylmälä 2021-09-19 13:20:58 UTC
So can we just obsolete those two patches? Let me know when this is ready for QA.
Comment 14 Kyle M Hall 2021-09-21 12:22:34 UTC
(In reply to Joonas Kylmälä from comment #13)
> So can we just obsolete those two patches? Let me know when this is ready
> for QA.

I think all three are needed. The first one fixes the koha structure file, the second one fixes existing installs, and the third one reverts the changes in the first patch to the original db update that creates the table.
Comment 15 Martin Renvoize 2021-09-24 15:20:38 UTC
Created attachment 125265 [details] [review]
Bug 28534: Fix engine for pending_offline_circulations table

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 16 Martin Renvoize 2021-09-24 15:20:43 UTC
Created attachment 125266 [details] [review]
Bug 28534: Atomic update to fix existing installs

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 17 Martin Renvoize 2021-09-24 15:20:47 UTC
Created attachment 125267 [details] [review]
Bug 28534: (follow-up) Don't adjust original DB update

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 18 Martin Renvoize 2021-09-24 15:21:10 UTC
Nice work, Passing QA
Comment 19 Jonathan Druart 2021-09-28 10:32:28 UTC
Created attachment 125393 [details] [review]
Bug 28534: Fix engine for pending_offline_circulations table

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 28534: (follow-up) Don't adjust original DB update

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 20 Jonathan Druart 2021-09-28 13:38:24 UTC
Pushed to master for 21.11, thanks to everybody involved!