Re: fix tablespace handling in pg_combinebackup

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andres Freund <andres(at)anarazel(dot)de>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: fix tablespace handling in pg_combinebackup
Date: 2024-04-19 19:14:54
Message-ID: CA+Tgmoa4DrubMNqigtxOuRCWMLTUj7GT35BpHVwgDH-BWdW3AQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Apr 19, 2024 at 2:44 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > On Thu, Apr 18, 2024 at 1:45 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
> >> Just to be clear: I don't want the above to block merging your test. If you
> >> think you want to do it the way you did, please do.
>
> > I think I will go ahead and do that soon, then.
>
> This patch failed to survive contact with the buildfarm. It looks
> like the animals that are unhappy are choking like this:
>
> pg_basebackup: error: backup failed: ERROR: symbolic link target too long for tar format: file name "pg_tblspc/16415", target "/home/bf/bf-build/olingo/HEAD/pgsql.build/testrun/pg_combinebackup/002_compare_backups/data/tmp_test_bV72/ts"
>
> So whether it works depends on how long the path to the animal's build
> root is.
>
> This is not good at all, because if the buildfarm is hitting this
> limit then actual users are likely to hit it as well. But doesn't
> POSIX define some way to get longer symlink paths into tar format?
> (If not POSIX, I bet there's a widely-accepted GNU extension.)

Ah, crap. That sucks. As far as I've been able to find, we have no
code in the tree that knows how to generate symlinks longer than 99
characters (see tarCreateHeader). I can search around and see if I can
find something else out there on the Internet.

I feel like this is not a new problem but one I've had to dodge
before. In fact, I think I had to dodge it locally when developing
this patch. I believe that in various test cases, we rely on the fact
that PostgreSQL::Test::Utils::tempdir() produces pathnames that tend
to be shorter than the ones you get if you generate a path using
$node->backup_dir or $node->basedir or whatever. And I think if I
don't do it that way, it fails even locally on my machine. But, I
thought that were using that workaround elsewhere successfully, so I
expected it to be OK.

But I think in 010_pg_basebackup.pl we actually work harder to avoid
the problem than I had realized:

my $sys_tempdir = PostgreSQL::Test::Utils::tempdir_short;
...
# Test backup of a tablespace using tar format.
# Symlink the system located tempdir to our physical temp location.
# That way we can use shorter names for the tablespace directories,
# which hopefully won't run afoul of the 99 character length limit.
my $real_sys_tempdir = "$sys_tempdir/tempdir";
dir_symlink "$tempdir", $real_sys_tempdir;

mkdir "$tempdir/tblspc1";
my $realTsDir = "$real_sys_tempdir/tblspc1";

Maybe I need to clone that workaround here.

--
Robert Haas
EDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2024-04-19 19:24:17 Re: Support a wildcard in backtrace_functions
Previous Message Robert Haas 2024-04-19 19:04:27 Re: Support a wildcard in backtrace_functions