Horton

Thread

  1. Is Patch Ok for deferred trigger disk queue?

    Stuart <deststar@blueyonder.co.uk> — 2003-06-30T12:16:25Z

    Hi,
I noticed  the patch:
http://archives.postgresql.org/pgsql-patches/2003-06/msg00366.php
isn't in the patch queue. Is the patch OK?
If not please say what is wrong with it.
Thank you,
- Stuart

  2. Re: Is Patch Ok for deferred trigger disk queue?

    Stephan Szabo <sszabo@megazone23.bigpanda.com> — 2003-06-30T19:37:39Z

    On Mon, 30 Jun 2003, deststar wrote:

> Hi,
> I noticed  the patch:
> http://archives.postgresql.org/pgsql-patches/2003-06/msg00366.php
> isn't in the patch queue. Is the patch OK?

I think it was just that Bruce hasn't gotten to it.

> If not please say what is wrong with it.

I just checked out a new cvs copy and applied the patch, and did something
like the following:
create table a1(a int unique, b int, c int, unique(b,c));
insert into a1 values (1,1,1);
create table a2(a int references a1(a), b int, c int, foreign
key(b,c) references a1(b,c) initially deferred );
begin;
insert into a2 values (1,1,1);
insert into a2 select * from a2;
[repeated a bunch of times until it'd be inserting 64k rows]

and got an error on writing the disk event handle and a signal 11:

ERROR:  Can not open first disk event file handle for
/usr/local/pgsql/data/base/17139/pgsql_tmp/pgsql_tmpdeftrig_555-000000001
The connection to the server was lost. Attempting reset: LOG:  server
process (pid 32125) was terminated by signal 11

The backtrace from the core looked like:
#0  0x42062867 in fclose@@GLIBC_2.1 () from /lib/i686/libc.so.6
#1  0x080d61a8 in deferredTriggerClean () at trigger.c:1864
#2  0x080d728e in DeferredTriggerAbortXact () at trigger.c:2642
#3  0x0808c214 in AbortTransaction () at xact.c:1042
#4  0x08141332 in PostgresMain (argc=4, argv=0x826a2f0, username=0x826a2c0
"sszabo") at postgres.c:2610
#5  0x0812320e in BackendFork (port=0x8277080) at postmaster.c:2471
#6  0x08122d1e in BackendStartup (port=0x8277080) at postmaster.c:2118
#7  0x081218ab in ServerLoop () at postmaster.c:1090
#8  0x08121358 in PostmasterMain (argc=3, argv=0x82693e0) at
postmaster.c:872
#9  0x080f9e30 in main (argc=3, argv=0xbffffa94) at main.c:211
#10 0x420158f7 in __libc_start_main () from /lib/i686/libc.so.6

looks like it was passing a NULL file handle if it couldn't be opened.

  3. Re: Is Patch Ok for deferred trigger disk queue?

    Stephan Szabo <sszabo@megazone23.bigpanda.com> — 2003-06-30T21:00:05Z

    On Mon, 30 Jun 2003, Stephan Szabo wrote:

> On Mon, 30 Jun 2003, deststar wrote:
>
> > Hi,
> > I noticed  the patch:
> > http://archives.postgresql.org/pgsql-patches/2003-06/msg00366.php
> > isn't in the patch queue. Is the patch OK?
>
> I think it was just that Bruce hasn't gotten to it.
>
> > If not please say what is wrong with it.
>
> I just checked out a new cvs copy and applied the patch, and did something
> like the following:
> create table a1(a int unique, b int, c int, unique(b,c));
> insert into a1 values (1,1,1);
> create table a2(a int references a1(a), b int, c int, foreign
> key(b,c) references a1(b,c) initially deferred );
> begin;
> insert into a2 values (1,1,1);
> insert into a2 select * from a2;
> [repeated a bunch of times until it'd be inserting 64k rows]
>
> and got an error on writing the disk event handle and a signal 11:
>
> ERROR:  Can not open first disk event file handle for
> /usr/local/pgsql/data/base/17139/pgsql_tmp/pgsql_tmpdeftrig_555-000000001
> The connection to the server was lost. Attempting reset: LOG:  server
> process (pid 32125) was terminated by signal 11
>
> The backtrace from the core looked like:
> #0  0x42062867 in fclose@@GLIBC_2.1 () from /lib/i686/libc.so.6
> #1  0x080d61a8 in deferredTriggerClean () at trigger.c:1864
> #2  0x080d728e in DeferredTriggerAbortXact () at trigger.c:2642
> #3  0x0808c214 in AbortTransaction () at xact.c:1042
> #4  0x08141332 in PostgresMain (argc=4, argv=0x826a2f0, username=0x826a2c0
> "sszabo") at postgres.c:2610
> #5  0x0812320e in BackendFork (port=0x8277080) at postmaster.c:2471
> #6  0x08122d1e in BackendStartup (port=0x8277080) at postmaster.c:2118
> #7  0x081218ab in ServerLoop () at postmaster.c:1090
> #8  0x08121358 in PostmasterMain (argc=3, argv=0x82693e0) at
> postmaster.c:872
> #9  0x080f9e30 in main (argc=3, argv=0xbffffa94) at main.c:211
> #10 0x420158f7 in __libc_start_main () from /lib/i686/libc.so.6
>
> looks like it was passing a NULL file handle if it couldn't be opened.

