Date: Sun, 20 Apr 2025 15:15:59 -0700
From: Noah Misch <noah@leadboat.com>
To: Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Nathan Bossart <nathandbossart@gmail.com>,
	Michael Paquier <michael@paquier.xyz>,
	Robert Pang <robertpang@google.com>,
	Kyotaro Horiguchi <horikyota.ntt@gmail.com>, robertmhaas@gmail.com,
	pgsql-hackers@postgresql.org
Subject: Re: Back-patch of: avoid multiple hard links to same WAL file after
 a crash
Message-ID: <20250420221559.ea.nmisch@google.com>
References: <20250403002900.5d.nmisch@google.com>
 <Z_ECg-lN2xQbqkLc@paquier.xyz>
 <2461391.1743865633@sss.pgh.pa.us>
 <20250405191339.8d.nmisch@google.com>
 <Z_GxukueQn7lDs1u@paquier.xyz>
 <20250406020958.95.nmisch@google.com>
 <20250413153312.12.nmisch@google.com>
 <850092.1744559517@sss.pgh.pa.us>
 <Z_xUl6EyD3Txa9V3@paquier.xyz>
 <20250420215339.e8.nmisch@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20250420215339.e8.nmisch@google.com>
User-Agent: Mutt/2.2.12 (2023-09-09)
Archived-At: <https://www.postgresql.org/message-id/20250420221559.ea.nmisch%40google.com>
Precedence: bulk

On Sun, Apr 20, 2025 at 02:53:39PM -0700, Noah Misch wrote:
> On Mon, Apr 14, 2025 at 09:19:35AM +0900, Michael Paquier wrote:
> > On Sun, Apr 13, 2025 at 11:51:57AM -0400, Tom Lane wrote:
> > > Noah Misch <noah@leadboat.com> writes:
> > > > Tom and Michael, do you still object to the test addition, or not?  If there
> > > > are no new or renewed objections by 2025-04-20, I'll proceed to add the test.
> 
> Pushed as commit 714bd9e.  The failure so far is
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2025-04-20%2015%3A36%3A35
> with these highlights:
> 
> pg_ctl: server does not shut down
> 
> 2025-04-20 17:27:35.735 UTC [1576688][postmaster][:0] LOG:  received immediate shutdown request
> 2025-04-20 17:27:35.969 UTC [1577386][archiver][:0] FATAL:  archive command was terminated by signal 3: Quit
> 2025-04-20 17:27:35.969 UTC [1577386][archiver][:0] DETAIL:  The failed archive command was: cp "pg_wal/00000001000000000000006D" "/home/bf/bf-build/skink-master/HEAD/pgsql.build/testrun/recovery/045_archive_restartpoint/data/t_045_archive_restartpoint_primary_data/archives/00000001000000000000006D"
> 
> The checkpoints and WAL creation took 30s, but archiving was only 20% done
> (based on file name 00000001000000000000006D) at the 360s PGCTLTIMEOUT.  I can
> reproduce this if I test with valgrind --trace-children=yes.  With my normal
> valgrind settings, the whole test file takes only 18s.  I recommend one of
> these changes to skink:
> 
> - Add --trace-children-skip='/bin/*,/usr/bin/*' so valgrind doesn't instrument
>   "sh" and "cp" commands.
> - Remove --trace-children=yes

I gave that more thought.  One can be more surgical than that, via
--trace-children-skip-by-arg='*cp "*' or similar.  My previous message's two
options stop valgrind instrumentation at boundaries like pg_dumpall calling
system(pg_dump ...), since that execs /bin/sh to run pg_dump.  If we wanted to
make it even more explicit and surgical, skink could use
--trace-children-skip-by-arg='*valgrind-ignore-child*' combined with:

--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -1404 +1404 @@ sub enable_restoring
-	  : qq{cp "$path/%f" "%p"};
+	  : qq{cp "$path/%f" "%p"  # valgrind-ignore-child};
@@ -1474 +1474 @@ sub enable_archiving
-	  : qq{cp "%p" "$path/%f"};
+	  : qq{cp "%p" "$path/%f"  # valgrind-ignore-child};

What's your preference?

> Andres, what do you think about making one of those skink configuration
> changes?  Alternatively, I could make the test poll until archiving catches
> up.  However, that would take skink about 30min, and I expect little value
> from 30min of valgrind instrumenting the "cp" command.