xfs: add heuristic to flush on rename
RFC PATCH 1/1 xfs: add heuristic to flush on rename
Add a heuristic to flush data to a file which looks like it's going through a tmpfile/rename dance, but not fsynced.
I had a report of a system with many 0-length files after package updates; as it turns out, the user had basically done 'yum update' and punched the power button when it was done.
Granted, the admin should not do this. Granted, the package manager should ensure persistence of files it updated.
Ext4, however, added a heuristic like this for just this case; someone who writes file.tmp, then renames over file, but never issues an fsync.
Now, this does smack of O_PONIES, but I would hope that it's fairly benign. If someone already synced the tmpfile, it's a no-op.
And it's not THAT far off our "flush on close if the file was truncated" heuristic.
Comments? Flames? Testing anyone would like to see?
---
fs/xfs/xfs_iops.c +13 -0
@@ -371,6 +371,19 @@ xfs_vn_rename(xfs_dentry_to_name (&oname, odentry,0 );xfs_dentry_to_name (&nname, ndentry, odentry->d_inode->i_mode); +/* +* If we are renaming a just-written file over an existing +* file, be pedantic and flush it out if it looks like somebody +* is doing a tmpfile dance, and didn't fsync. Best effort; +* ignore errors. +*/ +if (new_inode) { +xfs_inode_t *ip =XFS_I (odentry->d_inode); + +if (VN_DIRTY (VFS_I (ip)) && ip->i_delayed_blks >0 ) +filemap_flush (new_inode->i_mapping); + } +return -xfs_rename (XFS_I (odir), &oname,XFS_I (odentry->d_inode),XFS_I (ndir), &nname, new_inode ?XFS_I (new_inode) :NULL );
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
fs/xfs/xfs_iops.c xfs_vn_rename( · line 385
Quoted 12 lines
fs/xfs/xfs_iops.c +12
-0+/* +* If we are renaming a just-written file over an existing +* file, be pedantic and flush it out if it looks like somebody +* is doing a tmpfile dance, and didn't fsync. Best effort; +* ignore errors. +*/ +if (new_inode) { +xfs_inode_t *ip =XFS_I (odentry->d_inode); + +if (VN_DIRTY (VFS_I (ip)) && ip->i_delayed_blks >0 ) +filemap_flush (new_inode->i_mapping); + }
IFF we want a heuristic it should be a proper filemap_write_and_wait. a non-blocking start of I/O without waiting for it is snake oil.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 7 lines · Christoph Hellwig
Quoted 7 lines
fs/xfs/xfs_iops.c +7
-0+/* +* If we are renaming a just-written file over an existing +* file, be pedantic and flush it out if it looks like somebody +* is doing a tmpfile dance, and didn't fsync. Best effort; +* ignore errors. +*/ +if (new_inode) {
Quoted 8 lines
fs/xfs/xfs_iops.c xfs_vn_rename( · line 385
Quoted 5 lines
fs/xfs/xfs_iops.c +5
-0+xfs_inode_t *ip =XFS_I (odentry->d_inode); + +if (VN_DIRTY (VFS_I (ip)) && ip->i_delayed_blks >0 ) +filemap_flush (new_inode->i_mapping); + }IFF we want a heuristic it should be a proper
filemap_write_and_wait. a non-blocking start of I/O without waiting for it is snake oil.
Hi Christoph -
It's the same thing we do on truncated and written files, FWIW:
truncated = xfs_iflags_test_and_clear(ip, XFS_ITRUNCATED);
if (truncated) {
xfs_iflags_clear(ip, XFS_IDIRTY_RELEASE);
if (VN_DIRTY(VFS_I(ip)) && ip->i_delayed_blks > 0) {
error = -filemap_flush(VFS_I(ip)->i_mapping);
if (error)
return error;
}
}and that's served us pretty well in the past years.
I don't know if we're looking for rock-solid guarantees, or just do a little to try to save the user from themselves... blocking would make this a lot more heavyweight I suppose. Just depends on what we are willing / trying to do.
thanks,
-Eric
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 36 lines · Eric Sandeen
Add a heuristic to flush data to a file which looks like it's going through a tmpfile/rename dance, but not fsynced.
I had a report of a system with many 0-length files after package updates; as it turns out, the user had basically done 'yum update' and punched the power button when it was done.
Granted, the admin should not do this. Granted, the package manager should ensure persistence of files it updated.
Ext4, however, added a heuristic like this for just this case; someone who writes file.tmp, then renames over file, but never issues an fsync.
Now, this does smack of
O_PONIES, but I would hope that it's fairly benign. If someone already synced the tmpfile, it's a no-op.And it's not THAT far off our "flush on close if the file was truncated" heuristic.
Comments? Flames? Testing anyone would like to see?
Signed-off-by Eric Sandeen <[email protected]>---
fs/xfs/xfs_iops.c +1
-0@@ -371,6 +371,19 @@ xfs_vn_rename(xfs_dentry_to_name (&oname, odentry,0 );xfs_dentry_to_name (&nname, ndentry, odentry->d_inode->i_mode); +/*
fs/xfs/xfs_iops.c xfs_vn_rename( · line 384
Quoted 10 lines
fs/xfs/xfs_iops.c +10
-0+* If we are renaming a just-written file over an existing +* file, be pedantic and flush it out if it looks like somebody +* is doing a tmpfile dance, and didn't fsync. Best effort; +* ignore errors. +*/ +if (new_inode) { +xfs_inode_t *ip =XFS_I (odentry->d_inode); + +if (VN_DIRTY (VFS_I (ip)) && ip->i_delayed_blks >0 ) +filemap_flush (new_inode->i_mapping);
Uhhh I flushed the wrong inode (thanks Brian!) but you get the idea ;)
should be:
+ filemap_flush(odentry->d_inode->i_mapping);
-Eric
Quoted 11 lines
fs/xfs/xfs_iops.c +2
-0+ } +return -xfs_rename (XFS_I (odir), &oname,XFS_I (odentry->d_inode),XFS_I (ndir), &nname, new_inode ?XFS_I (new_inode) :NULL );_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 7 lines · Eric Sandeen
Add a heuristic to flush data to a file which looks like it's going through a tmpfile/rename dance, but not fsynced.
I had a report of a system with many 0-length files after package updates; as it turns out, the user had basically done 'yum update' and punched the power button when it was done.
So yum didn't run sync() on completion of the update? That seems rather dangerous to me - IMO system updates need to be guaranteed to be stable by the update mechanisms, not to leave the system state to chance if power fails or the system crashes immediately after an update...
Quoted 2 lines
Granted, the admin should not do this. Granted, the package manager should ensure persistence of files it updated.
Yes, yes it should. Problem solved without needing to touch XFS.
Quoted 3 lines
Ext4, however, added a heuristic like this for just this case; someone who writes file.tmp, then renames over file, but never issues an fsync.
You mean like rsync does all the time for every file it copies?
Quoted 3 lines
Now, this does smack of
O_PONIES, but I would hope that it's fairly benign. If someone already synced the tmpfile, it's a no-op.
I'd suggest it will greatly impact rsync speed and have impact on the resultant filesystem layout as it guarantees interleaving of metadata and data on disk....
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 16 lines · Dave Chinner
Quoted 7 lines · Eric Sandeen
Add a heuristic to flush data to a file which looks like it's going through a tmpfile/rename dance, but not fsynced.
I had a report of a system with many 0-length files after package updates; as it turns out, the user had basically done 'yum update' and punched the power button when it was done.
So yum didn't run
sync()on completion of the update? That seems rather dangerous to me - IMO system updates need to be guaranteed to be stable by the update mechanisms, not to leave the system state to chance if power fails or the system crashes immediately after an update...
Quoted 4 lines
Quoted 2 lines
Granted, the admin should not do this. Granted, the package manager should ensure persistence of files it updated.
Yes, yes it should. Problem solved without needing to touch XFS.
Right, I first suggested it 5 years or so ago for RPM. But hey, who knows, someday maybe.
So no need to touch XFS, just every godawful userspace app out there...
Somebody should bring up the topic to wider audience, I'm sure they'll all get fixed in short order. Wait, or did we try that already? :)
Quoted 5 lines
Quoted 3 lines
Ext4, however, added a heuristic like this for just this case; someone who writes file.tmp, then renames over file, but never issues an fsync.
You mean like rsync does all the time for every file it copies?
Yeah, I guess rsync doesn't fsync either. ;)
Quoted 7 lines
Quoted 3 lines
Now, this does smack of
O_PONIES, but I would hope that it's fairly benign. If someone already synced the tmpfile, it's a no-op.I'd suggest it will greatly impact rsync speed and have impact on the resultant filesystem layout as it guarantees interleaving of metadata and data on disk....
Ok, well, based on the responses thus far, sounds like a non-starter.
I'm not wedded to it, just thought I'd float the idea.
OTOH, it is an interesting juxtaposition to say the open O_TRUNC case is worth catching, but the tempfile overwrite case is not.
-Eric
Quoted 4 lines
Cheers,
Dave.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 17 lines · Eric Sandeen
Quoted 16 lines · Dave Chinner
Quoted 7 lines · Eric Sandeen
Add a heuristic to flush data to a file which looks like it's going through a tmpfile/rename dance, but not fsynced.
I had a report of a system with many 0-length files after package updates; as it turns out, the user had basically done 'yum update' and punched the power button when it was done.
So yum didn't run
sync()on completion of the update? That seems rather dangerous to me - IMO system updates need to be guaranteed to be stable by the update mechanisms, not to leave the system state to chance if power fails or the system crashes immediately after an update...
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
Granted, the admin should not do this. Granted, the package manager should ensure persistence of files it updated.
Yes, yes it should. Problem solved without needing to touch XFS.
Right, I first suggested it 5 years or so ago for RPM. But hey, who knows, someday maybe.
grrrrr.
Quoted 4 lines
So no need to touch XFS, just every godawful userspace app out there...
Somebody should bring up the topic to wider audience, I'm sure they'll all get fixed in short order. Wait, or did we try that already? :)
I'm not talking about any random application. Package managers are *CRITICAL SYSTEM INFRASTRUCTURE*. They should be architectected to handle failures gracefully; following *basic data integrity rules* is a non-negotiable requirement for a system upgrade procedure. Leaving the system in an indeterminate and potentially inoperable state after a successful upgrade completion is reported is a completely unacceptable outcome for any system management operation.
Critical infrastructure needs to Do Things Right, not require other people to hack around it's failings and hope that they might be able to save the system when shit goes wrong. There is no excuse for critical infrastructure developers failing to acknowledge and address the data integrity requirements of their infrastructure.
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Ext4, however, added a heuristic like this for just this case; someone who writes file.tmp, then renames over file, but never issues an fsync.
You mean like rsync does all the time for every file it copies?
Yeah, I guess rsync doesn't fsync either. ;)
That's because rsync doesn't need to sync until it completes all of the data writes. A failed
rsync can simply be re-run after the system comes back up and nothing is lost. That's a very different situation to a package manager replacing binaries that the system may need to boot, yes?
Quoted 8 lines
Quoted 7 lines
Quoted 3 lines
Now, this does smack of
O_PONIES, but I would hope that it's fairly benign. If someone already synced the tmpfile, it's a no-op.I'd suggest it will greatly impact rsync speed and have impact on the resultant filesystem layout as it guarantees interleaving of metadata and data on disk....
Quoted 6 lines
Ok, well, based on the responses thus far, sounds like a non-starter.
I'm not wedded to it, just thought I'd float the idea.
OTOH, it is an interesting juxtaposition to say the open
O_TRUNCcase is worth catching, but the tempfile overwrite case is not.
We went through this years ago - the O_TRUNC case is dealing with direct overwrite of data which we can reliably detect, usually only occurs one file at a time, has no major performance impact and data loss is almost entirely mitigated by the flush-on-close behaviour. It's a pretty reliable mitigation mechanism.
Rename often involves many files (so much larger writeback delay on async flush), it has cases we can't catch (e.g. rename of a directory containing unsynced data files) and has much more unpredictable behaviour (e.g. rename of files being actively written to). There's nothing worse than having unpredictable/non-repeatable data loss scenarios - if we can't handle all rename cases with the same guarantees, then we shouldn't provide any data integrity guarantees at all.
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 41 lines · Dave Chinner
Quoted 17 lines · Eric Sandeen
Quoted 16 lines · Dave Chinner
Quoted 7 lines · Eric Sandeen
Add a heuristic to flush data to a file which looks like it's going through a tmpfile/rename dance, but not fsynced.
I had a report of a system with many 0-length files after package updates; as it turns out, the user had basically done 'yum update' and punched the power button when it was done.
So yum didn't run
sync()on completion of the update? That seems rather dangerous to me - IMO system updates need to be guaranteed to be stable by the update mechanisms, not to leave the system state to chance if power fails or the system crashes immediately after an update...Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
Granted, the admin should not do this. Granted, the package manager should ensure persistence of files it updated.
Yes, yes it should. Problem solved without needing to touch XFS.
Right, I first suggested it 5 years or so ago for RPM. But hey, who knows, someday maybe.
grrrrr.
Quoted 4 lines
So no need to touch XFS, just every godawful userspace app out there...
Somebody should bring up the topic to wider audience, I'm sure they'll all get fixed in short order. Wait, or did we try that already? :)
I'm not talking about any random application. Package managers are *CRITICAL SYSTEM INFRASTRUCTURE*. They should be architectected to handle failures gracefully; following *basic data integrity rules* is a non-negotiable requirement for a system upgrade procedure. Leaving the system in an indeterminate and potentially inoperable state after a successful upgrade completion is reported is a completely unacceptable outcome for any system management operation.
Quoted 5 lines
Critical infrastructure needs to Do Things Right, not require other people to hack around it's failings and hope that they might be able to save the system when shit goes wrong. There is no excuse for critical infrastructure developers failing to acknowledge and address the data integrity requirements of their infrastructure.
Yeah, I know - choir, preaching, etc.
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
Quoted 3 lines
Ext4, however, added a heuristic like this for just this case; someone who writes file.tmp, then renames over file, but never issues an fsync.
You mean like rsync does all the time for every file it copies?
Quoted 7 lines
Quoted 1 line
Yeah, I guess rsync doesn't fsync either. ;)
That's because rsync doesn't need to sync until it completes all of the data writes. A failed
rsync can simply be re-run after the system comes back up and nothing is lost. That's a very different situation to a package manager replacing binaries that the system may need to boot, yes?
yeah, my point is that rsync overwrites exiting files and _never_ syncs. Not per-file, not at the end, not with any available option, AFAICT.
Different situation, yes, but arguably just as bad under the wrong circumstances.
Quoted 12 lines
Quoted 12 lines
Quoted 7 lines
Quoted 3 lines
Now, this does smack of
O_PONIES, but I would hope that it's fairly benign. If someone already synced the tmpfile, it's a no-op.I'd suggest it will greatly impact rsync speed and have impact on the resultant filesystem layout as it guarantees interleaving of metadata and data on disk....
Ok, well, based on the responses thus far, sounds like a non-starter.
I'm not wedded to it, just thought I'd float the idea.
Quoted 8 lines
Quoted 2 lines
OTOH, it is an interesting juxtaposition to say the open
O_TRUNCcase is worth catching, but the tempfile overwrite case is not.We went through this years ago - the
O_TRUNCcase is dealing with direct overwrite of data which we can reliably detect, usually only occurs one file at a time, has no major performance impact and data loss is almost entirely mitigated by the flush-on-close behaviour. It's a pretty reliable mitigation mechanism.
[citation needed] for a some of that, but *shrug*
Quoted 8 lines
Rename often involves many files (so much larger writeback delay on async flush), it has cases we can't catch (e.g. rename of a directory containing unsynced data files) and has much more unpredictable behaviour (e.g. rename of files being actively written to). There's nothing worse than having unpredictable/non-repeatable data loss scenarios - if we can't handle all rename cases with the same guarantees, then we shouldn't provide any data integrity guarantees at all.
Ok, so it's a NAK.
I'm over it already,
-Eric
Quoted 4 lines
Cheers,
Dave.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 12 lines · Eric Sandeen
Quoted 11 lines · Dave Chinner
Quoted 9 lines · Eric Sandeen
Quoted 6 lines · Dave Chinner
Quoted 3 lines · Eric Sandeen
Ext4, however, added a heuristic like this for just this case; someone who writes file.tmp, then renames over file, but never issues an fsync.
You mean like rsync does all the time for every file it copies?
Yeah, I guess rsync doesn't fsync either. ;)
Quoted 8 lines
Quoted 5 lines
That's because rsync doesn't need to sync until it completes all of the data writes. A failed
rsync can simply be re-run after the system comes back up and nothing is lost. That's a very different situation to a package manager replacing binaries that the system may need to boot, yes?yeah, my point is that rsync overwrites exiting files and _never_ syncs. Not per-file, not at the end, not with any available option, AFAICT.
But which a user can easily add.
Quoted 2 lines
Different situation, yes, but arguably just as bad under the wrong circumstances.
Which is why rsync provides this:
$ zcat /usr/share/doc/rsync/scripts/atomic-rsync.gz .... This script lets you update a hierarchy of files in an atomic way by first creating a new hierarchy (using hard-links to leverage the existing files), and then swapping the new hierarchy into place. ....
Yes, it doesn't have a sync in it but, again, that can easily be added. The point being is that rename safety and atomic renames are something that can be solved at the application level....
Quoted 15 lines
Quoted 15 lines
Quoted 14 lines
Quoted 7 lines
Quoted 3 lines
Now, this does smack of
O_PONIES, but I would hope that it's fairly benign. If someone already synced the tmpfile, it's a no-op.I'd suggest it will greatly impact rsync speed and have impact on the resultant filesystem layout as it guarantees interleaving of metadata and data on disk....
Ok, well, based on the responses thus far, sounds like a non-starter.
I'm not wedded to it, just thought I'd float the idea.
OTOH, it is an interesting juxtaposition to say the open
O_TRUNCcase is worth catching, but the tempfile overwrite case is not.
Quoted 7 lines
Quoted 5 lines
We went through this years ago - the
O_TRUNCcase is dealing with direct overwrite of data which we can reliably detect, usually only occurs one file at a time, has no major performance impact and data loss is almost entirely mitigated by the flush-on-close behaviour. It's a pretty reliable mitigation mechanism.[citation needed] for a some of that, but *shrug*
It was internal to SGI, mainly related to Irix, unfortunately, which is where all this "avoid NULL files" stuff came from originally...
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs