Re: cleanup temporary files after crash

2021-06-24 Thread Michael Paquier
On Wed, Jun 23, 2021 at 08:46:12AM +0900, Michael Paquier wrote: > Looks adapted to me. Applied as it was still on the stack. Please note that GUC_NOT_IN_SAMPLE was missing. -- Michael signature.asc Description: PGP signature

Re: cleanup temporary files after crash

2021-06-22 Thread Michael Paquier
On Tue, Jun 22, 2021 at 04:12:51PM -0300, Euler Taveira wrote: > Was it? I seem to have missed this suggestion. > > I'm attaching a patch to fix it. Looks adapted to me. -- Michael signature.asc Description: PGP signature

Re: cleanup temporary files after crash

2021-06-22 Thread Euler Taveira
On Fri, Jun 11, 2021, at 9:43 PM, Justin Pryzby wrote: > On Sat, Oct 31, 2020 at 09:01:15PM -0300, Euler Taveira wrote: > > > > The current behavior is only useful for debugging purposes. > > On Wed, 28 Oct 2020 at 15:42, Tomas Vondra > wrote: > > > One thing I'm not sure about is whether we sho

Re: cleanup temporary files after crash

2021-06-11 Thread Justin Pryzby
On Sat, Oct 31, 2020 at 09:01:15PM -0300, Euler Taveira wrote: > > > The current behavior is only useful for debugging purposes. On Wed, 28 Oct 2020 at 15:42, Tomas Vondra wrote: > > One thing I'm not sure about is whether we should have the GUC as > > proposed, or have a negative "keep_temp_file

Re: cleanup temporary files after crash

2021-03-19 Thread Euler Taveira
On Fri, Mar 19, 2021, at 2:27 PM, Tomas Vondra wrote: > I've pushed this version, with the plpgsql block. I've tested it on all > the machines I have here, hopefully it'll make buildfarm happy too. Thanks for taking care of this issue. > AFAICS I was mistaken about what the pump() functions do - i

Re: cleanup temporary files after crash

2021-03-19 Thread Tomas Vondra
On 3/19/21 5:26 PM, Tomas Vondra wrote: > On 3/19/21 5:23 PM, Tomas Vondra wrote: >> ... >> >> If I replace this with a wait loop in a plpgsql block, that works >> perfectly fine (no infinite loops). Tested both on x86_64 and rpi. >> > > For the record, here's the version with plpgsql block, which

Re: cleanup temporary files after crash

2021-03-19 Thread Tomas Vondra
On 3/19/21 5:23 PM, Tomas Vondra wrote: > ... > > If I replace this with a wait loop in a plpgsql block, that works > perfectly fine (no infinite loops). Tested both on x86_64 and rpi. > For the record, here's the version with plpgsql block, which seems to be working just fine. regards -- Toma

Re: cleanup temporary files after crash

2021-03-19 Thread Tomas Vondra
On 3/19/21 5:23 PM, Tomas Vondra wrote: > ... > > If I replace this with a wait loop in a plpgsql block, that works > perfectly fine (no infinite loops). Tested both on x86_64 and rpi. > For the record, here's the version with plpgsql block, which seems to be working just fine. regards -- To

Re: cleanup temporary files after crash

2021-03-19 Thread Tomas Vondra
On 3/19/21 3:17 PM, Euler Taveira wrote: > 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 i

Re: cleanup temporary files after crash

2021-03-19 Thread Euler Taveira
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 >

Re: cleanup temporary files after crash

