Re: [gentoo-portage-dev] Plugin backport PATCH (1/2)/(2/2)

[Brian Harring]

On Sun, Nov 20, 2005 at 12:50:25AM +0900, Jason Stubbs wrote:
<regarding FsLocks and keeping fds open while unlocked>
> > > Also, with a long running portage
> > > process and badly written code, it'd be quite easy to run out of fds...
> >
> > All locking code (stable included) has this potential though- it's not
> > something limited to this implementation.
>
> I still don't see why fds should remain open after all locks are released.
> What my point above is about is that if some external process creates a bunch
> of FsLock objects that it keeps around to obtain future locks (bad design,
> but) fds will quickly become used up when they are not actually being used
> for anything.
How about a configurable?

From where I'm sitting, running out of fds *really* shouldn't occur.  
This locking is strictly fs orientated, synchronization points on the 
fs shouldn't be too many.

Via a configurable, we can avoid open/close for every unlocked -> lock 
and lock -> unlocked transition; if fd exhaustion becomes an issue, 
we can just modify the class to ignore the configurable, so that it 
falls back to open/close during transitions.

Compromise of sorts, plus it's probably useful for the class. :)

Offhand... it's a bit nuts, but conversion of stable to the class 
based instance might be worthwhile, rather then the crap func route we 
have right now.

One issue though is that portage_locks will cycle through flock/lockf, 
then falling back to the evil hardlink locking- this code is strictly 
flock.  Thoughts on trying to clean up that crap in stable, or just 
leave the mess as it is and try and replace it down the line?

> > Code needs further work if it's going into stable imo.
> >
> > Reason I wanted the code split off from 3.0 and backported was to show
> > a file based approach rather then trying to do scans at load up.
>
> So, off topic then, eh? ;)  I should actually take the code for a spin then to
> see how it works.
Not off topic, just clarifying why I nudged alec to split this off for 
stable. :)

I'd like to keep the plugins handling on disk the same between 2.0 and 
3.0, so if we're adding a true pluging registry to 2.0, common code 
_hopefully_ can be shared. :)

<relevant code>
            if plugin_type == None:
                ptypes = [x for x in os.listdir(plugins_dir) if x.endswith(PLUGINS_EXTENSION) ]

            len_exten = len(PLUGINS_EXTENSION)
            for x in ptypes:
                c = RawConfigParser()
                c.read(os.path.join(plugins_dir, x.lstrip(os.path.sep)))
                if raw:
                    d[x[:-len_exten]] = c
                else:
                    d[x[:-len_exten]] = dict([(y, dict(c.items(y))) for y in c.sections()])

> Hmm.. I know the dict zipping bit, but no I can't see what it's doing. "x" is
> defined 6 lines above and seems to be a ptype. From what I know so far, I
> guess an example would be "cache"?
Yep, cache would be an example.

> raw is a parameter/undocumented and d is the return type. This is what I mean
> by not readable. (Sorry to be harsh/dumb)
Nah, the code honestly was something of a dump.
A refactoring would be useful, as would code feedback :)

>
> > > > > +               finally:
> > > > > +                       if locking:
> > > > > +                               plug_lock.release_read_lock()
> > > > > +               if plugin_type != None:
> > > > > +                       return d[plugin_type]
> > > > > +               return d
> > > > >
> > > > > Not sure about having different return types...
> > > >
> > > > Agreed.
> > >
> > > So one method for querying what plugin types are available and another
> > > for querying plugins of a given type?
> >
> > Or one for querying the full tree, one for pulling a specific
> > branch...
>
> Or all three? ;)
>
> This is a difference in style, I guess. A list of plugin types plus a method
> to enumerate registered plugins for each type allows one to create a tree of
> plugins on one's own. Providing the tree of plugins means one has to extract
> information from a return and then throw away the rest. Performance-wise
> they're probably both about the same when talking the non-catered case...
Offhand... list of plugin types isn't known.  The class is generic, 
locking down the plugin types allowed should/would be implemented in a 
derivative class, or via some proxy_it evilness.

> > Curious, qualms about using a massively cleaned up version of this?
> > The reason I wanted this seperated out and backported was to go with
> > an actual registry, and abuse the existing code- specific complaints
> > with that route, beyond patch sucking?
>
> None here. For what it's worth at this late stage, this patch is much better
> than my attempt. :)

This is version 3 of the savior plugins registry.  You *really* do not 
want to see the first two I created (hence them never being in cvs).  
It's a fun bit to attempt :)

So... assuming a plugin registry with on disk files is an agreeable 
approach, what would be the initial targets?  Cache comes to mind, but 
what else?

The problem with a registry is it just points at the func/code to load 
up; for elog (fex), it's not binding any configuration information to 
it- it *could* be managed by pointing at a func that holds the 
configuration info, but that's massively unclean (plus it'll stomp on 
3.0 goals of code/config seperation).
~harring