There's a serious problem with Koha - it is CGI-centric. So all that singleton badness (as in context, database connection, cache) is to make global vars and  optimise performance of the existing code under CGI conditions.  I think better idea would be an app container, and develop with that in mind, rather then improving existing code base performance and then doing triple somersaults to get it going in modern setups. Even Koha:: namespace is very in line with the one CGI process paradigm.

This is an attempt to control the damage while we decide how to move on. The idea is to enable context-less module load, and improve on clunky set_context -> restore context mechanism. I have tried many pages with caching situation, and it seems to work in both old CGI and new plack setup. But I did not go to every corner.

I strongly suggest we start moving towards Koha::App container (a persistant glorified context without singletons).

Hi Srdjan,
How can we test this patch, could you provide a test plan?
Is this patch already in [pre-]production somewhere?

I have no specific plan for this. Existing code change should be transparent and is mainly in two core files
C4:Context:
* in the way it is used (initialised  and imported)
* the way sysprefs are cached
* default cache is now stored in context rather than being a singleton
Koha::Database:
* new_schema() now takes optional context param
There's a number of files which have as a consequence all Koha::Cache->get_instance calls replaced with C4::Context->cache

Arguably I should have replaced Koha::Database->[new()]->schema calls with C4::Context->schema, to get it in line with the cache change, (and maybe I will), but that would mean changing every file. Besides Koha::Database->new()->schema hurts your eyes and soul much more than C4::Context->schema so we may feel some pressure to change the way we are using the database handle.

I assume all relevant functionality is covered in the tests, and tests passing with MEMCACHED_SERVERS set is a good starting point.

Otherwise what I tried was
* going to (many) pages, checking that context is loaded properly, and the database connection is ok
* checking that sysprefs caching works

I can only suggest checking other pages which use cache.

The change is not deployed on our test/demo sites yet, but will be shortly.

What about $use_syspref_cache variable and disable_syspref_cache() method?
Shouldn't it be removed? What is your test environment? Do you use
debian/templates/plack.psgi?

