Thread: bgwriter, inherited temp tables TODO items?
I'm switching the aftermath of this thread -- http://archives.postgresql.org/pgsql-general/2005-07/msg00501.php -- to -hackers since it raised issues of potential concern to developers.
At various points in the thread, Tom Lane said the following:
"I have an old note to myself that persistent write errors could "clog"
the bgwriter, because I was worried that after an error it would
stupidly try to write the same buffer again instead of trying to make
progress elsewhere. (CVS tip might be better about this, I'm not sure.)
A dirty buffer for a file that doesn't exist anymore would certainly
qualify as a persistent failure."
and
"Hmm ... a SELECT from one of the "actual tables" would then scan the
temp tables too, no?
Thinking about this, I seem to recall that we had agreed to make the
planner ignore temp tables of other backends when expanding an
inheritance list --- but I don't see anything in the code implementing
that, so it evidently didn't get done yet."
I don't immediately see TODO items correpsonding to these. Should there be some? Or do these qualify as bugs and should they be submitted to that queue?
--
Thomas F. O'Connell
Co-Founder, Information Architect
Sitening, LLC
Strategic Open Source: Open Your i™
110 30th Avenue North, Suite 6
Nashville, TN 37203-6320
615-260-0005
Thomas F. O'Connell wrote: > I'm switching the aftermath of this thread -- http:// > archives.postgresql.org/pgsql-general/2005-07/msg00501.php -- to - > hackers since it raised issues of potential concern to developers. > > At various points in the thread, Tom Lane said the following: > > "I have an old note to myself that persistent write errors could "clog" > the bgwriter, because I was worried that after an error it would > stupidly try to write the same buffer again instead of trying to make > progress elsewhere. (CVS tip might be better about this, I'm not sure.) > A dirty buffer for a file that doesn't exist anymore would certainly > qualify as a persistent failure." > > and > > "Hmm ... a SELECT from one of the "actual tables" would then scan the > temp tables too, no? > > Thinking about this, I seem to recall that we had agreed to make the > planner ignore temp tables of other backends when expanding an > inheritance list --- but I don't see anything in the code implementing > that, so it evidently didn't get done yet." > > I don't immediately see TODO items correpsonding to these. Should > there be some? Or do these qualify as bugs and should they be > submitted to that queue? Would you show a query that causes the problem so I can properly word the TODO item for inheritance and temp tables? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
On Jul 21, 2005, at 1:22 PM, Bruce Momjian wrote:
> Thomas F. O'Connell wrote:
>
>> I'm switching the aftermath of this thread -- http://
>> archives.postgresql.org/pgsql-general/2005-07/msg00501.php -- to -
>> hackers since it raised issues of potential concern to developers.
>>
>> At various points in the thread, Tom Lane said the following:
>>
>> "I have an old note to myself that persistent write errors could
>> "clog"
>> the bgwriter, because I was worried that after an error it would
>> stupidly try to write the same buffer again instead of trying to make
>> progress elsewhere. (CVS tip might be better about this, I'm not
>> sure.)
>> A dirty buffer for a file that doesn't exist anymore would certainly
>> qualify as a persistent failure."
>>
>> and
>>
>> "Hmm ... a SELECT from one of the "actual tables" would then scan the
>> temp tables too, no?
>>
>> Thinking about this, I seem to recall that we had agreed to make the
>> planner ignore temp tables of other backends when expanding an
>> inheritance list --- but I don't see anything in the code
>> implementing
>> that, so it evidently didn't get done yet."
>>
>> I don't immediately see TODO items correpsonding to these. Should
>> there be some? Or do these qualify as bugs and should they be
>> submitted to that queue?
>>
>
> Would you show a query that causes the problem so I can properly word
> the TODO item for inheritance and temp tables?
It's really more of a timing issue than a specific query issue.
Here's a scenario:
CREATE TABLE parent ( ... );
begin thread1:
CREATE TEMP TABLE child ( ... ) INHERITS FROM ( parent );
begin thread2:
while( 1 ) { SELECT ... FROM parent WHERE ...;
}
end thread1 (thereby dropping the temp table at the end of session)
At this point, the file is gone, but, as I understand it, the planner
not ignoring temp tables of other backends means that thread2 is
inappropriately accessing the temp table "child" as it performs
SELECTS, thus causing potential dirty buffers in bgwriter, which at
some point during the heavy activity of the tight SELECT loop, will
have the file yanked out from under it and will throw a "No such
file" error.
So I guess the core issue is the failure of the planner to limit
access to temp tables.
Tom seems to come pretty close to a TODO item in his analysis in my
opinion. Something like:
"Make the planner ignore temp tables of other backends when expanding
an inheritance list."
--
Thomas F. O'Connell
Co-Founder, Information Architect
Sitening, LLC
Strategic Open Source: Open Your i™
http://www.sitening.com/
110 30th Avenue North, Suite 6
Nashville, TN 37203-6320
615-260-0005
Added to TODO:
* Prevent inherited tables from expanding temporary subtables of other sessions
---------------------------------------------------------------------------
Thomas F. O'Connell wrote:
>
> On Jul 21, 2005, at 1:22 PM, Bruce Momjian wrote:
>
> > Thomas F. O'Connell wrote:
> >
> >> I'm switching the aftermath of this thread -- http://
> >> archives.postgresql.org/pgsql-general/2005-07/msg00501.php -- to -
> >> hackers since it raised issues of potential concern to developers.
> >>
> >> At various points in the thread, Tom Lane said the following:
> >>
> >> "I have an old note to myself that persistent write errors could
> >> "clog"
> >> the bgwriter, because I was worried that after an error it would
> >> stupidly try to write the same buffer again instead of trying to make
> >> progress elsewhere. (CVS tip might be better about this, I'm not
> >> sure.)
> >> A dirty buffer for a file that doesn't exist anymore would certainly
> >> qualify as a persistent failure."
> >>
> >> and
> >>
> >> "Hmm ... a SELECT from one of the "actual tables" would then scan the
> >> temp tables too, no?
> >>
> >> Thinking about this, I seem to recall that we had agreed to make the
> >> planner ignore temp tables of other backends when expanding an
> >> inheritance list --- but I don't see anything in the code
> >> implementing
> >> that, so it evidently didn't get done yet."
> >>
> >> I don't immediately see TODO items correpsonding to these. Should
> >> there be some? Or do these qualify as bugs and should they be
> >> submitted to that queue?
> >>
> >
> > Would you show a query that causes the problem so I can properly word
> > the TODO item for inheritance and temp tables?
>
> It's really more of a timing issue than a specific query issue.
> Here's a scenario:
>
> CREATE TABLE parent ( ... );
>
> begin thread1:
> CREATE TEMP TABLE child ( ... ) INHERITS FROM ( parent );
>
> begin thread2:
> while( 1 ) {
> SELECT ... FROM parent WHERE ...;
> }
>
> end thread1 (thereby dropping the temp table at the end of session)
>
> At this point, the file is gone, but, as I understand it, the planner
> not ignoring temp tables of other backends means that thread2 is
> inappropriately accessing the temp table "child" as it performs
> SELECTS, thus causing potential dirty buffers in bgwriter, which at
> some point during the heavy activity of the tight SELECT loop, will
> have the file yanked out from under it and will throw a "No such
> file" error.
>
> So I guess the core issue is the failure of the planner to limit
> access to temp tables.
>
> Tom seems to come pretty close to a TODO item in his analysis in my
> opinion. Something like:
>
> "Make the planner ignore temp tables of other backends when expanding
> an inheritance list."
>
> --
> Thomas F. O'Connell
> Co-Founder, Information Architect
> Sitening, LLC
>
> Strategic Open Source: Open Your i?
>
> http://www.sitening.com/
> 110 30th Avenue North, Suite 6
> Nashville, TN 37203-6320
> 615-260-0005
>
-- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610)
359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square,
Pennsylvania19073
Great! Is background writer clogging worthy? That's the one that put postgres in a nearly unusable state after this bug was tripped. Thanks! -- Thomas F. O'Connell Co-Founder, Information Architect Sitening, LLC Strategic Open Source: Open Your i™ http://www.sitening.com/ 110 30th Avenue North, Suite 6 Nashville, TN 37203-6320 615-260-0005 On Jul 29, 2005, at 10:49 PM, Bruce Momjian wrote: > Added to TODO: > > * Prevent inherited tables from expanding temporary subtables > of other > sessions > > > ---------------------------------------------------------------------- > ----- > > Thomas F. O'Connell wrote: > >> >> On Jul 21, 2005, at 1:22 PM, Bruce Momjian wrote: >> >> >>> Thomas F. O'Connell wrote: >>> >>> >>>> I'm switching the aftermath of this thread -- http:// >>>> archives.postgresql.org/pgsql-general/2005-07/msg00501.php -- to - >>>> hackers since it raised issues of potential concern to developers. >>>> >>>> At various points in the thread, Tom Lane said the following: >>>> >>>> "I have an old note to myself that persistent write errors could >>>> "clog" >>>> the bgwriter, because I was worried that after an error it would >>>> stupidly try to write the same buffer again instead of trying to >>>> make >>>> progress elsewhere. (CVS tip might be better about this, I'm not >>>> sure.) >>>> A dirty buffer for a file that doesn't exist anymore would >>>> certainly >>>> qualify as a persistent failure." >>>> >>>> and >>>> >>>> "Hmm ... a SELECT from one of the "actual tables" would then >>>> scan the >>>> temp tables too, no? >>>> >>>> Thinking about this, I seem to recall that we had agreed to make >>>> the >>>> planner ignore temp tables of other backends when expanding an >>>> inheritance list --- but I don't see anything in the code >>>> implementing >>>> that, so it evidently didn't get done yet." >>>> >>>> I don't immediately see TODO items correpsonding to these. Should >>>> there be some? Or do these qualify as bugs and should they be >>>> submitted to that queue? >>>> >>>> >>> >>> Would you show a query that causes the problem so I can properly >>> word >>> the TODO item for inheritance and temp tables? >>> >> >> It's really more of a timing issue than a specific query issue. >> Here's a scenario: >> >> CREATE TABLE parent ( ... ); >> >> begin thread1: >> CREATE TEMP TABLE child ( ... ) INHERITS FROM ( parent ); >> >> begin thread2: >> while( 1 ) { >> SELECT ... FROM parent WHERE ...; >> } >> >> end thread1 (thereby dropping the temp table at the end of session) >> >> At this point, the file is gone, but, as I understand it, the planner >> not ignoring temp tables of other backends means that thread2 is >> inappropriately accessing the temp table "child" as it performs >> SELECTS, thus causing potential dirty buffers in bgwriter, which at >> some point during the heavy activity of the tight SELECT loop, will >> have the file yanked out from under it and will throw a "No such >> file" error. >> >> So I guess the core issue is the failure of the planner to limit >> access to temp tables. >> >> Tom seems to come pretty close to a TODO item in his analysis in my >> opinion. Something like: >> >> "Make the planner ignore temp tables of other backends when expanding >> an inheritance list." >> >> -- >> Thomas F. O'Connell >> Co-Founder, Information Architect >> Sitening, LLC >> >> Strategic Open Source: Open Your i? >> >> http://www.sitening.com/ >> 110 30th Avenue North, Suite 6 >> Nashville, TN 37203-6320 >> 615-260-0005 >> >> > > -- > Bruce Momjian | http://candle.pha.pa.us > pgman@candle.pha.pa.us | (610) 359-1001 > + If your life is a hard drive, | 13 Roberts Road > + Christ can be your backup. | Newtown Square, > Pennsylvania 19073 >
"Thomas F. O'Connell" <tfo@sitening.com> writes:
> Tom seems to come pretty close to a TODO item in his analysis in my
> opinion. Something like:
> "Make the planner ignore temp tables of other backends when expanding
> an inheritance list."
I've done this in CVS tip. I'm not sure whether it should be considered
a backpatchable bug fix, though.
If you want to apply the patch locally, it's attached --- should work
fine in 8.0, but I'm not sure about 7.4 or earlier, which have slightly
different logic here.
regards, tom lane
*** src/backend/optimizer/prep/prepunion.c.orig Thu Jul 28 18:27:00 2005
--- src/backend/optimizer/prep/prepunion.c Tue Aug 2 16:21:41 2005
***************
*** 22,27 ****
--- 22,28 ---- #include "access/heapam.h"
+ #include "catalog/namespace.h" #include "catalog/pg_type.h" #include "nodes/makefuncs.h" #include
"optimizer/clauses.h"
***************
*** 808,813 ****
--- 809,824 ---- Index childRTindex; /*
+ * It is possible that the parent table has children that are
+ * temp tables of other backends. We cannot safely access such
+ * tables (because of buffering issues), and the best thing to do
+ * seems to be to silently ignore them.
+ */
+ if (childOID != parentOID &&
+ isOtherTempNamespace(get_rel_namespace(childOID)))
+ continue;
+
+ /* * Build an RTE for the child, and attach to query's rangetable * list. We copy most
fieldsof the parent's RTE, but replace * relation OID, and set inh = false.
***************
*** 818,823 ****
--- 829,845 ---- parse->rtable = lappend(parse->rtable, childrte); childRTindex =
list_length(parse->rtable); inhRTIs = lappend_int(inhRTIs, childRTindex);
+ }
+
+ /*
+ * If all the children were temp tables, pretend it's a non-inheritance
+ * situation. The duplicate RTE we added for the parent table is harmless.
+ */
+ if (list_length(inhRTIs) < 2)
+ {
+ /* Clear flag to save repeated tests if called again */
+ rte->inh = false;
+ return NIL; } /*