[dev] Recent notification system init changes break rpc access to at least kronolith.

Michael M Slusarz slusarz at horde.org
Mon Feb 13 02:56:36 UTC 2012


Quoting Michael J Rubinsky <mrubinsk at horde.org>:

> This commit:
>   
> https://github.com/horde/horde/commit/cf3fd14107fba9d77ca83171206055cda7383a3a#framework/Core
>
> Breaks accessing kronolith via rpc.php for things like syncing and  
> api calls via RPC. I've spent a few hours tracing the code, but I'm  
> getting lost in a combination of authentication and notification  
> code...what I *do* know is:
>
> During rpc access, the 'horde' application is initialized via  
> Horde_Registry::appInit(), this instantiates $registry, during which  
> the notification system is initialized. Since we now no longer  
> filter out unauthenticated apps, every application with a  
> setupNotification method has this method called...which initializes  
> each application. This access occurs before the registry is even  
> fully constructed. In Kronolith, this leads to other applications'  
> API attempting to be called to setup e.g., timeobject calendars.

This is NOT good in _init():

     $GLOBALS['kronolith_shares'] =  
$GLOBALS['injector']->getInstance('Horde_Core_Factory_Share')->create();

_init() should not be setting up these kind of systems (since _init()  
will often times be run when not authenticated).  It should only be  
setting up the basic environment to use the application -- e.g.  
factory bindings; essential features that are fully self-contained  
within the application.

(The timezone call and the linkTags code should be moved out of init also.)

This is an offshoot of what I discovered w/xdebug a few weeks ago:  
having kronolith in the registry hammers the server, even if you  
aren't directly doing anything with it.  See, e.g.:

commit 71c28d8264b1451ea610e58a109ed97ad88e151a
Author: Michael M Slusarz <slusarz at horde.org>
Date:   Sat Jan 28 23:59:42 2012 -0700

     Begin to optimize Kronolith intialization code.

     Kronolith initialization is major performance killer according to
     xdebug.

     Still has a ways to go.  We are essentially doing both session  
initialization
     and GC on every pageload; this kind of code should only be run once per
     session though.

Removing kronolith from my registry saw a 30%+ increase in response  
time for things such as, e.g., polling the mailboxes in IMP (this was  
before implementing the notification caching).

Alternatively, I guess you could keep this stuff in _init() if you  
explicitly check that $initialApp from the registry is kronolith.

Also, the commit referenced above can't be reverted, because it fixes  
the bug of checking for notification handlers from all applications,  
regardless of whether they are authenticated to.  That should not  
change, because there has never been a limitation requiring an  
application to be authenticated to define a notification handler.

michael

___________________________________
Michael Slusarz [slusarz at horde.org]



More information about the dev mailing list