Saso, thanks for your continued work to clean this up. Here are some more comments:
- terminology: let's use similar terms as with the intent log: there are not several logs on each l2arc device. There is one log composed of a linked list of log blocks. (So, s/log/log block/ in the code/comments) - why do we need to persist the L2ARC_PERSIST_FLAGS? (e.g. L2CACHE - aren't they all in l2arc) - L2ARC_CHECK_REBUILD_TIMEOUT - unused macro - l2arc_rebuild_timeout: comment implies that rebuild happens before import completes; i thought it rebuilt in background? - calling it an uberblock is confusing (e.g. line 4415); maybe l2arc_dev_hdr_t? - reconstruct time is not that critical. We could simplify the code and on-disk format, and make it easier to convince ourselves that the code is correct, if we remove the prefetch & double pointer stuff from reconstruct code. It should be sufficient to issue one i/o at a time (rather than 2), perhaps issuing the i/o for the next zio while creating l2arc_hdr_t’s for this blocks' entries. - how can there be infinite loops of logs? validity should be determined by not straddling invalid range (between end and begin) - Consider storing the “uberblock” in the MOS - then don’t have to worry about atomic overwrite, self-checksum, magic number. Update the in-memory version as we do now, and just write it into the MOS every txg (if it's dirty). On Thu, Oct 10, 2013 at 6:49 PM, Saso Kiselkov <[email protected]>wrote: > I've decided to resurrect my original persistent L2ARC webrev from about > a year ago and rework it with additional design advice kindly provided > by Matt Ahrens. Interested folks, please take a preliminary view of this > changeset and let's get the ball rolling on getting this integrated. > > http://cr.illumos.org/~webrev/skiselkov/3525_simplified/ > > This partial rewrite accomplishes several important objectives: > > 1) Makes the design simpler and (I hope) easier to understand. I've cut > it by ~400 lines and it's now following proper naming nomenclature, > plus I got rid of all the encoding/decoding functions. > > 2) More robust. We no longer "taste" the L2ARC device to see if there's > an uberblock there. Instead we store this attribute in the vdev > config (magic numbers are kept as fail safes, of course). > > 3) Faster rebuilds. As before, rebuilds are done in background threads > and in parallel on all available L2ARC devices, so it doesn't block > pool import, but thanks to some restructuring of the way we link the > metadata chain I was able to optimize this phase, so that we saturate > the devices much better (rebuilding ~100GB worth of L2ARC takes ~2 > seconds on my crappy little system). > > Please consider this code alpha for now, as it hasn't seen much exposure > in production yet. I'm currently beating the crap out of it on my backup > machine. Any volunteers willing to help with testing are very much > welcome, especially if you reboot your machine a lot, or have > shared-storage pools capable of simulating ungraceful takeovers. I have > reasonable confidence that it won't trash your data, so worst case is > you'll see a kernel panic (make sure you can capture that if need be). > Please do not deploy into production if you value uptime. > > Cheers, > -- > Saso > > > ------------------------------------------- > illumos-zfs > Archives: https://www.listbox.com/member/archive/182191/=now > RSS Feed: > https://www.listbox.com/member/archive/rss/182191/21635000-ebd1d460 > Modify Your Subscription: > https://www.listbox.com/member/?member_id=21635000&id_secret=21635000-73dc201a > Powered by Listbox: http://www.listbox.com >
_______________________________________________ developer mailing list [email protected] http://lists.open-zfs.org/mailman/listinfo/developer
