Re: [PERFORM] Re: [HACKERS] [COMMITTERS] pgsql: Fix TransactionIdIsCurrentTransactionId() to use binary search

2008-04-26 Thread Robert Treat
On Friday 25 April 2008 17:32, Tom Lane wrote:
 Robert Treat [EMAIL PROTECTED] writes:
  Oddly some dtrace profiling gave me this, which is pretty different, but
  certainly doesn't have concerns about TransactionIdIsCurrentTransactionId

  which seems to pretty much destroy your thesis, no?


How so? Before the patch we bog down for hours, spending 99% of our time in  
TransactionIdIsCurrentTransactionId, after the patch everything performs well 
(really better than before) and we spend so little time in 
TransactionIdIsCurrentTransactionId it barely shows up on the radar. 

Note I'm open to the idea that TransactionIdIsCurrentTransactionId itself is 
not the problem, but that something else changed between 8.1 and 8.3 that 
exposes TransactionIdIsCurrentTransactionId as a problem.  Changing to a 
binary search for TransactionIdIsCurrentTransactionId  makes that a non-issue 
though. 

-- 
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL

-- 
Sent via pgsql-performance mailing list (pgsql-performance@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-performance


Re: [PERFORM] Re: [HACKERS] [COMMITTERS] pgsql: Fix TransactionIdIsCurrentTransactionId() to use binary search

2008-04-26 Thread Tom Lane
Robert Treat [EMAIL PROTECTED] writes:
 On Friday 25 April 2008 17:32, Tom Lane wrote:
 Robert Treat [EMAIL PROTECTED] writes:
 Oddly some dtrace profiling gave me this, which is pretty different, but
 certainly doesn't have concerns about TransactionIdIsCurrentTransactionId
 
  which seems to pretty much destroy your thesis, no?

 How so? Before the patch we bog down for hours, spending 99% of our time in  
 TransactionIdIsCurrentTransactionId, after the patch everything performs well
 (really better than before) and we spend so little time in 
 TransactionIdIsCurrentTransactionId it barely shows up on the radar. 

Oh, you failed to state that the dtrace output was post-patch.  You need
to show *pre* patch dtrace output if you want us to think it relevant.

regards, tom lane

-- 
Sent via pgsql-performance mailing list (pgsql-performance@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-performance


Re: [PERFORM] Re: [HACKERS] [COMMITTERS] pgsql: Fix TransactionIdIsCurrentTransactionId() to use binary search

2008-04-26 Thread Robert Treat
On Saturday 26 April 2008 13:26, Tom Lane wrote:
 Robert Treat [EMAIL PROTECTED] writes:
  On Friday 25 April 2008 17:32, Tom Lane wrote:
  Robert Treat [EMAIL PROTECTED] writes:
  Oddly some dtrace profiling gave me this, which is pretty different,
  but certainly doesn't have concerns about
  TransactionIdIsCurrentTransactionId
 
   which seems to pretty much destroy your thesis, no?
 
  How so? Before the patch we bog down for hours, spending 99% of our time
  in TransactionIdIsCurrentTransactionId, after the patch everything
  performs well (really better than before) and we spend so little time in
  TransactionIdIsCurrentTransactionId it barely shows up on the radar.

 Oh, you failed to state that the dtrace output was post-patch.  You need
 to show *pre* patch dtrace output if you want us to think it relevant.


Please read up-thread. 

-- 
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL

-- 
Sent via pgsql-performance mailing list (pgsql-performance@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-performance


Re: [PERFORM] Re: [HACKERS] [COMMITTERS] pgsql: Fix TransactionIdIsCurrentTransactionId() to use binary search

2008-04-26 Thread Tom Lane
Robert Treat [EMAIL PROTECTED] writes:
 On Saturday 26 April 2008 13:26, Tom Lane wrote:
 Oh, you failed to state that the dtrace output was post-patch.  You need
 to show *pre* patch dtrace output if you want us to think it relevant.

 Please read up-thread. 

Sorry, I'd forgotten your previous post.

I poked around for calls to TransactionIdIsCurrentTransactionId that
are in current code and weren't in 8.1.  I found these:

src/backend/commands/analyze.c: 965:if 
(TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(targtuple.t_data)))
src/backend/commands/analyze.c: 984:if 
(TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmax(targtuple.t_data)))
src/backend/commands/cluster.c: 803:if 
(!TransactionIdIsCurrentTransactionId(
src/backend/commands/cluster.c: 816:if 
(!TransactionIdIsCurrentTransactionId(
src/backend/storage/ipc/procarray.c: 374:   if 
(TransactionIdIsCurrentTransactionId(xid))
src/backend/utils/time/combocid.c: 108: 
Assert(TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(tup)));
src/backend/utils/time/combocid.c: 123: 
Assert(TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmax(tup)));
src/backend/utils/time/combocid.c: 156: 
TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(tup)))

The ANALYZE and CLUSTER calls are not likely to be your issue, but the
one in HeapTupleHeaderAdjustCmax could get called a lot, and the one
in TransactionIdIsInProgress definitely will get called a lot.
Neither of those calls existed in 8.2.

So I think that explains why TransactionIdIsCurrentTransactionId has
become more performance-critical in 8.3 than it was before.  Will
apply the back-patch.

regards, tom lane

-- 
Sent via pgsql-performance mailing list (pgsql-performance@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-performance


Re: [PERFORM] Re: [HACKERS] [COMMITTERS] pgsql: Fix TransactionIdIsCurrentTransactionId() to use binary search

2008-04-25 Thread Robert Treat
On Monday 21 April 2008 12:54, Alvaro Herrera wrote:
 Robert Treat wrote:
  Unfortunatly I don't have the 8.1 system to bang on anymore for this,
  (though anecdotaly speaking, I never saw this behavior in 8.1) however I
  do now have a parallel 8.3 system crunching the data, and it is showing
  the same symptom (yes, 2 8.3 servers, crunching the same data, both
  bogged down now), so I do feel this is something specific to 8.3.
 
  I am mostly wondering if anyone else has encountered behavior like this
  on 8.3 (large sets of insertupdate exception block in plpgsql bogging
  down), or if anyone has any thoughts on which direction I should poke at
  it from here. TIA.

 Perhaps what you could do is backpatch the change and see if the problem
 goes away.

So, after some more digging, we ended up backpatching the change. Results as 
follows:

= hanging job before patch

 elapsed | status
-+
 00:00:00.024075 | OK/starting with 2008-04-25 08:20:02
 00:00:00.611411 | OK/processing 624529 hits up until 2008-04-25 10:20:02
 03:48:02.748319 | ??/Processed 65000 aggregated rows so far
(3 rows)

= successful job after patch

 elapsed | status
-+-
 00:00:00.026809 | OK/starting with 2008-04-25 08:20:02
 00:00:03.921532 | OK/processing 2150115 hits up until 2008-04-25 15:00:02
 00:24:45.439081 | OK/Processed 334139 aggregated rows
 00:00:00.019433 | OK/
(4 rows)

Note the second run had to do all the rows from the first run, plus additional 
rows that accumulated while the first job was running. 

Oddly some dtrace profiling gave me this, which is pretty different, but 
certainly doesn't have concerns about TransactionIdIsCurrentTransactionId

snip
postgres`hash_search_with_hash_value  536   2.3%
postgres`SearchCatCache   538   2.3%
postgres`hash_seq_search  577   2.4%
postgres`MemoryContextAllocZeroAligned610   2.6%
postgres`_bt_compare  671   2.8%
libc.so.1`memcpy  671   2.8%
postgres`XLogInsert   755   3.2%
postgres`LockReassignCurrentOwner 757   3.2%
postgres`base_yyparse1174   5.0%
postgres`AllocSetAlloc   1244   5.3%

We still have one of our 8.3 servers running stock 8.3.1, so we'll see how 
long before this bites us again.  Would certainly be nice to get this fixed 
in the mainline code. 

-- 
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL

-- 
Sent via pgsql-performance mailing list (pgsql-performance@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-performance


Re: [PERFORM] Re: [HACKERS] [COMMITTERS] pgsql: Fix TransactionIdIsCurrentTransactionId() to use binary search

2008-04-25 Thread Tom Lane
Robert Treat [EMAIL PROTECTED] writes:
 Oddly some dtrace profiling gave me this, which is pretty different, but 
 certainly doesn't have concerns about TransactionIdIsCurrentTransactionId

... which seems to pretty much destroy your thesis, no?

regards, tom lane

-- 
Sent via pgsql-performance mailing list (pgsql-performance@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-performance


Re: [PERFORM] Re: [HACKERS] [COMMITTERS] pgsql: Fix TransactionIdIsCurrentTransactionId() to use binary search

2008-04-21 Thread Alvaro Herrera
Robert Treat wrote:

 Unfortunatly I don't have the 8.1 system to bang on anymore for this, (though 
 anecdotaly speaking, I never saw this behavior in 8.1) however I do now have 
 a parallel 8.3 system crunching the data, and it is showing the same symptom 
 (yes, 2 8.3 servers, crunching the same data, both bogged down now), so I do 
 feel this is something specific to 8.3.  
 
 I am mostly wondering if anyone else has encountered behavior like this on 
 8.3 
 (large sets of insertupdate exception block in plpgsql bogging down), or 
 if anyone has any thoughts on which direction I should poke at it from here. 
 TIA.

Perhaps what you could do is backpatch the change and see if the problem
goes away.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

-- 
Sent via pgsql-performance mailing list (pgsql-performance@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-performance