On Tue, Oct 29, 2024 at 09:48:50AM +0100, Stefano Brivio wrote:On Tue, 29 Oct 2024 15:20:56 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:I don't entirely follow. I see why you disable O_APPEND across the rotation, but I'm not clear on why it's opened with O_APPEND in the first place, if it's not for the typical logging reason.On Mon, Oct 28, 2024 at 11:00:40AM +0100, Stefano Brivio wrote:That's not the reason why I originally added it though: it was there because I thought I would lseek() to do the rotation and possibly end up with the cursor somewhere before the end. Then restart writing, and the write would happen in the middle of the file:We open the log file with O_APPEND, but switch it off before seeking, and turn it back on afterwards. We never seek when O_APPEND is on, so we don't actually need it, as its only function is to override the offset for writes so that they are always performed at the end regardless of the current offset (which is at the end anyway, for us).Sorry, this sounded fishy to me on the call, but I figured I was just missing something. But looking at this the reasoning doesn't make sense to me. We don't seek with O_APPEND, but we do write(), which is exactly where it matters. AIUI the point of O_APPEND is that if you have multiple processes writing to the same file, they won't clobber each others writes because of a stale file pointer.$ cat append.c #include <stdio.h> #include <unistd.h> #include <string.h> #include <fcntl.h> int main(int argc, char **argv) { int flags = O_CREAT | O_TRUNC | O_WRONLY | ((argc == 3) ? O_APPEND : 0); int fd = open(argv[1], flags, S_IRUSR | S_IWUSR); char buf[BUFSIZ]; memset(buf, 'a', BUFSIZ); write(fd, buf, 10); lseek(fd, 1, SEEK_SET); memset(buf, 'b', BUFSIZ); write(fd, buf, 10); write(fd, (char *){ "\n" }, 1); return 0; } $ gcc -o append{,.c} $ ./append test append $ cat test aaaaaaaaaabbbbbbbbbb $ ./append test $ cat test abbbbbbbbbbNot necessarily. It certainly can get garbled, but individual writes of reasonable size - such as a single log line will generally complete atomically. With a text logging format, that's not ideal but often pretty decipherable. Particularly if each writer includes a prefix identifying itself.That's usually not _necessary_ for us as such, but it's perhaps valuable since it reduces the likelihood of data loss if somehow you do get two instances logging to the same file.The result will be completely unreadable anyway, so I don't think it matters for us.I mean that in the case that there are multiple writers, the rotation breaks that "no data loss, and probably readable-ish" property of O_APPEND. -- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibsonOf course the rotation process *can* clobber things (which is exactly why I was always a bit sceptical of this "in place" rotation, not that we really have other options).Why would it clobber things? logfile_rotate_fallocate() and logfile_rotate_move() take care of cutting cleanly at a line boundary, and tests check that.