pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  

help / color / mirror / Atom feed
From: Euler Taveira <euler@eulerto.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: Thomas Munro <thomas.munro@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Euler Taveira <euler.taveira@2ndquadrant.com>
Cc: Anastasia Lubennikova <a.lubennikova@postgrespro.ru>
Cc: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: cleanup temporary files after crash
Date: Fri, 19 Mar 2021 11:17:50 -0300
Message-ID: <6eff9861-b059-4eaa-ae70-e5ff877b6448@www.fastmail.com> (raw)
In-Reply-To: <4030089.1616124234@sss.pgh.pa.us>
References: <CAH503wDKdYzyq7U-QJqGn=Gm6XmoK+6_6xTJ-Yn5WSvoHLY1Ww@mail.gmail.com>
	<363cb85c-3a03-9a49-b830-8b645c1241d9@enterprisedb.com>
	<YEdqlL3Gr3rZ/aC8@paquier.xyz>
	<CA+hUKGJtZqpMZtYtJZ2cAGfQT5ELEZGKpnK_TjV-fNeB=rYSNw@mail.gmail.com>
	<4245d284-97b2-4056-96c2-455792d9b564@www.fastmail.com>
	<8740f056-a7c0-ff12-0a50-de5324a928ee@enterprisedb.com>
	<822e3a60-56a1-219d-c473-365751c3e108@enterprisedb.com>
	<a32a7f28-570c-24a2-aa2a-9f7c96abd88b@enterprisedb.com>
	<54663718-1d87-46f2-a27b-e7edacbfae0d@www.fastmail.com>
	<1ec33f3d-ea4c-28c8-502c-3eb0a008a6ac@enterprisedb.com>
	<34226249-c5ba-4780-a1bb-f3433d19716e@www.fastmail.com>
	<05d307fc-8391-4091-b279-1b3214658f10@www.fastmail.com>
	<049a2752-fc99-194d-31b2-b15ffa914cd4@enterprisedb.com>
	<a26a35c6-8816-4c2b-91b9-7a428e430a57@www.fastmail.com>
	<482a6498-7713-e937-c554-7d235c415fba@enterprisedb.com>
	<112f0847-a71!a-eec2-e561-2d39dc26127a@enterprisedb.com>
	<4028905.1616122655@sss.pgh.pa.us>
	<19bc3276-6280-43bd-d868-ab3cb146266a@enter!prisedb.com>
	<4030089.1616124234@sss.pgh.pa.us>

On Fri, Mar 19, 2021, at 12:23 AM, Tom Lane wrote:
> [ reads code... ]
> ... no, I think the problem is the test is still full of race conditions.
> 
> In the first place, waiting till you see the output of a SELECT that's
> before the useful query is not enough to guarantee that the useful query
> is done, or even started.  That's broken on both sessions.
That's an ugly and fragile mechanism to workaround the fact that pump_until
reacts after you have the query return.

> In the second place, even if the second INSERT has started, you don't know
> that it's reached the point of blocking on the tuple conflict yet.
> Which in turn means that it might not've filled its tuplestore yet.
> 
> In short, this script is designed to ensure that the test query can't
> finish too soon, but that proves nothing about whether the test query
> has even started.  And since you also haven't really guaranteed that the
> intended-to-be-blocking query is done, I don't think that the first
> condition really holds either.
In order to avoid the race condition between filling the tuplestore and killing
the backend, we could use a pool_query_until() before SIGKILL to wait the
temporary file being created. Do you think this modification will make this
test more stable?


--
Euler Taveira
EDB   https://www.enterprisedb.com/

Attachments:


v3-fix-crash-temp-files.patch (text/x-patch, 1.0K) download
diff --git a/src/test/recovery/t/022_crash_temp_files.pl b/src/test/recovery/t/022_crash_temp_files.pl
index 8044849b73..41a91ebd06 100644
--- a/src/test/recovery/t/022_crash_temp_files.pl
+++ b/src/test/recovery/t/022_crash_temp_files.pl
@@ -100,6 +100,11 @@ ok(pump_until($killme, \$killme_stdout, qr/in-progress-before-sigkill/m),
 $killme_stdout = '';
 $killme_stderr = '';
 
+# Wait till a temporary file is created
+$node->poll_query_until(
+	'postgres',
+	'SELECT COUNT(1) FROM pg_ls_dir($$base/pgsql_tmp$$)', '1');
+
 # Kill with SIGKILL
 my $ret = TestLib::system_log('pg_ctl', 'kill', 'KILL', $pid);
 is($ret, 0, 'killed process with KILL');
@@ -168,6 +173,11 @@ ok(pump_until($killme, \$killme_stdout, qr/in-progress-before-sigkill/m),
 $killme_stdout = '';
 $killme_stderr = '';
 
+# Wait till a temporary file is created
+$node->poll_query_until(
+	'postgres',
+	'SELECT COUNT(1) FROM pg_ls_dir($$base/pgsql_tmp$$)', '1');
+
 # Kill with SIGKILL
 $ret = TestLib::system_log('pg_ctl', 'kill', 'KILL', $pid);
 is($ret, 0, 'killed process with KILL');


view thread (35+ messages)  latest in thread

Message-ID: <6eff9861-b059-4eaa-ae70-e5ff877b6448@www.fastmail.com>
Permalink:  ../6eff9861-b059-4eaa-ae70-e5ff877b6448@www.fastmail.com/
Also on:    postgresql.org/message-id/6eff9861-b059-4eaa-ae70-e5ff877b6448@www.fastmail.com

 ·  · 

reply
Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: euler@eulerto.com, tgl@sss.pgh.pa.us, tomas.vondra@enterprisedb.com, thomas.munro@gmail.com, michael@paquier.xyz, euler.taveira@2ndquadrant.com, a.lubennikova@postgrespro.ru, tomas.vondra@2ndquadrant.com, pgsql-hackers@lists.postgresql.org
  Subject: Re: cleanup temporary files after crash
  In-Reply-To: <6eff9861-b059-4eaa-ae70-e5ff877b6448@www.fastmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox