[issue11877] Change os.fsync() to support physical backing store syncs

STINNER Victor added the comment: What is the status of this issue? This issue is interesting in the implementation of #8604 (add shutil.atomic_write()). -- ___ Python tracker _

[issue11877] Change os.fsync() to support physical backing store syncs

Steffen Daode Nurpmeso added the comment: > Are you willing to update your patch accordingly? I'm a vain rooster! I've used fullfsync() in 11877-standalone.1.diff! Sorry for that, haypo. :-/ 11877.fullsync-1.diff uses fullsync() and will instead always be provided when fsync() is available,

[issue11877] Change os.fsync() to support physical backing store syncs

Steffen Daode Nurpmeso added the comment: > > Even PEP 3151 won't help. > > I don't understand. If the syscall supposed to flush the disk's buffer > cache fails - be it fcntl() or sync_file_range() - I think the error > should be propagated, not silently ignored and replaced with another > sysc

[issue11877] Change os.fsync() to support physical backing store syncs

Charles-François Natali added the comment: > One could argue that something had happened before the fsync(2), > so that code which blindly did so is too dumb to do any right > decision anyway. Even PEP 3151 won't help. I don't understand. If the syscall supposed to flush the disk's buffe

[issue11877] Change os.fsync() to support physical backing store syncs

Steffen Daode Nurpmeso added the comment: Here is something unsorted and loose: - @neologix: One could argue that something had happened before the fsync(2), so that code which blindly did so is too dumb to do any right decision anyway. Even PEP 3151 won't help. - I favour haypos fullsy

[issue11877] Change os.fsync() to support physical backing store syncs

Charles-François Natali added the comment: Trying to revive this issue, here's a comment I left on Rietveld: """ > I don't agree, the documentation states that full_sync will cause a flush to > stable storage where supported and a plain fsync where it isn't. This code does > just that. > I thi

[issue11877] Change os.fsync() to support physical backing store syncs

Steffen Daode Nurpmeso added the comment: (This was an attachment to an empty mail message.) -- Added file: http://bugs.python.org/file22046/11877-standalone.1.diff ___ Python tracker _

[issue11877] Change os.fsync() to support physical backing store syncs

Steffen Daode Nurpmeso added the comment: diff --git a/Doc/library/os.rst b/Doc/library/os.rst --- a/Doc/library/os.rst +++ b/Doc/library/os.rst @@ -810,6 +810,35 @@ Availability: Unix, and Windows. +.. function:: fullfsync(fd) + + The POSIX standart requires that :c:func:`fsync` must tra

[issue11877] Change os.fsync() to support physical backing store syncs

STINNER Victor added the comment: See also issue #12102. -- ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: ht

[issue11877] Change os.fsync() to support physical backing store syncs

