Bug 36569 - Compact action_logs indicies
Summary: Compact action_logs indicies
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Database (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-04-10 17:17 UTC by Andreas Jonsson
Modified: 2024-04-11 06:58 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 36569: Compact action log/combined index (4.75 KB, patch)
2024-04-10 17:55 UTC, Andreas Jonsson
Details | Diff | Splinter Review
Bug 36569: Compact action log/combined index (4.75 KB, patch)
2024-04-10 18:02 UTC, Andreas Jonsson
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Jonsson 2024-04-10 17:17:00 UTC
With cataloguing logging enabled on the action_logs table it will become large.  The module and action columns in action_logs are MEDIUMTEXT (max ~16 million bytes) with 196 first characters used for the index.  Since these columns holds names that are only used programatically we could reduce the size of these columns to make the index more compact.  Since module and action is often used together we can instead create a combined index and still have a smaller table.

> SELECT module, action, count(*) FROM action_logs GROUP BY module, action;
+------------------+----------------+----------+
| module           | action         | count(*) |
+------------------+----------------+----------+
| CATALOGUING      | ADD            |   543638 |
| CATALOGUING      | DELETE         |   573712 |
| CATALOGUING      | MODIFY         |  6264928 |
               .
               .
               .
| SYSTEMPREFERENCE | MODIFY         |      554 |
+------------------+----------------+----------+
33 rows in set (6 min 49,652 sec)

-rw-rw---- 1 mysql mysql 27439136768 10 apr 16.45  action_logs.ibd
-rw-rw---- 1 mysql mysql 24368906240 10 apr 17.33 action_logs.ibd

(/ (- 27439136768 24368906240) 27439136768.0)
(/ (- 27439136768 24914165760) 27439136768.0)

Changing the column sizes to 30 bytes each reduce the size of the action_logs table by 11%.

> ALTER TABLE action_logs MODIFY COLUMN module VARCHAR(30) CHARSET ascii, MODIFY COLUMN action VARCHAR(30) CHARSET ascii;
Query OK, 8725398 rows affected (18 min 32,541 sec)
Records: 8725398  Duplicates: 0  Warnings: 0

After creating a combined index the size is still 9% less than before:

CREATE INDEX IF NOT EXISTS module_action_idx ON action_logs(module, action);

Performance improvement 22024%:

+------------------+----------------+----------+
| module           | action         | count(*) |
+------------------+----------------+----------+
| CATALOGUING      | ADD            |   543638 |
| CATALOGUING      | DELETE         |   573712 |
| CATALOGUING      | MODIFY         |  6264928 |
.
.
.
| SYSTEMPREFERENCE | MODIFY         |      554 |
+------------------+----------------+----------+
33 rows in set (1,860 sec)
Comment 1 Andreas Jonsson 2024-04-10 17:55:45 UTC
Created attachment 164631 [details] [review]
Bug 36569: Compact action log/combined index

With cataloguing logging enabled on the action_logs table it will
become large.  The module and action columns in action_logs are
MEDIUMTEXT (max ~16 million bytes) with 196 first characters used for
the index.  Since these columns holds names that are only used
programatically we could reduce the size of these columns to make the
index more compact.  Since module and action is often used together we
can instead create a combined index and still have a smaller table.

Test plan:

* Start koha-testing-docker without patch applied (ku)
* Apply patch
* Run the command: sudo koha-upgrade-schema kohadev
* Verify database schema:
  > sudo koha-mysql kohadev -t -e 'SHOW CREATE TABLE action_logs' | \
       grep -E '`action`|`module`'

  The output should be (truncated long lines here):

  `module` varchar(30) CHARACTER SET ascii COLLATE ascii_general_ci ...
  `action` varchar(30) CHARACTER SET ascii COLLATE ascii_general_ci ...
  KEY `module_idx` (`module`),
  KEY `action_idx` (`action`),
  KEY `module_action_idx` (`module`,`action`),

* Run unit test: prove t/db_dependent/Koha/ActionLogs.t
* Stop koha-testing-docker and delete the containers (kd)
* Start koha-testing docher with patch applied
* Verify database schema:
  > sudo koha-mysql kohadev -t -e 'SHOW CREATE TABLE action_logs' | \
      grep -E '`action`|`module`'
  The output should be same as above
Comment 2 Andreas Jonsson 2024-04-10 18:02:58 UTC
Created attachment 164632 [details] [review]
Bug 36569: Compact action log/combined index

With cataloguing logging enabled on the action_logs table it will
become large.  The module and action columns in action_logs are
MEDIUMTEXT (max ~16 million bytes) with 196 first characters used for
the index.  Since these columns holds names that are only used
programatically we could reduce the size of these columns to make the
index more compact.  Since module and action is often used together we
can instead create a combined index and still have a smaller table.

Test plan:

* Start koha-testing-docker without patch applied (ku)
* Apply patch
* Run the command: sudo koha-upgrade-schema kohadev
* Verify database schema:
  > sudo koha-mysql kohadev -t -e 'SHOW CREATE TABLE action_logs' | \
       grep -E '`action`|`module`'

  The output should be (truncated long lines here):

  `module` varchar(30) CHARACTER SET ascii COLLATE ascii_general_ci ...
  `action` varchar(30) CHARACTER SET ascii COLLATE ascii_general_ci ...
  KEY `module_idx` (`module`),
  KEY `action_idx` (`action`),
  KEY `module_action_idx` (`module`,`action`),

* Run unit test: prove t/db_dependent/Koha/ActionLogs.t
* Stop koha-testing-docker and delete the containers (kd)
* Start koha-testing-docker with patch applied
* Verify database schema:
  > sudo koha-mysql kohadev -t -e 'SHOW CREATE TABLE action_logs' | \
      grep -E '`action`|`module`'
  The output should be same as above
Comment 3 David Cook 2024-04-11 00:27:34 UTC
First, I think this is awesome. Lately, we've been talking at the office a lot about this exact topic, so this is great to see.

However, I think we'll want to be very careful with this change. For instance, I don't think it should be backported, since any library with a large action_logs table will take a long time to upgrade. Or - as you note - they could have a volume full if they don't have enough disk space. 

I wonder if this is a case where we should update kohastructure.sql so that new databases have a good default, but not have an automatic upgrade of the action_logs. We are working on displaying a database audit in about.pl so that could be a good place to give people information about how to manually fix the action_logs indexes at a better time of their choosing...

I suppose another option would be to have an arbitrary threshold where an automatic database fix does happen for action_logs tables under a certain size... I don't know. 

Maybe we should put this one to the koha-devel listserv...

--

Questions:

1. How did you decide on 30 for the character length? 

I think SYSTEMPREFERENCE should be the longest "module" officially, and that's only 16 letters, so 30 seems pretty good. I suppose we're relying on plugins and customizations not having added any "module" longer than 30 characters...

I'm tempted to do query some prod databases to see if I can find anything unexpected in those columns.

2. Is the use of "CHARACTER SET ascii COLLATE ascii_general_ci" designed to keep the bytes allocated as small as possible? 

I think there's merit to that. We default to utf8mb4 which is going to be overkill for those columns, which should never be anything but ASCII, so I think it's good thinking.

It might be worth adding a comment to kohastructure explaining why the choice of ASCII so that someone in the future doesn't undo it by accident. 

3. The collation is missing in the "bug_36569-compact-action_logs-indices.pl" update. I haven't tested, but is that OK? Is it implied in MySQL from the charset?
Comment 4 David Cook 2024-04-11 00:28:19 UTC
Once you answer my questions, I'll have a look at testing this.

Even if I don't think it's ready for QA, I think I'd be willing to sign it off, if it works. Just to keep it moving...
Comment 5 Andreas Jonsson 2024-04-11 06:47:06 UTC
1. I believe 30 is sufficient, although even smaller might be possible?
2. I think restricting to ascii makes sense for values that are only accessed programatically although I don't know exactly much the overhead is for utf8mb4.
3. It should default to the general collation.  But setting it explicitly would not hurt.  I don't believe the collation is very important for these columns, though.
Comment 6 David Cook 2024-04-11 06:58:46 UTC
(In reply to Andreas Jonsson from comment #5)
> 1. I believe 30 is sufficient, although even smaller might be possible?

I don't mind erring on the side of caution using 30. 

> 2. I think restricting to ascii makes sense for values that are only
> accessed programatically although I don't know exactly much the overhead is
> for utf8mb4.

I think I follow. I just wanted to make sure that it was an intentional change.

> 3. It should default to the general collation.  But setting it explicitly
> would not hurt.  I don't believe the collation is very important for these
> columns, though.

Well, my point is that the kohastructure.sql and the database update are different. Regardless of the collation used, they need to be consistent/the same.