Bug 19016 - Add a script to fix corrupted data
Summary: Add a script to fix corrupted data
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Target Milestone: ---
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 18966
Blocks:
  Show dependency treegraph
 
Reported: 2017-08-01 13:03 UTC by Jonathan Druart
Modified: 2018-09-24 07:36 UTC (History)
17 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 19016: Add a script to fix corrupted data (6.79 KB, patch)
2017-08-01 17:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19016: Update other values (4.72 KB, patch)
2017-08-01 17:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19016 [Followup] - Fix bad column name (896 bytes, patch)
2017-08-03 10:41 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19016: Check and fix 'biblioitems' (2.22 KB, patch)
2017-08-09 13:28 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19016: Trigger reindex on fixing biblios (1.62 KB, patch)
2017-10-12 14:59 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2017-08-01 13:03:17 UTC
We must provide a fix to deal with corrupted data.

For more information, see bug 18966 and https://wiki.koha-community.org/wiki/DBMS_auto_increment_fix
Comment 1 Marcel de Rooy 2017-08-01 14:20:19 UTC
Would be great imo if this fix could be used in cleanup_database somehow too. Put the core fix in some module?
Comment 2 Jonathan Druart 2017-08-01 17:29:31 UTC
Created attachment 65402 [details] [review]
Bug 19016: Add a script to fix corrupted data

This patch add two new options to the cleanup_database.pl script:
* --list-corrupted-data to list the different rows that are affected
* --fix-corrupted-data to fix and reassign an id to the corrupted rows

TO NOT USE IT IN PRODUCTION YET!
Comment 3 Jonathan Druart 2017-08-01 17:29:37 UTC
Created attachment 65403 [details] [review]
Bug 19016: Update other values

Here we have a problem!
If we assign a new id to the different rows that cannot be moved
safetly, we need to modify the other tables that do not have a foreign
key (for historical or laziness reasons).

For instance:
John is borrowernumber=42 and create a suggestion
(suggestion.suggestedby=42)
Jane is borrowernumber=42 in the deletedborrowers table. She created a
suggestion (same suggestedby value).