(In reply to Frédéric Demians from comment #12)
> What about $use_syspref_cache variable and disable_syspref_cache() method?
> Shouldn't it be removed?

I have no idea.  disable_syspref_cache() is used in some places that do not affect this patch, ie stand-alone scripts and tests, and I'm reluctant to remove it. Although I agree that the whole disable_syspref_cache() thing does not sound right and it should have been done different way.

> What is your test environment?

Two complete OPAC + intranet sites, running under starman. We are trying to break it.

> Do you use debian/templates/plack.psgi?

No. It is suited for single site plack app servers. What we wanted here is to serve multiple sites with only one app server thus sharing the code.
I cannot see debian/templates/plack.psgi being used at all. Did I miss anything?

(In reply to Srdjan Jankovic from comment #13)
> (In reply to Frédéric Demians from comment #12)
> > What about $use_syspref_cache variable and disable_syspref_cache() method?
> > Shouldn't it be removed?
> 
> I have no idea.  disable_syspref_cache() is used in some places that do not
> affect this patch, ie stand-alone scripts and tests, and I'm reluctant to
> remove it. Although I agree that the whole disable_syspref_cache() thing
> does not sound right and it should have been done different way.

See bug 13805 to know what it's needed using Plack (yes, this is terrible and another would be greatly appreciated).

> See bug 13805 to know what it's needed using Plack (yes, this is terrible
> and another would be greatly appreciated).

It is needed as it stands now, but not needed after my patches. In fact, the most laborious part was to reorganise Context and sysprefs caching so it makes much less trouble.

Srdjan, did you have a look at bug 16044?

No, I must admit. In my  defence this one came out before it. On the bright side, 16044 covers my first patch, so once 16044 gets accepted, I will just drop the first patch. You may wish to block this bug, but I sincerely doubt that this will move before 16044 and cause trouble.
Or alternatively I can make it dependant on that patch, but not sure how "final" it is, ie how heated is the discussion.

16044 is in master now. So the first patch of this set is obsolete?

Created attachment 50713 [details] [review]
[SIGNED-OFF] bug_15562: Use do() rather than system() to execute updatedatabase.pl from installer.pl

That way:
* no external process is spawned
* code executes in the same perl process, which is required for plack
  multi-site

I have a dream. A dream that one day all code from .pl's will be in some
.pm's.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 50714 [details] [review]
[SIGNED-OFF] bug_15562: Revert "Bug 13690: use Koha::Schema only when it's needed"

This reverts commit c52bc9756a5fa1e4f6bba7ce5c58124f150c5efc.

I don't see any place where we would use KOha::Database without wanting
Koha::Schema.
require Koha::Schema interferes with some plack setups - for some reason
it calls C4::Context->import() even when C4::Context is already used.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Getting errors now when using a fresh install of kohadevbox with plack:

Could not compile /home/vagrant/kohaclone/mainpage.pl: Can't call method "get_from_cache" on an undefined value at /home/vagrant/kohaclone/C4/Context.pm line 593.
BEGIN failed--compilation aborted at /home/vagrant/kohaclone/C4/Auth.pm line 91.
Compilation failed in require at /home/vagrant/kohaclone/mainpage.pl line 25.
BEGIN failed--compilation aborted at /home/vagrant/kohaclone/mainpage.pl line 25.

(In reply to Kyle M Hall from comment #34)
> Getting errors now when using a fresh install of kohadevbox with plack:
> 
> Could not compile /home/vagrant/kohaclone/mainpage.pl: Can't call method
> "get_from_cache" on an undefined value at
> /home/vagrant/kohaclone/C4/Context.pm line 593.
> BEGIN failed--compilation aborted at /home/vagrant/kohaclone/C4/Auth.pm line
> 91.
> Compilation failed in require at /home/vagrant/kohaclone/mainpage.pl line 25.
> BEGIN failed--compilation aborted at /home/vagrant/kohaclone/mainpage.pl
> line 25.

I should note, I can get to the staff login screen just fine, but when I log in I get the error.

On the OPAC side, I can get to the main page, but all other pages cause errors even logged out.

On the OPAC side I'm getting:

Could not compile /home/vagrant/kohaclone/opac/opac-tags.pl: Attempt to reload C4/Auth.pm aborted.
Compilation failed in require at /home/vagrant/kohaclone/opac/opac-tags.pl line 38.
BEGIN failed--compilation aborted at /home/vagrant/kohaclone/opac/opac-tags.pl line 38.

(In reply to Kyle M Hall from comment #34)
> Getting errors now when using a fresh install of kohadevbox with plack:
> 
> Could not compile /home/vagrant/kohaclone/mainpage.pl: Can't call method
> "get_from_cache" on an undefined value at
> /home/vagrant/kohaclone/C4/Context.pm line 593.
> BEGIN failed--compilation aborted at /home/vagrant/kohaclone/C4/Auth.pm line
> 91.
> Compilation failed in require at /home/vagrant/kohaclone/mainpage.pl line 25.
> BEGIN failed--compilation aborted at /home/vagrant/kohaclone/mainpage.pl
> line 25.

That will teach me to test with both memcached on and off.

(In reply to Kyle M Hall from comment #36)
> On the OPAC side I'm getting:
> 
> Could not compile /home/vagrant/kohaclone/opac/opac-tags.pl: Attempt to
> reload C4/Auth.pm aborted.
> Compilation failed in require at /home/vagrant/kohaclone/opac/opac-tags.pl
> line 38.
> BEGIN failed--compilation aborted at
> /home/vagrant/kohaclone/opac/opac-tags.pl line 38.

I'm not getting that now with the amended patch. Please let me know if it persists in your env.

I think that may be a devbox-specific problem, Kyle; this works for me, at least in a single-site (OPAC/intranet) git installation.

Though note that I used koha-multi.psgi, after adding:

use Koha::Handler::Plack::CGI;

(In reply to Jesse Weaver from comment #40)
> I think that may be a devbox-specific problem, Kyle; this works for me, at
> least in a single-site (OPAC/intranet) git installation.
> 
> Though note that I used koha-multi.psgi, after adding:
> 
> use Koha::Handler::Plack::CGI;

Excellent! Do you think you can sign off on the patch set?

Created attachment 55577 [details] [review]
[SIGNED-OFF] bug_15562: Multi-host helper for plack installations

Sort of an apocalypse
* C4::Context->new() must be called with at least config file param.
  If you want current context, call C4::Context->current().
  C4::Context->some_method() will still work as is.
* Koha::Database->new_schema() now takes optional context param.
* C4::Context->set_context() and restore_context() are synched with
  database set_schema() and restore_schema().
  Created run_within_context() that wraps set_context() and
  restore_context() around code.
* Created Koha::Handler::Plack* to facilitate running same code within
  different (database) contexts.
* This initial version does not run with memcached turned on. Next patch
  will correct that.

https://bugs.koha-community.org/show_bug.cgi?id=15562

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 55578 [details] [review]
bug_15562: Removed Koha::Caches->get_instance()

There should be no cache singletons, full stop. If Koha is to move away
from .pl scripts that is.
As an interim measure Koha::Caches->get_instance() is replaced with
C4::Context->cache. In that respect it will continue to work in the
singleton-ish way if context is used as a singleton, but supports cache-per-context.

Koha::Handler::Plack->app_per_host() cache sysprefs using Context memcached.

(In reply to Srdjan Jankovic from comment #47)
> Created attachment 55578 [details] [review] [review]
> bug_15562: Removed Koha::Caches->get_instance()
> 
> There should be no cache singletons, full stop. If Koha is to move away
> from .pl scripts that is.
> As an interim measure Koha::Caches->get_instance() is replaced with
> C4::Context->cache. In that respect it will continue to work in the
> singleton-ish way if context is used as a singleton, but supports
> cache-per-context.
> 
> Koha::Handler::Plack->app_per_host() cache sysprefs using Context memcached.

Did you notice that Koha::Caches->get_instance could take a subnamespace parameter?
Currently we have 3 different subnamespaces: the default one (""), syspref and config.

Just looking at the code, I'd say it introduces a regressions: if the memcached configuration (defined in koha-conf.xml) is different than the default one (localhost:11211) it won't be used.

(In reply to Jonathan Druart from comment #48)

> Did you notice that Koha::Caches->get_instance could take a subnamespace
> parameter?
> Currently we have 3 different subnamespaces: the default one (""), syspref
> and config.

I did not see "syspref", just config, and removed it. I obviously didn't understand namespace/subnamespace thing. The pain that I'm trying to relief is using the same code container for multiple installations. I take that Koha::Caches is no different in that respect, it just hints that in the future we may support variety of ways different things can be cached. As it stands now, subnamespace only creates/uses a new instance of Koha::Cache that is same as the default one with different namespace.
I'll put it back, but just out of curiosity, there was a params hash to instantiate Koha::Cache already, why did we push subnamespaces into a separate one?
> 
> Just looking at the code, I'd say it introduces a regressions: if the
> memcached configuration (defined in koha-conf.xml) is different than the
> default one (localhost:11211) it won't be used.

I'll have a another look. The intention was to prefer ENV to koha-conf.xml.

I've been thinking about this a bit lately, and this seems like a huge task.

I support a legacy mod_perl app, and the original developers of that had difficulty with caching just within the single app itself. They weren't the best developers, but I wonder if we could really handle 2+ apps via a single Plack instance. 

Has anyone seen examples of this elsewhere on the Internet? I've never heard of anything like it before (outside CGI), but I'd be curious to hear about any.

I foresee problems with global variables in modules. Take XML::LibXSLT for example. There are several global options which are set per process, so all your Kohas would share those same options, and one Koha changing those options would affect all Kohas. I suppose we probably don't play with XML::LibXSLT too much. I think I added the ability to register functions, which is a global XML::LibXSLT option, in XSLT::Handler, but I don't think I ever used it. 

Anyway, I obviously haven't investigated very deeply into this, but I've been thinking more about using Plack locally for our multi-host environments, and I'm struggling conceptually to see how it would work. 


http://search.cpan.org/~shlomif/XML-LibXSLT-1.95/LibXSLT.pm#OPTIONS

Perhaps this would be useful: http://search.cpan.org/dist/Plack/lib/Plack/App/URLMap.pm

I guess we're already using Plack::Builder in koha.psgi... and it looks like we're using that idea in koha-multi.psgi for the most part...

I wish you all luck in this endeavour. These patches look interesting!

(In reply to David Cook from comment #52)
> I wish you all luck in this endeavour. These patches look interesting!

Thank you, I'll need it :)

Re URLMap, I am using it in Koha::Handler::Plack.
Also, thanks for the XML::LibXSLT thoughts. I think we can live with global opts same for all in this case, but I'll have a closer look.

(In reply to Srdjan Jankovic from comment #49)
> I'll put it back, but just out of curiosity, there was a params hash to
> instantiate Koha::Cache already, why did we push subnamespaces into a
> separate one?

We have 3 different "namespaces": "syspref" (for sysprefs), "config" (for things from koha-conf.xml) and the default "" one (for everything else).
That way we will be able to flush easily the sysprefs or the config and to use another namespace for tests for instance.

Created attachment 60723 [details] [review]
[SIGNED-OFF] bug_15562: Use do() rather than system() to execute updatedatabase.pl from installer.pl

That way:
* no external process is spawned
* code executes in the same perl process, which is required for plack
  multi-site

I have a dream. A dream that one day all code from .pl's will be in some
.pm's.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 60724 [details] [review]
[SIGNED-OFF] bug_15562: Revert "Bug 13690: use Koha::Schema only when it's needed"

This reverts commit c52bc9756a5fa1e4f6bba7ce5c58124f150c5efc.

I don't see any place where we would use KOha::Database without wanting
Koha::Schema.
require Koha::Schema interferes with some plack setups - for some reason
it calls C4::Context->import() even when C4::Context is already used.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 60725 [details] [review]
bug_15562: Multi-host helper for plack installations

Sort of an apocalypse
* C4::Context->new() must be called with at least config file param.
  If you want current context, call C4::Context->current().
  C4::Context->some_method() will still work as is.
* Koha::Database->new_schema() now takes optional context param.
* C4::Context->set_context() and restore_context() are synched with
  database set_schema() and restore_schema().
  Created run_within_context() that wraps set_context() and
  restore_context() around code.
* Created Koha::Handler::Plack* to facilitate running same code within
  different (database) contexts.
* This initial version does not run with memcached turned on. Next patch
  will correct that.

https://bugs.koha-community.org/show_bug.cgi?id=15562

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 60726 [details] [review]
bug_15562: Removed Koha::Caches->get_instance()

There should be no cache singletons, full stop. If Koha is to move away
from .pl scripts that is.
As an interim measure Koha::Caches->get_instance() is replaced with
C4::Context->cache. In that respect it will continue to work in the
singleton-ish way if context is used as a singleton, but supports cache-per-context.

Koha::Handler::Plack->app_per_host() cache sysprefs using Context memcached.

Created attachment 66374 [details] [review]
[SIGNED-OFF] bug_15562: Use do() rather than system() to execute updatedatabase.pl from installer.pl

That way:
* no external process is spawned
* code executes in the same perl process, which is required for plack
  multi-site

I have a dream. A dream that one day all code from .pl's will be in some
.pm's.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 66375 [details] [review]
[SIGNED-OFF] bug_15562: Revert "Bug 13690: use Koha::Schema only when it's needed"

This reverts commit c52bc9756a5fa1e4f6bba7ce5c58124f150c5efc.

I don't see any place where we would use KOha::Database without wanting
Koha::Schema.
require Koha::Schema interferes with some plack setups - for some reason
it calls C4::Context->import() even when C4::Context is already used.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 66376 [details] [review]
bug_15562: Multi-host helper for plack installations

Sort of an apocalypse
* C4::Context->new() must be called with at least config file param.
  If you want current context, call C4::Context->current().
  C4::Context->some_method() will still work as is.
* Koha::Database->new_schema() now takes optional context param.
* C4::Context->set_context() and restore_context() are synched with
  database set_schema() and restore_schema().
  Created run_within_context() that wraps set_context() and
  restore_context() around code.
* Created Koha::Handler::Plack* to facilitate running same code within
  different (database) contexts.
* This initial version does not run with memcached turned on. Next patch
  will correct that.

https://bugs.koha-community.org/show_bug.cgi?id=15562

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 66377 [details] [review]
bug_15562: Removed Koha::Caches->get_instance()

There should be no cache singletons, full stop. If Koha is to move away
from .pl scripts that is.
As an interim measure Koha::Caches->get_instance() is replaced with
C4::Context->cache. In that respect it will continue to work in the
singleton-ish way if context is used as a singleton, but supports cache-per-context.

Koha::Handler::Plack->app_per_host() cache sysprefs using Context memcached.

I keep thinking about this abstractly when I'm waiting for public transport, and still wishing you all the best Srdjan. 

I keep wondering what the threshold is for number of Koha sites on a single Plack server...

I suppose a person would use Apache or Nginx as the reverse proxy (so many virtual hosts configured there) and have them funneling traffic to a particular Plack server (or servers I suppose if we wanted to scale up)?

Anyway, Srdjan++.

Well did not think of that... It should be limited by resources available, not the application itself. It should...

I'm quite sure this will never see the light of day (although I did get it going), just too hard. However, I want to keep it alive for various reasons. The main one is: the fact that this *is* too hard tells that we are doing some things fundamentally wrong. Workarounds devised here (and most of it is exactly that - workarounds) should help understanding the problems better. Koha is being heavily optimised for the container per site scenario, but "abstractly thinking" I don't see why we should not be able to share the code. In a real world scenario you could imagine a cluster of small poor schools benefiting from that.

One day we will get rid of C4 and re-engineer Context, no CGI .pl... Maybe even move to static pages and API... Then this will become obsolete. But good to have it as a reference, until that happens.

And thanks for your wishes :) Public transport is useful even when it's horrible.

(In reply to Srdjan Jankovic from comment #64)
> Well did not think of that... It should be limited by resources available,
> not the application itself. It should...
> 

I suppose I was just thinking in terms of any leaks, but hopefully it won't leak anywhere!

> I'm quite sure this will never see the light of day (although I did get it
> going), just too hard. However, I want to keep it alive for various reasons.
> The main one is: the fact that this *is* too hard tells that we are doing
> some things fundamentally wrong. Workarounds devised here (and most of it is
> exactly that - workarounds) should help understanding the problems better.
> Koha is being heavily optimised for the container per site scenario, but
> "abstractly thinking" I don't see why we should not be able to share the
> code. In a real world scenario you could imagine a cluster of small poor
> schools benefiting from that.
> 

Well, I was thinking about that from a security perspective. At the moment, we're already sharing the code, but since it's CGI it loads it up in separate processes on the fly. If we're using multiple sites with Plack managed by a single process, then we're loading the code into memory once (yay for efficiency!) but then we're also keeping all the data for all the different sites accessible in memory by the same process which could allow things to leak across if we're not careful (booo). 

It would be too bad if this didn't see the light of day since I imagine there would be lots of people wanting to host multiple Koha instances with the same Plack instance, but I do wonder a bit how feasible it would be, although surely people do it with other apps or URLMap wouldn't exist, yeah? 

Actually, I've been thinking about Tomcat a bit lately as well. I reckon it would work similarly to Plack. You define a number of URL routes and then specify the code that you want to run for each route. So you can have lots of Java apps running under the same single Tomcat instance. I don't know how it separates them so that they don't overlap of course. 

I suppose Tomcat might do it by virtue of Java apps being written to be run on Tomcat whereas we've been shoehorning Koha to work with Plack. 

> One day we will get rid of C4 and re-engineer Context, no CGI .pl... Maybe
> even move to static pages and API... Then this will become obsolete. But
> good to have it as a reference, until that happens.
> 
> And thanks for your wishes :) Public transport is useful even when it's
> horrible.

I've been thinking for a while how nice it would be to separate the staff client and the OPAC. If I'm not mistaken, there are far fewer OPAC scripts than Staff Client scripts, so in theory writing a new OPAC would be easier than writing a new staff client. So we could build a new OPAC from the ground up that works with Plack natively. I figure library patrons should probably be the ones that demand the best performance, so why not start there, yeah? (Alternatively, if a separate OPAC is possible, it wouldn't even necessarily have to be written in Perl. Also, if we used APIs, we could use static pages with Javascript or people could access the APIs from their own websites and then we don't even have to worry about providing an OPAC - just maintaining the API.)

Then maybe once the OPAC is separated out, we could work on overhauling the staff client bit by bit. 

That's my pie in the sky dream ;). 

Alas, I have too much other work to do, but it's a dream I have nonetheless...

> but then we're also keeping all the data for all the
> different sites accessible in memory by the same process which could allow
> things to leak across if we're not careful (booo). 

That was exactly my point with Context. There should be no references to *anything* external (db, cache etc handles) outside Context. Now because we are still in this CGI frame of mind, we have all this horrible singletons. Once we are able to instantiate contexts and keep them in app containers, and pass them to request handlers we will be safe.

(In reply to Srdjan Jankovic from comment #66)
> That was exactly my point with Context. There should be no references to
> *anything* external (db, cache etc handles) outside Context. Now because we
> are still in this CGI frame of mind, we have all this horrible singletons.
> Once we are able to instantiate contexts and keep them in app containers,
> and pass them to request handlers we will be safe.

Agreed!

Even if these patches don't go in, I figure it's worthwhile getting rid of the singletons even for the sake of testing and flexibility. 

(On an unrelated note, I just noticed Starman can listen on Unix sockets, and I recall Nginx being able to reverse proxy to Unix sockets. Apparently Apache 2.4.7+ can as well. I wonder what kind of performance boost that would give. Actually, say the OPAC was separate but still server-side and using an API, calls to a Koha API over a Unix socket would probably be rather fast. I wonder if you could have a single Koha API process which needs the most resources and then have separate smaller processes for the OPAC (and perhaps staff client). Or not even have an OPAC and just let other services access the API. Although I suppose as a project we'd want to have a default OPAC app for an all-in-one solution. I suppose the architecture depends on the scale needed for Koha too. Probably easier to scale up by having lighter clients connecting to APIs and have it all service based. But then also more complicated to set up and maintain. Actually, I was also pondering having a single persistent Koha API process and then serving the OPAC (and staff client) via CGI. If they're lightweight and most of the heavy lifting is done by the API, maybe that would be a reasonable way to have a large multitenant system. But maybe that just sounds better in theory than it would be in practice. For things like record uploads, that's probably not feasible and would just involve double-handling and double-parsing. Oh well. I'll stop rambling now...)

I suggest you discuss tuits in koha-devel. 
BTW, Koha uses Starman listening on a Unix socket and it sets apache to use that.
On Debian.
We still didn't find someone to make it work on OpenSuSE.

(In reply to Tomás Cohen Arazi from comment #68)
> I suggest you discuss tuits in koha-devel. 

Good idea, Tomás. While I don't have the tuits, maybe someone else would find it interesting.

> BTW, Koha uses Starman listening on a Unix socket and it sets apache to use
> that.

According to http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=misc/plack/plackup.sh;h=2c7f5990852107408c10b096c24cdf6df8277abc;hb=HEAD, it appears to be using TCP sockets to me. 

Ah, you must be meaning http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=debian/scripts/koha-plack;h=5e1fc9cf65534628514f1b37db7192af7076c26d;hb=HEAD. That's cool.

> On Debian.
> We still didn't find someone to make it work on OpenSuSE.

I don't 100% understand what you  mean here. I don't think the Koha community ever tries to make Koha work outside of Debian (and Ubuntu). If it did, I think the debian/scripts folder would look a lot different. I certainly don't recall anyone speaking about openSUSE or any other non-Debian based distro. (I figure I'll just end up writing my own service when we get around to implementing Plack anyway.)

In this case, are you referring to this particular service script or packaging in general?

I found this bug report after being mentioned on IRC, and as a sysadmin working with Koha, I'm quite interested in this. At our public College, we like to provide an autonomous Koha instance to every student so they can work on assignments and expriment and learn the software. The instances are regularly thrown away and recreated.

We can do it without Plack, but it's somewhat painful to use, even more so with recent versions. We can have up to 120 instances available at any one time on a single server!

I was able to apply the patchset on top of master with only very few adjustments and make it work with 5 instances. But I only did very cursory test, just making sure login works and the catalogue search was available. I soon hit problems, though. For example, rebuild_zebra.pl throws a segfault.

While I understand that a following the path laid out with this patch through to the end might seem like a huge task, I would be very happy to see this patchset maintained and improved.

Created attachment 76062 [details] [review]
bug 15562: Roll-up and rebase

(In reply to Jerome Charaoui from comment #71)
> Created attachment 76062 [details] [review] [review]
> bug 15562: Roll-up and rebase

What's a "roll-up and rebase"? That's a roll-up of the existing patches as applied against a rebased branch?

Are you still having problems with Zebra with your updated patch?

I'm super interested in this issue. I'm time poor these days, but looking forward to any work that people do on it!

(In reply to David Cook from comment #72)
> What's a "roll-up and rebase"? That's a roll-up of the existing patches as
> applied against a rebased branch?

Yes. I've combined the previous patches and rebased them on top of the current master.

(In reply to David Cook from comment #73)
> Are you still having problems with Zebra with your updated patch?

Yeah, I wasn't able to fix that problem with rebuild_zebra.pl, which is quite a blocker for me. I suspect many scripts like this one would need patching for this to work correctly across the code base.

I've had more success with the patch proposed in bug 20630, which doesn't require patching the application code, and hence all support scripts like rebuild_zebra.pl work correctly.

Marking this as In discussion given none of the patches apply and appear to need some more work still.

Due to the age of this one and the nature of the bug, I'm going to mark it as a duplicate of Bug 20630.

*** This bug has been marked as a duplicate of bug 20630 ***

Or maybe even better drop this one. Things have moved on obviously. Take those small patches if you find them useful.