postmaster errors with index on temp table?

Philip Warner <pjw@rhyme.com.au> writes:

Is this a known problem?

pjw=# create temporary table tt(f int4);
CREATE
pjw=# create index tt_ix1 on tt(f);
CREATE
pjw=# \q

The postmaster says:

NOTICE: mdopen: couldn't open pg_temp.31633.1: No such file or directory
pq_flush: send() failed: Bad file descriptor
NOTICE: RelationIdBuildRelation: smgropen(pg_temp.31633.1): Bad file
descriptor
pq_flush: send() failed: Bad file descriptor

I see the same. "DROP INDEX tt_ix1" seems to do the right things, but
maybe temp-file cleanup fails to delink the index from its table.
Or, could temp-file cleanup be trying to delete these in the wrong
order?

The notices look pretty harmless, and AFAICT the tables do get cleaned
up, but it's ugly nonetheless...

regards, tom lane

Is this a known problem?

pjw=# create temporary table tt(f int4);
CREATE
pjw=# create index tt_ix1 on tt(f);
CREATE
pjw=# \q

The postmaster says:

NOTICE: mdopen: couldn't open pg_temp.31633.1: No such file or directory
pq_flush: send() failed: Bad file descriptor
NOTICE: RelationIdBuildRelation: smgropen(pg_temp.31633.1): Bad file
descriptor
pq_flush: send() failed: Bad file descriptor

It only happens if you create an index on a temporary table...

Yikes, I never looked in the postmaster log to see this.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

At 09:15 10/07/00 -0400, Bruce Momjian wrote:

Is this a known problem?

pjw=# create temporary table tt(f int4);
CREATE
pjw=# create index tt_ix1 on tt(f);
CREATE
pjw=# \q

The postmaster says:

NOTICE: mdopen: couldn't open pg_temp.31633.1: No such file or directory
pq_flush: send() failed: Bad file descriptor
NOTICE: RelationIdBuildRelation: smgropen(pg_temp.31633.1): Bad file
descriptor
pq_flush: send() failed: Bad file descriptor

It only happens if you create an index on a temporary table...

Yikes, I never looked in the postmaster log to see this.

As Tom Lane said, it's non-fatal, and probably not important, but I worried
me. I'll see if I can see the cause while I wait for people to test my new
pg_dump with BLOB support.

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.C.N. 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

Yikes, I never looked in the postmaster log to see this.

As Tom Lane said, it's non-fatal, and probably not important, but I worried
me. I'll see if I can see the cause while I wait for people to test my new
pg_dump with BLOB support.

I will certainly find the cause and fix it.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

At 09:27 10/07/00 -0400, Bruce Momjian wrote:

Yikes, I never looked in the postmaster log to see this.

As Tom Lane said, it's non-fatal, and probably not important, but I worried
me. I'll see if I can see the cause while I wait for people to test my new
pg_dump with BLOB support.

I've had a look at temprel.c, and remove_all_temp_relations has the following:

if (temp_rel->relkind != RELKIND_INDEX)
{
char relname[NAMEDATALEN];

/* safe from deallocation */
strcpy(relname, temp_rel->user_relname);
heap_drop_with_catalog(relname, allowSystemTableMods);
}
else
index_drop(temp_rel->relid);

But, when a temp rel is dropped it seems that heap_drop_with_catalog also
drops the indexes, so the error occurs when index_drop is called (at least
I think this is the case). Certainly commenting out the last two lines
*seems* to work.

These lines were changed in rev 1.18 from 'heap_destroy*' calls to
'heap_drop' calls...did 'heap_destroy*' also delete related objects as
heap_drop* now does?

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.C.N. 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

At 09:27 10/07/00 -0400, Bruce Momjian wrote:

Yikes, I never looked in the postmaster log to see this.

As Tom Lane said, it's non-fatal, and probably not important, but I worried
me. I'll see if I can see the cause while I wait for people to test my new
pg_dump with BLOB support.

I've had a look at temprel.c, and remove_all_temp_relations has the following:

if (temp_rel->relkind != RELKIND_INDEX)
{
char relname[NAMEDATALEN];

/* safe from deallocation */
strcpy(relname, temp_rel->user_relname);
heap_drop_with_catalog(relname, allowSystemTableMods);
}
else
index_drop(temp_rel->relid);