Steffen Daode Nurpmeso added the comment: Excusing myself seems to be the only "probates Mittel". @Antoine Pitrou: It was a real shame to read your mail. (It's sometimes so loud that "i don't even hear what i write".) -- ___ Python tracker

[issue11877] Change os.fsync() to support physical backing store syncs

STINNER Victor added the comment: I don't like the API because it gives a different behaviour depending on the OS and I don't see how to check that the function does really a full sync. I would prefer a new option os.fullsync() function which is like your os.fsync(fd, full_sync=False), except

[issue11877] Change os.fsync() to support physical backing store syncs

Antoine Pitrou added the comment: > I've dropped wet-her! > I hope now you're satisfied! > So the buffer cache is all which remains hot. > How deserted! I'm not sure I'm always understanding your messages well (I'm not a native English speaker), but I don't think this kind of joke is approp

[issue11877] Change os.fsync() to support physical backing store syncs

Changes by Steffen Daode Nurpmeso : Removed file: http://bugs.python.org/file21986/11877.8.diff ___ Python tracker ___ ___ Python-bugs-list ma

[issue11877] Change os.fsync() to support physical backing store syncs

Steffen Daode Nurpmeso added the comment: I've dropped wet-her! I hope now you're satisfied! So the buffer cache is all which remains hot. How deserted! > And you could also add a test (I guess that just calling fsync > with full_sync=True on a valid FD would be enough. I was able to add t

[issue11877] Change os.fsync() to support physical backing store syncs

Steffen Daode Nurpmeso added the comment: Thank you, thank you, thank you. I'm a bit irritated that a french man treats a "wet-her" as a typo! What if *i* like it?? In fact it is a fantastic physical backing store. Unbeatable. Well and about dropping the fsync() in case the fcntl() fails with

[issue11877] Change os.fsync() to support physical backing store syncs

Charles-François Natali added the comment: (I'm not sure Rietveld sent the message so I post it here, sorry in case of duplicate). Steffen, I've made a quick review of your patch, in case you're interested. I think that this functionality can be really useful to some people, and it'd be nice i

[issue11877] Change os.fsync() to support physical backing store syncs

Steffen Daode Nurpmeso added the comment: > Finally, depending on the workload, it could have a significant > performance impact. Oh yes (first replaces os.fsync() with an in-python wrapper): 18:12 ~/tmp $ ll mail ls: mail: No such file or directory 18:12 ~/tmp $ ll X-MAIL 312 -rw-r- 1 st

[issue11877] Change os.fsync() to support physical backing store syncs

Charles-François Natali added the comment: > and if they do they thus really strive for data integrity, so call > fsync() as a fallback for the security which Apple provides. Why? If I ask a full sync and it fails, I'd rather have an error returned so that I can take the appropriate decision (

[issue11877] Change os.fsync() to support physical backing store syncs

Changes by Steffen Daode Nurpmeso : Removed file: http://bugs.python.org/file21973/11877.7.diff ___ Python tracker ___ ___ Python-bugs-list ma

[issue11877] Change os.fsync() to support physical backing store syncs

Steffen Daode Nurpmeso added the comment: > So I think you should stick with the previous version (well, if the > full sync fails on other FDs, then it's another story, but in that > case it should just be dropped altogether if it's not reliable...). Strong stuff. *This* is the version which sh

[issue11877] Change os.fsync() to support physical backing store syncs

Steffen Daode Nurpmeso added the comment: [.] > OSError: [Errno 22] Invalid argument Sorry, i didn't know that. Mac OS X (2.5 and 2.6 Apple shipped): 21:43 ~/tmp $ python2.5 -c 'import os; os.fsync(1)'; echo $? 0 21:43 ~/tmp $ python2.6 -c 'import os; os.fsync(1)'; echo $? 0 21:43 ~/tmp $ pyt

[issue11877] Change os.fsync() to support physical backing store syncs

Charles-François Natali added the comment: Calling fsync on a file descriptor referring to a tty doesn't make much sense. On Linux, this fails with EINVAL: $ python -c 'import os; os.fsync(1)' Traceback (most recent call last): File "", line 1, in OSError: [Errno 22] Invalid argument So if t

[issue11877] Change os.fsync() to support physical backing store syncs

Steffen Daode Nurpmeso added the comment: Just adding more notes on that by reactivating one of haypo's links from #8604. (And: maybe some Linux documentation should be updated?) From Theodore Ts'o, http://www.linuxfoundation.org/news-media/blogs/browse/2009/03/don’t-fear-fsync: As the Ea

[issue11877] Change os.fsync() to support physical backing store syncs

Changes by Steffen Daode Nurpmeso : Removed file: http://bugs.python.org/file21953/11877.6.diff ___ Python tracker ___ ___ Python-bugs-list ma

[issue11877] Change os.fsync() to support physical backing store syncs

Changes by Steffen Daode Nurpmeso : Removed file: http://bugs.python.org/file21924/11877.5.diff ___ Python tracker ___ ___ Python-bugs-list ma

[issue11877] Change os.fsync() to support physical backing store syncs

Steffen Daode Nurpmeso added the comment: Ouch, ouch, ouch!! I'll have to send 11877.7.diff which extends 11877.6.diff. This is necessary because using fcntl(2) with F_FULLFSYNC may fail with ENOTTY (inapprobiate ioctl for device) in situations where a normal fsync(2) succeeds (e.g. STDOUT_FILEN

[issue11877] Change os.fsync() to support physical backing store syncs

Steffen Daode Nurpmeso added the comment: I don't agree with you and i don't believe it is implemented like that. But it seems i am the only one on this issue who sees it like that. Thus i apply 11877.6.diff. > Declaring variables as "auto" is not necessary in C code and not > used anywhere e

[issue11877] Change os.fsync() to support physical backing store syncs

Charles-François Natali added the comment: Steffen, you changed the default to doing a "full sync" in your last patch: while I was favoring that initially, I now agree with Ronald and Antoine, and think that we shouldn't change the default behaviour. The reason being that Apple and NetBSD fol

[issue11877] Change os.fsync() to support physical backing store syncs

Steffen Daode Nurpmeso added the comment: Ronald Oussoren wrote (2011-05-08 10:33+0200): > Steffen, I don't understand your comment about "auto". Declaring > variables as "auto" is not necessary in C code and not used > anywhere else in Python's source code. Well, as long as i can keep my under

[issue11877] Change os.fsync() to support physical backing store syncs

Ronald Oussoren added the comment: Steffen, I don't understand your comment about "auto". Declaring variables as "auto" is not necessary in C code and not used anywhere else in Python's source code. -- ___ Python tracker

[issue11877] Change os.fsync() to support physical backing store syncs

Changes by Steffen Daode Nurpmeso : Removed file: http://bugs.python.org/file21771/11877.4.diff ___ Python tracker ___ ___ Python-bugs-list ma

[issue11877] Change os.fsync() to support physical backing store syncs

Changes by Steffen Daode Nurpmeso : Removed file: http://bugs.python.org/file21749/11877.3.diff ___ Python tracker ___ ___ Python-bugs-list ma

[issue11877] Change os.fsync() to support physical backing store syncs

Steffen Daode Nurpmeso added the comment: 11877.5.diff incorporates all changes suggested by Charles-François except for the 'auto' keyword, which is extremely important and which would need to be invented if it would not yet exist. I'm dropping the old stuff. And i think this is the final ve

[issue11877] Change os.fsync() to support physical backing store syncs

Steffen Daode Nurpmeso added the comment: On Sat, 7 May 2011 14:20:51 +0200, Charles-François Natali wrote: > # ifdef __APPLE__ > res = fcntl(fd, F_FULLFSYNC); > # endif > # ifdef __NetBSD__ > res = fsync_range(fd, FFILESYNC|FDISKSYNC, 0, 0); > # endif > > Since __APPLE__ and __NetBSD_

[issue11877] Change os.fsync() to support physical backing store syncs

Steffen Daode Nurpmeso added the comment: On Sat, 7 May 2011 14:20:51 +0200, Charles-François Natali wrote: > I really can't see a good reason for using it here (and > anywhere, see http://c-faq.com/decl/auto.html). You're right. > Why are you using the "auto" storage class specifier? I know

[issue11877] Change os.fsync() to support physical backing store syncs

Charles-François Natali added the comment: > I'll attach 11877.4.diff A couple comments: static PyObject * posix_fsync(PyObject *self, PyObject *args, PyObject *kwargs) { PyObject *retval = NULL; auto PyObject *fdobj; auto int full_fsync = 1; Why are you using the "auto" storage c

[issue11877] Change os.fsync() to support physical backing store syncs

Steffen Daode Nurpmeso added the comment: I'll attach 11877.4.diff: - Docu change (i hope to be better) - Kept NetBSD after looking into http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/kern/vfs_syscalls.c?rev=1.423&content-type=text/x-cvsweb-markup&only_with_tag=MAIN because fsync_range() wi

[issue11877] Change os.fsync() to support physical backing store syncs

Charles-Francois Natali added the comment: > and it seems - as far as i understand what i read - that you're > still right; and, furthermore, that fsync() does everything > anyway.  (But here an idiot is talking about *very* complicated > stuff.) > I just double-checked, and indeed, fsync does

[issue11877] Change os.fsync() to support physical backing store syncs

Steffen Daode Nurpmeso added the comment: Charles-Francois Natali wrote: > I'm -10 on sync_file_range on Linux: > [...] last time I checked [...] I just looked at http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=fs/sync.c;h=c38ec163da6ccba00a0146c75606c1b548b31343;

[issue11877] Change os.fsync() to support physical backing store syncs

Charles-Francois Natali added the comment: I'm -10 on sync_file_range on Linux: - it doesn't update the file metadata, so there's a high chance of corruption after a crash - last time I checked, it didn't flush the disk cache (well, it probably does if barriers are enabled, but that's also the

[issue11877] Change os.fsync() to support physical backing store syncs

Changes by Santoso Wijaya : -- nosy: +santa4nt ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.pyth

[issue11877] Change os.fsync() to support physical backing store syncs

Changes by Steffen Daode Nurpmeso : Removed file: http://bugs.python.org/file21748/11877.2.diff ___ Python tracker ___ ___ Python-bugs-list ma

[issue11877] Change os.fsync() to support physical backing store syncs

Changes by Steffen Daode Nurpmeso : Removed file: http://bugs.python.org/file21742/11877.optarg-1.diff ___ Python tracker ___ ___ Python-bugs-

[issue11877] Change os.fsync() to support physical backing store syncs

Steffen Daode Nurpmeso added the comment: Ok, 11877.3.diff uses either-or. -- Added file: http://bugs.python.org/file21749/11877.3.diff ___ Python tracker ___diff --git a/Doc/li

[issue11877] Change os.fsync() to support physical backing store syncs

STINNER Victor added the comment: 11877.2.diff: On Mac OS X, fsync(fd, full_sync=True); fsync(fd) do a full sync twice. You should restore the old file flags at fsync() exit. Or this surprising behaviour should be documented. -- ___ Python tracker

[issue11877] Change os.fsync() to support physical backing store syncs

Steffen Daode Nurpmeso added the comment: P.S.: dito Linux and NetBSD. I've reused preprocessor tests we've discovered internally over the past years, but i cannot test this here. I could test Linux next Tuesday, though. -- ___ Pyt

[issue11877] Change os.fsync() to support physical backing store syncs

Changes by Steffen Daode Nurpmeso : Removed file: http://bugs.python.org/file21718/fsync.diff ___ Python tracker ___ ___ Python-bugs-list mail

[issue11877] Change os.fsync() to support physical backing store syncs

Steffen Daode Nurpmeso added the comment: I'm convinced. And i've rewritten the patch. Now fsync(2) is always called first *regardless* of the new optional argument. The documentation is (a little bit) better now. And i've added support for NetBSD, AIX and Linux; though: - for AIX i'm testin