Lists: | pgsql-committerspgsql-hackers |
---|
From: | stark(at)postgresql(dot)org (Greg Stark) |
---|---|
To: | pgsql-committers(at)postgresql(dot)org |
Subject: | pgsql: Oops, don't forget to rewind the directory before scanning it to |
Date: | 2010-02-22 00:11:05 |
Message-ID: | 20100222001105.66F7F7541D0@cvs.postgresql.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-committers pgsql-hackers |
Log Message:
-----------
Oops, don't forget to rewind the directory before scanning it to fsync files in CREATE DATABASE
Modified Files:
--------------
pgsql/src/port:
copydir.c (r1.28 -> r1.29)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/port/copydir.c?r1=1.28&r2=1.29)
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Greg Stark <stark(at)mit(dot)edu> |
Cc: | pgsql-committers(at)postgresql(dot)org |
Subject: | Re: pgsql: Oops, don't forget to rewind the directory before scanning it to |
Date: | 2010-02-22 02:30:58 |
Message-ID: | 3507.1266805858@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-committers pgsql-hackers |
stark(at)postgresql(dot)org (Greg Stark) writes:
> Oops, don't forget to rewind the directory before scanning it to fsync files in CREATE DATABASE
This is certainly not right:
+ AllocateDir(fromdir);
if (xldir == NULL)
ereport(ERROR,
There's no guarantee AllocateDir will hand back the same pointer as
it did the previous time.
regards, tom lane
From: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
---|---|
To: | Greg Stark <stark(at)postgresql(dot)org> |
Cc: | pgsql-committers(at)postgresql(dot)org |
Subject: | Re: pgsql: Oops, don't forget to rewind the directory before scanning it to |
Date: | 2010-02-22 02:38:52 |
Message-ID: | 3f0b79eb1002211838p634bd86dm2a6e42450326a13@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-committers pgsql-hackers |
On Mon, Feb 22, 2010 at 9:11 AM, Greg Stark <stark(at)postgresql(dot)org> wrote:
> Log Message:
> -----------
> Oops, don't forget to rewind the directory before scanning it to fsync files in CREATE DATABASE
}
+ Free(xldir);
s/Free/FreeDir ?
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
Cc: | Greg Stark <stark(at)postgresql(dot)org>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: [COMMITTERS] pgsql: Oops, don't forget to rewind the directory before scanning it to |
Date: | 2010-02-22 03:58:29 |
Message-ID: | 4956.1266811109@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-committers pgsql-hackers |
Fujii Masao <masao(dot)fujii(at)gmail(dot)com> writes:
> + Free(xldir);
> s/Free/FreeDir ?
Yeah, that too. I think it's all good now, but please test.
One thing I was wondering was whether the stat-wrong-file problem
could explain the buildfarm failures that we thought were evidence
of a portability issue. I was tempted to re-enable the #ifdef NOTYET
code, but didn't want to pull that trigger while there were other
problems outstanding.
regards, tom lane
From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | pgsql-hackers(at)postgresql(dot)org |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Greg Stark <stark(at)postgresql(dot)org> |
Subject: | Re: [COMMITTERS] pgsql: Oops, don't forget to rewind the directory before scanning it to |
Date: | 2010-02-22 07:59:36 |
Message-ID: | 201002220859.42437.andres@anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-committers pgsql-hackers |
On Monday 22 February 2010 04:58:29 Tom Lane wrote:
> Fujii Masao <masao(dot)fujii(at)gmail(dot)com> writes:
> > + Free(xldir);
> >
> > s/Free/FreeDir ?
>
> Yeah, that too. I think it's all good now, but please test.
At least I havent seen any of the problems pointed out in "fsync fun".
> One thing I was wondering was whether the stat-wrong-file problem
> could explain the buildfarm failures that we thought were evidence
> of a portability issue. I was tempted to re-enable the #ifdef NOTYET
> code, but didn't want to pull that trigger while there were other
> problems outstanding.
I unfortunately dont think so. The mkdir above should not have been bothered
by the stat issue - especially as it was only introduced by the commit to
disable the fsyncing.
> I also think it should scan the todir not the fromdir, just on
> general principles to avoid any possibility of race conditions.
That one actually was my idea/code and intentional with the idea to error out
at one more place if anything goes wrong in the copy loop - I could not think
of any race issues created by that which were not there before.
On the other hand its unlikely to catch anything so I dont mind.
Andres