From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202410 header.b=owE/2br4; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 7293B5A004C for ; Wed, 30 Oct 2024 03:34:05 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202410; t=1730255627; bh=bFEIBPB+IpEXaZLPBu0t9oHrHLrcn+zKCc7Qh+gzA1Q=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=owE/2br43kKOar7zA5rVcltCJeVXq9QddyJzCO7Zp+RFzzhBr0MqfzOLaS6xM01kJ xHbNH69lGF8VaeI14WWvyLodgPj5JC0ywVeqsLhcXnzcoZAjAkirJhdKgGElQ/BMel TypnwhRK1XPgWjn5xXDKU53NoYReSh4jEQBNVRJ44aLmXOqxMAm3aCu0b4hpGAO19P kJoywnhoY/X+z7U9uy52a7RkAawV6cSPFLJFT6rocrsa3o0jvHUUlPOkgf9f087C4V Aj3Hpo1h50zZsI7bd6Glz4cymwmR0apBr4CKvyZKWcLO+95NoL/nVsYdzNS73UrXwC I6lJDB9LOXzNg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4XdWQg24gpz4xFb; Wed, 30 Oct 2024 13:33:47 +1100 (AEDT) Date: Wed, 30 Oct 2024 13:33:43 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v3 5/9] log: Don't use O_APPEND at all Message-ID: References: <20241028100044.939714-1-sbrivio@redhat.com> <20241028100044.939714-6-sbrivio@redhat.com> <20241029094850.206c06bc@elisabeth> <20241029112329.25f2503b@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="X339oDA8w9cNZrB1" Content-Disposition: inline In-Reply-To: <20241029112329.25f2503b@elisabeth> Message-ID-Hash: XDZEHSHQDEB7NOPKQH5Y6VH7KWCD4VKZ X-Message-ID-Hash: XDZEHSHQDEB7NOPKQH5Y6VH7KWCD4VKZ X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --X339oDA8w9cNZrB1 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Oct 29, 2024 at 11:23:29AM +0100, Stefano Brivio wrote: > On Tue, 29 Oct 2024 20:32:40 +1100 > David Gibson wrote: >=20 > > On Tue, Oct 29, 2024 at 09:48:50AM +0100, Stefano Brivio wrote: > > > On Tue, 29 Oct 2024 15:20:56 +1100 > > > David Gibson wrote: > > > =20 > > > > On Mon, Oct 28, 2024 at 11:00:40AM +0100, Stefano Brivio wrote: =20 > > > > > We open the log file with O_APPEND, but switch it off before seek= ing, > > > > > and turn it back on afterwards. > > > > >=20 > > > > > 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 th= ey > > > > > are always performed at the end regardless of the current offset > > > > > (which is at the end anyway, for us). =20 > > > >=20 > > > > Sorry, this sounded fishy to me on the call, but I figured I was ju= st > > > > missing something. But looking at this the reasoning doesn't make > > > > sense to me. > > > >=20 > > > > We don't seek with O_APPEND, but we do write(), which is exactly wh= ere > > > > 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. =20 > > >=20 > > > 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: =20 > >=20 > > 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. >=20 > I initially opened it with O_APPEND because I _thought_ I would set the > offset to a possibly inconsistent value around the rotation. >=20 > Then I dropped O_APPEND around the rotation, forgetting about the > initial reason why I added it at all. So it makes no sense to have > O_APPEND at all. Ok, that makes sense. Except that maybe there is a reason to use O_APPEND (the multiple writer thing), even if it's not the one you thought of initially. [snip] > > > > Of course the rotation process *can* clobber things (which is exact= ly > > > > why I was always a bit sceptical of this "in place" rotation, not t= hat > > > > we really have other options). =20 > > >=20 > > > 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. =20 > >=20 > > 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. >=20 > Ah, sure. But I think that supporting multiple writers would need more > work anyway (at least adding a prefix as you mentioned). That's fair. I wonder if it might make sense to flock() the logfile, to (somewhat) enforce that only one process uses it at a time. > Well, anyway, if you think this might add a regression with multiple > writers, I can add an extra flag to output_file_open() and keep > O_APPEND for the log file. But I really struggle to see the actual use > case. >=20 --=20 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/~dgibson --X339oDA8w9cNZrB1 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmchmwYACgkQzQJF27ox 2GcLXhAApPlDyI9Z06K5YIToc8VbDhqJyMJ1KzGgqouusZ17iU09dpJSsoKaJPDZ FEN3srRldHK8gQ7TK1FgrstC6zrqvBv9usWjw9Wv8zDOPK4PVmb+YH/WmlOAJsCJ sWl2fqm3KqjBFyRP17ue+hERmTqLP4lYaroXGRnwzsSP5kgeI9w07W76E2fnglnK diQfnGnUhvMXB8qW7j6o5o3EbD/Oo14x/CpaAlIsZ08SICp6NhvXtoOaCk0HMiGA v70fOMcz/JHGfuiFTsD6/37BKanAUXY3eHnYcMVEGmjqn9LDvRRuu6hgvs739H1N /pe/iXySIE2hxsWCmsCG54pXRE4Szo+c+IAzDJTl80N9Vq4bETZfbSCmg3NhfSf/ 36zYHYQ9xPzkxGRmSu7CEpcJYfWj+PTL0jdHjZ4lHI50QDFW977/QFgxc3l1t+Oz ZoHo+ttfqCfX241z2ktQpMSmamX9l/y+WBqQnTEniwKcRcEIhyREpzrlhwo96roy BIVXn9Gk9a5alKqtCdq5spRywDtqCmk2xXQRGsemXkLVuT6ybIeZQ54BZIuhwpDs /b+WahmJW6jx9VJjLcl9LUSJkUM4w04xgqG19TjVzX23aZX+MQuy623UIv/aR+p0 kNRXsh7d6NHlBfZKj8XdlPi3v5hWUmPflW3q7/OBPLWkBtXTUBs= =JTvZ -----END PGP SIGNATURE----- --X339oDA8w9cNZrB1--