Mike Shapiro wrote: > 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,
Good recommendations. I wil go ahead make changes and send out update when it is done. Thanks Steve > -Mike > >
