Feedback-ID: i0fe9450f:Fastmail
Date: Fri, 8 Nov 2024 13:21:08 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Aleksander Alekseev <aleksander@timescale.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: [PATCH] Refactor SLRU to always use long file names
Message-ID: <Zy2RtPwzjdtgjLnE@paquier.xyz>
References: <CAJ7c6TOy7fUW9MuNeOWor3cSFnQg9tgz=mjXHDb94GORtM_Eyg@mail.gmail.com>
 <ZuIt9-loAPa8tvcY@paquier.xyz>
 <CAJ7c6TNs8RY4Yvng4m375mAvsS4wHeUcqR8t4b2_i2j=PE6iNQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="tFBNSxGKvnxzwgES"
Content-Disposition: inline
In-Reply-To: <CAJ7c6TNs8RY4Yvng4m375mAvsS4wHeUcqR8t4b2_i2j=PE6iNQ@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/Zy2RtPwzjdtgjLnE%40paquier.xyz>
Precedence: bulk


--tFBNSxGKvnxzwgES
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Thu, Sep 12, 2024 at 12:33:14PM +0300, Aleksander Alekseev wrote:
> It wouldn't hurt re-checking the segment file names in the TAP test
> but this would mean hardcoding catalog names which as I understand you
> want to avoid. With high probability PG wouldn't start if the
> corresponding piece of pg_upgrade is wrong (I checked more than once
> :). So I'm not entirely sure if it's worth the effort, but let's see
> what others think.

+        segno = strtoi64(de->d_name, NULL, 16);
+        snprintf(new_path, MAXPGPATH, "%s/%015llX", dir_path,
+                    (long long) segno);
+        snprintf(old_path, MAXPGPATH, "%s/%s", dir_path, de->d_name);
+
+        if (pg_mv_file(old_path, new_path) != 0)
+            pg_fatal("could not rename file \"%s\" to \"%s\": %m",
+                     old_path, new_path);

Your patch is just doing a rename() of the files from short to long
names.  How about adding a new TAP script in pg_upgrade that creates a
couple of empty files with short files names in each path that needs
to do the transfer?  Then the test could run one pg_upgrade command
and check that the new names are in place.  You could use a array of
objects, with the base path, the old name and the new name, then loop
over it.  With the check in check_slru_segment_filenames() based on
SLRU_SEG_FILENAMES_CHANGE_CAT_VER, that should work.

+	static const char* dirs[] = {
+		"pg_xact",
+		"pg_commit_ts",
+		"pg_multixact/offsets",
+		"pg_multixact/members",
+		"pg_subtrans",
+		"pg_serial",
+	};

Hardcoding that is always annoying, but well, that's not going to
change.  So living with this is not going to be a maintenance burden.
--
Michael

--tFBNSxGKvnxzwgES
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmctkbQACgkQnvQgOdby
QH2dMQ/+J3JYJDnopkt0fiVbMFYjqqK0zc5589ury95j2CbPWBKAwyspbJDTg5++
lTh0OMHT4sY3Kc2MpF6I3svrFRu1oWFsO8mVQ2EISZQuKHdtK1hnYTNUbsYbIzrz
7NiKC+H/a3NbImJ51P60LNzRXhdgd6e9SAptGmnjEhJNGlDpDtnNFynCnoYdlj0v
xPr4N4bXt5QA1xvqhGflyy8XCGJUkvvDP2SMGeGmUvxjvzcqjiNj+pZJEFLycCPW
eKzF1J+3qEK0qcaoc/q49Y3X84R0IKbJkxexQQGEpV7o109EYnV7csS2B4Rkb4W0
ZD4pbmilj5CYOpBK3oXiVcKcY4JQH9dY2rfZraj56YvTYhur4ZgHwQZNcTGgGPbt
qk+4lAPN0T6ufpc+w4YUK6Ekmf2eHwe/HlOAyp6ONIPea3HNFAKdH7V28deh3/Df
cmP8lV6VP2zde/QxBBNfG5FoI+rXNnx8rxgFmk+S8oEoBZRZ8rb/sT/GruP5Y8o9
eWjd9GV+lYTPRJwI4HQBMrKIbius3dZsb9q7kL8ewpkHAWfBUjExwz+Y1rAu8TJF
bwfO4jC4tcbotN9masWkPPB19MuiHrSwXfDIj8apRWlsfaXa3cUQ+jY2Gxeptuip
+E6Uoq9zvQ8UF1+g2NHy0IGK7/22hq30aXO+pTKNqG6VDLpuwLw=
=WSsa
-----END PGP SIGNATURE-----

--tFBNSxGKvnxzwgES--