On Wed, Jan 13, 2010 at 06:03:43PM -0600, Shawn Walker wrote: > >image.py: > > > > - Can you explain the use case for the allow_unprivileged option? > > Yes, to ensure these commands could work (assuming everything else > is ok) if they are not a privileged user: > > pkg install -nv > pkg image-update -nv > > ...etc. > > That's the only reason. If that isn't worthwhile, I can remove that > bit. Of course, this only works right now if all the manifests have > already been retrieved anyway.
I'm trying to discern the use for the image's NRLock in this situation. If the install/image-update -nv can't modify the image, is it necessary for the client to hold this lock without making use of flock? Would it be better to instead set a read-lock with flock in this case? > > - I don't think you need the timeout code in this set of locks. IMO, > > it complicates the code path and makes it harder for callers to get > > the lock in contended cases. Having the argument block=True/False > > would probably be sufficient for most cases. > > > > As an example, if updatemanager goes nuts and gets stuck in a > > refresh loop, the image locking code may never get the lock. This is > > because instead of blocking on the lock and getting a turn, the > > client goes to sleep and may get woken up at an arbitraily later > > point in time. If you block on the lock, you'll at least ensure > > that you get to run in between updatemanager refreshes. > > > > This code is also doing its own spinning for the NRLock, which is > > similarly less than ideal. In this case, threads within the API > > that are contending for this lock don't get the benefit of the > > mutex's fairness algorithms, nor are they awakened when the lock is > > released. > > This code does its own spinning on purpose to ensure that the total > amount of time waited includes both acquiring the NRLock and > acquiring the file lock. If clients don't want to use that logic at > all, that's what timeout=0 is for. > > I'm still not convinced that there isn't value here in the timeout > logic. Since the logic here is nearly identical to that used by > python's own mechanism for threading conditions, couldn't the same > problems happen? I don't understand why the timeout is necessary. Either the caller wants to grab the lock and is willing to wait its turn, or the caller wants to try the lock and return a suitable error message if it is already locked. The timeout doesn't do application code any good, since there's no way to know how long an operation might take. A user who wants to install now, immediately, should not block so that corrective action can occur as soon as possible. A user who is willing to wait for a pending operation to complete can't know the time and must simply block. What value am I missing? I'm not endorsing Python's locking strategy. If the interpreter puts itself to sleep during timeouts instead of using the underlying C syncrhonization objects, then yes, Python's threading is going to suffer from the same kind of fairness and scheduling problems when locks are contended. > > - line 370: If you've opened the file for reading but not writing and > > try to get an exclusive lock, you'll get an EBADF. However, that > > case isn't handled here. > > The file is always opened with 'a+', what am I missing? The pydoc > for lockf says I should check for EAGAIN and EACCES. If you don't ever expect to open/lock the file in read-only mode then you can ignore this. -j _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