2021-03-18 Thread Tom Lane
[ 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. In

Re: cleanup temporary files after crash

2021-03-18 Thread Tomas Vondra
On 3/19/21 3:57 AM, Tom Lane wrote: > Tomas Vondra writes: >> So crake and florican seem to be happy now. Not sure about lapwing yet. >> But interestingly enough, prion and curculio got unhappy. They worked >> fine with the older test, but now it fails with the "no such file or >> directory" messa

Re: cleanup temporary files after crash

2021-03-18 Thread Tom Lane
Tomas Vondra writes: > So crake and florican seem to be happy now. Not sure about lapwing yet. > But interestingly enough, prion and curculio got unhappy. They worked > fine with the older test, but now it fails with the "no such file or > directory" message. I wonder what makes them different fro

Re: cleanup temporary files after crash

2021-03-18 Thread Tomas Vondra
On 3/19/21 2:21 AM, Tomas Vondra wrote: > On 3/19/21 1:54 AM, Euler Taveira wrote: >> On Thu, Mar 18, 2021, at 8:34 PM, Tomas Vondra wrote: >>> Well, that's better, bit it still does not do the trick on the 32-bit >>> machine - in that case a 1000 rows with int4 still fit into work_mem, so >>> the

Re: cleanup temporary files after crash

2021-03-18 Thread Tomas Vondra
On 3/19/21 1:54 AM, Euler Taveira wrote: > On Thu, Mar 18, 2021, at 8:34 PM, Tomas Vondra wrote: >> Well, that's better, bit it still does not do the trick on the 32-bit >> machine - in that case a 1000 rows with int4 still fit into work_mem, so >> the temp file is not created. Per my experiments a

Re: cleanup temporary files after crash

2021-03-18 Thread Euler Taveira
On Thu, Mar 18, 2021, at 8:34 PM, Tomas Vondra wrote: > Well, that's better, bit it still does not do the trick on the 32-bit > machine - in that case a 1000 rows with int4 still fit into work_mem, so > the temp file is not created. Per my experiments about 1040 rows are > needed - s close ;-)

Re: cleanup temporary files after crash

2021-03-18 Thread Tomas Vondra
On 3/18/21 10:44 PM, Euler Taveira wrote: > On Thu, Mar 18, 2021, at 6:00 PM, Euler Taveira wrote: >> On Thu, Mar 18, 2021, at 5:51 PM, Tomas Vondra wrote: >>> OK. Can you prepare a patch with the proposed test approach? >> I'm on it. > What do you think about this patch? > Well, that's better, b

Re: cleanup temporary files after crash

2021-03-18 Thread Euler Taveira
On Thu, Mar 18, 2021, at 6:00 PM, Euler Taveira wrote: > On Thu, Mar 18, 2021, at 5:51 PM, Tomas Vondra wrote: >> OK. Can you prepare a patch with the proposed test approach? > I'm on it. What do you think about this patch? -- Euler Taveira EDB https://www.enterprisedb.com/ diff --git a/src/tes

Re: cleanup temporary files after crash

2021-03-18 Thread Euler Taveira
On Thu, Mar 18, 2021, at 5:51 PM, Tomas Vondra wrote: > OK. Can you prepare a patch with the proposed test approach? I'm on it. > FWIW I can reproduce this on a 32-bit ARM system (rpi4), where 500 rows > simply does not use a temp file, and with 1000 rows it works fine. On > the x86_64 the temp fi

Re: cleanup temporary files after crash

2021-03-18 Thread Tomas Vondra
On 3/18/21 9:06 PM, Euler Taveira wrote: > On Thu, Mar 18, 2021, at 4:20 PM, Tomas Vondra wrote: >> I think a better way to test this would be to use a tuple lock: > I predicated such issues with this test. Your suggestion works for me. Maybe > you should use less rows in the session 2 query. >

Re: cleanup temporary files after crash

2021-03-18 Thread Euler Taveira
On Thu, Mar 18, 2021, at 4:20 PM, Tomas Vondra wrote: > I think a better way to test this would be to use a tuple lock: I predicated such issues with this test. Your suggestion works for me. Maybe you should use less rows in the session 2 query. > setup: > > create table t (a int unique); > >

Re: cleanup temporary files after crash

2021-03-18 Thread Tomas Vondra
On 3/18/21 6:51 PM, Tomas Vondra wrote: > Hmm, > > crake and florican seem to have failed because of this commit, with this > error in the new TAP test: > > error running SQL: 'psql::1: ERROR: could not open directory > "base/pgsql_tmp": No such file or directory' > while running 'psql -XAtq -d

Re: cleanup temporary files after crash

2021-03-18 Thread Tomas Vondra
Hmm, crake and florican seem to have failed because of this commit, with this error in the new TAP test: error running SQL: 'psql::1: ERROR: could not open directory "base/pgsql_tmp": No such file or directory' while running 'psql -XAtq -d port=64336 host=/tmp/sv1WjSvj3P dbname='postgres' -f - -

Re: cleanup temporary files after crash

2021-03-18 Thread Tomas Vondra
On 3/17/21 2:34 AM, Euler Taveira wrote: > On Sun, Mar 14, 2021, at 11:01 PM, Thomas Munro wrote: >> On Wed, Mar 10, 2021 at 1:31 AM Michael Paquier > > wrote: >> > On Tue, Mar 09, 2021 at 02:28:43AM +0100, Tomas Vondra wrote: >> > > Let's move this patch forward. Bas

Re: cleanup temporary files after crash

2021-03-16 Thread Euler Taveira
On Sun, Mar 14, 2021, at 11:01 PM, Thomas Munro wrote: > On Wed, Mar 10, 2021 at 1:31 AM Michael Paquier wrote: > > On Tue, Mar 09, 2021 at 02:28:43AM +0100, Tomas Vondra wrote: > > > Let's move this patch forward. Based on the responses, I agree the > > > default behavior should be to remove the

Re: cleanup temporary files after crash

2021-03-14 Thread Thomas Munro
On Wed, Mar 10, 2021 at 1:31 AM Michael Paquier wrote: > On Tue, Mar 09, 2021 at 02:28:43AM +0100, Tomas Vondra wrote: > > Let's move this patch forward. Based on the responses, I agree the > > default behavior should be to remove the temp files, and I think we > > should have the GUC (on the off

Re: cleanup temporary files after crash

2021-03-09 Thread Euler Taveira
On Tue, Mar 9, 2021, at 9:31 AM, Michael Paquier wrote: > On Tue, Mar 09, 2021 at 02:28:43AM +0100, Tomas Vondra wrote: > > Let's move this patch forward. Based on the responses, I agree the > > default behavior should be to remove the temp files, and I think we > > should have the GUC (on the off

Re: cleanup temporary files after crash

2021-03-09 Thread Michael Paquier
On Tue, Mar 09, 2021 at 02:28:43AM +0100, Tomas Vondra wrote: > Let's move this patch forward. Based on the responses, I agree the > default behavior should be to remove the temp files, and I think we > should have the GUC (on the off chance that someone wants to preserve > the temporary files for

Re: cleanup temporary files after crash

2021-03-08 Thread Tomas Vondra
node->safe_psql( + 'postgres', + q[CREATE TABLE tab_crash (a text); + INSERT INTO tab_crash (a) SELECT gen_random_uuid() FROM generate_series(1, 500);]); + +# +# Test cleanup temporary files after crash +# + +# Run psql, keeping session alive, so we have an alive backend to kill. +my ($killme_std

Re: cleanup temporary files after crash

2020-11-26 Thread Euler Taveira
On Thu, 26 Nov 2020 at 05:48, Anastasia Lubennikova < a.lubennik...@postgrespro.ru> wrote: > > I also think that the GUC is not needed here. This 'feature' was > internal from the very beginning, so users shouldn't care about > preserving old behavior. Without the GUC the patch is very simple, > p

Re: cleanup temporary files after crash

2020-11-26 Thread Anastasia Lubennikova
On 01.11.2020 04:25, Michael Paquier wrote: On Sat, Oct 31, 2020 at 09:01:15PM -0300, Euler Taveira wrote: I thought about not providing a GUC at all or provide it in the developer section. I've never heard someone saying that they use those temporary files to investigate an issue. Regarding a c

Re: cleanup temporary files after crash

2020-10-31 Thread Michael Paquier
On Sat, Oct 31, 2020 at 09:01:15PM -0300, Euler Taveira wrote: > I thought about not providing a GUC at all or provide it in the developer > section. I've never heard someone saying that they use those temporary > files to investigate an issue. Regarding a crash, all information is already > availa

Re: cleanup temporary files after crash

2020-10-31 Thread Euler Taveira
On Wed, 28 Oct 2020 at 15:42, Tomas Vondra wrote: > > I did a quick review and the patch seems fine to me. Let's wait for a > bit and see if there are any objections - if not, I'll get it committed > in the next CF. > > Tomas, thanks for your review. > One thing I'm not sure about is whether we

Re: cleanup temporary files after crash

2020-10-28 Thread Tomas Vondra
Hi Euler, On Wed, Oct 28, 2020 at 11:16:26AM -0300, Euler Taveira wrote: Hi, Bug #16427 mentioned that temporary files are not removed after a crash. I heard similar complaints from some customers. In the bug report [1], I proposed a new GUC to control whether temporary files are removed after

cleanup temporary files after crash

2020-10-28 Thread Euler Taveira
e, insert rows +$node->safe_psql( + 'postgres', + q[CREATE TABLE tab_crash (a text); + INSERT INTO tab_crash (a) SELECT gen_random_uuid() FROM generate_series(1, 500);]); + +# +# Test cleanup temporary files after crash +# + +# Run psql, keeping session alive, so we have an alive b