Bug 9066 - VirtualShelves database access not Plack + MariaDB compatible
Summary: VirtualShelves database access not Plack + MariaDB compatible
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Database (show other bugs)
Version: 3.10
Hardware: All All
: P5 - low critical (vote)
Target Milestone: Oldversions
Assignee: Jared Camins-Esakov
QA Contact: Paul Poulain
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-13 03:08 UTC by Jared Camins-Esakov
Modified: 2018-12-16 19:33 UTC (History)
3 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 9066: VirtualShelves db access not Plack+MariaDB compatible (4.97 KB, patch)
2012-11-13 14:35 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9066: VirtualShelves db access not Plack+MariaDB compatible (5.08 KB, patch)
2012-11-16 05:20 UTC, Mason James
Details | Diff | Splinter Review
Bug 9066: VirtualShelves db access not Plack+MariaDB compatible (5.07 KB, patch)
2012-11-20 10:49 UTC, Paul Poulain
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jared Camins-Esakov 2012-11-13 03:08:30 UTC
It would seem that the default timeout for MariaDB is much shorter than for MySQL, which leads to frequent connection failures under Starman. By wrapping the $dbh->ping call in an eval, Starman is able to reconnect without any problems.
Comment 1 Jared Camins-Esakov 2012-11-13 13:56:54 UTC
Further research has led me to conclude that the problem is *not* the ping, but rather the way the database handle is handled in VirtualShelves. $dbh must be reinitialized in each subroutine.

Upping severity because now that I understand the problem I realize that this same issue could happen with MySQL.
Comment 2 Jared Camins-Esakov 2012-11-13 14:35:22 UTC Comment hidden (obsolete)
Comment 3 Mason James 2012-11-16 05:20:03 UTC Comment hidden (obsolete)
Comment 4 Mason James 2012-11-16 05:21:42 UTC
(In reply to comment #3)
> Created attachment 13476 [details] [review]
> Bug 9066: VirtualShelves db access not Plack+MariaDB compatible
> 
> To test:
> Confirm that Virtual Shelves functionality still works as expected. The
> problem is not reproducible when using Apache or Plack+MySQL, but if
> everything works after the patch has been applied, the changes are safe.
> 
> Signed-off-by: Mason James <mtj@kohaaloha.com>


tested this on my shiny new plack+mariadb koha

Virtual Shelves functionality still works as expected :)
Comment 5 Paul Poulain 2012-11-20 10:49:24 UTC
QA comment:
 * small patch that just move a variable for better accessibility
 * passes koha-qa.pl
 * no string change
 * no noticeable change

passed QA
Comment 6 Paul Poulain 2012-11-20 10:49:28 UTC
Created attachment 13545 [details] [review]
Bug 9066: VirtualShelves db access not Plack+MariaDB compatible

The $dbh database handle in C4::VirtualShelves was declared at the
module level, which means under Plack it is initialized only once, when
the server first starts. With the default MySQL configuration this is
not a problem, since the MySQL connection does not time out, but the
MariaDB default configuration does time out, resulting in every page
that calls any sub in C4::VirtualShelves failing after a certain period.
This patch eliminates the module-level $dbh variable and replaces it
with $dbh handles in each subroutine that requires it.

To test:
Confirm that Virtual Shelves functionality still works as expected. The
problem is not reproducible when using Apache or Plack+MySQL, but if
everything works after the patch has been applied, the changes are safe.

Signed-off-by: Mason James <mtj@kohaaloha.com>
Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Comment 7 Jared Camins-Esakov 2012-11-20 12:13:54 UTC
This patch has been pushed to master.
Comment 8 Paul Poulain 2012-11-20 15:23:06 UTC
Patch pushed to branch 3.10.x
Comment 9 Jared Camins-Esakov 2012-12-03 18:51:55 UTC
This is now working on master.