On Thu, Dec 06, 2007 at 03:54:21PM -0800, Dan Price wrote:
> On Thu 06 Dec 2007 at 02:05PM, Steve Peng wrote:
> > 
> > Please code review the following changes which address the service
> > import performance issue:
> > 
> > 6351623 Initial manifest-import is slow
> > 
> > It is under http://cr.opensolaris.org/~stevep/6351623
> > 
> > The fix is to copy the repository to the fast tmpfs, import the
> > manifests and once the import is completed copy the repository
> > back to /etc/svc.  During the verification cycle on both sparc
> > and amd64 systems, I see ~85 percent performance improvement.
> > 
> > Any comment/suggestion is greatly appreciated.
> > 
> > Thanks
> 
> Hurray!  This is going to make a huge difference for users of zones,
> who hit the slow repo import problem fairly often.  As such, I took
> a look at the code.  Mostly, I have nits.   See below.
> 
>         -dp

Continuning on Dan's comments:

The backend_switch_copy() routine needs to be more careful and more
informative on failures.  My recommendations would be as follows:

1. Every time a read or write fails you should be logging strerror()
   and perhaps also the byte offset of your location.  As a specific
   example, you need to make it very obvious to the caller when this
   operation failed for such reasons as: read-only root, out-of-disk-space.

2. You should fstat64() the backend before the copy and make sure your
   read and write passes read and wrote that many bytes.  This protects
   against bugs in this code and also against something asynchronous
   to this code mucking with the database in the middle of the operation
   (at least in a way that grows or shrinks it).

Send out an updated webrev when you have one.

-Mike

-- 
Mike Shapiro, Solaris Kernel Development. blogs.sun.com/mws/

Reply via email to