Thread: Streaming basebackups vs pg_stat_tmp

On 10/28/16 11:53 AM, Magnus Hagander wrote:
> In 9.6 and earlier, if you change pg_stat_tmp to be a symlink,
> basebackups no longer work. That's because we create symlink entry in
> the tarfile for it instead of an empty directory, but with no data,
> which Breaks Everything (TM).
>
> This was fixed in head in 6ad8ac60, which introduced "more excludes",
> due to the refactoring. That commit message refers to it also fixing
> this bug, but it seems the bugfix was never backpatched.
>
> Or did I miss something?

I don't think so.  I guess it got lost in the CF rush and also slipped 
my mind when I reviewed the final commit.

> Attached patch fixds this (based on 9.5 which is where I ran into it,
> but it needs to go in other back branches as well) by bringing back a
> (modified) version of the functoin _tarWriteDir() to the back branches.
>
> I'd appreciate a look-over before committing, but it works fine in my tests.

The patch looks sane to me, but I think it would be good to backpatch 
the TAP test from the exclusion patch that tests pg_replslot as a symlink.

-- 
-David
david@pgmasters.net

On 10/28/16 3:49 PM, Magnus Hagander wrote:
> On Fri, Oct 28, 2016 at 2:44 PM, David Steele <david@pgmasters.net
> <mailto:david@pgmasters.net>> wrote:
>     The patch looks sane to me, but I think it would be good to
>     backpatch the TAP test from the exclusion patch that tests
>     pg_replslot as a symlink.
>
> So that's the test that's in that same patch, 6ad8ac60, right? How much
> of the code for that is actually needed? (like the row which changes a
> 10 to a 11? which probably means something, but is it relevant here?) Or
> all of it?

The change from 10 to 11 increases the tests that are skipped on 
Windows, which is necessary because one extra symlink test is added.

I think you need:

-use Test::More tests => 54;
+use Test::More tests => 55;

and:
 SKIP: {
-   skip "symlinks not supported on Windows", 10 if ($windows_os);
+   skip "symlinks not supported on Windows", 11 if ($windows_os);
+
+   # Move pg_replslot out of $pgdata and create a symlink to it.
+   $node->stop;
+
+   rename("$pgdata/pg_replslot", "$tempdir/pg_replslot")
+       or BAIL_OUT "could not move $pgdata/pg_replslot";
+   symlink("$tempdir/pg_replslot", "$pgdata/pg_replslot")
+       or BAIL_OUT "could not symlink to $pgdata/pg_replslot";
+
+   $node->start;
    # Create a temporary directory in the system location and symlink it    # to our physical temp location.  That way
wecan use shorter names
 
@@ -148,6 +186,8 @@ SKIP:        "tablespace symlink was updated");    closedir $dh;

+   ok(-d "$tempdir/backup1/pg_replslot", 'pg_replslot symlink copied as 
directory');
+    mkdir "$tempdir/tbl=spc2";

The rest of the tests are for exclusions.

-- 
-David
david@pgmasters.net

On Fri, Oct 28, 2016 at 9:57 PM, David Steele <david@pgmasters.net> wrote:
> On 10/28/16 3:49 PM, Magnus Hagander wrote:
> The change from 10 to 11 increases the tests that are skipped on Windows,
> which is necessary because one extra symlink test is added.
>
> I think you need:
>
> [...]
>
> The rest of the tests are for exclusions.

Indeed, giving the attached for REL9_6_STABLE. You could as well have
a test for pg_stat_tmp but honestly that's not worth it. One thing I
have noticed is that the patch does not use _tarWriteDir() for
pg_xlog. I think it should even if that's not addressing directly a
bug...
--
Michael

Hi Magnus,

On 11/7/16 2:07 PM, Magnus Hagander wrote:
> On Sat, Oct 29, 2016 at 4:12 PM, Michael Paquier
>     Indeed, giving the attached for REL9_6_STABLE. You could as well have
>     a test for pg_stat_tmp but honestly that's not worth it. One thing I
>     have noticed is that the patch does not use _tarWriteDir() for
>     pg_xlog. I think it should even if that's not addressing directly a
>     bug...
>
> Applied and backported, thanks. Backported to 9.4, as this is where that
> exclusion code appeared.

I reviewed the three back-patches and they look sensible to me.

> I did not backport the tests, as we don't have the $node stuff available
> in 9.5 and earlier.

That's unfortunate but the changes are pretty straightforward and the 
testing is as good as it was before...

-- 
-David
david@pgmasters.net