The open error seems to have been errno=13 (EACCES).  I was able to get
past that by changing pgsql_tmp's permissions to 700 rather than 600.
The 64k insert one worked, but the next insert ... select failed with:

ERROR:  Attempt to read from disk deferred trigger queue before
initialisation.

---

As a side question, it looks to me that the code stores the first trigger
records in memory and then after some point starts storing all new records
on disk.  Is this correct?  I'd wonder if that's really what you want in
general, since I'd think that the earliest ones are the ones you're least
likely to need until end of transaction (or set constraints in the fk
case) whereas the most recent ones are possibly going to be immediate
triggers which you're going to need as soon as the statement is done.

  4. Re: Is Patch Ok for deferred trigger disk queue?

    Tom Lane <tgl@sss.pgh.pa.us> — 2003-06-30T21:38:21Z

    Stephan Szabo <sszabo@megazone23.bigpanda.com> writes:
> As a side question, it looks to me that the code stores the first trigger
> records in memory and then after some point starts storing all new records
> on disk.  Is this correct?  I'd wonder if that's really what you want in
> general, since I'd think that the earliest ones are the ones you're least
> likely to need until end of transaction (or set constraints in the fk
> case) whereas the most recent ones are possibly going to be immediate
> triggers which you're going to need as soon as the statement is done.

Good point.  It would be better to push out stuff from the head of the
queue, hoping that stuff near the end might never need to be written
at all.

			regards, tom lane

  5. Re: Is Patch Ok for deferred trigger disk queue?

    Stuart <deststar@blueyonder.co.uk> — 2003-07-01T12:22:45Z

    Tom Lane wrote:

> Stephan Szabo <sszabo@megazone23.bigpanda.com> writes:
> 
>>As a side question, it looks to me that the code stores the first trigger
>>records in memory and then after some point starts storing all new records
>>on disk.  Is this correct?  I'd wonder if that's really what you want in
>>general, since I'd think that the earliest ones are the ones you're least
>>likely to need until end of transaction (or set constraints in the fk
>>case) whereas the most recent ones are possibly going to be immediate
>>triggers which you're going to need as soon as the statement is done.
> 
> 
> Good point.  It would be better to push out stuff from the head of the
> queue, hoping that stuff near the end might never need to be written
> at all.
> 
> 			regards, tom lane
Hmmm.... I see your point. I will change the patch to write the head to
disk and reenter when the development branch splits off.
Also I've noticed that there is an fd.h which has file routines which I
should be using rather than the stdio routines.
I will also clean up those errors.
Thank you,
- Stuart

  6. Re: Is Patch Ok for deferred trigger disk queue?

    Stephan Szabo <sszabo@megazone23.bigpanda.com> — 2003-07-01T20:20:04Z

    On Tue, 1 Jul 2003, Stuart wrote:

> Tom Lane wrote:
>
> > Stephan Szabo <sszabo@megazone23.bigpanda.com> writes:
> >
> >>As a side question, it looks to me that the code stores the first trigger
> >>records in memory and then after some point starts storing all new records
> >>on disk.  Is this correct?  I'd wonder if that's really what you want in
> >>general, since I'd think that the earliest ones are the ones you're least
> >>likely to need until end of transaction (or set constraints in the fk
> >>case) whereas the most recent ones are possibly going to be immediate
> >>triggers which you're going to need as soon as the statement is done.
> >
> >
> > Good point.  It would be better to push out stuff from the head of the
> > queue, hoping that stuff near the end might never need to be written
> > at all.
> >
> > 			regards, tom lane
> Hmmm.... I see your point. I will change the patch to write the head to
> disk and reenter when the development branch splits off.
> Also I've noticed that there is an fd.h which has file routines which I
> should be using rather than the stdio routines.
> I will also clean up those errors.

Hmm, it also looks like the original patch broke deferred foreign keys.
You'd not have noticed it since I'd forgotten the case in question for
the foreign key regression tests.  I'm going to make a patch for the
tests since we should be testing that in any case.

  7. Re: Is Patch Ok for deferred trigger disk queue?

    Jan Wieck <janwieck@yahoo.com> — 2003-07-01T21:57:18Z

    Stuart wrote:
