Re: A test for replay of regression tests

From: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Anastasia Lubennikova <lubennikovaav(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: A test for replay of regression tests
Date: 2021-12-04 04:21:08
Message-ID: CA+hUKG+nk_WCqi_y+9TentqwXvy=1dW0DgBB-8yTtgdTTLtP0Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Nov 30, 2021 at 3:14 AM Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
> choco install -y Carbon
> Import-Module Carbon
> Grant-CPrivilege -Identity myuser -Privilege SeCreateSymbolicLinkPrivilege

Interesting. Well, I found the problem with my last patch (to wit:
junction points must be absolute, unlike real symlinks, which I'd
considered already but I missed that tmp_check's DataDir had a stray
internal \.\), and now I'm wondering whether these newer real symlinks
could help. The constraints are pretty hard to work with... I thought
about a couple of options:

1. We could try to use real symlinks, and fall back to junction
points if that fails. That means that these new tests I'm proposing
would fail unless you grant that privilege or run in developer mode as
you were saying. It bothers me a bit that developers and the BF would
be testing a different code path than production databases run...
unless you're thinking we should switch to symlinks with no fallback,
and require that privilege to be granted by end users to production
servers at least if they want to use tablespaces, and also drop
pre-Win10 support in the same release? That's bigger than I was
thinking.

2. We could convert relative paths to absolute paths at junction
point creation time, which I tried, and "check" now passes. Problems:
(1) now you can't move pgdata around, (2) the is-the-path-too-long
check performed on a primary is not sufficient to check if the
transformed absolute path will be too long on a replica.

The most conservative simple idea I have so far is: go with option 2,
but make this whole thing an undocumented developer-only mode, and
turn it on in the regression tests. Here's a patch set like that.
Thoughts?

Another option would be to stop using operating system symlinks, and
build the target paths ourselves; I didn't investigate that as it
seemed like a bigger change than this warrants.

Next problem: The below is clearly not the right way to find the
pg_regress binary and bindir, and doesn't work on Windows or VPATH.
Any suggestions for how to do this? I feel like something like
$node->installed_command() or similar logic is needed...

# Run the regression tests against the primary.
# XXX How should we find the pg_regress binary and bindir?
system_or_bail("../regress/pg_regress",
"--bindir=../../bin/psql",
"--port=" . $node_primary->port,
"--schedule=../regress/parallel_schedule",
"--dlpath=../regress",
"--inputdir=../regress");

BTW 0002 is one of those renaming patches from git that GNU patch
doesn't seem to apply correctly, sorry cfbot...

Attachment Content-Type Size
v7-0001-Allow-restricted-relative-tablespace-paths.patch text/x-patch 7.7 KB
v7-0002-Use-relative-paths-for-tablespace-regression-test.patch text/x-patch 10.4 KB
v7-0003-Test-replay-of-regression-tests.patch text/x-patch 5.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2021-12-04 04:35:17 Re: Skip vacuum log report code in lazy_scan_heap() if possible
Previous Message Tom Lane 2021-12-04 01:22:00 Re: SPI TupTable memory context