Bug 7857 - database upgrade fails with plack
Summary: database upgrade fails with plack
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: 3.8
Hardware: All All
: P5 - low normal (vote)
Assignee: Dobrica Pavlinusic
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on:
Blocks: 7172
  Show dependency treegraph
 
Reported: 2012-03-30 20:05 UTC by Dobrica Pavlinusic
Modified: 2013-12-05 20:06 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 7857 - database upgrade fails with plack (1.62 KB, patch)
2012-03-30 20:17 UTC, Dobrica Pavlinusic
Details | Diff | Splinter Review
Bug 7857 - database upgrade fails with plack (1.96 KB, patch)
2012-03-30 22:56 UTC, Dobrica Pavlinusic
Details | Diff | Splinter Review
Bug 7857 - invalidate cached preferences after SetVersion (1.00 KB, patch)
2012-04-05 12:18 UTC, Dobrica Pavlinusic
Details | Diff | Splinter Review
Bug 7857 - database upgrade fails with plack (2.05 KB, patch)
2012-06-01 17:35 UTC, Paul Poulain
Details | Diff | Splinter Review
Bug 7857 - fix PERL5LIB followup (810 bytes, patch)
2012-06-14 21:35 UTC, Dobrica Pavlinusic
Details | Diff | Splinter Review
Bug 7857 - [SIGNED-OFF] database upgrade fails with plack (925 bytes, patch)
2012-06-18 08: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 Dobrica Pavlinusic 2012-03-30 20:05:34 UTC

    
Comment 1 Dobrica Pavlinusic 2012-03-30 20:17:43 UTC Comment hidden (obsolete)
Comment 2 Dobrica Pavlinusic 2012-03-30 22:56:19 UTC Comment hidden (obsolete)
Comment 3 Dobrica Pavlinusic 2012-03-31 20:25:16 UTC
At end of upgrade you need to manually restart plack since C4::Context is keeping cache of sysprefs and database upgrade insert to new version from updatedatabase.pl get ignored.

This will be fixed in separate bug.
Comment 4 Chris Cormack 2012-03-31 20:29:48 UTC
Thats actually a general syspref issue it either needs to be a true global cache (shared between threads)  or an external store like reddis or memcached. And flushed on change. There are quite a few gotchas like this.
Comment 5 Dobrica Pavlinusic 2012-03-31 22:59:25 UTC
This problem could be solved with simple Koha::SysPrefs which would wrap systempreferences table into intelligent caching.

But rest of code would benefit from simple our $cache per-request cache which is safe from cache invalidation problems.

Then we could start pushing it to memcache (after we flush all invalidation issues), but from my profiling so far, it seems biggest low-hanging fruit is caching within single request based on Bug 7177

This is next think I plan to work on, after all quirks with plack are cleaned up.
Comment 6 Dobrica Pavlinusic 2012-04-05 12:18:34 UTC
Created attachment 8912 [details] [review]
Bug 7857 - invalidate cached preferences after SetVersion

This allows plack to correctly move to login page after database upgrade
Comment 7 Dobrica Pavlinusic 2012-04-05 12:21:04 UTC
It turns out that we already have method to invoke cache invalidation so call to it is provided as second patch. I think this change is generally useful for all Koha versions, so I'm providing it as separate patch so it can be applied to older versions if needed.
Comment 8 Paul Poulain 2012-06-01 17:35:40 UTC
Created attachment 9888 [details] [review]
Bug 7857 - database upgrade fails with plack

Updating database structure

Update errors :

    Can't locate C4/Context.pm in @INC (@INC contains: /etc/perl /usr/local/lib/perl/5.10.1 /usr/local/share/perl/5.10.1 /usr/lib/perl5 /usr/share/perl5 /usr/lib/perl/5.10 /usr/share/perl/5.10 /usr/local/lib/site_perl /usr/local/lib/perl/5.10.0 /usr/local/share/perl/5.10.0 .) at /srv/koha/installer/data/mysql/updatedatabase.pl line 37.
    BEGIN failed--compilation aborted at /srv/koha/installer/data/mysql/updatedatabase.pl line 37.

