On Sat, Mar 26, 2022 at 6:33 PM Michael Paquier <mich...@paquier.xyz> wrote: > Ah, good point. I have not tested on Windows so I am not 100% sure, > but indeed it would make sense to reverse both conditions if a > junction point happens to be marked as both FILE_ATTRIBUTE_DIRECTORY > and FILE_ATTRIBUTE_REPARSE_POINT when scanning a directory. Based on > a read of the the upstream docs, I guess that this is the case: > https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-fscc/ca28ec38-f155-4768-81d6-4bfeb8586fc9 > > Note the "A file or directory that has an associated reparse point." > for the description of FILE_ATTRIBUTE_REPARSE_POINT.
That leads to the attached patches, the first of which I'd want to back-patch. Unfortunately while testing this I realised there is something else wrong here: if you take a basebackup using tar format, in-place tablespaces are skipped (they should get their own OID.tar file, but they don't, also no error). While it wasn't one of my original goals for in-place tablespaces to work in every way (and I'm certain some external tools would be confused by them), it seems we're pretty close so we should probably figure out that piece of the puzzle. It may be obvious why but I didn't have time to dig into that today... perhaps instead of just skipping the readlink() we should be writing something different into the mapfile and then restoring as appropriate...
From 68883f29d86b7cbbc9120e0c42a9f5bd8146645f Mon Sep 17 00:00:00 2001 From: Thomas Munro <thomas.mu...@gmail.com> Date: Wed, 30 Mar 2022 17:34:43 +1300 Subject: [PATCH 1/2] Fix get_dirent_type() for Windows junction points. Commit 87e6ed7c8 included code that purported to report Windows "junction" points as DT_LNK (ie the same way we report symlinks on Unix). Windows junction points are also directories according to the Windows attributes API, and we were reporting them as as DT_DIR. Change the order we check the attribute flags, to prioritize DT_LNK. If at some point we start using Windows' recently added real symlinks and need to distinguish them from junction points, we may need to rethink this, but for now this continues the tradition of wrapper functions that treat junction points as symlinks. Back-patch to 14, where get_dirent_type() landed. Discussion: https://postgr.es/m/CA%2BhUKGLzLK4PUPx0_AwXEWXOYAejU%3D7XpxnYE55Y%2Be7hB2N3FA%40mail.gmail.com --- src/port/dirent.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/port/dirent.c b/src/port/dirent.c index f67bbf7f71..b5a66ca93c 100644 --- a/src/port/dirent.c +++ b/src/port/dirent.c @@ -107,12 +107,12 @@ readdir(DIR *d) strcpy(d->ret.d_name, fd.cFileName); /* Both strings are MAX_PATH long */ d->ret.d_namlen = strlen(d->ret.d_name); /* The only identified types are: directory, regular file or symbolic link */ - if ((fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0) - d->ret.d_type = DT_DIR; /* For reparse points dwReserved0 field will contain the ReparseTag */ - else if ((fd.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0 && - (fd.dwReserved0 == IO_REPARSE_TAG_MOUNT_POINT)) + if ((fd.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0 && + (fd.dwReserved0 == IO_REPARSE_TAG_MOUNT_POINT)) d->ret.d_type = DT_LNK; + else if ((fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0) + d->ret.d_type = DT_DIR; else d->ret.d_type = DT_REG; -- 2.35.1
From f0d1dc19fd340e6206cf7b8e26d1ca6edeb62c7c Mon Sep 17 00:00:00 2001 From: Thomas Munro <thomas.mu...@gmail.com> Date: Wed, 30 Mar 2022 16:52:26 +1300 Subject: [PATCH 2/2] Remove unnecessary Windows-specific basebackup code. Commit c6f2f016 added an explicit check for a Windows "junction point", instead of the get_dirent_type() check we use for Unix. That was necessary at the time, but that turned out to be because get_dirent_type() was not working as originally intended. That's been fixed, so now we can remove the special Windows code. Add a TAP-test to demonstrate that in-place tablespaces are copied by pg_basebackup. Discussion: https://postgr.es/m/CA%2BhUKGLzLK4PUPx0_AwXEWXOYAejU%3D7XpxnYE55Y%2Be7hB2N3FA%40mail.gmail.com --- src/backend/access/transam/xlog.c | 5 ----- src/bin/pg_basebackup/t/010_pg_basebackup.pl | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 17a56152f1..3fd253413f 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -8325,13 +8325,8 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p, * we sometimes use allow_in_place_tablespaces to create * directories directly under pg_tblspc, which would fail below. */ -#ifdef WIN32 - if (!pgwin32_is_junction(fullpath)) - continue; -#else if (get_dirent_type(fullpath, de, false, ERROR) != PGFILETYPE_LNK) continue; -#endif #if defined(HAVE_READLINK) || defined(WIN32) rllen = readlink(fullpath, linkpath, sizeof(linkpath)); diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl index 47f3d00ac4..107aa8a3d8 100644 --- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl +++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl @@ -856,4 +856,21 @@ ok(pump_until($sigchld_bb, $sigchld_bb_timeout, \$sigchld_bb_stderr, 'background process exit message'); $sigchld_bb->finish(); +# Test that we can back up an in-place tablespace +$node->safe_psql('postgres', + "SET allow_in_place_tablespaces = on; CREATE TABLESPACE tblspc2 LOCATION '';"); +$node->safe_psql('postgres', + "CREATE TABLE test2 (a int) TABLESPACE tblspc2;" + . "INSERT INTO test2 VALUES (1234);"); +my $tblspc_oid = $node->safe_psql('postgres', + "SELECT oid FROM pg_tablespace WHERE spcname = 'tblspc2';"); +$node->backup('backup3'); +$node->safe_psql('postgres', "DROP TABLE test2;"); +$node->safe_psql('postgres', "DROP TABLESPACE tblspc2;"); + +# check that the in-place tablespace exists in the backup +my $backupdir = $node->backup_dir . '/backup3'; +my @dst_tblspc = glob "$backupdir/pg_tblspc/$tblspc_oid/PG_*"; +is(@dst_tblspc, 1, 'tblspc directory copied'); + done_testing(); -- 2.35.1
