|
Description
Julian Maurice
2018-04-13 16:31:55 UTC
Created attachment 74173 [details] [review] Bug 20582: Turn Koha into a Mojolicious application (?) This patch is a proof-of-concept of Koha as a Mojolicious application This has several benefits: - Run `morbo script/koha` and you're set : fully functional Koha without apache2/nginx (URL rewrites and static files are handled by the app) - apache2/nginx configuration is much simpler (an example of nginx configuration is included in the patch) - starman and plack middlewares can still be used for debug or gzip compression for instance (see script/koha.psgi) - All that is needed to run koha is in the source code and not hidden in an obscure directory (I'm looking at you, debian/templates/plack.psgi) - Using Test::Mojo we can test the whole application, as we do with the REST API (which is a Mojolicious application too) - It's another step in the direction of dropping CGI support - It also opens a way for converting CGI scripts into Mojolicious controllers and actions (even if that's not possible at the moment because of the authentication code) - It requires zero changes to existing files, so it should apply nicely forever (no rebase needed \o/) A few downsides too: - It is not complete : some things from debian/templates/plack.psgi are missing, like the L1 cache flush or Koha::Middleware::SetEnv. But that shouldn't be too hard to add. - Already mentioned above, but the actual state of get_template_and_user and checkauth completely forbids the use of Mojolicious controllers. We really need to rewrite C4::Auth - Not sure if it's a downside or not, but as OPAC and Intranet are on the same host:port, if you authenticate on OPAC you will be authenticated on Intranet too. This can probably be changed if it's really a problem. - ... see FIXME in the code Other remarks: - It uses the same mechanism as Plack::App::CGIBin to deal with CGI scripts, so it should be equivalent in terms of performance - I did not test the install and upgrade process (yet) How to test ? - Run `morbo script/koha`, then go to: - http://localhost:3000/ - A basic page that redirects you to OPAC - Then navigate in the OPAC, searching for bugs - http://localhost:3000/intranet - The same page that redirects you to intranet - Then navigate in the Intranet, still searching for bugs - http://localhost:3000/api/v1/... - The REST API - Read the code (and the comments), it's not very long Hi Julian, a bit over my head here, but sounds like something that should go on the mailing list or the next dev meeting agenda too. (In reply to Katrin Fischer from comment #2) > Hi Julian, a bit over my head here, but sounds like something that should go > on the mailing list or the next dev meeting agenda too. I wanted to but didn't have the time. I will try to write something later today (In reply to Julian Maurice from comment #3) > I wanted to but didn't have the time. I will try to write something later > today http://lists.koha-community.org/pipermail/koha-devel/2018-April/044465.html https://framavox.org/d/tTGf0Bzd/bug-20582-turn-koha-into-a-mojolicious-application I love this! Moving Koha into a Mojolicious would make Koha a first class plack enabled application! I think Koha would benefit greatly in the long term from using a good framework like Mojolicious. Created attachment 74517 [details] [review] Bug 20582: Turn Koha into a Mojolicious application This patch is a proof-of-concept of Koha as a Mojolicious application This has several benefits: - Run `morbo bin/koha` and you're set : fully functional Koha without apache2/nginx (URL rewrites and static files are handled by the app) - apache2/nginx configuration is much simpler (an example of nginx configuration is included in the patch) - starman and plack middlewares can still be used for debug or gzip compression for instance (see bin/koha.psgi) - All that is needed to run koha is in the source code and not hidden in an obscure directory (I'm looking at you, debian/templates/plack.psgi) - Using Test::Mojo we can test the whole application, as we do with the REST API (which is a Mojolicious application too) - It's another step in the direction of dropping CGI support - It also opens a way for converting CGI scripts into Mojolicious controllers and actions (even if that's not possible at the moment because of the authentication code) - It requires zero changes to existing files, so it should apply nicely forever (no rebase needed \o/) A few downsides too: - It is not complete : some things from debian/templates/plack.psgi are missing, like the L1 cache flush or Koha::Middleware::SetEnv. But that shouldn't be too hard to add. - Already mentioned above, but the actual state of get_template_and_user and checkauth completely forbids the use of Mojolicious controllers. We really need to rewrite C4::Auth - Not sure if it's a downside or not, but as OPAC and Intranet are on the same host:port, if you authenticate on OPAC you will be authenticated on Intranet too. This can probably be changed if it's really a problem. - ... see FIXME in the code Other remarks: - It uses the same mechanism as Plack::App::CGIBin to deal with CGI scripts, so it should be equivalent in terms of performance How to test ? - Run `morbo bin/koha`, then go to: - http://localhost:3000/ - Redirects to OPAC main page - Then navigate in the OPAC, searching for bugs - http://localhost:3000/intranet - Redirects to intranet main page - Then navigate in the Intranet, still searching for bugs - http://localhost:3000/api/v1/... - The REST API - Test the web installer - Read the code (and the comments), it's not very long Created attachment 74518 [details] [review] Bug 20582: Add routes for some svc/ scripts svc/club/enroll, svc/club/cancel_enrollment and svc/report are available in different versions: one for the OPAC, one for the intranet This patch creates new routes that override the CGI scripts and that work for both OPAC and intranet Created attachment 74519 [details] [review] Bug 20582: Enable SetEnv middleware in bin/koha.psgi and make sure the psgi env is passed to CGI scripts Created attachment 74520 [details] [review] Bug 20582: Flush memory caches before every request I did a bit of cleanup in the initial patch and added some missing parts in additional patches. I think it's now ready for testing. After reviewing the patches, this seems awesome! It seems like a great way to move Koha into the 21st century and allow for incremental improvements. I see it doesn't need PERL5LIB but what about KOHA_CONF? (In reply to David Cook from comment #11) > After reviewing the patches, this seems awesome! It seems like a great way > to move Koha into the 21st century and allow for incremental improvements. > > I see it doesn't need PERL5LIB but what about KOHA_CONF? That's a good question... I've done nothing to pass this environment variable to the CGI environment. I'm wondering what value it actually uses... probably the /etc/koha/koha-conf.xml (because it defaults to that), and that would be a bug. I will look at it. Anyway, it is still required. Actually, C4::Context get compiled before the execution of a CGI script, so it gets the same environment as the Mojolicious app (so if you run `env KOHA_CONF=foo morbo bin/koha` C4::Context.pm will load foo) I think it's the same with debian/templates/plack.psgi Comment on attachment 74517 [details] [review] Bug 20582: Turn Koha into a Mojolicious application Review of attachment 74517 [details] [review]: ----------------------------------------------------------------- ::: Koha/App/Koha.pm @@ +1,3 @@ > +package Koha::App::Koha; > + > +use Modern::Perl; This really isn't required as you're using Mojo::Base below which turns on pretty much all the useful stuff from Modern::Perl and is more stable in it's actions... use strict; use warnings; use utf8; use feature ':5.10'; use IO::Handle (); require Mojolicious; push @ISA, 'Mojolicious'; sub has { Mojo::Base::attr(__PACKAGE__, @_) } Any progress on this one in the Koha community? (In reply to Martin Renvoize from comment #14) > This really isn't required as you're using Mojo::Base below which turns on > pretty much all the useful stuff from Modern::Perl and is more stable in > it's actions... If it doesn't do any harm I'd say we should keep using Modern::Perl for consistency, but it's not a strong opinion. (In reply to David Cook from comment #15) > Any progress on this one in the Koha community? The bug status is Needs Signoff, which means it's waiting for testers ;) Created attachment 79648 [details] [review] Bug 20582: Fix memory cache flush Cache flush should be in around_action hook instead of before_dispatch Otherwise it is flushed before every request (even static files requests) Also, make sure the Cache::Memory cache is really flushed (not only L1 cache) Has anyone tested this with file uploads? For some reason I'm unable to upload any files. For example, when I try to stage a MARC upload, the application responds with an HTTP 200 OK with the following content: {"fileid":null,"errors":null,"status":"failed"}
Comment on attachment 74517 [details] [review] Bug 20582: Turn Koha into a Mojolicious application Review of attachment 74517 [details] [review]: ----------------------------------------------------------------- ::: Koha/App/Plugin/CGIBinKoha.pm @@ +44,5 @@ > + $c->rendered; > + })->name('cgi'); > +} > + > +sub _psgi_env { Could we use https://mojolicious.org/perldoc/Mojo/Message/Request#env to just get a PSGI environment hash from Mojolicious? I haven't delved into these patches, but aren't we basically just using Mojolicious as a proxy to CGI::Emulate::PSGI, which looks like it comes straight out of Plack::App::WrapCGI anyway? Jerome and I were chatting in IRC and it seems like $c->req->body might not be properly handling multipart/form-data data. But could we just take the psgi.input straight from Mojolicious and plug it into CGI::Emulate::PSGI's app callback? I don't see why we couldn't do that in theory. (In reply to David Cook from comment #20) > Jerome and I were chatting in IRC and it seems like $c->req->body might not > be properly handling multipart/form-data data. > > But could we just take the psgi.input straight from Mojolicious and plug it > into CGI::Emulate::PSGI's app callback? I don't see why we couldn't do that > in theory. I don't think we can, psgi.input is defined only when running in a PSGI environment (app run by starman for instance) and $c->req->env is an empty hashref when the app is run by morbo or hypnotoad. Created attachment 81687 [details] [review] Bug 20582: Handle multipart content-type Mojo::Message::Request's body attribute does not contain anything when the content-type is multipart/* We have to rebuild the multipart body for CGI scripts Test plan: 1. Go to Tools » Upload and try to upload a file (In reply to Julian Maurice from comment #21) > I don't think we can, psgi.input is defined only when running in a PSGI > environment (app run by starman for instance) and $c->req->env is an empty > hashref when the app is run by morbo or hypnotoad. Ahhh, I see. I forgot about morbo and hypnotoad. Neither CGI nor PSGI, they're totally bespoke Mojolicious web servers it seems? I guess that's why they would be more optimized for Mojolicious... (In reply to Julian Maurice from comment #22) > Created attachment 81687 [details] [review] [review] > Bug 20582: Handle multipart content-type > > Mojo::Message::Request's body attribute does not contain anything when > the content-type is multipart/* > We have to rebuild the multipart body for CGI scripts > > Test plan: > 1. Go to Tools » Upload and try to upload a file Seems such a shame that Mojolicious doesn't just provide the actual original body of the message... I was just idly curious about this bug so I'll leave testing to Jerome. Created attachment 81857 [details] [review] Fix PDF generation Here's a patch to fix generating PDFs from the Acquisitions module. (In reply to Julian Maurice from comment #22) > Created attachment 81687 [details] [review] [review] > Bug 20582: Handle multipart content-type > > Mojo::Message::Request's body attribute does not contain anything when > the content-type is multipart/* > We have to rebuild the multipart body for CGI scripts > > Test plan: > 1. Go to Tools » Upload and try to upload a file Thank you for being so quick to provide a patch! I can confirm that it fixes upload functions within Koha. (In reply to Jerome Charaoui from comment #26) > Thank you for being so quick to provide a patch! > > I can confirm that it fixes upload functions within Koha. Thank you for testing it ;) (In reply to Jerome Charaoui from comment #25) > Created attachment 81857 [details] [review] [review] > Fix PDF generation > > Here's a patch to fix generating PDFs from the Acquisitions module. How did you generate this patch ? It's modifying a path that doesn't exist (b/site/profile/files/koha/lib/Koha/App/Koha.pm), I bet it won't apply nicely with `git bz apply`. Also the fix in itself references a path that only exist on 'standard' install (intranet/cgi-bin/acqui) so it won't work on a 'dev' install. Can you provide steps to reproduce the bug ? (In reply to Julian Maurice from comment #27) > (In reply to Jerome Charaoui from comment #26) > > Thank you for being so quick to provide a patch! > > > > I can confirm that it fixes upload functions within Koha. > > Thank you for testing it ;) > > (In reply to Jerome Charaoui from comment #25) > > Created attachment 81857 [details] [review] [review] [review] > > Fix PDF generation > > > > Here's a patch to fix generating PDFs from the Acquisitions module. > > How did you generate this patch ? It's modifying a path that doesn't exist > (b/site/profile/files/koha/lib/Koha/App/Koha.pm), I bet it won't apply > nicely with `git bz apply`. > Also the fix in itself references a path that only exist on 'standard' > install (intranet/cgi-bin/acqui) so it won't work on a 'dev' install. Can > you provide steps to reproduce the bug ? Oops, sorry for fumbling that patch, my bad. The following error message is generated when exporting a acquisitions basketgroup in PDF format (https://imgur.com/a/UmXpppW) : [debug] GET "/cgi-bin/koha/acqui/basketgroup.pl" [debug] Routing to a callback [error] Undefined subroutine &CGI::Compile::ROOT::usr_share_koha_intranet_cgi_2dbin_acqui_basketgroup_2epl::printpdf called at /usr/share/koha/intranet/cgi-bin/acqui/basketgroup.pl line 213. I'd like to share some results of our first Koha/Mojolicious lab that happened earlier today. Functionality-wise everything was up to par. One thing I noticed is that the 10 hypnotoad workers quickly grew memory hungry. Here is the smemstat output:
PID Swap USS PSS RSS User Command
8719 28.3 M 501.0 M 502.7 M 525.6 M root /usr/share/koha/bin/koha
8718 71.6 M 484.9 M 486.6 M 509.4 M root /usr/share/koha/bin/koha
8726 28.3 M 483.7 M 485.4 M 508.3 M root /usr/share/koha/bin/koha
8724 28.3 M 479.7 M 481.4 M 504.4 M root /usr/share/koha/bin/koha
8721 28.3 M 454.2 M 456.0 M 478.9 M root /usr/share/koha/bin/koha
8720 28.6 M 452.0 M 453.7 M 476.6 M root /usr/share/koha/bin/koha
8723 28.3 M 427.4 M 429.1 M 451.9 M root /usr/share/koha/bin/koha
14932 28.3 M 306.5 M 308.2 M 330.9 M root /usr/share/koha/bin/koha
23653 28.4 M 247.6 M 249.4 M 272.3 M root /usr/share/koha/bin/koha
23103 28.3 M 240.3 M 242.1 M 264.9 M root /usr/share/koha/bin/koha
We can see all the workers using up between 240 to 500M of unshared memory. When hypnotoad starts fresh, each worker uses up about 125M.
This is the current hypnotoad config:
{
basepath => '/usr/share/koha',
hypnotoad => {
listen => ['http://127.0.0.1:3000'],
workers => 10,
pid_file => '/run/koha/koha-app.pid'
}
};
In order to deal with this I modified the config by adding "accepts => 200" so that workers get recycled more quickly. If you have any other ideas as to how we could deal with memory usage, please let us know!
(In reply to Jerome Charaoui from comment #29) > In order to deal with this I modified the config by adding "accepts => 200" > so that workers get recycled more quickly. If you have any other ideas as to > how we could deal with memory usage, please let us know! I think we'd have to do more profiling, but I have a couple of thoughts. 1) I think memory usage will always go up, because when the worker first starts, it's only pre-loading a small selection of modules. As more work gets done, I think more modules get loaded into memory, and they stay there for the life of the worker. But how much memory is used by loading modules... that's something I don't know. I have looked a bit at memory use based on modules being loaded and noticed big differences. 2) I'm guessing maybe we have memory leaks. My first guess would DBIx::Class objects hanging around longer than they should. Some of that might be scoping and some of that might be caching in DBIx::Class. Mind you that's just a guess. This is something that could maybe be investigated and changed. But yeah... both just guesses. I'd say it would need application level profiling to really know. But considering how a multi-threaded Tomcat process can eat up GBs of memory, a multi-process hyponotoad eating up GBs of memory collectively doesn't really shock me either. -- Oh one thought... if we pre-loaded more modules into the master hypnotoad process, in theory there would be less in the unshared memory, since they'd be using the shared memory for the modules. I once encountered a legacy mod_perl app which wasn't preloading enough modules, and it caused performance problems when new workers were forked because they had to reload lots of modules. I preloaded everything important into the master, and performance went way up. Memory wasn't really a big deal for the app as we had lots to spare, but I assume copy-on-write was used so that the preloaded modules were in shared memory. Just my 2 cents. Thanks David for your input, much appreciated. I'm currently testing with Mojolicious::Plugin::SizeLimit. I'm hoping this will avoid frequest restarts while also keeping memory usage under control. I'll report back when I have some results. Comment on attachment 81857 [details] [review] Fix PDF generation Obsoleting this patch. It doesn't apply and the bug it tries to fix is in master (it is not related to this bug). I opened bug 21853 Created attachment 82423 [details] [review] Bug 20582: Handle multipart content-type Mojo::Message::Request's body attribute does not contain anything when the content-type is multipart/* We have to rebuild the multipart body for CGI scripts Test plan: 1. Go to Tools » Upload and try to upload a file (In reply to David Cook from comment #24) > Seems such a shame that Mojolicious doesn't just provide the actual original > body of the message... Actually it does. I rewrote the last patch in a much simpler way. (In reply to Julian Maurice from comment #34) > (In reply to David Cook from comment #24) > > Seems such a shame that Mojolicious doesn't just provide the actual original > > body of the message... > > Actually it does. I rewrote the last patch in a much simpler way. I think build_body reassembles the body from its component parts after taking apart the body, which seems efficient, but if it works, it works. Created attachment 84526 [details] [review] Bug 20582: Turn Koha into a Mojolicious application This patch is a proof-of-concept of Koha as a Mojolicious application This has several benefits: - Run `morbo bin/koha` and you're set : fully functional Koha without apache2/nginx (URL rewrites and static files are handled by the app) - apache2/nginx configuration is much simpler (an example of nginx configuration is included in the patch) - starman and plack middlewares can still be used for debug or gzip compression for instance (see bin/koha.psgi) - All that is needed to run koha is in the source code and not hidden in an obscure directory (I'm looking at you, debian/templates/plack.psgi) - Using Test::Mojo we can test the whole application, as we do with the REST API (which is a Mojolicious application too) - It's another step in the direction of dropping CGI support - It also opens a way for converting CGI scripts into Mojolicious controllers and actions (even if that's not possible at the moment because of the authentication code) - It requires zero changes to existing files, so it should apply nicely forever (no rebase needed \o/) A few downsides too: - It is not complete : some things from debian/templates/plack.psgi are missing, like the L1 cache flush or Koha::Middleware::SetEnv. But that shouldn't be too hard to add. - Already mentioned above, but the actual state of get_template_and_user and checkauth completely forbids the use of Mojolicious controllers. We really need to rewrite C4::Auth - Not sure if it's a downside or not, but as OPAC and Intranet are on the same host:port, if you authenticate on OPAC you will be authenticated on Intranet too. This can probably be changed if it's really a problem. - ... see FIXME in the code Other remarks: - It uses the same mechanism as Plack::App::CGIBin to deal with CGI scripts, so it should be equivalent in terms of performance How to test ? - Run `morbo bin/koha`, then go to: - http://localhost:3000/ - Redirects to OPAC main page - Then navigate in the OPAC, searching for bugs - http://localhost:3000/intranet - Redirects to intranet main page - Then navigate in the Intranet, still searching for bugs - http://localhost:3000/api/v1/... - The REST API - Test the web installer - Read the code (and the comments), it's not very long Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 84527 [details] [review] Bug 20582: Add routes for some svc/ scripts svc/club/enroll, svc/club/cancel_enrollment and svc/report are available in different versions: one for the OPAC, one for the intranet This patch creates new routes that override the CGI scripts and that work for both OPAC and intranet Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 84528 [details] [review] Bug 20582: Enable SetEnv middleware in bin/koha.psgi and make sure the psgi env is passed to CGI scripts Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 84529 [details] [review] Bug 20582: Flush memory caches before every request Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 84530 [details] [review] Bug 20582: Fix memory cache flush Cache flush should be in around_action hook instead of before_dispatch Otherwise it is flushed before every request (even static files requests) Also, make sure the Cache::Memory cache is really flushed (not only L1 cache) Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 84531 [details] [review] Bug 20582: Handle multipart content-type Mojo::Message::Request's body attribute does not contain anything when the content-type is multipart/* We have to rebuild the multipart body for CGI scripts Test plan: 1. Go to Tools » Upload and try to upload a file Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed off as it works as advertised and it would be great to see it moving ;) Just one note: we need to add Mojolicious::Plugin::TemplateToolkit to dependencies and make a debian package (adding Mirko to CC) I am moving this to "Milestone Future" and "In Discussion" - to be put back into the QA queue for 19.11 (see discussion during yesterday's Dev meeting) We're in the 19.11 cycle now and nearing my 'adventurous pushing' window where I hope to move things like this along. It would be great to see a bit more testing and get some collective QA eyes on it over the next couple of weeks with an aim to having a clear way forward for pushing it and end users (well, system administrators) tentatively enabling this deployment method. I'm time poor at the moment but interested in this report and especially #20630. Will keep it on my "to test" list and try to get to it. (In reply to Josef Moravec from comment #42) > Signed off as it works as advertised and it would be great to see it moving > ;) > > Just one note: we need to add Mojolicious::Plugin::TemplateToolkit to > dependencies and make a debian package (adding Mirko to CC) It is not really needed. I added this module as a 'will be useful in the future' stuff, but it can be removed. (In reply to Martin Renvoize from comment #44) > We're in the 19.11 cycle now and nearing my 'adventurous pushing' window > where I hope to move things like this along. It would be great to see a bit > more testing and get some collective QA eyes on it over the next couple of > weeks with an aim to having a clear way forward for pushing it and end users > (well, system administrators) tentatively enabling this deployment method. I'm glad you have an "adventurous pushing" window :) I'll try to be as reactive as possible to answer questions and to fix bugs. Created attachment 91009 [details] [review] Bug 20582: Remove usage of Mojo plugin TemplateToolkit It is not used anywhere yet (it will be useful if we start rewriting CGI scripts as Mojo controllers) These patches introduce new files without the GPL license header which is mandatory as per our coding guidelines: <https://wiki.koha-community.org/wiki/Coding_Guidelines>. The patches should be re-done so that there is the license header. Setting this as 'Failed QA' until this is resolved. Created attachment 99267 [details] [review] Bug 20582: Turn Koha into a Mojolicious application This patch is a proof-of-concept of Koha as a Mojolicious application This has several benefits: - Development setup is easier. No need for apache or nginx. Just run `morbo bin/intranet` or `morbo bin/opac` and go to http://localhost:3000 (URL rewrites and static files are handled by the app) - apache2/nginx configuration is simpler too (an example of nginx configuration is included in the patch) - starman and plack middlewares can still be used for debug or gzip compression for instance (see app.psgi) - Using Test::Mojo we can test the whole application, as we do with the REST API (which is a Mojolicious application too) - It opens a way for converting CGI scripts into Mojolicious controllers and actions (even if that's not possible at the moment because of the authentication code) It uses the same mechanism as Plack::App::CGIBin to deal with CGI scripts, so it should be equivalent in terms of performance How to test ? - Run `morbo bin/intranet`, then go to http://localhost:3000/ and try to find bugs. Check the REST API at http://localhost:3000/api/v1 - Run `morbo bin/opac`, then go to http://localhost:3000/ and try to find bugs. Check the REST API at http://localhost:3000/api/v1 - Run `starman -l :5000 -l :5001` and verify that intranet (http://localhost:5000) and opac (http://localhost:5001) work normally - Read the code (and the comments), it's not very long I squashed all patches, added the missing license headers, and made small changes to emphasize the fact that opac and intranet are two different apps. This makes the patch smaller and removes one of the FIXMEs. I also had to fix OPAC login when running app.psgi with starman (see comment in Koha::App::Plugin::CGIBinKoha::_psgi_env) Back to needs signoff. Does anyone manages to import a staged record? The progress bar stays to 0% Even with plack stopped (in case it blocks something), the behviour is the same. ## Logs (plack stopped) ### confirm staging [Mon Apr 20 23:39:54 2020] [debug] POST "/cgi-bin/koha/tools/stage-marc-import.pl" [Mon Apr 20 23:39:54 2020] [debug] Routing to a callback Subroutine staging_progress_callback redefined at /kohadevbox/koha/tools/stage-marc-import.pl line 234. Subroutine matching_progress_callback redefined at /kohadevbox/koha/tools/stage-marc-import.pl line 243. Use of uninitialized value in string eq at /kohadevbox/koha/bin/../Koha/App/Plugin/CGIBinKoha.pm line 88. Use of uninitialized value in concatenation (.) or string at /usr/share/perl5/MARC/File/XML.pm line 397, <__ANONIO__> chunk 2. [Mon Apr 20 23:39:54 2020] [debug] 200 OK (0.098054s, 10.198/s) seek() on closed filehandle _GEN_4 at /usr/share/perl5/CGI/Emulate/PSGI.pm line 34. [Mon Apr 20 23:39:54 2020] [error] Can't seek stdout handle: Bad file descriptor at /kohadevbox/koha/bin/../Koha/App/Plugin/CGIBinKoha.pm line 56. [Mon Apr 20 23:39:54 2020] [debug] Template "exception.development.html.ep" not found [Mon Apr 20 23:39:54 2020] [debug] Template "exception.html.ep" not found [Mon Apr 20 23:39:54 2020] [debug] Rendering template "mojo/debug.html.ep" [Mon Apr 20 23:39:54 2020] [debug] Rendering template "mojo/menubar.html.ep" [Mon Apr 20 23:39:54 2020] [debug] Your secret passphrase needs to be changed [Mon Apr 20 23:39:54 2020] [debug] 500 Internal Server Error (0.258607s, 3.867/s) [Mon Apr 20 23:39:55 2020] [debug] GET "/cgi-bin/koha/tools/background-job-progress.pl" [Mon Apr 20 23:39:55 2020] [debug] Routing to a callback Use of uninitialized value in string eq at /kohadevbox/koha/bin/../Koha/App/Plugin/CGIBinKoha.pm line 88. [Mon Apr 20 23:39:55 2020] [debug] 200 OK (0.028605s, 34.959/s) [Mon Apr 20 23:39:55 2020] [debug] POST "/cgi-bin/koha/tools/stage-marc-import.pl" [Mon Apr 20 23:39:55 2020] [debug] Routing to a callback Subroutine staging_progress_callback redefined at /kohadevbox/koha/tools/stage-marc-import.pl line 234. Subroutine matching_progress_callback redefined at /kohadevbox/koha/tools/stage-marc-import.pl line 243. Use of uninitialized value in string eq at /kohadevbox/koha/bin/../Koha/App/Plugin/CGIBinKoha.pm line 88. [Mon Apr 20 23:39:55 2020] [debug] 200 OK (0.099609s, 10.039/s) ### confirm import [Mon Apr 20 23:40:53 2020] [debug] POST "/cgi-bin/koha/tools/manage-marc-import.pl" [Mon Apr 20 23:40:53 2020] [debug] Routing to a callback Subroutine redo_matching redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 140. Subroutine create_labelbatch_from_importbatch redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 184. Subroutine import_batches_list redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 204. Subroutine commit_batch redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 232. Subroutine revert_batch redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 271. Subroutine put_in_background redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 309. Subroutine progress_callback redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 344. Subroutine add_results_to_template redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 354. Subroutine add_saved_job_results_to_template redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 360. Subroutine import_records_list redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 368. Subroutine batch_info redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 390. Subroutine add_matcher_list redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 422. Subroutine add_page_numbers redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 435. Use of uninitialized value in string eq at /kohadevbox/koha/bin/../Koha/App/Plugin/CGIBinKoha.pm line 88. DBD::mysql::db begin_work failed: Already in a transaction at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1588. [Mon Apr 20 23:40:53 2020] [debug] 200 OK (0.094207s, 10.615/s) I retried with the patches applied before the container was started (in case that made a difference)
I don't think it's related, but the end changed a bit and now I get:
> rollback ineffective with AutoCommit enabled
Can someone else confirm? Maybe my setup is wrong or I missed an implict step.
### confirm staging
[Fri Apr 24 00:03:10 2020] [debug] POST "/cgi-bin/koha/tools/stage-marc-import.pl"
[Fri Apr 24 00:03:10 2020] [debug] Routing to a callback
Subroutine staging_progress_callback redefined at /kohadevbox/koha/tools/stage-marc-import.pl line 234.
Subroutine matching_progress_callback redefined at /kohadevbox/koha/tools/stage-marc-import.pl line 243.
Use of uninitialized value in string eq at /kohadevbox/koha/bin/../Koha/App/Plugin/CGIBinKoha.pm line 88.
Use of uninitialized value in concatenation (.) or string at /usr/share/perl5/MARC/File/XML.pm line 397, <__ANONIO__> chunk 2.
[Fri Apr 24 00:03:10 2020] [debug] 200 OK (0.105929s, 9.440/s)
seek() on closed filehandle _GEN_5 at /usr/share/perl5/CGI/Emulate/PSGI.pm line 34.
[Fri Apr 24 00:03:10 2020] [error] Can't seek stdout handle: Bad file descriptor at /kohadevbox/koha/bin/../Koha/App/Plugin/CGIBinKoha.pm line 56.
[Fri Apr 24 00:03:10 2020] [debug] Template "exception.development.html.ep" not found
[Fri Apr 24 00:03:10 2020] [debug] Template "exception.html.ep" not found
[Fri Apr 24 00:03:10 2020] [debug] Rendering template "mojo/debug.html.ep"
[Fri Apr 24 00:03:10 2020] [debug] Rendering template "mojo/menubar.html.ep"
[Fri Apr 24 00:03:10 2020] [debug] Your secret passphrase needs to be changed
[Fri Apr 24 00:03:10 2020] [debug] 500 Internal Server Error (0.269868s, 3.706/s)
[Fri Apr 24 00:03:11 2020] [debug] GET "/cgi-bin/koha/tools/background-job-progress.pl"
[Fri Apr 24 00:03:11 2020] [debug] Routing to a callback
Use of uninitialized value in string eq at /kohadevbox/koha/bin/../Koha/App/Plugin/CGIBinKoha.pm line 88.
[Fri Apr 24 00:03:11 2020] [debug] 200 OK (0.029011s, 34.470/s)
[Fri Apr 24 00:03:11 2020] [debug] POST "/cgi-bin/koha/tools/stage-marc-import.pl"
[Fri Apr 24 00:03:11 2020] [debug] Routing to a callback
Subroutine staging_progress_callback redefined at /kohadevbox/koha/tools/stage-marc-import.pl line 234.
Subroutine matching_progress_callback redefined at /kohadevbox/koha/tools/stage-marc-import.pl line 243.
Use of uninitialized value in string eq at /kohadevbox/koha/bin/../Koha/App/Plugin/CGIBinKoha.pm line 88.
[Fri Apr 24 00:03:11 2020] [debug] 200 OK (0.085216s, 11.735/s)
### manage-marc-import
[Fri Apr 24 00:03:20 2020] [debug] GET "/cgi-bin/koha/tools/manage-marc-import.pl"
[Fri Apr 24 00:03:20 2020] [debug] Routing to a callback
Use of uninitialized value in string eq at /kohadevbox/koha/bin/../Koha/App/Plugin/CGIBinKoha.pm line 88.
[Fri Apr 24 00:03:21 2020] [debug] 200 OK (0.219592s, 4.554/s)
[Fri Apr 24 00:03:22 2020] [debug] POST "/cgi-bin/koha/tools/batch_records_ajax.pl"
[Fri Apr 24 00:03:22 2020] [debug] Routing to a callback
Use of uninitialized value in string eq at /kohadevbox/koha/bin/../Koha/App/Plugin/CGIBinKoha.pm line 88.
[Fri Apr 24 00:03:22 2020] [debug] 200 OK (0.03023s, 33.080/s)
### confirm import
[Fri Apr 24 00:03:23 2020] [debug] POST "/cgi-bin/koha/tools/manage-marc-import.pl"
[Fri Apr 24 00:03:23 2020] [debug] Routing to a callback
Subroutine redo_matching redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 140.
Subroutine create_labelbatch_from_importbatch redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 184.
Subroutine import_batches_list redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 204.
Subroutine commit_batch redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 232.
Subroutine revert_batch redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 271.
Subroutine put_in_background redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 309.
Subroutine progress_callback redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 344.
Subroutine add_results_to_template redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 354.
Subroutine add_saved_job_results_to_template redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 360.
Subroutine import_records_list redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 368.
Subroutine batch_info redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 390.
Subroutine add_matcher_list redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 422.
Subroutine add_page_numbers redefined at /kohadevbox/koha/tools/manage-marc-import.pl line 435.
Use of uninitialized value in string eq at /kohadevbox/koha/bin/../Koha/App/Plugin/CGIBinKoha.pm line 88.
rollback ineffective with AutoCommit enabled at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1651.
[Fri Apr 24 00:03:24 2020] [debug] 200 OK (0.116588s, 8.577/s)
I think that's the same problem than with starman: background jobs don't work in this environment. See bug 17240 and bug 15218 Created attachment 103633 [details] [review] Bug 20582: Suppress warning in CGIBinKoha.pm > I think that's the same problem than with starman: background jobs don't work in this environment.
What do you mean by "in this environment" ?
Note: I can import on the same setup when using Koha via the apache2&CGI way.
(In reply to Victor Grousset/tuxayo from comment #55) > > I think that's the same problem than with starman: background jobs don't work in this environment. > > What do you mean by "in this environment" ? I mean starman/morbo/hypnotoad/... Background jobs work only in CGI mode. So it's not an issue for the current patch? IIUC: That would be an issue only is morbo/Mojolicious was the only way the new patch allows. Does anyone has an idea bout what specific stuff to test for this patch? @Julian: since it doesn't replace the old mode, we don't need to test all Koha right? ^^" So no worries about this moving forward a week before the feature freeze? (In reply to Victor Grousset/tuxayo from comment #58) > Does anyone has an idea bout what specific stuff to test for this patch? > > @Julian: since it doesn't replace the old mode, we don't need to test all > Koha right? ^^" > > So no worries about this moving forward a week before the feature freeze? In my opinion it could be pushed as an experimental feature. It is completely optional and doesn't break anything when using Plack or CGI mode. But I'm sure the QA team has other priorities at this point of the release cycle, so don't expect it in 20.05 ;) Created attachment 103720 [details] [review] Bug 20582: Turn Koha into a Mojolicious application This patch is a proof-of-concept of Koha as a Mojolicious application This has several benefits: - Development setup is easier. No need for apache or nginx. Just run `morbo bin/intranet` or `morbo bin/opac` and go to http://localhost:3000 (URL rewrites and static files are handled by the app) - apache2/nginx configuration is simpler too (an example of nginx configuration is included in the patch) - starman and plack middlewares can still be used for debug or gzip compression for instance (see app.psgi) - Using Test::Mojo we can test the whole application, as we do with the REST API (which is a Mojolicious application too) - It opens a way for converting CGI scripts into Mojolicious controllers and actions (even if that's not possible at the moment because of the authentication code) It uses the same mechanism as Plack::App::CGIBin to deal with CGI scripts, so it should be equivalent in terms of performance How to test ? - Run `morbo bin/intranet`, then go to http://localhost:3000/ and try to find bugs. Check the REST API at http://localhost:3000/api/v1 - Run `morbo bin/opac`, then go to http://localhost:3000/ and try to find bugs. Check the REST API at http://localhost:3000/api/v1 - Run `starman -l :5000 -l :5001` and verify that intranet (http://localhost:5000) and opac (http://localhost:5001) work normally - Read the code (and the comments), it's not very long Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 103721 [details] [review] Bug 20582: Suppress warning in CGIBinKoha.pm Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> - did some browsing - a check out - a check in - created a patron - deleted a patron - searched a patron - changed a syspref - searched the catalog - deleted an item - deleted a record - created a record from Z39.50 - created an item - created a record from SRU - OPAC - logged in - searched a record - placed a hold - canceled it - added items to a public list - classic ways of accessing the intranet and OPAC with koha-testing-docker still work - went to /api/v1 when running `morbo bin/opac` and `morbo bin/intranet` - ran out of relevant ideas It would be great to see this move forward now that we're at the start of a new release cycle. I've been thinking that a standalone Mojolicious-based OPAC would be great, but that would involve a huge amount of work (even if there are < 100 OPAC .pl scripts), so maybe it's more feasible to slowly move from CGI scripts to Mojolicious routes here... Julian, what's missing to have this ready for push? Can you provide a config for apache? If I understand correctly, we are not modifying any existing Koha core files (there are very few changes on bug 20630 that should be safe) and so we could have this pushed to be tested by developers, when production sites would continue to use the standard plack architecture. It that correct? (In reply to Jonathan Druart from comment #65) > Julian, what's missing to have this ready for push? > Can you provide a config for apache? > > If I understand correctly, we are not modifying any existing Koha core files > (there are very few changes on bug 20630 that should be safe) and so we > could have this pushed to be tested by developers, when production sites > would continue to use the standard plack architecture. It that correct? This patch can be pushed safely. It will not change anything for existing and new installations. I cannot provide an apache config right now (I'm on vacation), but it should not be difficult to write one that just does ProxyPass and ProxyPassReverse, like in the provided nginx config file. I will try to write one next week. Created attachment 109282 [details] [review] Bug 20582: Fix PSGI file when behind a reverse proxy ReverseProxy middleware can modify SERVER_PORT which is used to determines which app (opac or intranet) to run, so the choice should be made before ReverseProxy middleware Created attachment 109283 [details] [review] Bug 20582: Add very simple configuration file for Apache Created attachment 109284 [details] [review] Bug 20582: Fix a cache issue in Koha::App::{Opac,Intranet} This was a workaround I wrote when Koha::Caches->flush_L1_caches was not flushing all caches correctly. Now it's not needed and it causes problems, so it must be removed. Created attachment 109895 [details] [review] Bug 20582: Turn Koha into a Mojolicious application This patch is a proof-of-concept of Koha as a Mojolicious application This has several benefits: - Development setup is easier. No need for apache or nginx. Just run `morbo bin/intranet` or `morbo bin/opac` and go to http://localhost:3000 (URL rewrites and static files are handled by the app) - apache2/nginx configuration is simpler too (an example of nginx configuration is included in the patch) - starman and plack middlewares can still be used for debug or gzip compression for instance (see app.psgi) - Using Test::Mojo we can test the whole application, as we do with the REST API (which is a Mojolicious application too) - It opens a way for converting CGI scripts into Mojolicious controllers and actions (even if that's not possible at the moment because of the authentication code) It uses the same mechanism as Plack::App::CGIBin to deal with CGI scripts, so it should be equivalent in terms of performance How to test ? - Run `morbo bin/intranet`, then go to http://localhost:3000/ and try to find bugs. Check the REST API at http://localhost:3000/api/v1 - Run `morbo bin/opac`, then go to http://localhost:3000/ and try to find bugs. Check the REST API at http://localhost:3000/api/v1 - Run `starman -l :5000 -l :5001` and verify that intranet (http://localhost:5000) and opac (http://localhost:5001) work normally - Read the code (and the comments), it's not very long Signed-off-by: Jerome Charaoui <jcharaoui@cmaisonneuve.qc.ca> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109896 [details] [review] Bug 20582: Suppress warning in CGIBinKoha.pm Signed-off-by: Jerome Charaoui <jcharaoui@cmaisonneuve.qc.ca> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109897 [details] [review] Bug 20582: Fix PSGI file when behind a reverse proxy ReverseProxy middleware can modify SERVER_PORT which is used to determines which app (opac or intranet) to run, so the choice should be made before ReverseProxy middleware Signed-off-by: Jerome Charaoui <jcharaoui@cmaisonneuve.qc.ca> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109898 [details] [review] Bug 20582: Add very simple configuration file for Apache Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109899 [details] [review] Bug 20582: Fix a cache issue in Koha::App::{Opac,Intranet} This was a workaround I wrote when Koha::Caches->flush_L1_caches was not flushing all caches correctly. Now it's not needed and it causes problems, so it must be removed. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 109900 [details] [review] Bug 20582: (QA follow-up) Add POD to satisfy coding guidelines Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> This works well and is very self contained so I'm passing QA on it. It's designed for developers only at this point, for further testing and I would suggest we encourage devs to start running with this environment as a stepping stone to developing with Mojolicious in mind. Next steps: 1) We need to think about authentication; I would suggest writing a fresh Auth mechanism designed with Mojolicious best practices in mind which can be used initially with and intercept of the existing Koha cookies. 2) Once we have the above, we can start developing new controllers as we need them in a Mojo focused way and setup the router to pass such routes directly to the Mojo based controller. 3) Pick some simple controllers to migrate Question 1) Does anyone still run under CGI only.. i.e. not via plack? If we wish to continue supporting this, we will need to allow setting the apache/niginx configs to run the Mojo app in CGI emulation (which is known to be slow.. but then CGI in Koha is now known to be slow). I eventually see hypnotoad as replacing both Apache/CGI and Plack running environments, perhaps with Nginx/Apache for static resources/Proxying. > Question
> 1) Does anyone still run under CGI only.. i.e. not via plack? If we wish to
> continue supporting this, we will need to allow setting the apache/niginx
> configs to run the Mojo app in CGI emulation (which is known to be slow..
> but then CGI in Koha is now known to be slow). I eventually see hypnotoad
> as replacing both Apache/CGI and Plack running environments, perhaps with
> Nginx/Apache for static resources/Proxying.
I'm really looking forward to what we can do with this!
We have just a couple partners in CGI only mode for Shib, but that's all.
> I'm really looking forward to what we can do with this!
>
> We have just a couple partners in CGI only mode for Shib, but that's all.
We had one for the same reason, but with 18.11 we are able to use Shib with Plack. It appears to just work and was such a performance boost.
Totally agree with you, Martin! We still have a number of instances that are CGI-only, but they're all being migrated to Plack over time. Nothing new on CGI. The community still needs to get rid of CGI dependence (even when running Plack) for some scripts, but hopefully my discovery on bug 26400 might help a little bit with that. Plus the message queue in bug 22417 for background tasks. I think if/when we can get the Mojo side authentication routines nailed we can likely more easily add in native shibboleth handling at the same time which would save us the "nasty" headers work around too :) (In reply to Martin Renvoize from comment #80) > I think if/when we can get the Mojo side authentication routines nailed we > can likely more easily add in native shibboleth handling at the same time > which would save us the "nasty" headers work around too :) I'm not familiar with the Shibboleth work, but I'm already sold on the idea. Inside KTD, when I run `starman -l :5000 -l :5001` and then hit IP:5000 I get the intranet mainpage, but I don't get the OPAC when I hit IP:5001 (I get the intranet mainpage as well). What am I missing? (In reply to Jonathan Druart from comment #82) > Inside KTD, when I run `starman -l :5000 -l :5001` and then hit IP:5000 I > get the intranet mainpage, but I don't get the OPAC when I hit IP:5001 (I > get the intranet mainpage as well). > > What am I missing? koha-testing-docker defines environment variables KOHA_INTRANET_PORT and KOHA_OPAC_PORT which are used by app.psgi to determine which ports to use. You can try `env KOHA_INTRANET_PORT=5000 KOHA_OPAC_PORT=5001 starman -l :5000 -l :5001` The "we will loose -utf8 under plack" trick in plack.psgi is needed here (see bug 13815), we cannot search with "utf-8 characters" (try biblios and patrons search) Created attachment 111282 [details] [review] Bug 20582: Reset CGI::PARAM_UTF8 to 1 before each CGI request CGI::Compile calls CGI::initialize_globals before each request, which resets PARAM_UTF8 to 0 We need to set it back to the correct value This is the same trick used in debian/templates/plack.psgi (In reply to Jonathan Druart from comment #84) > The "we will loose -utf8 under plack" trick in plack.psgi is needed here > (see bug 13815), we cannot search with "utf-8 characters" (try biblios and > patrons search) Good catch, thanks. Fixed in Koha::App::Plugin::CGIBinKoha, so it works the same whether you use morbo/hypnotoad or starman. (In reply to Julian Maurice from comment #83) > (In reply to Jonathan Druart from comment #82) > > Inside KTD, when I run `starman -l :5000 -l :5001` and then hit IP:5000 I > > get the intranet mainpage, but I don't get the OPAC when I hit IP:5001 (I > > get the intranet mainpage as well). > > > > What am I missing? > > koha-testing-docker defines environment variables KOHA_INTRANET_PORT and > KOHA_OPAC_PORT which are used by app.psgi to determine which ports to use. > You can try `env KOHA_INTRANET_PORT=5000 KOHA_OPAC_PORT=5001 starman -l > :5000 -l :5001` Then I don't understand. With the default 8080 and 8081 I was expecting to hit :8080 and :8081 and get the interfaces. Can you explain a bit more please? > - It uses the same mechanism as Plack::App::CGIBin to deal with CGI
> scripts, so it should be equivalent in terms of performance
It's not what I am experiencing.
Apache+plack, hit mainpage: between 800 and 1000ms
Mojo, hit mainpage: 1400-1700ms
(In reply to Jonathan Druart from comment #87) > Then I don't understand. With the default 8080 and 8081 I was expecting to > hit :8080 and :8081 and get the interfaces. > Can you explain a bit more please? You can use these ports, but you need to tell starman to listen on them, with starman -l :8080 -l :8081 (In reply to Jonathan Druart from comment #88) > > - It uses the same mechanism as Plack::App::CGIBin to deal with CGI > > scripts, so it should be equivalent in terms of performance > > It's not what I am experiencing. > Apache+plack, hit mainpage: between 800 and 1000ms > Mojo, hit mainpage: 1400-1700ms What are you measuring exactly ? The GET request or the whole page load with static files ? Because I get a constant time of 200ms for the GET request. If you are measuring the whole page load, then it is expected to be slower, because it handles all static files requests too. Make apache/nginx handle the static files and it should be faster. Even without apache/nginx, if I run the app with hypnotoad, the whole page (HTML + css/js) loads under 200ms (In reply to Julian Maurice from comment #89) > (In reply to Jonathan Druart from comment #87) > > Then I don't understand. With the default 8080 and 8081 I was expecting to > > hit :8080 and :8081 and get the interfaces. > > Can you explain a bit more please? > > You can use these ports, but you need to tell starman to listen on them, > with starman -l :8080 -l :8081 Ok so the correct start command would be: % starman -l :${KOHA_OPAC_PORT} -l :${KOHA_INTRANET_PORT} > (In reply to Jonathan Druart from comment #88) > > > - It uses the same mechanism as Plack::App::CGIBin to deal with CGI > > > scripts, so it should be equivalent in terms of performance > > > > It's not what I am experiencing. > > Apache+plack, hit mainpage: between 800 and 1000ms > > Mojo, hit mainpage: 1400-1700ms > > What are you measuring exactly ? The GET request or the whole page load with > static files ? > Because I get a constant time of 200ms for the GET request. > If you are measuring the whole page load, then it is expected to be slower, > because it handles all static files requests too. Make apache/nginx handle > the static files and it should be faster. > > Even without apache/nginx, if I run the app with hypnotoad, the whole page > (HTML + css/js) loads under 200ms Makes sense, thanks for the quick answer! Pushed to master for 20.11, thanks to everybody involved! Julian, t/Makefile.t and xt/check_makefile.t are failing, can you check? Created attachment 111315 [details] [review] Bug 20582: Map app.psgi file and bin directory in Makefile.PL Not sure if SCRIPT_DIR is the correct destination for these files, but I haven't found a better one. (In reply to Julian Maurice from comment #93) > Created attachment 111315 [details] [review] [review] > Bug 20582: Map app.psgi file and bin directory in Makefile.PL Patch pushed to master, thank Julian! (In reply to Julian Maurice from comment #89) > Even without apache/nginx, if I run the app with hypnotoad, the whole page > (HTML + css/js) loads under 200ms That's amazing. I've been thinking a lot about static assets this year, and if we don't have to count on a reverse proxy to handle them, that would be great (especially in terms of Docker containerization). One way popular web servers serve static content so fast is by using the sendfile(2) system call. Both Nginx and Tomcat use sendfile by default (https://github.com/nginx/nginx/blob/master/conf/nginx.conf https://tomcat.apache.org/tomcat-8.0-doc/config/http.html#NIO_specific_configuration), although Apache 2.4+ has it disabled by default (http://httpd.apache.org/docs/current/mod/core.html#enablesendfile). I've tried to look into Wildfly's undertow web server which uses non-blocking I/O too and I don't think it uses sendfile(2). I suppose non-blocking servers like undertow and hypnotoad would allow for many concurrent requests... and the added overhead of doing read/write instead of sendfile(2) goes unnoticed. Really looking forward to more progress on the hypnotoad + Mojolicious front. Ah, it looks like Wildfly's undertow system overrides the transferTo() method from java.nio.channels.FileChannel, which uses sendfile under the hood (at least on Linux/UNIX). Hypnotoad seems to use the same file handling libraries as the rest of Mojolicious (see https://github.com/mojolicious/mojo/blob/master/lib/Mojo/Asset/File.pm), and it does a read/write, which is less efficient than letting the kernel copy directly from the file to the network socket. Since Apache httpd doesn't use sendfile either, both Apache and hypnotoad would have to buffer the contents of files to serve them, so hypnotoad's ability to handle many connections via non-blocking I/O probably would make the performance difference negligible. And like I was saying... I'd love to run Koha in containers just using hypnotoad as the only process running. One could then add a caching proxy/CDN in front of Koha to further improve performance. (I also think sometimes about packaging Koha's static assets to make it so you could upload/push to a CDN ahead of time and totally remove static assets from the Koha backend, but that's a more complex scenario...) I have created a merge request on koha-testing-docker to use this easily for devs: https://gitlab.com/koha-community/koha-testing-docker/-/issues/228 Created attachment 111374 [details] [review] Bug 20582 - Turn Koha into a Mojolicious application fix package builds, by ignoring the following example files... etc/koha/apache.conf etc/koha/nginx.conf (In reply to Mason James from comment #99) > Created attachment 111374 [details] [review] [review] > Bug 20582 - Turn Koha into a Mojolicious application > > fix package builds, by ignoring the following example files... > > etc/koha/apache.conf > etc/koha/nginx.conf Patch pushed to master. Comment on attachment 109895 [details] [review] Bug 20582: Turn Koha into a Mojolicious application Review of attachment 109895 [details] [review]: ----------------------------------------------------------------- ::: bin/intranet @@ +1,1 @@ > +#!/usr/bin/env perl I know it's a bit late but this creates an inconsistency with our other files that use the #!/usr/bin/perl shebang. (In reply to David Cook from comment #101) > I know it's a bit late but this creates an inconsistency with our other > files that use the #!/usr/bin/perl shebang. Bug 26658 enhancement will not be backported to 20.05.x I haven't posted anything yet, but I have some related ideas coming in Bug 23640, which will be integrated into any current production install. (In reply to Jonathan Druart from comment #98) > I have created a merge request on koha-testing-docker to use this easily for > devs: > https://gitlab.com/koha-community/koha-testing-docker/-/issues/228 I've worked a bit more on that this morning. Can I get feedback on it please? (In reply to Jonathan Druart from comment #105) > (In reply to Jonathan Druart from comment #98) > > I have created a merge request on koha-testing-docker to use this easily for > > devs: > > https://gitlab.com/koha-community/koha-testing-docker/-/issues/228 > > I've worked a bit more on that this morning. Can I get feedback on it please? Looks good I think. I've just added 1 comment about using the --pid flag with starman to make the restart_all code a bit more robust. Actually, here's another question: why are we using starman when we could be using hypnotoad? Finally, I don't actually see myself using KOHA_MOJO_APP in koha-testing-docker unless I was testing to see how a transition to using it in production would work? (In reply to David Cook from comment #106) > Finally, I don't actually see myself using KOHA_MOJO_APP in > koha-testing-docker unless I was testing to see how a transition to using it > in production would work? The idea is to provide an easy way for developer to test it in on a daily basis. We should first catch eventual issues before knowing it is ready for production. I am not sure I understand your question actually. (In reply to Jonathan Druart from comment #107) > (In reply to David Cook from comment #106) > > Finally, I don't actually see myself using KOHA_MOJO_APP in > > koha-testing-docker unless I was testing to see how a transition to using it > > in production would work? > > The idea is to provide an easy way for developer to test it in on a daily > basis. > We should first catch eventual issues before knowing it is ready for > production. > > I am not sure I understand your question actually. Well, if I'm testing patches that we're looking to push to master, I wouldn't want to use KOHA_MOJO_APP. I'd want to use the same setup that Koha uses for production. But I understand the point that we'd want to run KOHA_MOJO_APP in dev before production. I just don't know when we would do that on an individual developer basis. Unless we were looking to switch to using Koha as a Mojolicious application for 21.05? Realizing too that app.psgi will need to be updated to include the plack.psgi changes for using timestamped logs.. (In reply to Jonathan Druart from comment #100) > > Patch pushed to master. Is anyone using this in production? BibLibre perhaps? (In reply to David Cook from comment #110) > (In reply to Jonathan Druart from comment #100) > > > > Patch pushed to master. > > Is anyone using this in production? > > BibLibre perhaps? Not in production, but I'm using it daily for development since more than a year now. The only problems I have is when `fork` is used. If you want to try, there are Docker images here: https://hub.docker.com/r/julianmaurice/koha and you can find a docker-compose.yml at https://github.com/jajm/koha-docker/tree/master/master (In reply to Julian Maurice from comment #111) > Not in production, but I'm using it daily for development since more than a > year now. The only problems I have is when `fork` is used. Oh that's cool. Yeah, same problem as the Starman then too. Slowly but surely the task queue should eliminate that problem. > If you want to try, there are Docker images here: > https://hub.docker.com/r/julianmaurice/koha and you can find a > docker-compose.yml at https://github.com/jajm/koha-docker/tree/master/master I'm intrigued but I'm pretty invested in sticking to the mainstream :/. Julian for RM next time and a move to Mojolicious? ;) (In reply to David Cook from comment #113) > Julian for RM next time and a move to Mojolicious? ;) I'll vote for that! I think this is a logical step forward. It would be great information on how the change behaves in production, but I believe it is applicable. +1 (In reply to David Cook from comment #113) > Julian for RM next time and a move to Mojolicious? ;) What do you say, Julian? Koha 24.11? > Julian for RM next time and a move to Mojolicious? lol In the end, it's "only" about that, right?: > Not in production, but I'm using it daily for development > since more than a year now. The only problems I have is when `fork` is used. So what's missing? - patron imports to use task queue - what else needs to use background workers? - but before expanding task queue usage, there is Bug 35092 - Bug 26791 ? Not task queue but seems related oh I'd love to have hypnotoad's hot restart <3 (In reply to Victor Grousset/tuxayo from comment #117) > > Not in production, but I'm using it daily for development > > since more than a year now. The only problems I have is when `fork` is used. > > So what's missing? > - patron imports to use task queue > - what else needs to use background workers? Good question. At a glance, nothing outside of t/* and misc/* directly use "fork(". "Parallel::ForkManager" seems to be in t/., xt/, debian/, and misc/. Where does patron imports fork? There are a few scripts that are still run in CGI, but since they're not proxied they shouldn't be an immediate problem I think. /etc/koha/apache-shared-intranet-plack.conf: ProxyPass "/cgi-bin/koha/tools/export.pl" "!" /etc/koha/apache-shared-intranet-plack.conf: ProxyPass "/cgi-bin/koha/tools/upload-cover-image.pl" "!" /etc/koha/apache-shared-intranet-plack.conf: ProxyPass "/cgi-bin/koha/svc/cataloguing/metasearch" "!" |