But, when a temp rel is dropped it seems that heap_drop_with_catalog also
drops the indexes, so the error occurs when index_drop is called (at least
I think this is the case). Certainly commenting out the last two lines
*seems* to work.

These lines were changed in rev 1.18 from 'heap_destroy*' calls to
'heap_drop' calls...did 'heap_destroy*' also delete related objects as
heap_drop* now does?

Not sure why I introduced that bug in 1.18. Your suggestion was 100%
correct. I have applied the following patch.

Thanks for finding that.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Bruce Momjian <pgman@candle.pha.pa.us> writes:

But, when a temp rel is dropped it seems that heap_drop_with_catalog also
drops the indexes, so the error occurs when index_drop is called (at least
I think this is the case). Certainly commenting out the last two lines
*seems* to work.

Not sure why I introduced that bug in 1.18. Your suggestion was 100%
correct. I have applied the following patch.

Actually, I don't think this is the true explanation. index_drop'ing
the temp indexes may not be *necessary*, but it shouldn't *hurt* either.

Now that I think about it, the reason for the failure is probably that
there's no CommandCounterIncrement in this loop. Therefore, when we
index_drop an index, the resulting system-table updates are *not seen*
by heap_drop_with_catalog when it comes time to drop the owning table,
and so it tries to drop the index again.

Your solution of not doing index_drop at all is sufficient for the
table-and-index case, but I bet it is not sufficient for more complex
cases like RI checks between temp relations. I'd recommend doing
CommandCounterIncrement after each temp item is dropped, instead.

There is another potential bug here: remove_all_temp_relations() is
failing to consider the possibility that removing one list entry may
cause other ones (eg, indexes) to go away. It's holding onto a "next"
pointer to a list entry that may not be there by the time control comes
back from heap_drop_with_catalog. This is probably OK for tables and
indexes because the indexes will always be added after their table and
thus appear earlier in the list, but again it seems like trouble just
waiting to happen. I would recommend logic along the lines of

while (temp_rels != NIL)
{
get first entry in temp_rels,
and either heap_drop or index_drop it as appropriate;
CommandCounterIncrement();
}

This relies on the drop to come back and remove the temp_rels entry
(and, possibly, other entries); so it's not an infinite loop even
though it looks like one.

regards, tom lane

Attached is a patch for temp tables that implements Tom's requests.

Bruce Momjian <pgman@candle.pha.pa.us> writes:

But, when a temp rel is dropped it seems that heap_drop_with_catalog also
drops the indexes, so the error occurs when index_drop is called (at least
I think this is the case). Certainly commenting out the last two lines
*seems* to work.

Not sure why I introduced that bug in 1.18. Your suggestion was 100%
correct. I have applied the following patch.

Actually, I don't think this is the true explanation. index_drop'ing
the temp indexes may not be *necessary*, but it shouldn't *hurt* either.

Now that I think about it, the reason for the failure is probably that
there's no CommandCounterIncrement in this loop. Therefore, when we
index_drop an index, the resulting system-table updates are *not seen*
by heap_drop_with_catalog when it comes time to drop the owning table,
and so it tries to drop the index again.

Your solution of not doing index_drop at all is sufficient for the
table-and-index case, but I bet it is not sufficient for more complex
cases like RI checks between temp relations. I'd recommend doing
CommandCounterIncrement after each temp item is dropped, instead.

There is another potential bug here: remove_all_temp_relations() is
failing to consider the possibility that removing one list entry may
cause other ones (eg, indexes) to go away. It's holding onto a "next"
pointer to a list entry that may not be there by the time control comes
back from heap_drop_with_catalog. This is probably OK for tables and
indexes because the indexes will always be added after their table and
thus appear earlier in the list, but again it seems like trouble just
waiting to happen. I would recommend logic along the lines of

while (temp_rels != NIL)
{
get first entry in temp_rels,
and either heap_drop or index_drop it as appropriate;
CommandCounterIncrement();
}

This relies on the drop to come back and remove the temp_rels entry
(and, possibly, other entries); so it's not an infinite loop even
though it looks like one.

regards, tom lane

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026