pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedRe: CSStorm occurred again by postgreSQL8.2
22+ messages / 6 participants
[nested] [flat]
* Re: CSStorm occurred again by postgreSQL8.2
@ 2006-08-07 04:36 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-07 13:09 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-08-08 03:16 ` Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
0 siblings, 2 replies; 22+ messages in thread
From: ITAGAKI Takahiro @ 2006-08-07 04:36 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers; +Cc: Katsuhiko Okano <okano.katsuhiko@oss.ntt.co.jp>
Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > It does not solve, even if it increases the number of NUM_SUBTRANS_BUFFERS.
> > The problem was only postponed.
>
> Can you provide a reproducible test case for this?
This is the reproducible test case:
- Occurs on both 8.1.4 and HEAD.
- On smp machine. I used dual opterons.
CSStrom becomes worse on dual xeon with hyper-threading.
- Tuning parameters are default. Whole data are cached in shared buffers.
(shared_buffers=32MB, data of pgbench (scale=1) are less than 15MB.)
- Using custom pgbench. One client doing UPDATE with indexscan
and multiple clients doing SELECT with seqscan/indexscan.
$ pgbench -i
$ pgbench -n -c 1 -t 100000 -f cs_update.sql &
$ pgbench -n -c 50 -t 100000 -f cs_indexscan.sql &
$ pgbench -n -c 35 -t 100000 -f cs_seqscan.sql &
(The scripts are attached at end of this message.)
In above workload, context switches are 2000-10000/sec and cpu usage is
user=100%. Then, start a long open transaction on another connection.
$ psql
# begin; -- Long open transaction
After a lapse of 30-60 seconds, context switches become 50000/sec over
(120000 over on xeons) and cpu usage is user=66% / sys=21% / idle=13%.
If we increase the frequency of UPDATE, the duration becomes shorter.
This is a human-induced workload, but I can see the same condition in
TPC-W -- even though it is a benchmark. TPC-W requires full-text search
and it is implementd using "LIKE %foo%" in my implementation (DBT-1, too).
Also, it requires periodical aggregations. They might behave as long
transactions.
The cause seems to be a lock contention. The number of locks on
SubtransControlLock and SubTransBuffer are significantly increased
by comparison with BufMappingLocks.
# Before starting a long transaction.
kind | lwlock | sh_call | sh_wait | ex_call | ex_wait
------+---------------------+----------+---------+---------+---------
13 | SubtransControlLock | 28716 | 2 | 54 | 0
22 | BufMappingLock | 11637884 | 0 | 2492 | 0
27 | SubTransBuffer | 0 | 0 | 11 | 0
# After
kind | lwlock | sh_call | sh_wait | ex_call | ex_wait
------+---------------------+----------+---------+---------+---------
13 | SubtransControlLock | 4139111 | 65059 | 3926691 | 390838
22 | BufMappingLock | 32348073 | 0 | 2509 | 0
27 | SubTransBuffer | 939646 | 960341 | 1419152 | 61
The invokers of SubTrans module are two SubTransGetTopmostTransaction()
in HeapTupleSatisfiesSnapshot(). When I disabled the calls, CSStorm did
not occur. SubTransGetTopmostTransaction returns the argument without
change when we don't use SAVEPOINTs.
If we optimize for non-subtransactions, we can avoid to lock SubTrans
for check visiblities of tuples inserted by top transactions.
If we want to resolve the probmen fundamentally, we might have to
improve SubTrans using a better buffer management algorithm or so.
Do you have any idea to avoid such a problem?
-- cs_update.sql
\set naccounts 100000 * :tps
\setrandom aid 1 :naccounts
\setrandom delta -5000 5000
UPDATE accounts SET abalance = abalance + :delta WHERE aid = :aid;
SELECT pg_sleep(0.1);
-- cs_seqscan.sql
\set naccounts 100000 * :tps
\setrandom aid 1 :naccounts
SELECT abalance FROM accounts WHERE aid::int8 = :aid; -- cast to force seqscan
-- cs_indexscan.sql
\set naccounts 100000 * :tps
\setrandom aid 1 :naccounts
SELECT abalance FROM accounts WHERE aid = :aid;
Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CSStorm occurred again by postgreSQL8.2
2006-08-07 04:36 Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
@ 2006-08-07 13:09 ` Tom Lane <tgl@sss.pgh.pa.us>
2006-08-07 18:52 ` Re: CSStorm occurred again by postgreSQL8.2 Alvaro Herrera <alvherre@commandprompt.com>
1 sibling, 1 reply; 22+ messages in thread
From: Tom Lane @ 2006-08-07 13:09 UTC (permalink / raw)
To: ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>; +Cc: pgsql-hackers@postgresql.org, Katsuhiko Okano <okano.katsuhiko@oss.ntt.co.jp>
ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp> writes:
> The invokers of SubTrans module are two SubTransGetTopmostTransaction()
> in HeapTupleSatisfiesSnapshot(). When I disabled the calls, CSStorm did
> not occur. SubTransGetTopmostTransaction returns the argument without
> change when we don't use SAVEPOINTs.
> If we optimize for non-subtransactions, we can avoid to lock SubTrans
> for check visiblities of tuples inserted by top transactions.
Only for top transactions still in progress, so I doubt that would
help much.
I'm wondering about doing something similar to what
TransactionIdIsInProgress does, ie, make use of the PGPROC lists
of live subtransactions. Suppose that GetSnapshotData copies not
only top xids but live subxids into the snapshot, and adds a flag
indicating whether the subxids are complete (ie, none of the subxid
lists have overflowed). Then if the flag is set, tqual.c doesn't
need to do SubTransGetTopmostTransaction() before searching the
list.
regards, tom lane
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CSStorm occurred again by postgreSQL8.2
2006-08-07 04:36 Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-07 13:09 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
@ 2006-08-07 18:52 ` Alvaro Herrera <alvherre@commandprompt.com>
2006-08-07 19:42 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 22+ messages in thread
From: Alvaro Herrera @ 2006-08-07 18:52 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>; pgsql-hackers@postgresql.org, Katsuhiko Okano <okano.katsuhiko@oss.ntt.co.jp>
Tom Lane wrote:
> I'm wondering about doing something similar to what
> TransactionIdIsInProgress does, ie, make use of the PGPROC lists
> of live subtransactions. Suppose that GetSnapshotData copies not
> only top xids but live subxids into the snapshot, and adds a flag
> indicating whether the subxids are complete (ie, none of the subxid
> lists have overflowed). Then if the flag is set, tqual.c doesn't
> need to do SubTransGetTopmostTransaction() before searching the
> list.
Well, that sounds awfully more expensive than setting
local-to-my-database Xmins as well as global (all databases) Xmins :-)
On the other hand, ISTM as soon as one cache overflows, you have to go
check pg_subtrans which means the entire optimization buys nothing in
that case. It would be nice if the optimization degraded more
gracefully. I don't have any concrete suggestion though. The changes
proposed in the other CS-storm thread by the NTT person may help.
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CSStorm occurred again by postgreSQL8.2
2006-08-07 04:36 Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-07 13:09 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-08-07 18:52 ` Re: CSStorm occurred again by postgreSQL8.2 Alvaro Herrera <alvherre@commandprompt.com>
@ 2006-08-07 19:42 ` Tom Lane <tgl@sss.pgh.pa.us>
2006-08-07 20:10 ` Re: CSStorm occurred again by postgreSQL8.2 Alvaro Herrera <alvherre@commandprompt.com>
0 siblings, 1 reply; 22+ messages in thread
From: Tom Lane @ 2006-08-07 19:42 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@commandprompt.com>; +Cc: ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>; pgsql-hackers@postgresql.org, Katsuhiko Okano <okano.katsuhiko@oss.ntt.co.jp>
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Tom Lane wrote:
>> I'm wondering about doing something similar to what
>> TransactionIdIsInProgress does, ie, make use of the PGPROC lists
>> of live subtransactions.
> Well, that sounds awfully more expensive than setting
> local-to-my-database Xmins as well as global (all databases) Xmins :-)
Only when you've got a lot of subtransactions. The point of this
discussion is to optimize for the few-or-none case. In any case,
the problem with the local/global bit was that you'd be expending
foreground-process cycles without any foreground-process return.
Being able to use a snapshot without consulting pg_subtrans will
certainly buy back some cycles...
regards, tom lane
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CSStorm occurred again by postgreSQL8.2
2006-08-07 04:36 Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-07 13:09 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-08-07 18:52 ` Re: CSStorm occurred again by postgreSQL8.2 Alvaro Herrera <alvherre@commandprompt.com>
2006-08-07 19:42 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
@ 2006-08-07 20:10 ` Alvaro Herrera <alvherre@commandprompt.com>
2006-08-07 20:27 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-03 16:15 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 22+ messages in thread
From: Alvaro Herrera @ 2006-08-07 20:10 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>; pgsql-hackers@postgresql.org, Katsuhiko Okano <okano.katsuhiko@oss.ntt.co.jp>
Tom Lane wrote:
> Alvaro Herrera <alvherre@commandprompt.com> writes:
> > Tom Lane wrote:
> >> I'm wondering about doing something similar to what
> >> TransactionIdIsInProgress does, ie, make use of the PGPROC lists
> >> of live subtransactions.
>
> > Well, that sounds awfully more expensive than setting
> > local-to-my-database Xmins as well as global (all databases) Xmins :-)
>
> Only when you've got a lot of subtransactions. The point of this
> discussion is to optimize for the few-or-none case. In any case,
> the problem with the local/global bit was that you'd be expending
> foreground-process cycles without any foreground-process return.
> Being able to use a snapshot without consulting pg_subtrans will
> certainly buy back some cycles...
I can buy that. Some idle thoughts:
I was thinking at what time was the most appropiate to insert or remove
an Xid from the cache. We can do without any excl-locking because 1) we
already assume the storing of an Xid to be atomic, and 2) no one can be
interested in querying for an Xid before the originating transaction has
had the chance to write a tuple with that Xid anyway.
OTOH I think we only need to store live Xids and those finished that
wrote a WAL record; we can drop subaborted and subcommitted if they
didn't.
On the third hand, are we going to sh-acquire the ProcArray lock while a
GetSnapshotData copies all subxact Xids of all running transactions?
ProcArrayLock will become more of a contention point than it already is.
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CSStorm occurred again by postgreSQL8.2
2006-08-07 04:36 Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-07 13:09 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-08-07 18:52 ` Re: CSStorm occurred again by postgreSQL8.2 Alvaro Herrera <alvherre@commandprompt.com>
2006-08-07 19:42 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-08-07 20:10 ` Re: CSStorm occurred again by postgreSQL8.2 Alvaro Herrera <alvherre@commandprompt.com>
@ 2006-08-07 20:27 ` Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 22+ messages in thread
From: Tom Lane @ 2006-08-07 20:27 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@commandprompt.com>; +Cc: ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>; pgsql-hackers@postgresql.org, Katsuhiko Okano <okano.katsuhiko@oss.ntt.co.jp>
Alvaro Herrera <alvherre@commandprompt.com> writes:
> I was thinking at what time was the most appropiate to insert or remove
> an Xid from the cache. We can do without any excl-locking because 1) we
> already assume the storing of an Xid to be atomic, and 2) no one can be
> interested in querying for an Xid before the originating transaction has
> had the chance to write a tuple with that Xid anyway.
Actually ... that fails if GetSnapshotData is going to copy subtrans
XIDs. So this area needs more thought.
> On the third hand, are we going to sh-acquire the ProcArray lock while a
> GetSnapshotData copies all subxact Xids of all running transactions?
> ProcArrayLock will become more of a contention point than it already is.
Yeah, but sharelock is better than exclusive lock ...
regards, tom lane
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CSStorm occurred again by postgreSQL8.2
2006-08-07 04:36 Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-07 13:09 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-08-07 18:52 ` Re: CSStorm occurred again by postgreSQL8.2 Alvaro Herrera <alvherre@commandprompt.com>
2006-08-07 19:42 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-08-07 20:10 ` Re: CSStorm occurred again by postgreSQL8.2 Alvaro Herrera <alvherre@commandprompt.com>
@ 2006-09-03 16:15 ` Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 22+ messages in thread
From: Tom Lane @ 2006-09-03 16:15 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@commandprompt.com>; +Cc: ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>; pgsql-hackers@postgresql.org, Katsuhiko Okano <okano.katsuhiko@oss.ntt.co.jp>
Alvaro Herrera <alvherre@commandprompt.com> writes:
> OTOH I think we only need to store live Xids and those finished that
> wrote a WAL record; we can drop subaborted and subcommitted if they
> didn't.
While reviewing this thread, I see Alvaro already had the idea I just
came to...
regards, tom lane
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CSStorm occurred again by postgreSQL8.2
2006-08-07 04:36 Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
@ 2006-08-08 03:16 ` ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-26 03:46 ` Re: CSStorm occurred again by postgreSQL8.2 Bruce Momjian <bruce@momjian.us>
1 sibling, 1 reply; 22+ messages in thread
From: ITAGAKI Takahiro @ 2006-08-08 03:16 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers; +Cc: Katsuhiko Okano <okano.katsuhiko@oss.ntt.co.jp>
This is an additional information.
I wrote:
> If we want to resolve the probmen fundamentally, we might have to
> improve SubTrans using a better buffer management algorithm or so.
The above is maybe wrong. I checked each lwlock of pg_subtrans's buffers.
All lwlocks are uniformly acquired and I could not see any differences
among buffers. So the cause seems not to be a buffer management algorithm,
but just a lack of SLRU buffer pages.
NUM_SUBTRANS_BUFFERS is defined as 32 in HEAD. If we increase it,
we can avoid the old transaction problem for a certain time.
However, it doesn't help much on high-load -- for example, on a workload
with 2000 tps, we will use up 1000 pg_subtrans pages in 15 minites.
I suppose it is not enough for online and batch/maintenance mix.
Also, the simple scanning way in SLRU will likely cause another performance
issue when we highly increase the number of buffers. A sequential scanning
is used in SLRU, so it will not work well against many buffers.
I hope some cares in upper layer, snapshot, hitbits or something,
being discussed in the recent thread.
Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CSStorm occurred again by postgreSQL8.2
2006-08-07 04:36 Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-08 03:16 ` Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
@ 2006-08-26 03:46 ` Bruce Momjian <bruce@momjian.us>
2006-08-28 10:17 ` Re: [HACKERS] CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
0 siblings, 1 reply; 22+ messages in thread
From: Bruce Momjian @ 2006-08-26 03:46 UTC (permalink / raw)
To: ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers@postgresql.org, Katsuhiko Okano <okano.katsuhiko@oss.ntt.co.jp>
Is there anything to do for 8.2 here?
---------------------------------------------------------------------------
ITAGAKI Takahiro wrote:
> This is an additional information.
>
> I wrote:
> > If we want to resolve the probmen fundamentally, we might have to
> > improve SubTrans using a better buffer management algorithm or so.
>
> The above is maybe wrong. I checked each lwlock of pg_subtrans's buffers.
> All lwlocks are uniformly acquired and I could not see any differences
> among buffers. So the cause seems not to be a buffer management algorithm,
> but just a lack of SLRU buffer pages.
>
> NUM_SUBTRANS_BUFFERS is defined as 32 in HEAD. If we increase it,
> we can avoid the old transaction problem for a certain time.
> However, it doesn't help much on high-load -- for example, on a workload
> with 2000 tps, we will use up 1000 pg_subtrans pages in 15 minites.
> I suppose it is not enough for online and batch/maintenance mix.
>
> Also, the simple scanning way in SLRU will likely cause another performance
> issue when we highly increase the number of buffers. A sequential scanning
> is used in SLRU, so it will not work well against many buffers.
>
>
> I hope some cares in upper layer, snapshot, hitbits or something,
> being discussed in the recent thread.
>
> Regards,
> ---
> ITAGAKI Takahiro
> NTT Open Source Software Center
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo@postgresql.org so that your
> message can get through to the mailing list cleanly
--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [HACKERS] CSStorm occurred again by postgreSQL8.2
2006-08-07 04:36 Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-08 03:16 ` Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-26 03:46 ` Re: CSStorm occurred again by postgreSQL8.2 Bruce Momjian <bruce@momjian.us>
@ 2006-08-28 10:17 ` ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-09-03 16:08 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 22+ messages in thread
From: ITAGAKI Takahiro @ 2006-08-28 10:17 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; pgsql-patches@postgresql.org; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers
Bruce Momjian <bruce@momjian.us> wrote:
> Is there anything to do for 8.2 here?
I'm working on Tom's idea. It is not a feature and does not change
the on-disk-structures, so I hope it meet the 8.2 deadline...
Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I'm wondering about doing something similar to what
> TransactionIdIsInProgress does, ie, make use of the PGPROC lists
> of live subtransactions. Suppose that GetSnapshotData copies not
> only top xids but live subxids into the snapshot, and adds a flag
> indicating whether the subxids are complete (ie, none of the subxid
> lists have overflowed). Then if the flag is set, tqual.c doesn't
> need to do SubTransGetTopmostTransaction() before searching the
> list.
I added a subxid array to Snapshot and running subxids are gathered from
PGPROC->subxids cache. There are two overflowed case; any of PGPROC->subxids
are overflowed or the number of total subxids exceeds pre-allocated buffers.
If overflowed, we cannot avoid to call SubTransGetTopmostTransaction.
I tested the patch and did not see any context switch storm which comes
from pg_subtrans, but there may be some bugs in the visibility checking.
It would be very nice if you could review or test the patch.
Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center
Attachments:
[application/octet-stream] snapshot_subtrans.patch (7.9K, ../../20060828181312.5638.ITAGAKI.TAKAHIRO@oss.ntt.co.jp/2-snapshot_subtrans.patch)
download | inline diff:
diff -cpr pgsql-orig/src/backend/storage/ipc/procarray.c pgsql/src/backend/storage/ipc/procarray.c
*** pgsql-orig/src/backend/storage/ipc/procarray.c Mon Aug 28 09:36:08 2006
--- pgsql/src/backend/storage/ipc/procarray.c Mon Aug 28 17:38:05 2006
*************** GetSnapshotData(Snapshot snapshot, bool
*** 499,504 ****
--- 499,505 ----
TransactionId globalxmin;
int index;
int count = 0;
+ int subcount = 0;
Assert(snapshot != NULL);
*************** GetSnapshotData(Snapshot snapshot, bool
*** 599,604 ****
--- 600,642 ----
if (TransactionIdIsNormal(xid))
if (TransactionIdPrecedes(xid, globalxmin))
globalxmin = xid;
+
+ /* Query subtransaction ids */
+ if (subcount >= 0)
+ {
+ if (proc->subxids.overflowed)
+ {
+ subcount = -1; /* overflowed */
+ }
+ else if (proc->subxids.nxids > 0)
+ {
+ int sub;
+
+ /* XXX: or use some multiplied value? */
+ int maxSubCount = arrayP->maxProcs;
+
+ if (snapshot->subxip == NULL)
+ {
+ snapshot->subxip = (TransactionId *)
+ malloc(maxSubCount * sizeof(TransactionId));
+ if (snapshot->subxip == NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("out of memory")));
+ }
+
+ for (sub = 0; sub < proc->subxids.nxids; sub++)
+ {
+ snapshot->subxip[subcount] = proc->subxids.xids[sub];
+ subcount++;
+ if (subcount >= maxSubCount)
+ {
+ subcount = -1; /* overflowed */
+ break;
+ }
+ }
+ }
+ }
}
if (serializable)
*************** GetSnapshotData(Snapshot snapshot, bool
*** 621,626 ****
--- 659,665 ----
snapshot->xmin = xmin;
snapshot->xmax = xmax;
snapshot->xcnt = count;
+ snapshot->subxcnt = subcount;
snapshot->curcid = GetCurrentCommandId();
diff -cpr pgsql-orig/src/backend/utils/time/tqual.c pgsql/src/backend/utils/time/tqual.c
*** pgsql-orig/src/backend/utils/time/tqual.c Mon Aug 28 09:36:10 2006
--- pgsql/src/backend/utils/time/tqual.c Mon Aug 28 17:55:58 2006
*************** TransactionId TransactionXmin = InvalidT
*** 72,77 ****
--- 72,78 ----
TransactionId RecentXmin = InvalidTransactionId;
TransactionId RecentGlobalXmin = InvalidTransactionId;
+ static bool XidInSnapshot(TransactionId xid, Snapshot snapshot);
/*
* HeapTupleSatisfiesItself
*************** HeapTupleSatisfiesSnapshot(HeapTupleHead
*** 954,979 ****
if (TransactionIdFollowsOrEquals(HeapTupleHeaderGetXmin(tuple),
snapshot->xmin))
{
- TransactionId parentXid;
-
if (TransactionIdFollowsOrEquals(HeapTupleHeaderGetXmin(tuple),
snapshot->xmax))
return false;
! parentXid = SubTransGetTopmostTransaction(HeapTupleHeaderGetXmin(tuple));
!
! if (TransactionIdFollowsOrEquals(parentXid, snapshot->xmin))
! {
! uint32 i;
!
! /* no point in checking parentXid against xmax here */
!
! for (i = 0; i < snapshot->xcnt; i++)
! {
! if (TransactionIdEquals(parentXid, snapshot->xip[i]))
! return false;
! }
! }
}
if (tuple->t_infomask & HEAP_XMAX_INVALID) /* xid invalid or aborted */
--- 955,966 ----
if (TransactionIdFollowsOrEquals(HeapTupleHeaderGetXmin(tuple),
snapshot->xmin))
{
if (TransactionIdFollowsOrEquals(HeapTupleHeaderGetXmin(tuple),
snapshot->xmax))
return false;
! if (XidInSnapshot(HeapTupleHeaderGetXmin(tuple), snapshot))
! return false;
}
if (tuple->t_infomask & HEAP_XMAX_INVALID) /* xid invalid or aborted */
*************** HeapTupleSatisfiesSnapshot(HeapTupleHead
*** 1023,1048 ****
if (TransactionIdFollowsOrEquals(HeapTupleHeaderGetXmax(tuple),
snapshot->xmin))
{
- TransactionId parentXid;
-
if (TransactionIdFollowsOrEquals(HeapTupleHeaderGetXmax(tuple),
snapshot->xmax))
return true;
! parentXid = SubTransGetTopmostTransaction(HeapTupleHeaderGetXmax(tuple));
!
! if (TransactionIdFollowsOrEquals(parentXid, snapshot->xmin))
! {
! uint32 i;
!
! /* no point in checking parentXid against xmax here */
!
! for (i = 0; i < snapshot->xcnt; i++)
! {
! if (TransactionIdEquals(parentXid, snapshot->xip[i]))
! return true;
! }
! }
}
/* This is to be used only for disaster recovery and requires serious analysis. */
--- 1010,1021 ----
if (TransactionIdFollowsOrEquals(HeapTupleHeaderGetXmax(tuple),
snapshot->xmin))
{
if (TransactionIdFollowsOrEquals(HeapTupleHeaderGetXmax(tuple),
snapshot->xmax))
return true;
! if (XidInSnapshot(HeapTupleHeaderGetXmax(tuple), snapshot))
! return true;
}
/* This is to be used only for disaster recovery and requires serious analysis. */
*************** Snapshot
*** 1299,1309 ****
CopySnapshot(Snapshot snapshot)
{
Snapshot newsnap;
/* We allocate any XID array needed in the same palloc block. */
! newsnap = (Snapshot) palloc(sizeof(SnapshotData) +
! snapshot->xcnt * sizeof(TransactionId));
memcpy(newsnap, snapshot, sizeof(SnapshotData));
if (snapshot->xcnt > 0)
{
newsnap->xip = (TransactionId *) (newsnap + 1);
--- 1272,1290 ----
CopySnapshot(Snapshot snapshot)
{
Snapshot newsnap;
+ Size subxipoff;
+ Size size;
+
+ size = subxipoff = sizeof(SnapshotData) +
+ snapshot->xcnt * sizeof(TransactionId);
+ if (snapshot->subxcnt > 0)
+ size += snapshot->subxcnt * sizeof(TransactionId);
/* We allocate any XID array needed in the same palloc block. */
! newsnap = (Snapshot) palloc(size);
memcpy(newsnap, snapshot, sizeof(SnapshotData));
+
+ /* setup XID array */
if (snapshot->xcnt > 0)
{
newsnap->xip = (TransactionId *) (newsnap + 1);
*************** CopySnapshot(Snapshot snapshot)
*** 1313,1318 ****
--- 1294,1309 ----
else
newsnap->xip = NULL;
+ /* setup sub XID array */
+ if (snapshot->subxcnt > 0)
+ {
+ newsnap->subxip = (TransactionId *) ((char *) newsnap + subxipoff);
+ memcpy(newsnap->subxip, snapshot->subxip,
+ snapshot->subxcnt * sizeof(TransactionId));
+ }
+ else
+ newsnap->subxip = NULL;
+
return newsnap;
}
*************** FreeXactSnapshot(void)
*** 1346,1349 ****
--- 1337,1386 ----
SerializableSnapshot = NULL;
LatestSnapshot = NULL;
ActiveSnapshot = NULL; /* just for cleanliness */
+ }
+
+ static bool
+ XidInSnapshot(TransactionId xid, Snapshot snapshot)
+ {
+ TransactionId parentXid;
+
+ if (snapshot->subxcnt < 0)
+ {
+ /*
+ * The snapshot does not have enough information. We will get
+ * information from pg_subtrans log.
+ */
+ parentXid = SubTransGetTopmostTransaction(xid);
+ }
+ else
+ {
+ int32 i;
+
+ for (i = 0; i < snapshot->subxcnt; i++)
+ {
+ if (TransactionIdEquals(xid, snapshot->subxip[i]))
+ return true;
+ }
+
+ /*
+ * XID may not be a top transaction id. However, they will not match
+ * any top XIDs. So we can merely ignore any subxids not in subxip.
+ */
+ parentXid = xid;
+ }
+
+ if (TransactionIdFollowsOrEquals(parentXid, snapshot->xmin))
+ {
+ uint32 i;
+
+ /* no point in checking parentXid against xmax here */
+
+ for (i = 0; i < snapshot->xcnt; i++)
+ {
+ if (TransactionIdEquals(parentXid, snapshot->xip[i]))
+ return true;
+ }
+ }
+
+ return false;
}
diff -cpr pgsql-orig/src/include/utils/tqual.h pgsql/src/include/utils/tqual.h
*** pgsql-orig/src/include/utils/tqual.h Mon Aug 28 09:36:13 2006
--- pgsql/src/include/utils/tqual.h Mon Aug 28 17:24:04 2006
*************** typedef struct SnapshotData
*** 39,44 ****
--- 39,46 ----
TransactionId xmax; /* XID >= xmax are invisible to me */
uint32 xcnt; /* # of xact ids in xip[] */
TransactionId *xip; /* array of xact IDs in progress */
+ int32 subxcnt; /* # of xact ids in subxip[], -1 on overflow */
+ TransactionId *subxip; /* array of subxact IDs in progress */
/* note: all ids in xip[] satisfy xmin <= xip[i] < xmax */
CommandId curcid; /* in my xact, CID < curcid are visible */
} SnapshotData;
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CSStorm occurred again by postgreSQL8.2
2006-08-07 04:36 Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-08 03:16 ` Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-26 03:46 ` Re: CSStorm occurred again by postgreSQL8.2 Bruce Momjian <bruce@momjian.us>
2006-08-28 10:17 ` Re: [HACKERS] CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
@ 2006-09-03 16:08 ` Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 01:45 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 22+ messages in thread
From: Tom Lane @ 2006-09-03 16:08 UTC (permalink / raw)
To: ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>; +Cc: Bruce Momjian <bruce@momjian.us>; Alvaro Herrera <alvherre@alvh.no-ip.org>; pgsql-hackers
ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp> writes:
> I added a subxid array to Snapshot and running subxids are gathered from
> PGPROC->subxids cache. There are two overflowed case; any of PGPROC->subxids
> are overflowed or the number of total subxids exceeds pre-allocated buffers.
> If overflowed, we cannot avoid to call SubTransGetTopmostTransaction.
Applied after some editorialization (you really need to pay more
attention to keeping comments in sync with code ;-))
I cannot measure any consistent speed difference in plain pgbench
scenarios with and without the patch, so at least as a rough
approximation the extra cycles in GetSnapshotData aren't hurting.
And I confirm that the test case you posted before no longer exhibits
a context-swap storm.
This change makes it even more obvious than before that we really want
to stay out of the subxids-overflowed regime. I don't especially want
to make those arrays even bigger, but I wonder if there isn't more we
can do to use them efficiently. In particular, it seems like in the
case where RecordSubTransactionCommit detects that the subxact has not
stored its XID anywhere, we could immediately remove the XID from
the PGPROC array, just as if it had aborted. This would avoid chewing
subxid slots for cases such as exception blocks in plpgsql that are
not modifying the database, but just catching computational errors.
Comments?
regards, tom lane
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CSStorm occurred again by postgreSQL8.2
2006-08-07 04:36 Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-08 03:16 ` Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-26 03:46 ` Re: CSStorm occurred again by postgreSQL8.2 Bruce Momjian <bruce@momjian.us>
2006-08-28 10:17 ` Re: [HACKERS] CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-09-03 16:08 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
@ 2006-09-14 01:45 ` Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 11:05 ` Re: CSStorm occurred again by postgreSQL8.2 Gregory Stark <stark@enterprisedb.com>
0 siblings, 1 reply; 22+ messages in thread
From: Tom Lane @ 2006-09-14 01:45 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@alvh.no-ip.org>; +Cc: ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>; Bruce Momjian <bruce@momjian.us>; pgsql-hackers
I wrote:
> ... it seems like in the
> case where RecordSubTransactionCommit detects that the subxact has not
> stored its XID anywhere, we could immediately remove the XID from
> the PGPROC array, just as if it had aborted. This would avoid chewing
> subxid slots for cases such as exception blocks in plpgsql that are
> not modifying the database, but just catching computational errors.
(and later realized that Alvaro had had the same idea awhile back, but
I don't have his message at hand).
I looked into this a bit more; it seems like basically it should only
take addition of
else
XidCacheRemoveRunningXids(xid, 0, NULL);
to the bottom of RecordSubTransactionCommit(), plus suitable adjustment
of the comments in both routines. However, there's a problem: if we
delete a second-level subxact's XID from PGPROC, and later its upper
subtransaction aborts, XidCacheRemoveRunningXids will emit scary
warnings when it doesn't find the sub-subxact in PGPROC. This could
doubtless be fixed with sufficient jiggery-pokery --- simply removing
the debug warnings would be a brute-force answer, but I'd like to find
something a bit less brute-force. Maybe drop the sub-subxact from its
parent's list immediately, instead of carrying it forward?
Anyway, given that there's this one nonobvious gotcha, there might be
others. My recommendation is that we take this off the open-items list
for 8.2 and revisit it in the 8.3 cycle when there's more time.
regards, tom lane
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CSStorm occurred again by postgreSQL8.2
2006-08-07 04:36 Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-08 03:16 ` Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-26 03:46 ` Re: CSStorm occurred again by postgreSQL8.2 Bruce Momjian <bruce@momjian.us>
2006-08-28 10:17 ` Re: [HACKERS] CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-09-03 16:08 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 01:45 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
@ 2006-09-14 11:05 ` Gregory Stark <stark@enterprisedb.com>
2006-09-14 12:41 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 22+ messages in thread
From: Gregory Stark @ 2006-09-14 11:05 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>; ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>; Bruce Momjian <bruce@momjian.us>; pgsql-hackers
Tom Lane <tgl@sss.pgh.pa.us> writes:
> Anyway, given that there's this one nonobvious gotcha, there might be
> others. My recommendation is that we take this off the open-items list
> for 8.2 and revisit it in the 8.3 cycle when there's more time.
I wonder if Theo's recent reported problem with 4.3M child xids changes the
calculus on this.
--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CSStorm occurred again by postgreSQL8.2
2006-08-07 04:36 Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-08 03:16 ` Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-26 03:46 ` Re: CSStorm occurred again by postgreSQL8.2 Bruce Momjian <bruce@momjian.us>
2006-08-28 10:17 ` Re: [HACKERS] CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-09-03 16:08 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 01:45 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 11:05 ` Re: CSStorm occurred again by postgreSQL8.2 Gregory Stark <stark@enterprisedb.com>
@ 2006-09-14 12:41 ` Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 13:40 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 22+ messages in thread
From: Tom Lane @ 2006-09-14 12:41 UTC (permalink / raw)
To: Gregory Stark <stark@enterprisedb.com>; +Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>; ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>; Bruce Momjian <bruce@momjian.us>; pgsql-hackers
Gregory Stark <stark@enterprisedb.com> writes:
> Tom Lane <tgl@sss.pgh.pa.us> writes:
>> Anyway, given that there's this one nonobvious gotcha, there might be
>> others. My recommendation is that we take this off the open-items list
>> for 8.2 and revisit it in the 8.3 cycle when there's more time.
> I wonder if Theo's recent reported problem with 4.3M child xids changes the
> calculus on this.
Yeah, I was just looking at that. Removing useless entries from the
child-xid list would presumably help him. Considering we're not even
formally in beta yet, I'm probably being too conservative to recommend
we not touch it.
regards, tom lane
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CSStorm occurred again by postgreSQL8.2
2006-08-07 04:36 Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-08 03:16 ` Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-26 03:46 ` Re: CSStorm occurred again by postgreSQL8.2 Bruce Momjian <bruce@momjian.us>
2006-08-28 10:17 ` Re: [HACKERS] CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-09-03 16:08 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 01:45 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 11:05 ` Re: CSStorm occurred again by postgreSQL8.2 Gregory Stark <stark@enterprisedb.com>
2006-09-14 12:41 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
@ 2006-09-14 13:40 ` Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 13:57 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 22+ messages in thread
From: Tom Lane @ 2006-09-14 13:40 UTC (permalink / raw)
To: Gregory Stark <stark@enterprisedb.com>; +Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>; ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>; Bruce Momjian <bruce@momjian.us>; pgsql-hackers
I wrote:
> Yeah, I was just looking at that. Removing useless entries from the
> child-xid list would presumably help him. Considering we're not even
> formally in beta yet, I'm probably being too conservative to recommend
> we not touch it.
Actually ... wait a minute. We do not assign an XID to a subtransaction
at all unless it writes a tuple to disk (see GetCurrentTransactionId
and its callers). So this whole "optimization" idea is redundant.
I see a bug though, which is that RecordSubTransactionAbort() calls
GetCurrentTransactionId() before having verified that it needs to do
anything. This means that we'll generate and then discard an XID
uselessly in a failed subxact that didn't touch disk. Worth fixing,
but it doesn't look like this is Theo's problem.
Unless I'm missing something, Theo's problem must involve having done
tuple updates in 4.6M different subtransactions.
regards, tom lane
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CSStorm occurred again by postgreSQL8.2
2006-08-07 04:36 Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-08 03:16 ` Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-26 03:46 ` Re: CSStorm occurred again by postgreSQL8.2 Bruce Momjian <bruce@momjian.us>
2006-08-28 10:17 ` Re: [HACKERS] CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-09-03 16:08 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 01:45 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 11:05 ` Re: CSStorm occurred again by postgreSQL8.2 Gregory Stark <stark@enterprisedb.com>
2006-09-14 12:41 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 13:40 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
@ 2006-09-14 13:57 ` Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 14:22 ` Re: CSStorm occurred again by postgreSQL8.2 Alvaro Herrera <alvherre@commandprompt.com>
0 siblings, 1 reply; 22+ messages in thread
From: Tom Lane @ 2006-09-14 13:57 UTC (permalink / raw)
To: Gregory Stark <stark@enterprisedb.com>; +Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>; ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>; Bruce Momjian <bruce@momjian.us>; pgsql-hackers
I wrote:
> I see a bug though, which is that RecordSubTransactionAbort() calls
> GetCurrentTransactionId() before having verified that it needs to do
> anything. This means that we'll generate and then discard an XID
> uselessly in a failed subxact that didn't touch disk.
Well, it would be a bug except that RecordSubTransactionAbort isn't
called unless the current subxact has an XID. Perhaps a comment would
be appropriate but there's nothing to fix here.
I think Theo's problem is probably somewhere else, too --- apparently
it's not so much that TransactionIdIsCurrentTransactionId takes a long
time as that something is calling it lots of times with no check for
interrupt.
regards, tom lane
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CSStorm occurred again by postgreSQL8.2
2006-08-07 04:36 Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-08 03:16 ` Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-26 03:46 ` Re: CSStorm occurred again by postgreSQL8.2 Bruce Momjian <bruce@momjian.us>
2006-08-28 10:17 ` Re: [HACKERS] CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-09-03 16:08 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 01:45 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 11:05 ` Re: CSStorm occurred again by postgreSQL8.2 Gregory Stark <stark@enterprisedb.com>
2006-09-14 12:41 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 13:40 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 13:57 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
@ 2006-09-14 14:22 ` Alvaro Herrera <alvherre@commandprompt.com>
2006-09-14 14:43 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 22+ messages in thread
From: Alvaro Herrera @ 2006-09-14 14:22 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Gregory Stark <stark@enterprisedb.com>; ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>; Bruce Momjian <bruce@momjian.us>; pgsql-hackers
Tom Lane wrote:
> I wrote:
> > I see a bug though, which is that RecordSubTransactionAbort() calls
> > GetCurrentTransactionId() before having verified that it needs to do
> > anything. This means that we'll generate and then discard an XID
> > uselessly in a failed subxact that didn't touch disk.
>
> Well, it would be a bug except that RecordSubTransactionAbort isn't
> called unless the current subxact has an XID. Perhaps a comment would
> be appropriate but there's nothing to fix here.
>
> I think Theo's problem is probably somewhere else, too --- apparently
> it's not so much that TransactionIdIsCurrentTransactionId takes a long
> time as that something is calling it lots of times with no check for
> interrupt.
Could it be something like heap_lock_tuple? It calls MultiXactIdWait,
which calls GetMultXactIdMembers and TransactionIdIsCurrentTransactionId
on each member. (heap_update and heap_delete do the same thing). I
must admit I didn't read Theo's description on his scenario though.
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CSStorm occurred again by postgreSQL8.2
2006-08-07 04:36 Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-08 03:16 ` Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-26 03:46 ` Re: CSStorm occurred again by postgreSQL8.2 Bruce Momjian <bruce@momjian.us>
2006-08-28 10:17 ` Re: [HACKERS] CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-09-03 16:08 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 01:45 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 11:05 ` Re: CSStorm occurred again by postgreSQL8.2 Gregory Stark <stark@enterprisedb.com>
2006-09-14 12:41 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 13:40 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 13:57 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 14:22 ` Re: CSStorm occurred again by postgreSQL8.2 Alvaro Herrera <alvherre@commandprompt.com>
@ 2006-09-14 14:43 ` Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 15:15 ` Re: CSStorm occurred again by postgreSQL8.2 Gregory Stark <stark@enterprisedb.com>
0 siblings, 1 reply; 22+ messages in thread
From: Tom Lane @ 2006-09-14 14:43 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@commandprompt.com>; +Cc: Gregory Stark <stark@enterprisedb.com>; ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>; Bruce Momjian <bruce@momjian.us>; pgsql-hackers
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Tom Lane wrote:
>> I think Theo's problem is probably somewhere else, too --- apparently
>> it's not so much that TransactionIdIsCurrentTransactionId takes a long
>> time as that something is calling it lots of times with no check for
>> interrupt.
> Could it be something like heap_lock_tuple? It calls MultiXactIdWait,
> which calls GetMultXactIdMembers and TransactionIdIsCurrentTransactionId
> on each member. (heap_update and heap_delete do the same thing). I
> must admit I didn't read Theo's description on his scenario though.
He shows HeapTupleSatisfiesSnapshot as the next thing down the call
stack, so those scenarios don't seem quite right. I'm wondering about a
CHECK_FOR_INTERRUPTS-free loop in either plperl or trigger handling,
myself.
Anyway, I was thinking some more about Theo's original suggestion that
the linked-list representation of childXids was too inefficient. I'm
disinclined to use a hash as he suggests, but it strikes me that we
could very easily change the list into a dynamically extended array
--- and because the entries are surely added in increasing XID order,
such an array could be binary-searched. This wouldn't be a win for
very small numbers of child XIDs, but for large numbers it would.
OTOH, there are probably enough other inefficiencies in handling large
numbers of subxact XIDs that speeding up TransactionIdIsCurrentTransactionId
might be a useless exercise. It would be good to profile a test case
before spending much effort here.
regards, tom lane
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CSStorm occurred again by postgreSQL8.2
2006-08-07 04:36 Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-08 03:16 ` Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-26 03:46 ` Re: CSStorm occurred again by postgreSQL8.2 Bruce Momjian <bruce@momjian.us>
2006-08-28 10:17 ` Re: [HACKERS] CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-09-03 16:08 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 01:45 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 11:05 ` Re: CSStorm occurred again by postgreSQL8.2 Gregory Stark <stark@enterprisedb.com>
2006-09-14 12:41 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 13:40 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 13:57 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 14:22 ` Re: CSStorm occurred again by postgreSQL8.2 Alvaro Herrera <alvherre@commandprompt.com>
2006-09-14 14:43 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
@ 2006-09-14 15:15 ` Gregory Stark <stark@enterprisedb.com>
2006-09-14 15:30 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 22+ messages in thread
From: Gregory Stark @ 2006-09-14 15:15 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Alvaro Herrera <alvherre@commandprompt.com>; ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>; Bruce Momjian <bruce@momjian.us>; pgsql-hackers
Tom Lane <tgl@sss.pgh.pa.us> writes:
> --- and because the entries are surely added in increasing XID order,
> such an array could be binary-searched.
If they're only added if they write to disk then isn't it possible to add them
out of order? Start a child transaction, start a child of that one and write
to disk, then exit the grandchild and write to disk in the first child? I'm
just going on your description, I'm not familiar with this part of the code at
all.
--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CSStorm occurred again by postgreSQL8.2
2006-08-07 04:36 Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-08 03:16 ` Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-26 03:46 ` Re: CSStorm occurred again by postgreSQL8.2 Bruce Momjian <bruce@momjian.us>
2006-08-28 10:17 ` Re: [HACKERS] CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-09-03 16:08 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 01:45 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 11:05 ` Re: CSStorm occurred again by postgreSQL8.2 Gregory Stark <stark@enterprisedb.com>
2006-09-14 12:41 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 13:40 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 13:57 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 14:22 ` Re: CSStorm occurred again by postgreSQL8.2 Alvaro Herrera <alvherre@commandprompt.com>
2006-09-14 14:43 ` Re: CSStorm occurred again by postgreSQL8.2 Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 15:15 ` Re: CSStorm occurred again by postgreSQL8.2 Gregory Stark <stark@enterprisedb.com>
@ 2006-09-14 15:30 ` Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 22+ messages in thread
From: Tom Lane @ 2006-09-14 15:30 UTC (permalink / raw)
To: Gregory Stark <stark@enterprisedb.com>; +Cc: Alvaro Herrera <alvherre@commandprompt.com>; ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>; Bruce Momjian <bruce@momjian.us>; pgsql-hackers
Gregory Stark <stark@enterprisedb.com> writes:
> Tom Lane <tgl@sss.pgh.pa.us> writes:
>> --- and because the entries are surely added in increasing XID order,
>> such an array could be binary-searched.
> If they're only added if they write to disk then isn't it possible to add them
> out of order? Start a child transaction, start a child of that one and write
> to disk, then exit the grandchild and write to disk in the first
> child?
No, because we enforce child XID > parent XID. In the case above, the
child xact would be given an XID when the grandchild needs one --- see
recursion in AssignSubTransactionId(). The actually slightly shaky
assumption above is that children of the same parent xact must subcommit
in numerical order ... but as long as we have strict nesting of subxacts
I think this must be so.
regards, tom lane
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CSStorm occurred again by postgreSQL8.2
@ 2008-03-13 00:13 Bruce Momjian <bruce@momjian.us>
2008-03-13 08:24 ` Re: CSStorm occurred again by postgreSQL8.2 Simon Riggs <simon@2ndquadrant.com>
0 siblings, 1 reply; 22+ messages in thread
From: Bruce Momjian @ 2008-03-13 00:13 UTC (permalink / raw)
To: Simon Riggs <simon@2ndquadrant.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Alvaro Herrera <alvherre@alvh.no-ip.org>; ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>; pgsql-hackers
Is this a TODO? Tom's reply was:
> Nonsense. Main transaction exit also takes an exclusive lock, and is
> far more likely to be exercised in typical workloads than a
> subtransaction abort.
>
> In any case: there has still not been any evidence presented by anyone
> that optimizing XidCacheRemoveRunningXids will help one bit. Given the
> difficulty of measuring any benefit from the last couple of
> optimizations in this general area, I'm thinking that such evidence
> will be hard to come by. And we have got way more than enough on our
> plates already. Can we let go of this for 8.3, please?
---------------------------------------------------------------------------
Simon Riggs wrote:
> On Wed, 2006-09-13 at 21:45 -0400, Tom Lane wrote:
>
> > Anyway, given that there's this one nonobvious gotcha, there might be
> > others. My recommendation is that we take this off the open-items list
> > for 8.2 and revisit it in the 8.3 cycle when there's more time.
>
> Well, its still 8.3 just...
>
> As discussed in the other thread "Final Thoughts for 8.3 on LWLocking
> and Scalability", XidCacheRemoveRunningXids() is now the only holder of
> an X lock during normal processing, so I would like to remove it.
> Here's how:
>
> Currently, we take the lock, remove the subxact and then shuffle down
> all the other subxactIds so that the subxact cache is contiguous.
>
> I propose that we simply zero out the subxact entry without re-arranging
> the cache; this will be atomic, so we need not acquire an X lock. We
> then increment ndeletedxids. When we enter a new subxact into the cache,
> if ndeletedxids > 0 we scan the cache to find an InvalidTransactionId
> that we can use, then decrement ndeletedxids. So ndeletedxids is just a
> hint, not an absolute requirement. nxids then becomes the number of
> cache entries and never goes down until EOXact. The subxact cache is no
> longer in order, but then it doesn't need to be either.
>
> When we take a snapshot we will end up taking a copy of zeroed cache
> entries, so the snapshots will be slightly larger than previously.
> Though still no larger than the max. The size reduction was not so large
> as to make a significant difference across the whole array, so
> scalability is the main issue to resolve.
>
> The snapshots will be valid with no change, since InvalidTransactionId
> will never match against any recorded Xid.
>
> I would also like to make the size of the subxact cache configurable
> with a parameter such as subtransaction_cache_size = 64 (default), valid
> range 4-256.
>
> --
> Simon Riggs
> 2ndQuadrant http://www.2ndQuadrant.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 datatypes do not
> match
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CSStorm occurred again by postgreSQL8.2
2008-03-13 00:13 Re: CSStorm occurred again by postgreSQL8.2 Bruce Momjian <bruce@momjian.us>
@ 2008-03-13 08:24 ` Simon Riggs <simon@2ndquadrant.com>
0 siblings, 0 replies; 22+ messages in thread
From: Simon Riggs @ 2008-03-13 08:24 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Alvaro Herrera <alvherre@alvh.no-ip.org>; ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>; pgsql-hackers
On Wed, 2008-03-12 at 20:13 -0400, Bruce Momjian wrote:
> Is this a TODO? Tom's reply was:
The general topic, yes. The caveats still apply.
> > Nonsense. Main transaction exit also takes an exclusive lock, and is
> > far more likely to be exercised in typical workloads than a
> > subtransaction abort.
> >
> > In any case: there has still not been any evidence presented by anyone
> > that optimizing XidCacheRemoveRunningXids will help one bit. Given the
> > difficulty of measuring any benefit from the last couple of
> > optimizations in this general area, I'm thinking that such evidence
> > will be hard to come by. And we have got way more than enough on our
> > plates already. Can we let go of this for 8.3, please?
>
> ---------------------------------------------------------------------------
>
> Simon Riggs wrote:
> > On Wed, 2006-09-13 at 21:45 -0400, Tom Lane wrote:
> >
> > > Anyway, given that there's this one nonobvious gotcha, there might be
> > > others. My recommendation is that we take this off the open-items list
> > > for 8.2 and revisit it in the 8.3 cycle when there's more time.
> >
> > Well, its still 8.3 just...
> >
> > As discussed in the other thread "Final Thoughts for 8.3 on LWLocking
> > and Scalability", XidCacheRemoveRunningXids() is now the only holder of
> > an X lock during normal processing, so I would like to remove it.
> > Here's how:
> >
> > Currently, we take the lock, remove the subxact and then shuffle down
> > all the other subxactIds so that the subxact cache is contiguous.
> >
> > I propose that we simply zero out the subxact entry without re-arranging
> > the cache; this will be atomic, so we need not acquire an X lock. We
> > then increment ndeletedxids. When we enter a new subxact into the cache,
> > if ndeletedxids > 0 we scan the cache to find an InvalidTransactionId
> > that we can use, then decrement ndeletedxids. So ndeletedxids is just a
> > hint, not an absolute requirement. nxids then becomes the number of
> > cache entries and never goes down until EOXact. The subxact cache is no
> > longer in order, but then it doesn't need to be either.
> >
> > When we take a snapshot we will end up taking a copy of zeroed cache
> > entries, so the snapshots will be slightly larger than previously.
> > Though still no larger than the max. The size reduction was not so large
> > as to make a significant difference across the whole array, so
> > scalability is the main issue to resolve.
> >
> > The snapshots will be valid with no change, since InvalidTransactionId
> > will never match against any recorded Xid.
> >
> > I would also like to make the size of the subxact cache configurable
> > with a parameter such as subtransaction_cache_size = 64 (default), valid
> > range 4-256.
> >
> > --
> > Simon Riggs
> > 2ndQuadrant http://www.2ndQuadrant.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 datatypes do not
> > match
>
--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com
PostgreSQL UK 2008 Conference: http://www.postgresql.org.uk
^ permalink raw reply [nested|flat] 22+ messages in thread
end of thread, other threads:[~2008-03-13 08:24 UTC | newest]
Thread overview: 22+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2006-08-07 04:36 Re: CSStorm occurred again by postgreSQL8.2 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-07 13:09 ` Tom Lane <tgl@sss.pgh.pa.us>
2006-08-07 18:52 ` Alvaro Herrera <alvherre@commandprompt.com>
2006-08-07 19:42 ` Tom Lane <tgl@sss.pgh.pa.us>
2006-08-07 20:10 ` Alvaro Herrera <alvherre@commandprompt.com>
2006-08-07 20:27 ` Tom Lane <tgl@sss.pgh.pa.us>
2006-09-03 16:15 ` Tom Lane <tgl@sss.pgh.pa.us>
2006-08-08 03:16 ` ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-08-26 03:46 ` Bruce Momjian <bruce@momjian.us>
2006-08-28 10:17 ` ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2006-09-03 16:08 ` Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 01:45 ` Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 11:05 ` Gregory Stark <stark@enterprisedb.com>
2006-09-14 12:41 ` Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 13:40 ` Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 13:57 ` Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 14:22 ` Alvaro Herrera <alvherre@commandprompt.com>
2006-09-14 14:43 ` Tom Lane <tgl@sss.pgh.pa.us>
2006-09-14 15:15 ` Gregory Stark <stark@enterprisedb.com>
2006-09-14 15:30 ` Tom Lane <tgl@sss.pgh.pa.us>
2008-03-13 00:13 Re: CSStorm occurred again by postgreSQL8.2 Bruce Momjian <bruce@momjian.us>
2008-03-13 08:24 ` Simon Riggs <simon@2ndquadrant.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox