Thread: Lock partitions

Lock partitions

From
Bruce Momjian
Date:
I see this in the CVS commits for 8.2.  Did we determine the proper
number of lock partitions?  Should it be based on the number of buffers
or concurrent sessions allowed?
Divide the lock manager's shared state into 'partitions', so as toreduce contention for the former single LockMgrLock.
Permy recentproposal.  I set it up for 16 partitions, but on a pgbench test thisgives only a marginal further
improvementover 4 partitions --- we needto test more scenarios to choose the number of partitions.
 

--  Bruce Momjian   bruce@momjian.us EnterpriseDB    http://www.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +


Re: Lock partitions

From
Tom Lane
Date:
Bruce Momjian <bruce@momjian.us> writes:
> I see this in the CVS commits for 8.2.  Did we determine the proper
> number of lock partitions?  Should it be based on the number of buffers
> or concurrent sessions allowed?

No.  NUM_LOCK_PARTITIONS needs to be a compile-time constant for a
number of reasons, and there is absolutely zero evidence to justify
making any effort (and spending any cycles) on a variable value.

It would be nice to see some results from the OSDL tests with, say, 4,
8, and 16 lock partitions before we forget about the point though.
Anybody know whether OSDL is in a position to run tests for us?
        regards, tom lane


Re: Lock partitions

From
Tom Lane
Date:
Mark Wong <markw@osdl.org> writes:
> Tom Lane wrote:
>> It would be nice to see some results from the OSDL tests with, say, 4,
>> 8, and 16 lock partitions before we forget about the point though.
>> Anybody know whether OSDL is in a position to run tests for us?

> Yeah, I can run some dbt2 tests in the lab.  I'll get started on it. 
> We're still a little bit away from getting the automated testing for 
> PostgreSQL going again though.

Great, thanks.  The thing to twiddle is LOG2_NUM_LOCK_PARTITIONS in
src/include/storage/lwlock.h.  You need a full backend recompile
after changing it, but you shouldn't need to initdb, if that helps.
        regards, tom lane


Re: Lock partitions

From
Mark Wong
Date:
Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
>> I see this in the CVS commits for 8.2.  Did we determine the proper
>> number of lock partitions?  Should it be based on the number of buffers
>> or concurrent sessions allowed?
> 
> No.  NUM_LOCK_PARTITIONS needs to be a compile-time constant for a
> number of reasons, and there is absolutely zero evidence to justify
> making any effort (and spending any cycles) on a variable value.
> 
> It would be nice to see some results from the OSDL tests with, say, 4,
> 8, and 16 lock partitions before we forget about the point though.
> Anybody know whether OSDL is in a position to run tests for us?

Yeah, I can run some dbt2 tests in the lab.  I'll get started on it. 
We're still a little bit away from getting the automated testing for 
PostgreSQL going again though.

Mark


Re: Lock partitions

From
Simon Riggs
Date:
On Mon, 2006-09-11 at 11:29 -0400, Tom Lane wrote:
> Mark Wong <markw@osdl.org> writes:
> > Tom Lane wrote:
> >> It would be nice to see some results from the OSDL tests with, say, 4,
> >> 8, and 16 lock partitions before we forget about the point though.
> >> Anybody know whether OSDL is in a position to run tests for us?
> 
> > Yeah, I can run some dbt2 tests in the lab.  I'll get started on it. 
> > We're still a little bit away from getting the automated testing for 
> > PostgreSQL going again though.
> 
> Great, thanks.  The thing to twiddle is LOG2_NUM_LOCK_PARTITIONS in
> src/include/storage/lwlock.h.  You need a full backend recompile
> after changing it, but you shouldn't need to initdb, if that helps.

IIRC we did that already and the answer was 16...

--  Simon Riggs              EnterpriseDB   http://www.enterprisedb.com



Re: Lock partitions

From
Tom Lane
Date:
Simon Riggs <simon@2ndquadrant.com> writes:
> On Mon, 2006-09-11 at 11:29 -0400, Tom Lane wrote:
>> Great, thanks.  The thing to twiddle is LOG2_NUM_LOCK_PARTITIONS in
>> src/include/storage/lwlock.h.  You need a full backend recompile
>> after changing it, but you shouldn't need to initdb, if that helps.

> IIRC we did that already and the answer was 16...

No, no one has shown me any numbers from any "real" tests (anything
more than pgbench on a Dell PC ...).
        regards, tom lane


Re: Lock partitions

From
"Strong, David"
Date:
We can pass on what we've seen when running tests here with different
BufMapping and LockMgr partition sizes.

We use a TPC-C inspired benchmark. Currently it is configured to run 25
backend processes. The test runs for 16 minutes as this is the minimum
amount of time we can run and obtain useful information. This gives us
24,000 seconds (25 * 16 * 60) of processing time.

The following timings have been rounded to the nearest second and
represent the amount of time amongst all backend processes to acquire
and release locks. For example, a value of 2500 seconds would mean each
backend process (25) took ~100 seconds to acquire or release a lock.
Although, in reality, the time spent locking or releasing each partition
entry is not uniform and there are some definite hotspot entries. We can
pass on some of the lock output if anyone is interested.

When using 16 buffer and 16 lock partitions, we see that BufMapping
takes 809 seconds to acquire locks and 174 seconds to release locks. The
LockMgr takes 362 seconds to acquire locks and 26 seconds to release
locks.

When using 128 buffer and 128 lock partitions, we see that BufMapping
takes 277 seconds (532 seconds improvement) to acquire locks and 78
seconds (96 seconds improvement) to release locks. The LockMgr takes 235
seconds (127 seconds improvement) to acquire locks and 22 seconds (4
seconds improvement) to release locks.

Overall, 128 BufMapping partitions improves locking/releasing by 678
seconds, 128 LockMgr partitions improves locking/releasing by 131
seconds.

With the improvements in the various locking times, one might expect an
improvement in the overall benchmark result. However, a 16 partition run
produces a result of 198.74 TPS and a 128 partition run produces a
result of 203.24 TPS.

Part of the time saved from BufMapping and LockMgr partitions is
absorbed into the WALInsertLock lock. For a 16 partition run, the total
time to lock/release the WALInsertLock lock is 5845 seconds. For 128
partitions, the WALInsertLock lock takes 6172 seconds, an increase of
327 seconds. Perhaps we have our WAL configured incorrectly?

Other static locks are also affected, but not as much as the
WALInsertLock lock. For example, the ProcArrayLock lock increases from
337 seconds to 348 seconds. The SInvalLock lock increases from 317
seconds to 331 seconds.

Due to expansion of time in other locks, a 128 partition run only spends
403 seconds less in locking than a 16 partition run.

We can generate some OProfile statistics, but most of the time saved is
probably absorbed into functions such as HeapTupleSatisfiesSnapshot and
PinBuffer which seem to have a very high overhead.

David