> Tom Lane wrote:
> 
>> Stephan Szabo <sszabo@megazone23.bigpanda.com> writes:
>> 
>>>As a side question, it looks to me that the code stores the first trigger
>>>records in memory and then after some point starts storing all new records
>>>on disk.  Is this correct?  I'd wonder if that's really what you want in
>>>general, since I'd think that the earliest ones are the ones you're least
>>>likely to need until end of transaction (or set constraints in the fk
>>>case) whereas the most recent ones are possibly going to be immediate
>>>triggers which you're going to need as soon as the statement is done.
>> 
>> 
>> Good point.  It would be better to push out stuff from the head of the
>> queue, hoping that stuff near the end might never need to be written
>> at all.
>> 
>> 			regards, tom lane
> Hmmm.... I see your point. I will change the patch to write the head to
> disk and reenter when the development branch splits off.
> Also I've noticed that there is an fd.h which has file routines which I
> should be using rather than the stdio routines.
> I will also clean up those errors.

While you are still at it, can you make the arbitrarily choosen trigger 
queue size a config parameter? It is much easier to do tuning without 
the need to recompile the backend.


Jan

-- 
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.                                  #
#================================================== JanWieck@Yahoo.com #

  8. Re: Is Patch Ok for deferred trigger disk queue?

    Bruce Momjian <pgman@candle.pha.pa.us> — 2003-07-20T01:30:28Z

    Stuart, were are on this patch?  Seems we need GUC additions, though I
can do that for you, and changes to write the head to disk.  Was that
completed?

---------------------------------------------------------------------------

Stuart wrote:
> Tom Lane wrote:
> 
> > Stephan Szabo <sszabo@megazone23.bigpanda.com> writes:
> > 
> >>As a side question, it looks to me that the code stores the first trigger
> >>records in memory and then after some point starts storing all new records
> >>on disk.  Is this correct?  I'd wonder if that's really what you want in
> >>general, since I'd think that the earliest ones are the ones you're least
> >>likely to need until end of transaction (or set constraints in the fk
> >>case) whereas the most recent ones are possibly going to be immediate
> >>triggers which you're going to need as soon as the statement is done.
> > 
> > 
> > Good point.  It would be better to push out stuff from the head of the
> > queue, hoping that stuff near the end might never need to be written
> > at all.
> > 
> > 			regards, tom lane
> Hmmm.... I see your point. I will change the patch to write the head to
> disk and reenter when the development branch splits off.
> Also I've noticed that there is an fd.h which has file routines which I
> should be using rather than the stdio routines.
> I will also clean up those errors.
> Thank you,
> - Stuart
> 
> 
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
> 
>                http://archives.postgresql.org
> 

-- 
  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

  9. Re: Is Patch Ok for deferred trigger disk queue?

    Bruce Momjian <pgman@candle.pha.pa.us> — 2003-07-22T20:45:53Z

    I assume this will not be completed for 7.4.  I will keep the emails for
7.5.

One idea I had was to use the existing sort_mem parameter to control
when to force the deferred trigger queue to disk --- it doesn't have
anything to do with sorting, but it does have the same purpose, to force
thing to disk when we consume enough RAM.

---------------------------------------------------------------------------

Bruce Momjian wrote:
> 
> Stuart, were are on this patch?  Seems we need GUC additions, though I
> can do that for you, and changes to write the head to disk.  Was that
> completed?
> 
> ---------------------------------------------------------------------------
> 
> Stuart wrote:
> > Tom Lane wrote:
> > 
> > > Stephan Szabo <sszabo@megazone23.bigpanda.com> writes:
> > > 
> > >>As a side question, it looks to me that the code stores the first trigger
> > >>records in memory and then after some point starts storing all new records
> > >>on disk.  Is this correct?  I'd wonder if that's really what you want in
> > >>general, since I'd think that the earliest ones are the ones you're least
> > >>likely to need until end of transaction (or set constraints in the fk
> > >>case) whereas the most recent ones are possibly going to be immediate
> > >>triggers which you're going to need as soon as the statement is done.
> > > 
> > > 
> > > Good point.  It would be better to push out stuff from the head of the
> > > queue, hoping that stuff near the end might never need to be written
> > > at all.
> > > 
> > > 			regards, tom lane
> > Hmmm.... I see your point. I will change the patch to write the head to
> > disk and reenter when the development branch splits off.
> > Also I've noticed that there is an fd.h which has file routines which I
> > should be using rather than the stdio routines.
> > I will also clean up those errors.
> > Thank you,
> > - Stuart
> > 
> > 
> > 
> > ---------------------------(end of broadcast)---------------------------
> > TIP 6: Have you searched our list archives?
> > 
> >                http://archives.postgresql.org
> > 
> 
> -- 
>   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
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster
> 

