bgwriter, inherited temp tables TODO items?

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, Pennsylvania 19073

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, Pennsylvania 19073

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:

"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 fields of 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;
  	}

/*