-----Original Message-----
From: pgsql-hackers-owner@postgresql.org
[mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Simon Riggs
Sent: Tuesday, September 12, 2006 1:37 AM
To: Tom Lane
Cc: Mark Wong; Bruce Momjian; PostgreSQL-development
Subject: Re: [HACKERS] Lock partitions

On Mon, 2006-09-11 at 11:29 -0400, Tom Lane wrote:
> Mark Wong <markw@osdl.org> writes:
> > Tom Lane wrote:
> >> It would be nice to see some results from the OSDL tests with, say,
4,
> >> 8, and 16 lock partitions before we forget about the point though.
> >> Anybody know whether OSDL is in a position to run tests for us?
>
> > Yeah, I can run some dbt2 tests in the lab.  I'll get started on it.

> > We're still a little bit away from getting the automated testing for

> > PostgreSQL going again though.
>
> Great, thanks.  The thing to twiddle is LOG2_NUM_LOCK_PARTITIONS in
> src/include/storage/lwlock.h.  You need a full backend recompile
> after changing it, but you shouldn't need to initdb, if that helps.

IIRC we did that already and the answer was 16...

--  Simon Riggs              EnterpriseDB   http://www.enterprisedb.com


---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend


Re: Lock partitions

From
Tom Lane
Date:
"Strong, David" <david.strong@unisys.com> writes:
> When using 16 buffer and 16 lock partitions, we see that BufMapping
> takes 809 seconds to acquire locks and 174 seconds to release locks. The
> LockMgr takes 362 seconds to acquire locks and 26 seconds to release
> locks.

> When using 128 buffer and 128 lock partitions, we see that BufMapping
> takes 277 seconds (532 seconds improvement) to acquire locks and 78
> seconds (96 seconds improvement) to release locks. The LockMgr takes 235
> seconds (127 seconds improvement) to acquire locks and 22 seconds (4
> seconds improvement) to release locks.

While I don't see any particular penalty to increasing
NUM_BUFFER_PARTITIONS, increasing NUM_LOCK_PARTITIONS carries a very
significant penalty (increasing PGPROC size as well as the work needed
during LockReleaseAll, which is executed at every transaction end).
I think 128 lock partitions is probably verging on the ridiculous
... particularly if your benchmark only involves touching half a dozen
tables.  I'd be more interested in comparisons between 4 and 16 lock
partitions.  Also, please vary the two settings independently rather
than confusing the issue by changing them both at once.

> With the improvements in the various locking times, one might expect an
> improvement in the overall benchmark result. However, a 16 partition run
> produces a result of 198.74 TPS and a 128 partition run produces a
> result of 203.24 TPS.

> Part of the time saved from BufMapping and LockMgr partitions is
> absorbed into the WALInsertLock lock. For a 16 partition run, the total
> time to lock/release the WALInsertLock lock is 5845 seconds. For 128
> partitions, the WALInsertLock lock takes 6172 seconds, an increase of
> 327 seconds. Perhaps we have our WAL configured incorrectly?

I fear this throws your entire measurement procedure into question.  For
a fixed workload the number of acquisitions of WALInsertLock ought to be
fixed, so you shouldn't see any more contention for WALInsertLock if the
transaction rate didn't change materially.
        regards, tom lane


Re: Lock partitions

From
"Strong, David"
Date:
Tom,

Thanks for the feedback. We'll run a few tests with differing buffer and
lock partition sizes in the range you're interested in and we'll let you
know what we see.

Our workload is not fixed, however. Our benchmark does not follow the
strict TPC-C guideline of using think times etc. We throw as many
transactions at the database as we can. So, when any time is freed up,
we will fill it with another transaction. We simply want to stress as
much as we can. As one bottleneck is removed, the time saved obviously
flows to the next.

Postgres 8.2 moves some of the time that used to be consumed by single
BufMappingLock and LockMGRLock locks to the WALInsertLock lock. We have
run tests where we made XLogInsert a NOP, because we wanted to see where
the next bottleneck would be, and some of the time occupied by
WALInsertLock lock was absorbed by the SInvalLock lock. We have not
tried to remove the SInvalLock lock to see where time flows to next, but
we might.

David

-----Original Message-----
From: Tom Lane [mailto:tgl@sss.pgh.pa.us]
Sent: Tuesday, September 12, 2006 9:40 AM
To: Strong, David
Cc: PostgreSQL-development
Subject: Re: [HACKERS] Lock partitions

"Strong, David" <david.strong@unisys.com> writes:
> When using 16 buffer and 16 lock partitions, we see that BufMapping
> takes 809 seconds to acquire locks and 174 seconds to release locks.
The
> LockMgr takes 362 seconds to acquire locks and 26 seconds to release
> locks.

> When using 128 buffer and 128 lock partitions, we see that BufMapping
> takes 277 seconds (532 seconds improvement) to acquire locks and 78
> seconds (96 seconds improvement) to release locks. The LockMgr takes
235
> seconds (127 seconds improvement) to acquire locks and 22 seconds (4
> seconds improvement) to release locks.

While I don't see any particular penalty to increasing
NUM_BUFFER_PARTITIONS, increasing NUM_LOCK_PARTITIONS carries a very
significant penalty (increasing PGPROC size as well as the work needed
during LockReleaseAll, which is executed at every transaction end).
I think 128 lock partitions is probably verging on the ridiculous
... particularly if your benchmark only involves touching half a dozen
tables.  I'd be more interested in comparisons between 4 and 16 lock
partitions.  Also, please vary the two settings independently rather
than confusing the issue by changing them both at once.

> With the improvements in the various locking times, one might expect
an
> improvement in the overall benchmark result. However, a 16 partition
run
> produces a result of 198.74 TPS and a 128 partition run produces a
> result of 203.24 TPS.

> Part of the time saved from BufMapping and LockMgr partitions is
> absorbed into the WALInsertLock lock. For a 16 partition run, the
total
> time to lock/release the WALInsertLock lock is 5845 seconds. For 128
> partitions, the WALInsertLock lock takes 6172 seconds, an increase of
> 327 seconds. Perhaps we have our WAL configured incorrectly?

I fear this throws your entire measurement procedure into question.  For
a fixed workload the number of acquisitions of WALInsertLock ought to be
fixed, so you shouldn't see any more contention for WALInsertLock if the
transaction rate didn't change materially.
        regards, tom lane


Re: Lock partitions

From
Simon Riggs
Date:
On Tue, 2006-09-12 at 12:40 -0400, Tom Lane wrote:
> "Strong, David" <david.strong@unisys.com> writes:
> > When using 16 buffer and 16 lock partitions, we see that BufMapping
> > takes 809 seconds to acquire locks and 174 seconds to release locks. The
> > LockMgr takes 362 seconds to acquire locks and 26 seconds to release
> > locks.
> 
> > When using 128 buffer and 128 lock partitions, we see that BufMapping
> > takes 277 seconds (532 seconds improvement) to acquire locks and 78
> > seconds (96 seconds improvement) to release locks. The LockMgr takes 235
> > seconds (127 seconds improvement) to acquire locks and 22 seconds (4
> > seconds improvement) to release locks.
> 
> While I don't see any particular penalty to increasing
> NUM_BUFFER_PARTITIONS, increasing NUM_LOCK_PARTITIONS carries a very
> significant penalty (increasing PGPROC size as well as the work needed
> during LockReleaseAll, which is executed at every transaction end).
> I think 128 lock partitions is probably verging on the ridiculous
> ... particularly if your benchmark only involves touching half a dozen
> tables.  I'd be more interested in comparisons between 4 and 16 lock
> partitions.  Also, please vary the two settings independently rather
> than confusing the issue by changing them both at once.

Good thinking David. Even if 128 is fairly high, it does seem worth
exploring higher values - I was just stuck in "fewer == better"
thoughts.

> > With the improvements in the various locking times, one might expect an
> > improvement in the overall benchmark result. However, a 16 partition run
> > produces a result of 198.74 TPS and a 128 partition run produces a
> > result of 203.24 TPS.
> 
> > Part of the time saved from BufMapping and LockMgr partitions is
> > absorbed into the WALInsertLock lock. For a 16 partition run, the total
> > time to lock/release the WALInsertLock lock is 5845 seconds. For 128
> > partitions, the WALInsertLock lock takes 6172 seconds, an increase of
> > 327 seconds. Perhaps we have our WAL configured incorrectly?
> 
> I fear this throws your entire measurement procedure into question.  For
> a fixed workload the number of acquisitions of WALInsertLock ought to be
> fixed, so you shouldn't see any more contention for WALInsertLock if the
> transaction rate didn't change materially.

David's results were to do with lock acquire/release time, not the
number of acquisitions, so that in itself doesn't make me doubt these
measurements. Perhaps we can ask whether there was a substantially
different number of lock acquisitions? As Tom says, that would be an
issue.

It seems reasonable that relieving the bottleneck on BufMapping and
LockMgr locks that we would then queue longer on the next bottleneck,
WALInsertLock. So again, those tests seem reasonable to me so far.

These seem to be the beginnings of accurate wait time analysis, so I'm
listening closely.

Are you using a lightweight timer?

--  Simon Riggs              EnterpriseDB   http://www.enterprisedb.com



Re: Lock partitions

From
"Strong, David"
Date:
Simon,

In the 16/16 (16 buffer partitions/16 lock partitions) test, the
WALInsertLock lock had 14643080 acquisition attempts and 12057678
successful acquisitions on the lock. That's 2585402 retries on the lock.
That is to say that PGSemaphoreLock was invoked 2585402 times.

In the 128/128 test, the WALInsertLock lock had 14991208 acquisition
attempts and 12324765 successful acquisitions. That's 2666443 retries.

The 128/128 test attempted 348128 more lock acquisitions than the 16/16
test and retried 81041 times more than the 16/16 test. We attribute the
rise in WALInsertLock lock accesses to the reduction in time on
acquiring the BufMapping and LockMgr partition locks. Does this seem
reasonable?

The overhead of any monitoring is of great concern to us. We've tried
both clock_gettime () and getttimeofday () calls. They both seem to have
the same overhead ~1 us/call (measured against the TSC of the CPU) and
both seem to be accurate. We realize this can be a delicate point and so
we would be happy to rerun any tests with a different timing mechanism.

David

-----Original Message-----
From: Simon Riggs [mailto:simon@2ndquadrant.com]
Sent: Wednesday, September 13, 2006 2:22 AM
To: Tom Lane
Cc: Strong, David; PostgreSQL-development
Subject: Re: [HACKERS] Lock partitions

On Tue, 2006-09-12 at 12:40 -0400, Tom Lane wrote:
> "Strong, David" <david.strong@unisys.com> writes:
> > When using 16 buffer and 16 lock partitions, we see that BufMapping
> > takes 809 seconds to acquire locks and 174 seconds to release locks.
The
> > LockMgr takes 362 seconds to acquire locks and 26 seconds to release
> > locks.
>
> > When using 128 buffer and 128 lock partitions, we see that
BufMapping
> > takes 277 seconds (532 seconds improvement) to acquire locks and 78
> > seconds (96 seconds improvement) to release locks. The LockMgr takes
235
> > seconds (127 seconds improvement) to acquire locks and 22 seconds (4
> > seconds improvement) to release locks.
>
> While I don't see any particular penalty to increasing
> NUM_BUFFER_PARTITIONS, increasing NUM_LOCK_PARTITIONS carries a very
> significant penalty (increasing PGPROC size as well as the work needed
> during LockReleaseAll, which is executed at every transaction end).
> I think 128 lock partitions is probably verging on the ridiculous
> ... particularly if your benchmark only involves touching half a dozen
> tables.  I'd be more interested in comparisons between 4 and 16 lock
> partitions.  Also, please vary the two settings independently rather
> than confusing the issue by changing them both at once.

Good thinking David. Even if 128 is fairly high, it does seem worth
exploring higher values - I was just stuck in "fewer == better"
thoughts.

> > With the improvements in the various locking times, one might expect
an
> > improvement in the overall benchmark result. However, a 16 partition
run
> > produces a result of 198.74 TPS and a 128 partition run produces a
> > result of 203.24 TPS.
>
> > Part of the time saved from BufMapping and LockMgr partitions is
> > absorbed into the WALInsertLock lock. For a 16 partition run, the
total
> > time to lock/release the WALInsertLock lock is 5845 seconds. For 128
> > partitions, the WALInsertLock lock takes 6172 seconds, an increase
of
> > 327 seconds. Perhaps we have our WAL configured incorrectly?
>
> I fear this throws your entire measurement procedure into question.
For
> a fixed workload the number of acquisitions of WALInsertLock ought to
be
> fixed, so you shouldn't see any more contention for WALInsertLock if
the
> transaction rate didn't change materially.

David's results were to do with lock acquire/release time, not the
number of acquisitions, so that in itself doesn't make me doubt these
measurements. Perhaps we can ask whether there was a substantially
different number of lock acquisitions? As Tom says, that would be an
issue.

It seems reasonable that relieving the bottleneck on BufMapping and
LockMgr locks that we would then queue longer on the next bottleneck,
WALInsertLock. So again, those tests seem reasonable to me so far.

These seem to be the beginnings of accurate wait time analysis, so I'm
listening closely.

Are you using a lightweight timer?

--  Simon Riggs              EnterpriseDB   http://www.enterprisedb.com



Re: Lock partitions

From
"Strong, David"
Date:
Tom,

We have some results for you. We left the buffer partition locks at 128
as this did not seem to be a concern and we're still using 25 backend
processes. We ran tests for 4, 8 and 16 lock partitions.

For 4 lock partitions, it took 620 seconds to acquire locks and 32
seconds to release locks. The test produced 199.95 TPS.

For 8 lock partitions, it took 505 seconds to acquire locks and 31
seconds to release locks. The test produced 201.16 TPS.

For 16 lock partitions, it took 362 seconds to acquire locks and 22
seconds to release locks. The test produced 200.75 TPS.

And, just for grins, using 128 buffer and 128 lock partitions, took 235
seconds to acquire locks and 22 seconds to release locks. The test
produced 203.24 TPS.

Let me know if we can provide any additional information from these
tests and if there are any other tests that we can run.

David

-----Original Message-----
From: pgsql-hackers-owner@postgresql.org
[mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Strong, David
Sent: Wednesday, September 13, 2006 10:52 AM
To: PostgreSQL-development
Subject: Re: [HACKERS] Lock partitions

Simon,

In the 16/16 (16 buffer partitions/16 lock partitions) test, the
WALInsertLock lock had 14643080 acquisition attempts and 12057678
successful acquisitions on the lock. That's 2585402 retries on the lock.
That is to say that PGSemaphoreLock was invoked 2585402 times.

In the 128/128 test, the WALInsertLock lock had 14991208 acquisition
attempts and 12324765 successful acquisitions. That's 2666443 retries.

The 128/128 test attempted 348128 more lock acquisitions than the 16/16
test and retried 81041 times more than the 16/16 test. We attribute the
rise in WALInsertLock lock accesses to the reduction in time on
acquiring the BufMapping and LockMgr partition locks. Does this seem
reasonable?

The overhead of any monitoring is of great concern to us. We've tried
both clock_gettime () and getttimeofday () calls. They both seem to have
the same overhead ~1 us/call (measured against the TSC of the CPU) and
both seem to be accurate. We realize this can be a delicate point and so
we would be happy to rerun any tests with a different timing mechanism.

David

-----Original Message-----
From: Simon Riggs [mailto:simon@2ndquadrant.com]
Sent: Wednesday, September 13, 2006 2:22 AM
To: Tom Lane
Cc: Strong, David; PostgreSQL-development
Subject: Re: [HACKERS] Lock partitions

On Tue, 2006-09-12 at 12:40 -0400, Tom Lane wrote:
> "Strong, David" <david.strong@unisys.com> writes:
> > When using 16 buffer and 16 lock partitions, we see that BufMapping
> > takes 809 seconds to acquire locks and 174 seconds to release locks.
The
> > LockMgr takes 362 seconds to acquire locks and 26 seconds to release
> > locks.
>
> > When using 128 buffer and 128 lock partitions, we see that
BufMapping
> > takes 277 seconds (532 seconds improvement) to acquire locks and 78
> > seconds (96 seconds improvement) to release locks. The LockMgr takes
235
> > seconds (127 seconds improvement) to acquire locks and 22 seconds (4
> > seconds improvement) to release locks.
>
> While I don't see any particular penalty to increasing
> NUM_BUFFER_PARTITIONS, increasing NUM_LOCK_PARTITIONS carries a very
> significant penalty (increasing PGPROC size as well as the work needed
> during LockReleaseAll, which is executed at every transaction end).
> I think 128 lock partitions is probably verging on the ridiculous
> ... particularly if your benchmark only involves touching half a dozen
> tables.  I'd be more interested in comparisons between 4 and 16 lock
> partitions.  Also, please vary the two settings independently rather
> than confusing the issue by changing them both at once.

Good thinking David. Even if 128 is fairly high, it does seem worth
exploring higher values - I was just stuck in "fewer == better"
thoughts.

> > With the improvements in the various locking times, one might expect
an
> > improvement in the overall benchmark result. However, a 16 partition
run
> > produces a result of 198.74 TPS and a 128 partition run produces a
> > result of 203.24 TPS.
>
> > Part of the time saved from BufMapping and LockMgr partitions is
> > absorbed into the WALInsertLock lock. For a 16 partition run, the
total
> > time to lock/release the WALInsertLock lock is 5845 seconds. For 128
> > partitions, the WALInsertLock lock takes 6172 seconds, an increase
of
> > 327 seconds. Perhaps we have our WAL configured incorrectly?
>
> I fear this throws your entire measurement procedure into question.
For
> a fixed workload the number of acquisitions of WALInsertLock ought to
be
> fixed, so you shouldn't see any more contention for WALInsertLock if
the
> transaction rate didn't change materially.

David's results were to do with lock acquire/release time, not the
number of acquisitions, so that in itself doesn't make me doubt these
measurements. Perhaps we can ask whether there was a substantially
different number of lock acquisitions? As Tom says, that would be an
issue.

It seems reasonable that relieving the bottleneck on BufMapping and
LockMgr locks that we would then queue longer on the next bottleneck,
WALInsertLock. So again, those tests seem reasonable to me so far.

These seem to be the beginnings of accurate wait time analysis, so I'm
listening closely.

Are you using a lightweight timer?

--  Simon Riggs              EnterpriseDB   http://www.enterprisedb.com


---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to      choose an index scan if your joining column's
datatypesdo not      match 


Re: Lock partitions

From
Tom Lane
Date:
"Strong, David" <david.strong@unisys.com> writes:
> We have some results for you. We left the buffer partition locks at 128
> as this did not seem to be a concern and we're still using 25 backend
> processes. We ran tests for 4, 8 and 16 lock partitions. 

> For 4 lock partitions, it took 620 seconds to acquire locks and 32
> seconds to release locks. The test produced 199.95 TPS.

> For 8 lock partitions, it took 505 seconds to acquire locks and 31
> seconds to release locks. The test produced 201.16 TPS.

> For 16 lock partitions, it took 362 seconds to acquire locks and 22
> seconds to release locks. The test produced 200.75 TPS.

> And, just for grins, using 128 buffer and 128 lock partitions, took 235
> seconds to acquire locks and 22 seconds to release locks. The test
> produced 203.24 TPS.

[ itch... ]  I can't help thinking there's something wrong with this;
the wait-time measurements seem sane, but why is there essentially no
change in the TPS result?

The above numbers are only for the lock-partition LWLocks, right?
What are the totals --- that is, how much time is spent blocked
vs. processing overall?
        regards, tom lane


Re: Lock partitions

From
Jim Nasby
Date:
On Sep 13, 2006, at 2:46 PM, Strong, David wrote:
> We have some results for you. We left the buffer partition locks at  
> 128
> as this did not seem to be a concern and we're still using 25 backend
> processes. We ran tests for 4, 8 and 16 lock partitions.

Isn't having more lock partitions than buffer partitions pointless?
--
Jim Nasby                                            jim@nasby.net
EnterpriseDB      http://enterprisedb.com      512.569.9461 (cell)



Re: Lock partitions

From
Tom Lane
Date:
Jim Nasby <jim@nasby.net> writes:
> Isn't having more lock partitions than buffer partitions pointless?

AFAIK they're pretty orthogonal.  It's true though that a typical
transaction doesn't hold all that many locks, which is why I don't
see a need for a large number of lock partitions.
        regards, tom lane


Re: Lock partitions

From
"Strong, David"
Date:
Tom,

Taking the 4 lock vs 8 lock partitions, 4 LockMgr lock partitions spent
a total of 652 seconds in lock management (acquiring/releasing) and 8
LockMgr lock partitions spent a total of 536 in lock management. This is
an improvement of 116 seconds, but the TPS didn't improve by much - only
a 1.21 TPS improvement.

The improvement in the LockMgr processing is consumed by the next system
bottleneck downstream as more work is being let through. In this
particular case it's the WALInsertLock lock. The 4 LockMgr lock
partition test spent a total of 5868 seconds in WALInsertLock lock
management whereas the 8 LockMgr partition test spent 5945 seconds in
WALInsertLock lock management which is an increase of 77 seconds. But,
that's not the only static lock that increased in time, it's just the
most significant increase. The WALWriteLock lock increased by 12
seconds, ProcArrayLock increased by 8 seconds and SInvalLock increased
by 5 seconds. This takes the total time flowing to other locks to 102
seconds.

The locks are not the only part of the puzzle. As improvements are made
to various areas like the BufMapping and LockMgr lock partitions, other
parts of the system start to get exercised in ways that were not
possible in previous releases. We're still trying to get our arms around
all the functions that might become bottlenecks when other lock
contention is minimized.

And, improvements are being made. The locking changes from 8.0.x to
8.1.x made a significant difference in scalability. Again, the current
lock improvements in 8.2 have realized ~20% improvement over 8.1.x,
based on our testing.

We added monitoring code to the LWLockAcquire and LWLockRelease
functions. We track the total time taken to pass through LWLockAcquire
and LWLockRelease. So, if a particular backend process takes 1 second to
run through LWLockAcquire, we will track that as 1 second in lock
acquisition. Irrespective of whether my backend process was spinning or
in a semaphore wait, it's 1 second that was taken away from processing a
statement/request. We could also add timing for semaphore waits within
LWLockAcquire, if that would be a useful statistic.

Let me know if there are any other tests or metrics that would be
useful.

David

-----Original Message-----
From: Tom Lane [mailto:tgl@sss.pgh.pa.us]
Sent: Wednesday, September 13, 2006 1:36 PM
To: Strong, David
Cc: PostgreSQL-development
Subject: Re: [HACKERS] Lock partitions

"Strong, David" <david.strong@unisys.com> writes:
> We have some results for you. We left the buffer partition locks at
128
> as this did not seem to be a concern and we're still using 25 backend
> processes. We ran tests for 4, 8 and 16 lock partitions.

> For 4 lock partitions, it took 620 seconds to acquire locks and 32
> seconds to release locks. The test produced 199.95 TPS.

> For 8 lock partitions, it took 505 seconds to acquire locks and 31
> seconds to release locks. The test produced 201.16 TPS.

> For 16 lock partitions, it took 362 seconds to acquire locks and 22
> seconds to release locks. The test produced 200.75 TPS.

> And, just for grins, using 128 buffer and 128 lock partitions, took
235
> seconds to acquire locks and 22 seconds to release locks. The test
> produced 203.24 TPS.

[ itch... ]  I can't help thinking there's something wrong with this;
the wait-time measurements seem sane, but why is there essentially no
change in the TPS result?

The above numbers are only for the lock-partition LWLocks, right?
What are the totals --- that is, how much time is spent blocked
vs. processing overall?
        regards, tom lane


Re: Lock partitions

From
Mark Wong
Date:
Tom Lane wrote:
> Mark Wong <markw@osdl.org> writes:
>> Tom Lane wrote:
>>> It would be nice to see some results from the OSDL tests with, say, 4,
>>> 8, and 16 lock partitions before we forget about the point though.
>>> Anybody know whether OSDL is in a position to run tests for us?
> 
>> Yeah, I can run some dbt2 tests in the lab.  I'll get started on it. 
>> We're still a little bit away from getting the automated testing for 
>> PostgreSQL going again though.
> 
> Great, thanks.  The thing to twiddle is LOG2_NUM_LOCK_PARTITIONS in
> src/include/storage/lwlock.h.  You need a full backend recompile
> after changing it, but you shouldn't need to initdb, if that helps.

Sorry for the delay but looks like there's some data coming in.  It also 
looks like my kit is starting to be a little dated.  My stored libpq 
calls are failing.  I'm getting this message:

ERROR:  record type has not been registered
From PQerrorMessage() on line 41 from this bit of code:

        /* Create the query and execute it. */        sprintf(stmt, "SELECT * FROM order_status(%d, %d, %d, '%s')",
          data->c_id, data->c_w_id, data->c_d_id, data->c_last);        res = PQexec(dbc->conn, stmt);        if (!res
||(PQresultStatus(res) != PGRES_COMMAND_OK &&                PQresultStatus(res) != PGRES_TUPLES_OK)) {
LOG_ERROR_MESSAGE("%s",PQerrorMessage(dbc->conn));                PQclear(res);                return ERROR;        }
 

LOG_ERROR_MESSAGE() is just a macro for a function that does a printf().

Any suggestions?

Thanks,
Mark


Re: Lock partitions

From
Tom Lane
Date:
Mark Wong <markw@osdl.org> writes:
> Sorry for the delay but looks like there's some data coming in.  It also 
> looks like my kit is starting to be a little dated.  My stored libpq 
> calls are failing.  I'm getting this message:

> ERROR:  record type has not been registered

This is a server-side failure --- could we see how order_status()
is defined?  What PG version are you testing exactly?
        regards, tom lane


Re: Lock partitions

From
Mark Wong
Date:
Tom Lane wrote:
> Mark Wong <markw@osdl.org> writes:
>> Sorry for the delay but looks like there's some data coming in.  It also 
>> looks like my kit is starting to be a little dated.  My stored libpq 
>> calls are failing.  I'm getting this message:
> 
>> ERROR:  record type has not been registered
> 
> This is a server-side failure --- could we see how order_status()
> is defined?  What PG version are you testing exactly?

I took pgsqsl snapshot from cvs on Sept 11.  Due to the length of the 
file that order_status() is in and of order_status() itself, here's is a 
url for the file in the svn repository.  order_status() is defined 
starting on line 710:

http://svn.sourceforge.net/viewvc/osdldbt/trunk/dbt2/storedproc/pgsql/c/funcs.c?view=markup

Thanks,
Mark


Re: Lock partitions

From
Tom Lane
Date:
Mark Wong <markw@osdl.org> writes:
> Tom Lane wrote:
>> This is a server-side failure --- could we see how order_status()
>> is defined?  What PG version are you testing exactly?

> I took pgsqsl snapshot from cvs on Sept 11.  Due to the length of the 
> file that order_status() is in and of order_status() itself, here's is a 
> url for the file in the svn repository.  order_status() is defined 
> starting on line 710:

> http://svn.sourceforge.net/viewvc/osdldbt/trunk/dbt2/storedproc/pgsql/c/funcs.c?view=markup

Hmph.  I think we broke something --- the error implies that some
function tried to return a tuple that hadn't been properly "blessed",
but I can't see that order_status or any of the other functions in that
file are doing anything wrong.  In any case, if it used to work for you,
we had better figure out exactly why it stopped working.

I know you've posted info before on how to set up and run the dbt code,
but could you refresh my memory?  Is there a URL somewhere with the info?
        regards, tom lane


Re: Lock partitions

From
Mark Wong
Date:
Tom Lane wrote:
> Mark Wong <markw@osdl.org> writes:
>> Tom Lane wrote:
>>> This is a server-side failure --- could we see how order_status()
>>> is defined?  What PG version are you testing exactly?
> 
>> I took pgsqsl snapshot from cvs on Sept 11.  Due to the length of the 
>> file that order_status() is in and of order_status() itself, here's is a 
>> url for the file in the svn repository.  order_status() is defined 
>> starting on line 710:
> 
>> http://svn.sourceforge.net/viewvc/osdldbt/trunk/dbt2/storedproc/pgsql/c/funcs.c?view=markup
> 
> Hmph.  I think we broke something --- the error implies that some
> function tried to return a tuple that hadn't been properly "blessed",
> but I can't see that order_status or any of the other functions in that
> file are doing anything wrong.  In any case, if it used to work for you,
> we had better figure out exactly why it stopped working.
> 
> I know you've posted info before on how to set up and run the dbt code,
> but could you refresh my memory?  Is there a URL somewhere with the info?

Here's a readme: 
http://svn.sourceforge.net/viewvc/osdldbt/trunk/dbt2/README-POSTGRESQL?view=markup

But perhaps something much easier, using subversion:

mkdir /mnt/dbt2 # for pgdata
svn co https://svn.sourceforge.net/svnroot/osdldbt/trunk/dbt2 dbt2
cd dbt2
./configure --with-postgresql=<pgsql_dir>
make
cd scripts/pgsql/
./build_db -g -w 1
cd ..
./run_workload -w 1 -d 120 -c 20 -n

I think that should work.  That will create a 1 warehouse database and 
run a 120 second test with no-thinktimes.  That should be sufficient to 
run through each transaction more than several times.

Thanks,
Mark


Re: Lock partitions

From
Tom Lane
Date:
Mark Wong <markw@osdl.org> writes:
> But perhaps something much easier, using subversion:

> mkdir /mnt/dbt2 # for pgdata
> svn co https://svn.sourceforge.net/svnroot/osdldbt/trunk/dbt2 dbt2
> cd dbt2
> ./configure --with-postgresql=<pgsql_dir>

configure is not in the svn checkout.  I guessed that I needed to do
aclocal/automake/autoconf, but automake fails:

$ automake
configure.ac:11: required file `config.h.in' not found
$

and I don't see anyplace to get config.h.in from.
        regards, tom lane


Re: Lock partitions

From
Mark Wong
Date:
Tom Lane wrote:
> Mark Wong <markw@osdl.org> writes:
>> But perhaps something much easier, using subversion:
> 
>> mkdir /mnt/dbt2 # for pgdata
>> svn co https://svn.sourceforge.net/svnroot/osdldbt/trunk/dbt2 dbt2
>> cd dbt2
>> ./configure --with-postgresql=<pgsql_dir>
> 
> configure is not in the svn checkout.  I guessed that I needed to do
> aclocal/automake/autoconf, but automake fails:
> 
> $ automake
> configure.ac:11: required file `config.h.in' not found
> $
> 
> and I don't see anyplace to get config.h.in from.

Oops!  'autoreconf --install' is what I run to generate all that stuff.  Sorry about that.

Thanks,
Mark


Re: Lock partitions

From
Tom Lane
Date:
Mark Wong <markw@osdl.org> writes:
> Oops!  'autoreconf --install' is what I run to generate all that stuff. 

Ah, better.  I see at least part of the problem:

CREATE OR REPLACE FUNCTION stock_level (INTEGER, INTEGER, INTEGER) RETURNS INTEGER AS
'/home/tgl/dbt2/storedproc/pgsql/c/../../../storedproc/pgsql/c/funcs'LANGUAGE C STRICT;
 
psql:/home/tgl/dbt2/scripts/pgsql/../../storedproc/pgsql/c/stock_level.sql:7: ERROR:  incompatible library
"/home/tgl/dbt2/storedproc/pgsql/c/../../../storedproc/pgsql/c/funcs.so":missing magic block
 
HINT:  Extension libraries are now required to use the PG_MODULE_MAGIC macro.

You need to add something like this to funcs.c:
 #include <executor/spi.h> /* this should include most necessary APIs */ #include <executor/executor.h>  /* for
GetAttributeByName()*/ #include <funcapi.h> /* for returning set of rows in order_status */
 
+ 
+ #ifdef PG_MODULE_MAGIC
+ PG_MODULE_MAGIC;
+ #endif  /* #define DEBUG

With that change, I didn't see run_workload report any errors, but maybe
I don't know where to look.  I'm not sure how this bug could have led to
a "type not registered" error ... the query should've just failed outright.
        regards, tom lane


Re: Lock partitions

From
Mark Wong
Date:
Tom Lane wrote:
> Mark Wong <markw@osdl.org> writes:
>> Oops!  'autoreconf --install' is what I run to generate all that stuff. 
> 
> Ah, better.  I see at least part of the problem:
> 
> CREATE OR REPLACE FUNCTION stock_level (INTEGER, INTEGER, INTEGER) RETURNS INTEGER AS
'/home/tgl/dbt2/storedproc/pgsql/c/../../../storedproc/pgsql/c/funcs'LANGUAGE C STRICT;
 
> psql:/home/tgl/dbt2/scripts/pgsql/../../storedproc/pgsql/c/stock_level.sql:7: ERROR:  incompatible library
"/home/tgl/dbt2/storedproc/pgsql/c/../../../storedproc/pgsql/c/funcs.so":missing magic block
 
> HINT:  Extension libraries are now required to use the PG_MODULE_MAGIC macro.
> 
> You need to add something like this to funcs.c:
> 
>   #include <executor/spi.h> /* this should include most necessary APIs */
>   #include <executor/executor.h>  /* for GetAttributeByName() */
>   #include <funcapi.h> /* for returning set of rows in order_status */
> + 
> + #ifdef PG_MODULE_MAGIC
> + PG_MODULE_MAGIC;
> + #endif
>   
>   /*
>   #define DEBUG
> 
> With that change, I didn't see run_workload report any errors, but maybe
> I don't know where to look.  I'm not sure how this bug could have led to
> a "type not registered" error ... the query should've just failed outright.

The error is captured in dbt2/scripts/output/*/client/error.log, where * 
is the run directory.

Ok, I'll give it a shot on my system.

Thanks,
Mark


Re: Lock partitions

From
Tom Lane
Date:
Mark Wong <markw@osdl.org> writes:
> Tom Lane wrote:
>> With that change, I didn't see run_workload report any errors, but maybe
>> I don't know where to look.

> The error is captured in dbt2/scripts/output/*/client/error.log, where * 
> is the run directory.

Hm ... here's what I see in there:

Thu Sep 14 15:19:16 2006
tid:-1430387232 client.c:129
20 DB worker threads have started
Thu Sep 14 15:19:31 2006
tid:1087957312 libpq/dbc_new_order.c:111
ERROR:  deadlock detected
DETAIL:  Process 5334 waits for ShareLock on transaction 3505055; blocked by process 5363.
Process 5363 waits for ShareLock on transaction 3505049; blocked by process 5334.
CONTEXT:  SQL statement "UPDATE stock
SET s_quantity = s_quantity - 10
WHERE s_i_id = 48368 AND s_w_id = 1"

Thu Sep 14 15:21:18 2006
tid:1089960256 client_interface.c:33
socket closed on _receive
Thu Sep 14 15:21:18 2006
tid:1089689920 client_interface.c:33
socket closed on _receive
Thu Sep 14 15:21:18 2006
tid:1090636096 client_interface.c:33
socket closed on _receive
Thu Sep 14 15:21:18 2006
tid:1090230592 client_interface.c:33
socket closed on _receive
Thu Sep 14 15:21:18 2006
tid:1090365760 client_interface.c:33
socket closed on _receive
Thu Sep 14 15:21:18 2006
tid:1090095424 client_interface.c:33
socket closed on _receive
Thu Sep 14 15:21:18 2006
tid:1089825088 client_interface.c:33
socket closed on _receive
Thu Sep 14 15:21:18 2006
tid:1090500928 client_interface.c:33
socket closed on _receive
Thu Sep 14 15:21:18 2006
tid:1090906432 client_interface.c:33
socket closed on _receive
Thu Sep 14 15:21:18 2006
tid:1090771264 client_interface.c:33
socket closed on _receive

Is the deadlock failure expected?
        regards, tom lane


Re: Lock partitions

From
Mark Wong
Date:
Tom Lane wrote:
> Mark Wong <markw@osdl.org> writes:
>> Tom Lane wrote:
>>> With that change, I didn't see run_workload report any errors, but maybe
>>> I don't know where to look.
> 
>> The error is captured in dbt2/scripts/output/*/client/error.log, where * 
>> is the run directory.
> 
> Hm ... here's what I see in there:
> 
> Thu Sep 14 15:19:16 2006
> tid:-1430387232 client.c:129
> 20 DB worker threads have started
> Thu Sep 14 15:19:31 2006
> tid:1087957312 libpq/dbc_new_order.c:111
> ERROR:  deadlock detected
> DETAIL:  Process 5334 waits for ShareLock on transaction 3505055; blocked by process 5363.
> Process 5363 waits for ShareLock on transaction 3505049; blocked by process 5334.
> CONTEXT:  SQL statement "UPDATE stock
> SET s_quantity = s_quantity - 10
> WHERE s_i_id = 48368
>   AND s_w_id = 1"
[snip]
> 
> Is the deadlock failure expected?

Ooh, that's interesting.  Deadlock failure is possible although I think 
we would all prefer that it didn't happen.  In the scheme of the 
workload having failed transactions is ok.  So with respect to having an 
invalid test run it's something I wouldn't worry about too much if it's 
infrequent.

Mark


Re: Lock partitions

From
Tom Lane
Date:
Mark Wong <markw@osdl.org> writes:
> Curious, I'm still seeing the same behavior.  Maybe I'll take another 
> snapshot from CVS.

Hm, maybe I need to try a bit harder here.  Does the "not registered"
error happen immediately/reliably for you, or do you need to run the
test awhile?

> As for the deadlock issue you mention I've been told 
> I have some seeding/random number generation problems in the kit. 
> Perhaps that is related to the deadlock at least.

The nature of the locks suggests that you have two transactions trying
to update the same two rows in opposite orders.  The usually recommended
fix is to ensure you use a consistent processing order within a
transaction (eg, ascending primary keys --- but any well-defined row
ordering will work).
        regards, tom lane


Re: Lock partitions

From
markw@osdl.org
Date:
> Mark Wong <markw@osdl.org> writes:
>> Curious, I'm still seeing the same behavior.  Maybe I'll take another
>> snapshot from CVS.
>
> Hm, maybe I need to try a bit harder here.  Does the "not registered"
> error happen immediately/reliably for you, or do you need to run the
> test awhile?

It appears to occur for every order status transaction.  You can monitor
the transactions by watching dbt2/scripts/output/*/driver/mix.log.  A 'o'
(lowercase) indicates a successful order status transaction while a 'O'
(uppercase) indiciates an unsuccessful transaction.

or, perhaps a simplier way is to start the database then:

cd dbt2/src
./transaction_test -d localhost -l 5432 -t o

That will connect directly to the database and execute an order status
transaction.

Thanks,
Mark


Re: Lock partitions

From
Mark Wong
Date:
Tom Lane wrote:
> Mark Wong <markw@osdl.org> writes:
>> Curious, I'm still seeing the same behavior.  Maybe I'll take another 
>> snapshot from CVS.
> 
> Hm, maybe I need to try a bit harder here.  Does the "not registered"
> error happen immediately/reliably for you, or do you need to run the
> test awhile?

I did a gross test and my kit appears broken between the 8.0 and 8.1 
releases.  I'll try to narrow down the exact date.

Mark


Re: Lock partitions

From
Mark Wong
Date:
Mark Wong wrote:
> Tom Lane wrote:
>> Mark Wong <markw@osdl.org> writes:
>>> Curious, I'm still seeing the same behavior.  Maybe I'll take another
>>> snapshot from CVS.
>>
>> Hm, maybe I need to try a bit harder here.  Does the "not registered"
>> error happen immediately/reliably for you, or do you need to run the
>> test awhile?
>
> I did a gross test and my kit appears broken between the 8.0 and 8.1
> releases.  I'll try to narrow down the exact date.

I've narrowed it down between cvs pulls from Dec 14, 2005 and Dec 15,
2005.  Does the attached diff appear to be a plausible cause?

Thanks,
Mark
diff -urN pgsql-2005-12-14/src/backend/commands/prepare.c pgsql-2005-12-15/src/backend/commands/prepare.c
--- pgsql-2005-12-14/src/backend/commands/prepare.c    2005-11-28 17:25:49.000000000 -0800
+++ pgsql-2005-12-15/src/backend/commands/prepare.c    2005-12-14 09:06:27.000000000 -0800
@@ -10,7 +10,7 @@
  * Copyright (c) 2002-2005, PostgreSQL Global Development Group
  *
  * IDENTIFICATION
- *      $PostgreSQL: pgsql/src/backend/commands/prepare.c,v 1.43 2005/11/29 01:25:49 tgl Exp $
+ *      $PostgreSQL: pgsql/src/backend/commands/prepare.c,v 1.44 2005/12/14 17:06:27 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -448,6 +448,30 @@
 }

 /*
+ * Given a prepared statement, determine whether it will return tuples.
+ *
+ * Note: this is used rather than just testing the result of
+ * FetchPreparedStatementResultDesc() because that routine can fail if
+ * invoked in an aborted transaction.  This one is safe to use in any
+ * context.  Be sure to keep the two routines in sync!
+ */
+bool
+PreparedStatementReturnsTuples(PreparedStatement *stmt)
+{
+    switch (ChoosePortalStrategy(stmt->query_list))
+    {
+        case PORTAL_ONE_SELECT:
+        case PORTAL_UTIL_SELECT:
+            return true;
+
+        case PORTAL_MULTI_QUERY:
+            /* will not return tuples */
+            break;
+    }
+    return false;
+}
+
+/*
  * Given a prepared statement that returns tuples, extract the query
  * targetlist.    Returns NIL if the statement doesn't have a determinable
  * targetlist.
diff -urN pgsql-2005-12-14/src/backend/executor/execQual.c pgsql-2005-12-15/src/backend/executor/execQual.c
--- pgsql-2005-12-14/src/backend/executor/execQual.c    2005-11-22 10:17:10.000000000 -0800
+++ pgsql-2005-12-15/src/backend/executor/execQual.c    2005-12-14 08:28:32.000000000 -0800
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *      $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.185 2005/11/22 18:17:10 momjian Exp $
+ *      $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.186 2005/12/14 16:28:32 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -523,7 +523,7 @@
     Assert(variable->varno != OUTER);
     slot = econtext->ecxt_scantuple;

-    tuple = slot->tts_tuple;
+    tuple = ExecFetchSlotTuple(slot);
     tupleDesc = slot->tts_tupleDescriptor;

     /*
diff -urN pgsql-2005-12-14/src/backend/tcop/postgres.c pgsql-2005-12-15/src/backend/tcop/postgres.c
--- pgsql-2005-12-14/src/backend/tcop/postgres.c    2005-11-22 10:17:21.000000000 -0800
+++ pgsql-2005-12-15/src/backend/tcop/postgres.c    2005-12-14 09:06:27.000000000 -0800
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *      $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.470 2005/11/22 18:17:21 momjian Exp $
+ *      $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.471 2005/12/14 17:06:27 tgl Exp $
  *
  * NOTES
  *      this is the "main" module of the postgres backend and
@@ -1849,6 +1849,15 @@
     ListCell   *l;
     StringInfoData buf;

+    /*
+     * Start up a transaction command. (Note that this will normally change
+     * current memory context.) Nothing happens if we are already in one.
+     */
+    start_xact_command();
+
+    /* Switch back to message context */
+    MemoryContextSwitchTo(MessageContext);
+
     /* Find prepared statement */
     if (stmt_name[0] != '\0')
         pstmt = FetchPreparedStatement(stmt_name, true);
@@ -1862,6 +1871,22 @@
                      errmsg("unnamed prepared statement does not exist")));
     }

+    /*
+     * If we are in aborted transaction state, we can't safely create a result
+     * tupledesc, because that needs catalog accesses.  Hence, refuse to
+     * Describe statements that return data.  (We shouldn't just refuse all
+     * Describes, since that might break the ability of some clients to issue
+     * COMMIT or ROLLBACK commands, if they use code that blindly Describes
+     * whatever it does.)  We can Describe parameters without doing anything
+     * dangerous, so we don't restrict that.
+     */
+    if (IsAbortedTransactionBlockState() &&
+        PreparedStatementReturnsTuples(pstmt))
+        ereport(ERROR,
+                (errcode(ERRCODE_IN_FAILED_SQL_TRANSACTION),
+                 errmsg("current transaction is aborted, "
+                        "commands ignored until end of transaction block")));
+
     if (whereToSendOutput != DestRemote)
         return;                    /* can't actually do anything... */

@@ -1902,12 +1927,36 @@
 {
     Portal        portal;

+    /*
+     * Start up a transaction command. (Note that this will normally change
+     * current memory context.) Nothing happens if we are already in one.
+     */
+    start_xact_command();
+
+    /* Switch back to message context */
+    MemoryContextSwitchTo(MessageContext);
+
     portal = GetPortalByName(portal_name);
     if (!PortalIsValid(portal))
         ereport(ERROR,
                 (errcode(ERRCODE_UNDEFINED_CURSOR),
                  errmsg("portal \"%s\" does not exist", portal_name)));

+    /*
+     * If we are in aborted transaction state, we can't run
+     * SendRowDescriptionMessage(), because that needs catalog accesses.
+     * Hence, refuse to Describe portals that return data.  (We shouldn't just
+     * refuse all Describes, since that might break the ability of some
+     * clients to issue COMMIT or ROLLBACK commands, if they use code that
+     * blindly Describes whatever it does.)
+     */
+    if (IsAbortedTransactionBlockState() &&
+        portal->tupDesc)
+        ereport(ERROR,
+                (errcode(ERRCODE_IN_FAILED_SQL_TRANSACTION),
+                 errmsg("current transaction is aborted, "
+                        "commands ignored until end of transaction block")));
+
     if (whereToSendOutput != DestRemote)
         return;                    /* can't actually do anything... */

diff -urN pgsql-2005-12-14/src/include/commands/prepare.h pgsql-2005-12-15/src/include/commands/prepare.h
--- pgsql-2005-12-14/src/include/commands/prepare.h    2005-11-28 17:25:50.000000000 -0800
+++ pgsql-2005-12-15/src/include/commands/prepare.h    2005-12-14 09:06:28.000000000 -0800
@@ -6,7 +6,7 @@
  *
  * Copyright (c) 2002-2005, PostgreSQL Global Development Group
  *
- * $PostgreSQL: pgsql/src/include/commands/prepare.h,v 1.15 2005/11/29 01:25:50 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/commands/prepare.h,v 1.16 2005/12/14 17:06:28 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -60,6 +60,7 @@
 extern void DropPreparedStatement(const char *stmt_name, bool showError);
 extern List *FetchPreparedStatementParams(const char *stmt_name);
 extern TupleDesc FetchPreparedStatementResultDesc(PreparedStatement *stmt);
+extern bool PreparedStatementReturnsTuples(PreparedStatement *stmt);
 extern List *FetchPreparedStatementTargetList(PreparedStatement *stmt);

 #endif   /* PREPARE_H */
diff -urN pgsql-2005-12-14/src/interfaces/libpq/libpq.rc pgsql-2005-12-15/src/interfaces/libpq/libpq.rc
--- pgsql-2005-12-14/src/interfaces/libpq/libpq.rc    2006-09-20 10:05:18.000000000 -0700
+++ pgsql-2005-12-15/src/interfaces/libpq/libpq.rc    2006-09-19 17:31:48.000000000 -0700
@@ -1,8 +1,8 @@
 #include <winver.h>

 VS_VERSION_INFO VERSIONINFO
- FILEVERSION 8,2,0,6263
- PRODUCTVERSION 8,2,0,6263
+ FILEVERSION 8,2,0,6262
+ PRODUCTVERSION 8,2,0,6262
  FILEFLAGSMASK 0x3fL
  FILEFLAGS 0
  FILEOS VOS__WINDOWS32
diff -urN pgsql-2005-12-14/src/test/regress/expected/select.out pgsql-2005-12-15/src/test/regress/expected/select.out
--- pgsql-2005-12-14/src/test/regress/expected/select.out    2005-04-06 18:51:40.000000000 -0700
+++ pgsql-2005-12-15/src/test/regress/expected/select.out    2005-12-14 08:28:32.000000000 -0800
@@ -431,3 +431,24 @@
  mary    |   8
 (58 rows)

+--
+-- Test some cases involving whole-row Var referencing a subquery
+--
+select foo from (select 1) as foo;
+ foo
+-----
+ (1)
+(1 row)
+
+select foo from (select null) as foo;
+ foo
+-----
+ ()
+(1 row)
+
+select foo from (select 'xyzzy',1,null) as foo;
+    foo
+------------
+ (xyzzy,1,)
+(1 row)
+
diff -urN pgsql-2005-12-14/src/test/regress/sql/select.sql pgsql-2005-12-15/src/test/regress/sql/select.sql
--- pgsql-2005-12-14/src/test/regress/sql/select.sql    2005-04-06 18:51:41.000000000 -0700
+++ pgsql-2005-12-15/src/test/regress/sql/select.sql    2005-12-14 08:28:32.000000000 -0800
@@ -104,3 +104,9 @@
 --
 SELECT p.name, p.age FROM person* p ORDER BY age using >, name;

+--
+-- Test some cases involving whole-row Var referencing a subquery
+--
+select foo from (select 1) as foo;
+select foo from (select null) as foo;
+select foo from (select 'xyzzy',1,null) as foo;

Re: Lock partitions

From
Tom Lane
Date:
Mark Wong <markw@osdl.org> writes:
>> I did a gross test and my kit appears broken between the 8.0 and 8.1 
>> releases.  I'll try to narrow down the exact date.

> I've narrowed it down between cvs pulls from Dec 14, 2005 and Dec 15, 
> 2005.  Does the attached diff appear to be a plausible cause?

No, not particularly.  You sure about those dates?
        regards, tom lane


Re: Lock partitions

From
Mark Wong
Date:
Tom Lane wrote:
> Mark Wong <markw@osdl.org> writes:
>>> I did a gross test and my kit appears broken between the 8.0 and 8.1 
>>> releases.  I'll try to narrow down the exact date.
> 
>> I've narrowed it down between cvs pulls from Dec 14, 2005 and Dec 15, 
>> 2005.  Does the attached diff appear to be a plausible cause?
> 
> No, not particularly.  You sure about those dates?

Ugh, double checking tells me I'm wrong.  I'll keep testing.

Thanks,
Mark


Re: Lock partitions

From
Mark Wong
Date:
Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
>> I see this in the CVS commits for 8.2.  Did we determine the proper
>> number of lock partitions?  Should it be based on the number of buffers
>> or concurrent sessions allowed?
> 
> No.  NUM_LOCK_PARTITIONS needs to be a compile-time constant for a
> number of reasons, and there is absolutely zero evidence to justify
> making any effort (and spending any cycles) on a variable value.
> 
> It would be nice to see some results from the OSDL tests with, say, 4,
> 8, and 16 lock partitions before we forget about the point though.
> Anybody know whether OSDL is in a position to run tests for us?

I have a couple of bigger runs now against a CVS checkout on 2006-09-20 
(please forgive my NUM_BUFFER_PARTITIONS note if you notice that on the 
web pages):

Baseline (default NUM_LOCK_PARTITIONS=4):
notpm 6589
http://dbt.osdl.org/dbt/dbt2dev/results/dev4-015/184/

NUM_LOCK_PARTITIONS=8:
notpm 4471
http://dbt.osdl.org/dbt/dbt2dev/results/dev4-015/180/

NUM_LOCK_PARTITIONS=16:
Failed to run.


The number of transaction errors increased when I increased the 
NUM_LOCK_PARTITIONS, which I think is the reason it failed to run when I 
set it to 16.  And the throughput went down significantly (32%).  Should 
I try against a more recent checkout?

Mark


Re: Lock partitions

From
Tom Lane
Date:
Mark Wong <markw@osdl.org> writes:
> The number of transaction errors increased when I increased the 
> NUM_LOCK_PARTITIONS, which I think is the reason it failed to run when I 
> set it to 16.

Hmm, what sort of errors are we talking about?  I wonder if you've
exposed a bug.  Changing NUM_LOCK_PARTITIONS really shouldn't have any
semantic effect.
        regards, tom lane


Re: Lock partitions

From
Mark Wong
Date:
Tom Lane wrote:
> Mark Wong <markw@osdl.org> writes:
>> The number of transaction errors increased when I increased the 
>> NUM_LOCK_PARTITIONS, which I think is the reason it failed to run when I 
>> set it to 16.
> 
> Hmm, what sort of errors are we talking about?  I wonder if you've
> exposed a bug.  Changing NUM_LOCK_PARTITIONS really shouldn't have any
> semantic effect.

The libpq client (error log: 
http://dbt.osdl.org/dbt/dbt2dev/results/dev4-015/180/client/error.log) 
is saying things like this:

ERROR:  too many LWLocks taken
CONTEXT:  SQL statement "DELETE FROM new_order
WHERE no_o_id = 2101  AND no_w_id = 349  AND no_d_id = 1"

A grep through that file shows that all the unexpected errors appear to 
be due to "too many LWLocks taken".

Mark



Re: Lock partitions

From
Tom Lane
Date:
Mark Wong <markw@osdl.org> writes:
> Tom Lane wrote:
>> Hmm, what sort of errors are we talking about?

> ERROR:  too many LWLocks taken

That really shouldn't happen ... are you sure you did a full recompile
after changing NUM_LOCK_PARTITIONS?

Actually ... wait a moment.  The default value of NUM_LOCK_PARTITIONS
is already 16 (1 << LOG2_NUM_LOCK_PARTITIONS where the latter is 4).
Are you saying you set LOG2_NUM_LOCK_PARTITIONS to 16?  That would be
way too many partitions.  I was thinking of testing
LOG2_NUM_LOCK_PARTITIONS in the range of about 2 to 5.
        regards, tom lane


Re: Lock partitions

From
Mark Wong
Date:
Tom Lane wrote:
> Mark Wong <markw@osdl.org> writes:
>> Tom Lane wrote:
>>> Hmm, what sort of errors are we talking about?
> 
>> ERROR:  too many LWLocks taken
> 
> That really shouldn't happen ... are you sure you did a full recompile
> after changing NUM_LOCK_PARTITIONS?
> 
> Actually ... wait a moment.  The default value of NUM_LOCK_PARTITIONS
> is already 16 (1 << LOG2_NUM_LOCK_PARTITIONS where the latter is 4).
> Are you saying you set LOG2_NUM_LOCK_PARTITIONS to 16?  That would be
> way too many partitions.  I was thinking of testing
> LOG2_NUM_LOCK_PARTITIONS in the range of about 2 to 5.

Oops, I can't read bit shifting. =p  I'll do again.

Mark