It seems that plack cleans enviroment, so we have to re-instart PERL5LIB
so script invocation won't fail.

It also fixes exit 1 which fail with plack.

Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Comment 9 Paul Poulain 2012-06-01 17:37:16 UTC
Patch Bug 7857 - database upgrade fails with plack  signed off, please QA.

The patch "Bug 7857 - invalidate cached preferences after SetVersion" should be put "in discussion". Plus there is a CHI cache management that should hit Koha very soon, that could make the discussion obsolete
Comment 10 Jonathan Druart 2012-06-14 13:15:28 UTC
I don't know if it's normal but I have another error :

Can't locate InstallAuth.pm in @INC (@INC contains: /home/koha/src /etc/perl /usr/local/lib/perl/5.14.2 /usr/local/share/perl/5.14.2 /usr/lib/perl5 /usr/share/perl5 /usr/lib/perl/5.14 /usr/share/perl/5.14 /usr/local/lib/site_perl .) at /home/koha/src/installer/install.pl line 7.
BEGIN failed--compilation aborted at /home/koha/src/installer/install.pl line 7 (#1)



I start plack like this:
KOHA_CONF=/home/koha/etc/koha-conf.xml PERL5LIB=/home/koha/src; plackup --reload intranet.psgi --port=5001

I don't get this error if I start like:
KOHA_CONF=/home/koha/etc/koha-conf.xml PERL5LIB=/home/koha/src:/home/koha/src/installer plackup --reload intranet.psgi --port=5001

But it it that we want ?
Comment 11 Jonathan Druart 2012-06-14 13:16:27 UTC
Replace last sentence with:
  But is it what we want ?
Comment 12 Dobrica Pavlinusic 2012-06-14 21:34:28 UTC
No, of course not :-)

There is hidden -Iinstaller include in Bug 7844 - plack scripts for developers thanks for catching that!
Comment 13 Dobrica Pavlinusic 2012-06-14 21:35:11 UTC Comment hidden (obsolete)
Comment 14 Jonathan Druart 2012-06-18 08:32:03 UTC
Created attachment 10345 [details] [review]
Bug 7857 - [SIGNED-OFF] database upgrade fails with plack

This patch fixes PERL5LIB path injection to include installer dir
which is required

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 15 Jonathan Druart 2012-06-18 08:38:14 UTC
Last patch is trivial and makes sense.

I passed QA these 3 patchs.
Comment 16 Paul Poulain 2012-07-06 14:04:56 UTC
(In reply to comment #3)
> At end of upgrade you need to manually restart plack since C4::Context is
> keeping cache of sysprefs and database upgrade insert to new version from
> updatedatabase.pl get ignored.
> 
> This will be fixed in separate bug.

Hi Dobrica,

is this problem solved somewhere ? because it's still here...
Comment 17 Jared Camins-Esakov 2012-11-15 17:41:22 UTC
(In reply to comment #16)
> (In reply to comment #3)
> > At end of upgrade you need to manually restart plack since C4::Context is
> > keeping cache of sysprefs and database upgrade insert to new version from
> > updatedatabase.pl get ignored.
> > 
> > This will be fixed in separate bug.
> 
> Hi Dobrica,
> 
> is this problem solved somewhere ? because it's still here...

Paul,

You have to disable the syspref cache, as I made possible with bug 9005. There is no other way to deal with this at present. The solution -- to the extent that there is one -- is to use an update API (like in the non-linear updates bug) which handles syspref updating via C4::Context, and then if you're using a single-threaded Plack server it will work. As you know, it is my assertion that the syspref cache should always be disabled with PSGI.
Comment 18 Chris Cormack 2013-04-22 08:45:51 UTC
Released in 3.10.0