Bug 17240 - Allow processes that rely on background jobs run in Plack mode
Summary: Allow processes that rely on background jobs run in Plack mode
Status: CLOSED INVALID
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Fridolin Somers
QA Contact: Testopia
URL:
Keywords:
Depends on: 15218
Blocks:
  Show dependency treegraph
 
Reported: 2016-09-02 10:26 UTC by Fridolin Somers
Modified: 2022-12-12 21:25 UTC (History)
7 users (show)

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


Attachments
Bug 17240 - Allow processes that rely on background jobs run in Plack mode (1.67 KB, patch)
2016-09-02 10:33 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 17240 - Allow processes that rely on background jobs run in Plack mode - prepare (1.68 KB, patch)
2016-09-02 10:34 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 17240 - Allow processes that rely on background jobs run in Plack mode - prepare (3.36 KB, patch)
2016-09-02 13:31 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 17240 - Allow processes that rely on background jobs run in Plack mode (7.76 KB, patch)
2016-09-02 13:32 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 17240 - Allow processes that rely on background jobs run in Plack mode - prepare (3.36 KB, patch)
2016-09-02 14:15 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 17240 - Allow processes that rely on background jobs run in Plack mode (7.76 KB, patch)
2016-09-02 14:15 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 17240 - Allow processes that rely on background jobs run in Plack mode - followup (2.84 KB, patch)
2016-09-06 12:48 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 17240 - Allow processes that rely on background jobs run in Plack mode - followup 2 (1.22 KB, patch)
2016-12-28 08:55 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 17240 - Allow processes that rely on background jobs run in Plack mode - followup 3 (2.38 KB, patch)
2017-05-09 14:40 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 17240 - Allow processes that rely on background jobs run in Plack mode - followup 3 (3.26 KB, patch)
2017-05-15 06:44 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 17240 - Allow processes that rely on background jobs run in Plack mode - followup 3 (3.75 KB, patch)
2017-05-31 11:00 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 17240: (prepare) Allow processes that rely on background jobs run in Plack mode (3.44 KB, patch)
2018-06-27 10:07 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 17240: Allow processes that rely on background jobs run in Plack mode (7.47 KB, patch)
2018-06-27 10:07 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 17240: (followup 1) Allow processes that rely on background jobs run in Plack mode (2.82 KB, patch)
2018-06-27 10:07 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 17240: (followup 2) Allow processes that rely on background jobs run in Plack mode (1.22 KB, patch)
2018-06-27 10:08 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 17240: (followup 3) Allow processes that rely on background jobs run in Plack mode (4.06 KB, patch)
2018-06-27 10:08 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 17240: (followup 4) Allow processes that rely on background jobs run in Plack mode (2.50 KB, patch)
2018-06-27 10:08 UTC, Fridolin Somers
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2016-09-02 10:26:45 UTC
Since Plack mode (PSGI), the processes running with background jobs are broken.

Bug 15218 corrected by using CGI mode for concerned pages.

This patch proposes to allow running those pages in PSGI mode by disabling background jobs when PSGI is used.
Comment 1 Fridolin Somers 2016-09-02 10:33:51 UTC Comment hidden (obsolete)
Comment 2 Fridolin Somers 2016-09-02 10:34:38 UTC Comment hidden (obsolete)
Comment 3 Fridolin Somers 2016-09-02 13:31:40 UTC Comment hidden (obsolete)
Comment 4 Fridolin Somers 2016-09-02 13:32:00 UTC Comment hidden (obsolete)
Comment 5 Fridolin Somers 2016-09-02 14:15:12 UTC
Created attachment 55137 [details] [review]
Bug 17240 - Allow processes that rely on background jobs run in Plack mode - prepare

This adds prepares the main one by adding method "is_psgi" to C4::Context and using it in C4::Auth for all templates.
It is used by about.pl and Koha::AuthUtils.

Test plan :
- On a Koha running with Plack
- Go to about.pl
- Look at line "PGSI:"
=> You should see "Plack"
- Enable CAS authentication
- Go to OPAC auth page
- Look at the CAS link
=> You see the current page in this URL
Comment 6 Fridolin Somers 2016-09-02 14:15:24 UTC
Created attachment 55138 [details] [review]
Bug 17240 - Allow processes that rely on background jobs run in Plack mode

Since Plack mode (PSGI), the processes running with background jobs are broken.

Bug 15218 corrected by using CGI mode for concerned pages.

This patch proposes to allow running those pages in PSGI mode by disabling background jobs when PSGI is used.

Script are running so faster with Plack, there are less chances that the process will end in HTTP timeout.

Test plan :
 - on a Koha running with plack
 - revert Bug 15218 changes
 - test processes on pages :
     offline_circ/process_koc.pl
     tools/batchMod-edit.pl
     tools/batch_record_modification.pl
     tools/manage-marc-import.pl
     tools/stage-marc-import.pl
     tools/upload-images.pl