John will get a new id and the suggestion.suggestedby will not be
replaced without this patch. But with this fix, the 2 suggestions will
be marked as suggested by him.
Comment 4 Kyle M Hall 2017-08-03 10:23:32 UTC
What about the biblioitems table?
Comment 5 Kyle M Hall 2017-08-03 10:41:00 UTC
Created attachment 65456 [details] [review]
Bug 19016 [Followup] - Fix bad column name
Comment 6 Jonathan Druart 2017-08-03 14:26:38 UTC
(In reply to Kyle M Hall from comment #4)
> What about the biblioitems table?

I have no idea what to do with biblioitems.

What do you think about the second patch?
Comment 7 Tomás Cohen Arazi 2017-08-09 13:28:01 UTC
Created attachment 65710 [details] [review]
Bug 19016: Check and fix 'biblioitems'

This patch adds 'biblioitems' to the list of tables to be evaluated and fixed.

To test the problem exists:
- reset_all
- Run:
  $ sudo koha-mysql kohadev
  > SELECT biblionumber FROM biblio ORDER BY biblionumber DESC LIMIT 1;
- From the staff UI, delete the biblio with that biblionumber
- Restart mysql:
  $ sudo systemctl restart mysql.service
- Add a new biblio record
- Run:
  $ sudo koha-shell kohadev
 k> cd kohaclone
 k> misc/cronjobs/cleanup_database.pl --list-corrupted-data
=> FAIL: biblioitems issue is not higlighted
- Apply this patch
- Run:
  $ sudo koha-shell kohadev
 k> cd kohaclone
 k> misc/cronjobs/cleanup_database.pl --list-corrupted-data
=> SUCCESS: biblioitems issue is highlighted

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 8 Marcel de Rooy 2017-08-11 10:28:40 UTC
Created a problem in two tables:
 * Tables biblioitems/deletedbiblioitems: 2580, 2632
 * Tables reserves/old_reserves: 831, 833
Now fixing it with the script:
 * Tables biblioitems/deletedbiblioitems: 2580, 2632
Updating biblioitems.biblioitemnumber=2580 with new id 2634
Updating biblioitems.biblioitemnumber=2632 with new id 2635
 * Tables reserves/old_reserves: 831, 833
Updating reserves.reserve_id=831 with new id 835
Updating reserves.reserve_id=833 with new id 836
And now searching on opac. Crashes on opac-detail. Warnings on opac-search.pl
Can't call method "title" on an undefined value at /usr/share/koha/masterclone/opac/opac-detail.pl line 454.
GetCOinSBiblio called with undefined record at /usr/share/koha/masterclone/opac/opac-search.pl line 663.

We need to call ModZebra (or Elastic?) on these changed id's too.

Would it be safer to change id's in the old/deleted tables, and push the autoincrement on the original table ?
Comment 9 Marcel de Rooy 2017-08-11 10:59:38 UTC
(In reply to Marcel de Rooy from comment #8)
> Would it be safer to change id's in the old/deleted tables, and push the
> autoincrement on the original table ?

Inserting the new max into the original table (and deleting it) might be a faster workaround than alter table? Should be tested..
Comment 10 Katrin Fischer 2017-08-16 20:16:44 UTC
I wonder if it would be good to separate the different types of data. I feel pretty sure about the fix for issues, but borrowers and biblio* tables appear more dangerous. The issue_id one is also the most dangerous, as the items are kept on the accounts.

The list of tables for borrowers appears a bit short - are you relying on cascades in part there?

In general I am worried about maintaining the script - with every new table, this could cause new bugs. But as we don't have clean FK relationships, I don't see another way of doing it. :(
Comment 11 Christian McDonald 2017-08-29 21:51:42 UTC
When can expect a fix to be available via the stable Debian repo? I have 120 affected rows in my issues tables that is preventing my library from properly returning books. Otherwise, how can I manually fix "deal with" these affected rows?
Comment 12 dguidry 2017-09-08 19:52:08 UTC
(In reply to Christian McDonald from comment #11)
> When can expect a fix to be available via the stable Debian repo? I have 120
> affected rows in my issues tables that is preventing my library from
> properly returning books. Otherwise, how can I manually fix "deal with"
> these affected rows?

Seconded. I've applied the fix (DBMS auto increment fix on the Koha wiki site) to prevent further occurrences, but this doesn't appear to fix existing corruption. We have a handful of books that we're unable to check back in. How would one go about fixing this manually in lieu of the upcoming script?
Comment 13 Jonathan Druart 2017-09-12 13:12:03 UTC
(In reply to dguidry from comment #12)
> (In reply to Christian McDonald from comment #11)
> > When can expect a fix to be available via the stable Debian repo? I have 120
> > affected rows in my issues tables that is preventing my library from
> > properly returning books. Otherwise, how can I manually fix "deal with"
> > these affected rows?
> 
> Seconded. I've applied the fix (DBMS auto increment fix on the Koha wiki
> site) to prevent further occurrences, but this doesn't appear to fix
> existing corruption. We have a handful of books that we're unable to check
> back in. How would one go about fixing this manually in lieu of the upcoming
> script?

For issues/old_issues you could try the script, in a test environment(!)
You will see this kind of output:

  Updating issues.issue_id=24 with new id 42

When it is done, you should double-check the accountlines rows with the issue_id values that have been updated:

  SELECT borrowernumber FROM accountlines WHERE issue_id=42;

There are 2 possibilities: either the accountlines has been correctly updated, or not. Either fines has been attached to the correct patron account, or another patron has been charged (!)

The accountlines values have been updated with the following line of the cleanup script:
511 $dbh->do( q|UPDATE accountlines SET issue_id = ? WHERE issue_id = ?|, undef, $max, $old_id );
If you do not want accountlines to be updated, you can comment this line and execute the script again on a DB backup.

Without feedback I cannot tell you if it is preferable to comment this line or not.
By instinct I would comment the line and assume that check-ins are certainly  related to a fine rather than already checked out items.
Comment 14 Fridolin SOMERS 2017-09-14 10:18:39 UTC
We prefer to correct id in old_issues and old_reserves tables.

For deleted_xxx tables, we may just delete since it is too difficult to fix.
For example biblionumber, biblioitemnumber and itemnumber are in MARCXML also.
Comment 15 Tomás Cohen Arazi 2017-10-12 14:59:56 UTC
Created attachment 68015 [details] [review]
Bug 19016: Trigger reindex on fixing biblios

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 16 Oliver Behnke 2017-11-22 13:47:43 UTC
FWIW, I just tried the script with all the patches (up to now) and ran into a FK constraint violation:


% ./cleanup_database.pl --fix-corrupted-data

 * Tables biblio/deletedbiblio: 1455
Updating biblio.biblionumber=1455 with new id 1516
DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_koha`.`items`, CONSTRAINT `items_ibfk_5` FOREIGN KEY (`biblionumber`) REFERENCES `biblio` (`biblionumber`) ON DELETE CASCADE ON UPDATE CASCADE) [for Statement "UPDATE biblio SET biblionumber = ? WHERE biblionumber = ?" with ParamValues: 0=1516, 1=1455] at ./cleanup_database.pl line 510, <DATA> line 755.
Something went wrong, rolling back!

 * Tables biblioitems/deletedbiblioitems: 1455
Updating biblioitems.biblioitemnumber=1455 with new id 1516
Comment 17 Dobrica Pavlinusic 2017-12-18 14:33:41 UTC
This script doesn't update primary key auto increment after updating item numbers which results in duplicate primary key errors when Koha tries to create new items (until you go over items created by script).

This is not evident if you restart mysql server and have setup auto increment fixes according to wiki, after running this script, but it would be better if script would update auto increment after having used primary keys in first place.
Comment 18 sokunrotanak 2018-02-02 04:02:12 UTC
Hi, How do I download script to fix corrupted data?
Comment 19 Katrin Fischer 2018-02-02 06:12:16 UTC
The script is not finished and not recommended for production use.
Comment 20 Benjamin Rokseth 2018-09-21 07:18:56 UTC
The fix is in Bug 20271. Please push it forward
Comment 21 Jonathan Druart 2018-09-21 12:33:20 UTC
(In reply to Benjamin Rokseth from comment #20)
> The fix is in Bug 20271. Please push it forward

Hum, what do you mean?
Comment 22 Benjamin Rokseth 2018-09-24 07:36:27 UTC
Only an attempt to push focus to the core problem, and a proposed fix to that ;)
The problem is worse than shifted autoincrements, as we discovered to our dismay.