-- 
  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

  10. Re: Is Patch Ok for deferred trigger disk queue?

    Stuart <deststar@blueyonder.co.uk> — 2003-07-28T18:36:27Z

    Bruce Momjian wrote:
> I assume this will not be completed for 7.4.  I will keep the emails for
> 7.5.
> 
> One idea I had was to use the existing sort_mem parameter to control
> when to force the deferred trigger queue to disk --- it doesn't have
> anything to do with sorting, but it does have the same purpose, to force
> thing to disk when we consume enough RAM.
> 
> ---------------------------------------------------------------------------
> 
> Bruce Momjian wrote:
> 
>>Stuart, were are on this patch?  Seems we need GUC additions, though I
>>can do that for you, and changes to write the head to disk.  Was that
>>completed?
>>
>>---------------------------------------------------------------------------
>>
>>Stuart wrote:
>>
>>>Tom Lane wrote:
>>>
>>>
>>>>Stephan Szabo <sszabo@megazone23.bigpanda.com> writes:
>>>>
>>>>
>>>>>As a side question, it looks to me that the code stores the first trigger
>>>>>records in memory and then after some point starts storing all new records
>>>>>on disk.  Is this correct?  I'd wonder if that's really what you want in
>>>>>general, since I'd think that the earliest ones are the ones you're least
>>>>>likely to need until end of transaction (or set constraints in the fk
>>>>>case) whereas the most recent ones are possibly going to be immediate
>>>>>triggers which you're going to need as soon as the statement is done.
>>>>
>>>>
>>>>Good point.  It would be better to push out stuff from the head of the
>>>>queue, hoping that stuff near the end might never need to be written
>>>>at all.
>>>>
>>>>			regards, tom lane
>>>
>>>Hmmm.... I see your point. I will change the patch to write the head to
>>>disk and reenter when the development branch splits off.
>>>Also I've noticed that there is an fd.h which has file routines which I
>>>should be using rather than the stdio routines.
>>>I will also clean up those errors.
>>>Thank you,
>>>- Stuart
>>>

Sorry for the tardiness in replying, I've been away for the past week or so.
I didn't intend for 7.4 partly because I knew I'd be away & partly 
because I had seen there was a problem I hadn't realised with the 
previous patch and didn't want to submit something that may not be 
stable just before beta. Currently it compiles but there are some, er, 
issues - shouldn't take to long to fix but it might not be till 
wednesday as I've got a bit of a backlog to get through.
I could use sortmem, but if this is to be the case maybe there should be 
a change the call it something like max_local_mem with a way to register 
that you are using it. Maybe the memory allocs could automatically add 
to it and remove as memory is assigned. Alternativly just make a global 
to record the memory currently used by interested parties (currently the 
trigger & sortmem I'd guess). The only trouble with this that I can see 
is that the first one to claim the memory may claim it all, leaving 
nothing for the other. I'll carry on using the dedicated guc variable 
for the moment as I can't really see the correct way to solve this cleanly.
regards,
- Stuart

  11. Re: Is Patch Ok for deferred trigger disk queue?

    Bruce Momjian <pgman@candle.pha.pa.us> — 2003-07-31T00:38:21Z

    Stuart wrote:
> Sorry for the tardiness in replying, I've been away for the past week or so.
> I didn't intend for 7.4 partly because I knew I'd be away & partly 
> because I had seen there was a problem I hadn't realised with the 
> previous patch and didn't want to submit something that may not be 
> stable just before beta. Currently it compiles but there are some, er, 
> issues - shouldn't take to long to fix but it might not be till 
> wednesday as I've got a bit of a backlog to get through.
> I could use sortmem, but if this is to be the case maybe there should be 
> a change the call it something like max_local_mem with a way to register 
> that you are using it. Maybe the memory allocs could automatically add 
> to it and remove as memory is assigned. Alternativly just make a global 
> to record the memory currently used by interested parties (currently the 
> trigger & sortmem I'd guess). The only trouble with this that I can see 
> is that the first one to claim the memory may claim it all, leaving 
> nothing for the other. I'll carry on using the dedicated guc variable 
> for the moment as I can't really see the correct way to solve this cleanly.
> regards,

OK, we can do the trigger queue file for 7.5.  The issue with sortmem is
that its effect is to spill a sort out to file when it gets too large,
the same for the trigger queue representation.  We could rename it, but
because it is _mostly_ used for sorts, we would probably keep the name
the same and just mention the trigger queue effect in the docs.

-- 
  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