Comment 7 Jonathan Druart 2016-09-05 09:51:21 UTC
Not sure it makes sense to disable the background mode. Did you try and edit 100k bibliographic records or items or patrons?
Comment 8 Tomás Cohen Arazi 2016-09-05 14:58:03 UTC
I don't agree with the proposed solution (we should make it work). But I love the first patch, it should have it's own bug, something like "Add C4::Context->is_psgi" in the title :-D
Comment 9 Marcel de Rooy 2016-09-05 19:43:47 UTC
We should probably remove all fork calls. Resolve it with an ajax call (like the upload in one of these scripts) or via a queue serviced by some daemon.
In general, it seems better to prevent starting jobs directly by the web interface.
Comment 10 Fridolin Somers 2016-09-06 10:42:05 UTC
(In reply to Jonathan Druart from comment #7)
> Not sure it makes sense to disable the background mode. Did you try and edit
> 100k bibliographic records or items or patrons?

(In reply to Tomás Cohen Arazi from comment #8)
> I don't agree with the proposed solution (we should make it work).

But since in PSGI mode it is broken, why not make it work quickly ?
A revamping may take a while.
Comment 11 Fridolin Somers 2016-09-06 12:48:28 UTC
Created attachment 55257 [details] [review]
Bug 17240 - Allow processes that rely on background jobs run in Plack mode - followup

Followup
If not running in background mode, some calls are made on $job var.
Comment 12 Fridolin Somers 2016-12-28 08:55:57 UTC
Created attachment 58461 [details] [review]
Bug 17240 - Allow processes that rely on background jobs run in Plack mode - followup 2

Followup 2
If not running in background mode, some calls are made on $job var in tools/batch_record_modification.pl.
Comment 13 Fridolin Somers 2017-01-11 15:11:28 UTC
I should split this into several bugs.
Comment 14 Fridolin Somers 2017-05-09 14:40:16 UTC
Created attachment 63291 [details] [review]
Bug 17240 - Allow processes that rely on background jobs run in Plack mode - followup 3

Followup 3
If not running in background mode, items are not displayed in result page in tools/batchMod.pl.
Comment 15 Fridolin Somers 2017-05-15 06:44:55 UTC
Created attachment 63459 [details] [review]
Bug 17240 - Allow processes that rely on background jobs run in Plack mode - followup 3

Followup 3
If not running in background mode, items are not displayed in result page in tools/batchMod.pl.
Also the number of items and fields are missing in message.
Comment 16 Fridolin Somers 2017-05-31 11:00:55 UTC
Created attachment 63862 [details] [review]
Bug 17240 - Allow processes that rely on background jobs run in Plack mode - followup 3

Followup 3
If not running in background mode, items are not displayed in result page in tools/batchMod.pl.
Also the number of items and fields are missing in message.
Comment 17 Jonathan Druart 2018-04-10 20:44:12 UTC
Sounds like they are running under plack already, our apache2 config looks wrong

With:
  ProxyPass "/cgi-bin/koha/tools/batch_record_modification.pl" "!"
I see logs in plack log files

With:
  ProxyPass "/intranet/tools/batch_record_modification.pl" "!"
I see them in apache log files
Comment 18 Fridolin Somers 2018-06-27 10:07:14 UTC
Created attachment 76477 [details] [review]
Bug 17240: (prepare) Allow processes that rely on background jobs run in Plack mode

This adds prepares the main one by adding method "is_psgi" to C4::Context and using it in C4::Auth for all templates.
It is used by about.pl and Koha::AuthUtils.

Test plan :
- On a Koha running with Plack
- Go to about.pl
- Look at line "PGSI:"
=> You should see "Plack"
- Enable CAS authentication
- Go to OPAC auth page
- Look at the CAS link
=> You see the current page in this URL
Comment 19 Fridolin Somers 2018-06-27 10:07:32 UTC
Created attachment 76478 [details] [review]
Bug 17240: Allow processes that rely on background jobs run in Plack mode

Since Plack mode (PSGI), the processes running with background jobs are broken.

Bug 15218 corrected by using CGI mode for concerned pages.

This patch proposes to allow running those pages in PSGI mode by disabling background jobs when PSGI is used.

Script are running so faster with Plack, there are less chances that the process will end in HTTP timeout.

Test plan :
 - on a Koha running with plack
 - revert Bug 15218 changes
 - test processes on pages :
     offline_circ/process_koc.pl
     tools/batchMod-edit.pl
     tools/batch_record_modification.pl
     tools/manage-marc-import.pl
     tools/stage-marc-import.pl
     tools/upload-images.pl
Comment 20 Fridolin Somers 2018-06-27 10:07:48 UTC
Created attachment 76479 [details] [review]
Bug 17240: (followup 1) Allow processes that rely on background jobs run in Plack mode

Followup
If not running in background mode, some calls are made on $job var.
Comment 21 Fridolin Somers 2018-06-27 10:08:04 UTC
Created attachment 76480 [details] [review]
Bug 17240: (followup 2) Allow processes that rely on background jobs run in Plack mode

Followup 2
If not running in background mode, some calls are made on $job var in tools/batch_record_modification.pl.
Comment 22 Fridolin Somers 2018-06-27 10:08:22 UTC
Created attachment 76481 [details] [review]
Bug 17240: (followup 3) Allow processes that rely on background jobs run in Plack mode

Followup 3
If not running in background mode, items are not displayed in result page in tools/batchMod.pl.
Also the number of items and fields are missing in message.
Comment 23 Fridolin Somers 2018-06-27 10:08:37 UTC
Created attachment 76482 [details] [review]
Bug 17240: (followup 4) Allow processes that rely on background jobs run in Plack mode

Items batch deletion failed because after action, unlike batch modification, items are not displayed so code must not call BuildItemsData().
Comment 24 David Cook 2019-01-29 03:20:12 UTC
(In reply to Marcel de Rooy from comment #9)
> We should probably remove all fork calls. Resolve it with an ajax call (like
> the upload in one of these scripts) or via a queue serviced by some daemon.
> In general, it seems better to prevent starting jobs directly by the web
> interface.

I totally agree.
Comment 25 Fridolin Somers 2021-12-03 21:07:40 UTC
Solved by Bug 22417 for example items edition Bug 28445