I think the group in 4 is quite huge since the bug has the potential to already having hit everyone using filesystems with compression!=off (in case I got it correctly) - and I wouldn't count on everyone of them having knowledge of the problem.
Since the effect will only manifest when sending, ignoring this issue could lead to the equivalent to leftover landmines inside pools that detonate when the pool datasets get send to another systems (the hardware is eventually upgraded, for example) in the future. As this is a corruption causing issue IMHO the best possible effort should be made to limit the effects of it. And as a similar problem (partly corrupted metadata) might arise again from some feature or another, couldn't it make sense to have a mechanic in place that a could tag data created prior a specific TXG as tainted in some way or another? As an example with the currently existing hole birth problems, as soon as the issue is detected an update could be pushed that would set this to holebirth-taint=MAXINT (= defect currently existing in code, new data written is possibly affected) so that the data can be handled accordingly (effectively disabling prunging holes to send so the being caught by the defect is averted) and when the system is eventually updated to a known defect-free implementaion the new codebase can detect that it is importing a pool with holebirth-taint=MAXINT and set the value of that marker to the then current TXG - so data from now on is viewed as 'ok' while the data prior to the current TXG is suspected to have defects and is in need of special treatment. As a raw idea to throw into the discussion... Gregor Am 31.08.2016 um 17:58 schrieb Paul Dagnelie: > I think that the best approach, from my perspective, is to have an > off-by-default flag that lets you ignore the effects of hole birth. > While we're doing that, we should keep trying to fix all of these > issues; we've made good progress, and I suspect there aren't many > issues left. I think this approach does at least an ok job of > addressing the needs of everyone affected: > 1) Developers can keep developing, and this approach has a fairly low > implementation cost > 2) Users new to ZFS will get to use the feature by default, and will > be unaffected by any previous hole_birth bugs, because they have no > data from before the fixes > 3) Users who already have ZFS but didn't have corrupt data will get to > use the feature by default, and also will be unaffected by earlier > bugs because their data is not corrupt. > 4) Users who already have ZFS and do have corrupt data are probably > aware of the issue; they will hopefully be paying enough attention to > modify the flag when they upgrade to the new version. They won't be > able to use hole_birth until all their old data falls out of whatever > retention policies they have set up, or they recreate it. > > A flag (undocumented?) to zfs send that forced the use of hole_birth > might not be a bad idea, because it would let group 4 take advantage > of the feature on new data they create. The only people this approach > doesn't help are users who have corrupted data, but aren't aware of > it; if we think that's a large population, we might want to take a > different approach, like the "hole_birth_fixed" featureflag. > > On Wed, Aug 31, 2016 at 4:02 AM, Rich <[email protected] > <mailto:[email protected]>> wrote: > > Hi all, > I wanted to prod this discussion back to life again, given the > existence of two more hole_birth flaws in the intervening period, > namely Illumos #7176 and ZoL #4996. > > So, if we'd opted for a feature flag to mark "fixed" before this > point, we'd be in a position of needing another one (or a > mechanism to push the enabled_txg forward) to indicate that > "actually, after THIS POINT, we don't think there's any more known > bugs." > > I dislike the idea of whack-a-mole on revisions that are "safe", > or manual intervention to detect that, but I'm not sure there's a > nicer solution than either that or defaulting to discarding > hole_birth metadata without an explicit flag to zfs send like -e > or -L. > > If anyone has any nicer suggestions, or thoughts on how to better > handle this recurring problem, I'd love to have a better answer to > this. > > Thanks, > - Rich > > On Fri, Jul 8, 2016 at 1:31 PM, Boris <[email protected] > <mailto:[email protected]>> wrote: > > So, as we found out a short time ago, this is actually fixed > in ZoL as well, but after the 0.6.5.7 > > > ------------------------------------------------------------------------ > *From:* Rich <[email protected] <mailto:[email protected]>> > *Sent:* Friday, July 8, 2016 12:37:52 PM > *To:* [email protected] > <mailto:[email protected]> > *Cc:* developer; Developer Lists Illumos > > *Subject:* Re: [developer] Improvements to 6513 handling > > Hi Boris, > I now have working code that implements this feature, and > defaults to ignoring hole_birth data for sends if this feature > is not enabled; I'm going to post patches for it after I've > tested it on both ZoL and illumos. > > The latter portion of the above is easily changed, but "always > correct and marginally less efficient for old data" seemed a > better default than "usually correct". > > For a filesystem that prides itself on not allowing silent > corruption, requiring manual detection and intervention for > correctness seems unreasonable to me. > > - Rich > > On Fri, Jul 8, 2016 at 12:15 PM, Boris > <[email protected] <mailto:[email protected]>> wrote: > > Hi, Rich, > > > I agree that unconditional switch using the tunable is > heavy if done 'as a matter of standard practice' as > opposed to 'for a short time, to fixup the known corrupted > backups'. > > > To clarify the earlier suggestions, people with data > affected by the bug can do two things: > > > 1) install the code with 6513 fix and the patch with the > tunable, then temporarily turn off the hole birth > optimization, resend the 'difference' (a selected subset > of incrementals) affected by the problem, then turn the > optimization back on > > 2) install the code with 6513 fix without the patch, do a > full send of the affected snapshots > > > For 2) the non-incremental send would need to happen only > once per the affected snapshot lineage. Once the missed > holes are re-instated with the full send, the new fixed > code will perform proper incremental sends. > > > 1) is potentially more optimal in terms of resources > (network bw, etc.) > > 2) is potentially simpler from the operational standpoint, > does not require building/installing patched code, > twiddling the tunables, etc. > > > Boris. > > > > > > ------------------------------------------------------------------------ > *From:* Rich <[email protected] > <mailto:[email protected]>> > *Sent:* Friday, July 8, 2016 11:26:27 AM > *To:* developer > *Cc:* Developer Lists Illumos > *Subject:* Re: [developer] Improvements to 6513 handling > > Hi Boris, > A full send of the affected snapshots should be safe, AIUI > - but that means people would need to do non-incremental > snapshot sends to be certain of not hitting this bug, > which becomes increasingly infeasible as your datasets grow. > > If we're looking for the simplest solution without risk of > data corruption, unconditionally ignoring the hole_birth > data for doing a zfs send fits the bill, but seems a bit > heavy-handed. > > This seemed like the best way to permit people to safely > send older datasets while also permitting use of the > hole_birth data going forward. > > - Rich > > On Fri, Jul 8, 2016 at 11:03 AM, Boris > <[email protected] <mailto:[email protected]>> > wrote: > > Hi, Rich, > > > perhaps there is a simpler solution here. > > > I think for the datasets affected by this feature, a > full (not incremental) send of the source snapshot > that has some holes that have not been transmitted by > the faulty incremental send code, should fix the > issue, as far as the on-disk layout is concerned. > > > Boris. > > > ------------------------------------------------------------------------ > *From:* Rich <[email protected] > <mailto:[email protected]>> > *Sent:* Thursday, July 7, 2016 8:30:54 PM > *To:* developer; Developer Lists Illumos > *Subject:* [developer] Improvements to 6513 handling > > Hi all, > > So, ZFS on Linux just noticed it was getting bitten by > what ultimately turned out to be Illumos #6513, > partially filled holes losing birth time. > > Implementing that fix removes this problem for new > data, but on all platforms, this doesn't help data > already written on existing pools, getting munged > silently in incremental sends forever. > > pcd pointed out that a relatively trivial workaround > would be possible by simply ignoring the hole_birth > metadata with something like a global tunable, but > that seems too heavy-handed to me - either you're > disabling the feature everywhere because you don't > know when you can start trusting the birth times, or > you're risking silent mangling of affected files forever. > > I'd like to suggest using a read-compatible feature, > call it something like hole_birth_fix, in conjunction > with the enabled_txg feature, to permit a reasonable > default of ignoring hole_birth information before the > hole_birth_fix feature was enabled, but still > permitting use of it afterward. > > This has the unfortunate behavior of breaking write > support if you enable hole_birth_fix and then try to > go back to a prior codebase, but I can't think of a > reasonable way to avoid this. > > I filed illumos #7175 to track this proposal - I'll > happily write the code to implement this shortly. > > (Apologies if I've over-CCed or missed someone I > should be asking for comment, I've not done this > workflow before.) > > - Rich > > > > > > > > -- > Paul Dagnelie > *openzfs-developer* | Archives > <https://www.listbox.com/member/archive/274414/=now> > <https://www.listbox.com/member/archive/rss/274414/28015281-a0403a3e> > | Modify > <https://www.listbox.com/member/?&;> > Your Subscription [Powered by Listbox] <http://www.listbox.com> > ------------------------------------------- openzfs-developer Archives: https://www.listbox.com/member/archive/274414/=now RSS Feed: https://www.listbox.com/member/archive/rss/274414/28015062-cce53afa Modify Your Subscription: https://www.listbox.com/member/?member_id=28015062&id_secret=28015062-f966d51c Powered by Listbox: http://www.listbox.com
