pg.ddx.io pgsql-performance@postgresql.org mailing list archive
help / color / mirror / Atom feedVery slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
30+ messages / 8 participants
[nested] [flat]
* Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-10 09:55 Craig Ringer <craig@postnewspapers.com.au>
0 siblings, 1 reply; 30+ messages in thread
From: Craig Ringer @ 2008-03-10 09:55 UTC (permalink / raw)
To: pgsql-performance
Hi all
I'm encountering an odd issue with a bulk import query using PostgreSQL
8.3. After a 400,000 row import into a just-truncated table `booking', a
sequential scan run on the table in the same transaction is incredibly
slow, taking ~ 166738.047 ms. After a:
`COMMIT; BEGIN;'
the same query runs in 712.615 ms, with almost all the time difference
being in the sequential scan of the `booking' table [schema at end of post].
The table is populated by a complex pl/pgsql function that draws from
several other tables to convert data from another app's format. After
that function runs, here's what happens if I do a simple sequential
scan, then what happens after I commit and run it again:
craig=# explain analyze select * from booking;
QUERY PLAN
---------------------------------------------------------------------------------------------------------------------
Seq Scan on booking (cost=0.00..9871.60 rows=320160 width=139)
(actual time=0.017..166644.697 rows=341481 loops=1)
Total runtime: 166738.047 ms
(2 rows)
craig=# commit; begin;
COMMIT
BEGIN
craig=# explain analyze select * from booking;
QUERY PLAN
------------------------------------------------------------------------------------------------------------------
Seq Scan on booking (cost=0.00..9871.60 rows=320160 width=139)
(actual time=0.022..624.492 rows=341481 loops=1)
Total runtime: 712.615 ms
(2 rows)
SET client_min_messages = 'debug5';
does not shed any more light; the only extra output is (eg):
craig=# select count(distinct id) from booking;
DEBUG: StartTransactionCommand
DEBUG: CommitTransactionCommand
count
--------
341481
(1 row)
... which took 164558.961 ms to run, or about 2 tuples per second.
[Table schema at end of post]. By comparison, after commit the same
query read about 500 tuples/second.
This issue appears to affect any query that results in a sequential scan
on the newly populated table, and also affects maintenance operations
like ALTER TABLE ... ADD CONSTRAINT ... FOREIGN KEY that perform
sequential scans. ANALYZE is also really slow. I'm not yet sure if index
scans are affected.
I'm not using WAL logging.
It doesn't matter whether I truncate the table before the import using a
separate transaction or the same one that does the import.
I see essentially identical results, and runtimes, with other more
complex queries, but it seems to boil down to extremely slow sequential
scans.
The Linux 2.6.22 host these queries are running on runs PostgreSQL 8.3.
It has 4GB of RAM and shmmax set to 512MB. Tweaking the postgresql
memory parameters seems to make little difference, but the ones that I
adjusted away from defaults to see if this was a resource issue are:
shared_buffers = 32MB
temp_buffers = 128MB
work_mem = 128MB
maintenance_work_mem = 1024MB # min 1MB
(There are relatively few clients to this database, but they work it hard).
Is this huge speed difference in sequential scans expected behavior? Any
idea what might be causing it?
I'm presently working around it by just committing the transaction after
the bulk import - but there's lots more work to do after that and it
leaves the database in a rather messy interim state.
Here's the table's schema, pasted as a quote to stop Thunderbird
mangling it. There are no rules on this table except those that might be
created internally by postgresql.
> craig=# \d booking
> Table "public.booking"
> Column | Type | Modifiers
> ------------------------+--------------------------+------------------------------------------------------
> id | integer | not null default nextval('booking_id_seq'::regclass)
> customer_id | integer | not null
> edition_id | integer | not null
> description | character varying(255) | not null
> position | integer | not null
> loading_applied | boolean | not null default false
> loading_ratio | numeric(16,4) | not null
> size_type | integer | not null
> size_length | numeric(16,4) |
> base_price | numeric(16,4) | not null
> gst_factor | numeric(16,8) | not null default gst_factor()
> page_number | integer |
> invoiced | timestamp with time zone |
> contract_id | integer |
> old_customer_id | integer | not null
> booked_time | timestamp with time zone | not null
> booked_by | character varying(80) | not null
> cancelled | boolean | not null default false
> art_supplied | boolean | not null default false
> repeat_group | integer |
> notes | text |
> originally_from_system | character(1) |
> pe_booking_id | integer |
> pe_customer_id | integer |
> Indexes:
> "booking_pkey" PRIMARY KEY, btree (id)
> Check constraints:
> "base_price_nonnegative" CHECK (base_price >= 0::numeric)
> "gst_factor_nonnegative_and_sane" CHECK (gst_factor >= 0::numeric AND gst_factor < 9::numeric)
> "loading_ratio_sane" CHECK (loading_ratio > 0::numeric AND loading_ratio < 9::numeric)
> "page_no_sane" CHECK (page_number IS NULL OR page_number > 0 AND page_number <= 500)
> "size_length_nonnegative" CHECK (size_length IS NULL OR size_length >= 0::numeric)
> Foreign-key constraints:
> "booking_contract_id_fkey" FOREIGN KEY (contract_id) REFERENCES contract(id)
> "booking_customer_id_fkey" FOREIGN KEY (customer_id) REFERENCES customer(id)
> "booking_edition_id_fkey" FOREIGN KEY (edition_id) REFERENCES edition(id) ON DELETE CASCADE
> "booking_old_customer_id_fkey" FOREIGN KEY (old_customer_id) REFERENCES customer(id)
> "booking_position_fkey" FOREIGN KEY ("position") REFERENCES booking_position(id)
> "booking_repeat_group_fkey" FOREIGN KEY (repeat_group) REFERENCES booking_repeat(id) ON DELETE SET NULL
> "booking_size_type_fkey" FOREIGN KEY (size_type) REFERENCES booking_size_type(id)
> Triggers:
> booking_after_insert_update AFTER INSERT ON booking FOR EACH ROW EXECUTE PROCEDURE booking_after_trigger()
> booking_audit AFTER UPDATE ON booking FOR EACH ROW EXECUTE PROCEDURE booking_audit_trigger()
> booking_before_insert BEFORE INSERT ON booking FOR EACH ROW EXECUTE PROCEDURE booking_before_trigger()
--
Craig Ringer
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-10 11:01 Heikki Linnakangas <heikki@enterprisedb.com>
parent: Craig Ringer <craig@postnewspapers.com.au>
0 siblings, 5 replies; 30+ messages in thread
From: Heikki Linnakangas @ 2008-03-10 11:01 UTC (permalink / raw)
To: Craig Ringer <craig@postnewspapers.com.au>; +Cc: pgsql-performance
Craig Ringer wrote:
> I'm encountering an odd issue with a bulk import query using PostgreSQL
> 8.3. After a 400,000 row import into a just-truncated table `booking', a
> sequential scan run on the table in the same transaction is incredibly
> slow, taking ~ 166738.047 ms. After a:
> `COMMIT; BEGIN;'
> the same query runs in 712.615 ms, with almost all the time difference
> being in the sequential scan of the `booking' table [schema at end of post].
>
> The table is populated by a complex pl/pgsql function that draws from
> several other tables to convert data from another app's format.
You must be having an exception handler block in that pl/pgsql function,
which implicitly creates a new subtransaction on each invocation of the
exception handler block, so you end up with hundreds of thousands of
committed subtransactions. For each row in the seq scan, the list of
subtransactions is scanned, to see if the transaction that inserted the
row is part of the current top-level transaction. That's fine for a
handful of subtransactions, but it gets really slow with large numbers
of them, as you've seen. It's an O(n^2) operation, where n is the number
of rows inserted, so you'll be in even more trouble if the number of
rows increases.
As a work-around, avoid using exception handlers, or process more than 1
row per function invocation. Or COMMIT the transaction, as you did.
For 8.4, it would be nice to improve that. I tested that on my laptop
with a similarly-sized table, inserting each row in a pl/pgsql function
with an exception handler, and I got very similar run times. According
to oprofile, all the time is spent in TransactionIdIsInProgress. I think
it would be pretty straightforward to store the committed subtransaction
ids in a sorted array, instead of a linked list, and binary search. Or
to use a hash table. That should eliminate this problem, though there is
still other places as well where a large number of subtransactions will
hurt performance.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-10 11:29 Craig Ringer <craig@postnewspapers.com.au>
parent: Heikki Linnakangas <heikki@enterprisedb.com>
4 siblings, 1 reply; 30+ messages in thread
From: Craig Ringer @ 2008-03-10 11:29 UTC (permalink / raw)
To: pgsql-performance
Thanks for the extremely helpful response. I don't think I would've
spotted that one in a hurry.
> You must be having an exception handler block in that pl/pgsql
> function, which implicitly creates a new subtransaction on each
> invocation of the exception handler block, so you end up with hundreds
> of thousands of committed subtransactions.
Aah - yes, there is. I didn't realize it'd have such an impact. I can
work around the need for it by explicitly checking the table constraints
in the function - in which case an uncaught exception will terminate the
transaction, but should only arise when I've missed a constraint check.
> For 8.4, it would be nice to improve that. I tested that on my laptop
> with a similarly-sized table, inserting each row in a pl/pgsql
> function with an exception handler, and I got very similar run times.
> According to oprofile, all the time is spent in
> TransactionIdIsInProgress. I think it would be pretty straightforward
> to store the committed subtransaction ids in a sorted array, instead
> of a linked list, and binary search. Or to use a hash table. That
> should eliminate this problem, though there is still other places as
> well where a large number of subtransactions will hurt performance.
That does sound interesting - and it would be nice to be able to use
exception handlers this way without too huge a performance hit. In the
end though it's something that can be designed around once you're aware
of it - and I'm sure that other ways of storing that data have their own
different costs and downsides.
What might also be nice, and simpler, would be a `notice', `log', or
even `debug1' level warning telling the user they've reached an absurd
number of subtransactions that'll cripple PostgreSQL's performance - say
100,000. There's precedent for this in the checkpoint frequency warning
8.3 produces if checkpoints are becoming too frequent - and like that
warning it could be configurable for big sites. If you think that's sane
I might have a go at it - though I mostly work in C++ so the result
probably won't be too pretty initially.
--
Craig Ringer
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-10 12:16 Craig Ringer <craig@postnewspapers.com.au>
parent: Heikki Linnakangas <heikki@enterprisedb.com>
4 siblings, 0 replies; 30+ messages in thread
From: Craig Ringer @ 2008-03-10 12:16 UTC (permalink / raw)
To: Heikki Linnakangas <heikki@enterprisedb.com>; +Cc: pgsql-performance
Heikki Linnakangas wrote:
> You must be having an exception handler block in that pl/pgsql
> function, which implicitly creates a new subtransaction on each
> invocation of the exception handler block, so you end up with hundreds
> of thousands of committed subtransactions.
I've just confirmed that that was indeed the issue, and coding around
the begin block dramatically cuts the runtimes of commands executed
after the big import function.
Thanks again!
--
Craig Ringer
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-10 14:33 Tom Lane <tgl@sss.pgh.pa.us>
parent: Heikki Linnakangas <heikki@enterprisedb.com>
4 siblings, 2 replies; 30+ messages in thread
From: Tom Lane @ 2008-03-10 14:33 UTC (permalink / raw)
To: Heikki Linnakangas <heikki@enterprisedb.com>; +Cc: Craig Ringer <craig@postnewspapers.com.au>; pgsql-performance
"Heikki Linnakangas" <heikki@enterprisedb.com> writes:
> For 8.4, it would be nice to improve that. I tested that on my laptop
> with a similarly-sized table, inserting each row in a pl/pgsql function
> with an exception handler, and I got very similar run times. According
> to oprofile, all the time is spent in TransactionIdIsInProgress. I think
> it would be pretty straightforward to store the committed subtransaction
> ids in a sorted array, instead of a linked list, and binary search.
I think the OP is not complaining about the time to run the transaction
that has all the subtransactions; he's complaining about the time to
scan the table that it emitted. Presumably, each row in the table has a
different (sub)transaction ID and so we are thrashing the clog lookup
mechanism. It only happens once because after that the XMIN_COMMITTED
hint bits are set.
This probably ties into the recent discussions about eliminating the
fixed-size allocations for SLRU buffers --- I suspect it would've run
better if it could have scaled up the number of pg_clog pages held in
memory.
regards, tom lane
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-10 14:48 Craig Ringer <craig@postnewspapers.com.au>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 2 replies; 30+ messages in thread
From: Craig Ringer @ 2008-03-10 14:48 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Heikki Linnakangas <heikki@enterprisedb.com>; pgsql-performance
Tom Lane wrote:
> "Heikki Linnakangas" <heikki@enterprisedb.com> writes:
>> For 8.4, it would be nice to improve that. I tested that on my laptop
>> with a similarly-sized table, inserting each row in a pl/pgsql function
>> with an exception handler, and I got very similar run times. According
>> to oprofile, all the time is spent in TransactionIdIsInProgress. I think
>> it would be pretty straightforward to store the committed subtransaction
>> ids in a sorted array, instead of a linked list, and binary search.
>
> I think the OP is not complaining about the time to run the transaction
> that has all the subtransactions; he's complaining about the time to
> scan the table that it emitted.
Yes, but only in succeeding statements in the same transaction as the
procedure that creates all the subtransactions. Table scan times return
to normal after that transaction commits.
> Presumably, each row in the table has a
> different (sub)transaction ID and so we are thrashing the clog lookup
> mechanism. It only happens once because after that the XMIN_COMMITTED
> hint bits are set.
It seems to happen with every statement run in the same transaction as,
and after, the procedure with all the subtransactions. As soon as a
COMMIT is executed, operations return to normal speed. There's no
significant delay on the first statement after COMMIT as compared to
subsequent statements, nor do successive statements before the COMMIT
get faster.
In other words, if I repeatedly run one of the statements I used in
testing for my initial post, like:
EXPLAIN ANALYZE SELECT * FROM booking;
... after running the problem stored procedure, it takes just as long
for the second and third and so on runs as for the first.
As soon as I commit the transaction, the exact same statement returns to
running in less than a second, and doesn't significantly change in
runtime for subsequent executions.
I'll bang out a couple of examples at work tomorrow to see what I land
up with, since this is clearly something that can benefit from a neat
test case.
In any case, avoding the use of an exception block per record generated
worked around the performance issues, so it's clearly something to do
with the vast numbers of subtransactions - as Heikki Linnakangas
suggested and tested.
--
Craig Ringer
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-10 14:53 Heikki Linnakangas <heikki@enterprisedb.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 30+ messages in thread
From: Heikki Linnakangas @ 2008-03-10 14:53 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Craig Ringer <craig@postnewspapers.com.au>; pgsql-performance
Tom Lane wrote:
> "Heikki Linnakangas" <heikki@enterprisedb.com> writes:
>> For 8.4, it would be nice to improve that. I tested that on my laptop
>> with a similarly-sized table, inserting each row in a pl/pgsql function
>> with an exception handler, and I got very similar run times. According
>> to oprofile, all the time is spent in TransactionIdIsInProgress. I think
>> it would be pretty straightforward to store the committed subtransaction
>> ids in a sorted array, instead of a linked list, and binary search.
>
> I think the OP is not complaining about the time to run the transaction
> that has all the subtransactions; he's complaining about the time to
> scan the table that it emitted.
If you read the original post carefully, he complained that the seq scan
was slow when executed within the same transaction as populating the
table, and fast if he committed in between.
> Presumably, each row in the table has a
> different (sub)transaction ID and so we are thrashing the clog lookup
> mechanism. It only happens once because after that the XMIN_COMMITTED
> hint bits are set.
>
> This probably ties into the recent discussions about eliminating the
> fixed-size allocations for SLRU buffers --- I suspect it would've run
> better if it could have scaled up the number of pg_clog pages held in
> memory.
I doubt that makes any noticeable difference in this case. 300000
transaction ids fit on < ~100 clog pages, and the xmins on heap pages
are nicely in order.
Getting rid of the fixed-size allocations would be nice for other
reasons, of course.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-10 15:02 Tom Lane <tgl@sss.pgh.pa.us>
parent: Craig Ringer <craig@postnewspapers.com.au>
1 sibling, 1 reply; 30+ messages in thread
From: Tom Lane @ 2008-03-10 15:02 UTC (permalink / raw)
To: Craig Ringer <craig@postnewspapers.com.au>; +Cc: Heikki Linnakangas <heikki@enterprisedb.com>; pgsql-performance
Craig Ringer <craig@postnewspapers.com.au> writes:
> It seems to happen with every statement run in the same transaction as,
> and after, the procedure with all the subtransactions. As soon as a
> COMMIT is executed, operations return to normal speed.
Ah. I misread your post as saying that it happened only once.
regards, tom lane
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-10 15:03 Heikki Linnakangas <heikki@enterprisedb.com>
parent: Craig Ringer <craig@postnewspapers.com.au>
1 sibling, 1 reply; 30+ messages in thread
From: Heikki Linnakangas @ 2008-03-10 15:03 UTC (permalink / raw)
To: Craig Ringer <craig@postnewspapers.com.au>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-performance
Craig Ringer wrote:
> I'll bang out a couple of examples at work tomorrow to see what I land
> up with, since this is clearly something that can benefit from a neat
> test case.
Here's what I used to reproduce this:
postgres=# BEGIN;
BEGIN
postgres=# CREATE TABLE foo (id int4,t text);CREATE TABLE
postgres=# CREATE OR REPLACE FUNCTION insertfunc() RETURNS void LANGUAGE
plpgsql AS $$
begin
INSERT INTO foo VALUES ( 1, repeat('a',110));
exception when unique_violation THEN end;
$$;
CREATE FUNCTION
postgres=# SELECT COUNT(insertfunc()) FROM generate_series(1,300000);
count
--------
300000
(1 row)
postgres=# EXPLAIN ANALYZE SELECT COUNT(*) FROM foo;
QUERY PLAN
----------------------------------------------------------------------------------------------------------------------
Aggregate (cost=13595.93..13595.94 rows=1 width=0) (actual
time=239535.904..239535.906 rows=1 loops=1)
-> Seq Scan on foo (cost=0.00..11948.34 rows=659034 width=0)
(actual time=0.022..239133.898 rows=300000 loops=1)
Total runtime: 239535.974 ms
(3 rows)
The oprofile output is pretty damning:
samples % symbol name
42148 99.7468 TransactionIdIsCurrentTransactionId
If you put a COMMIT right before "EXPLAIN ANALYZE..." it runs in < 1s.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-10 15:10 Craig Ringer <craig@postnewspapers.com.au>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 30+ messages in thread
From: Craig Ringer @ 2008-03-10 15:10 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Heikki Linnakangas <heikki@enterprisedb.com>; pgsql-performance
Tom Lane wrote:
> Craig Ringer <craig@postnewspapers.com.au> writes:
>
>> It seems to happen with every statement run in the same transaction as,
>> and after, the procedure with all the subtransactions. As soon as a
>> COMMIT is executed, operations return to normal speed.
>>
>
> Ah. I misread your post as saying that it happened only once.
No worries - it's best to be sure.
Thanks for looking into it.
--
Craig Ringer
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-10 15:20 Tom Lane <tgl@sss.pgh.pa.us>
parent: Heikki Linnakangas <heikki@enterprisedb.com>
0 siblings, 1 reply; 30+ messages in thread
From: Tom Lane @ 2008-03-10 15:20 UTC (permalink / raw)
To: Heikki Linnakangas <heikki@enterprisedb.com>; +Cc: Craig Ringer <craig@postnewspapers.com.au>; pgsql-performance
"Heikki Linnakangas" <heikki@enterprisedb.com> writes:
> The oprofile output is pretty damning:
> samples % symbol name
> 42148 99.7468 TransactionIdIsCurrentTransactionId
Oh, I have no doubt that that could eat a lot of cycles inside the
originating transaction ;-). I just misread Craig's complaint as
being about the cost of the first table scan *after* that transaction.
Getting rid of the linked-list representation would be a win in a couple
of ways --- we'd not need the bogus "list of XIDs" support in pg_list.h,
and xactGetCommittedChildren would go away. OTOH AtSubCommit_childXids
would get more expensive.
regards, tom lane
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-10 16:55 Rainer Pruy <Rainer.Pruy@Acrys.COM>
parent: Craig Ringer <craig@postnewspapers.com.au>
0 siblings, 0 replies; 30+ messages in thread
From: Rainer Pruy @ 2008-03-10 16:55 UTC (permalink / raw)
To: Craig Ringer <craig@postnewspapers.com.au>; +Cc: pgsql-performance
We experienced a similar degradation,
when heavily using savepoints within a single transaction.
However, we had not yet enough time to really investigate the issue.
It also was not directly reproducible using a (small) set of statements from a script.
As the overall scenario "bulk loads with sub-transactions" is close to the scenario we do run, it might come down to the same reason, so.
Thus take my vote for a solution that does not end up with "don't use (sub-) transactions".
Regards,
Rainer
Craig Ringer schrieb:
> Thanks for the extremely helpful response. I don't think I would've
> spotted that one in a hurry.
>
>> You must be having an exception handler block in that pl/pgsql
>> function, which implicitly creates a new subtransaction on each
>> invocation of the exception handler block, so you end up with hundreds
>> of thousands of committed subtransactions.
>
> Aah - yes, there is. I didn't realize it'd have such an impact. I can
> work around the need for it by explicitly checking the table constraints
> in the function - in which case an uncaught exception will terminate the
> transaction, but should only arise when I've missed a constraint check.
>
>> For 8.4, it would be nice to improve that. I tested that on my laptop
>> with a similarly-sized table, inserting each row in a pl/pgsql
>> function with an exception handler, and I got very similar run times.
>> According to oprofile, all the time is spent in
>> TransactionIdIsInProgress. I think it would be pretty straightforward
>> to store the committed subtransaction ids in a sorted array, instead
>> of a linked list, and binary search. Or to use a hash table. That
>> should eliminate this problem, though there is still other places as
>> well where a large number of subtransactions will hurt performance.
>
> That does sound interesting - and it would be nice to be able to use
> exception handlers this way without too huge a performance hit. In the
> end though it's something that can be designed around once you're aware
> of it - and I'm sure that other ways of storing that data have their own
> different costs and downsides.
>
> What might also be nice, and simpler, would be a `notice', `log', or
> even `debug1' level warning telling the user they've reached an absurd
> number of subtransactions that'll cripple PostgreSQL's performance - say
> 100,000. There's precedent for this in the checkpoint frequency warning
> 8.3 produces if checkpoints are becoming too frequent - and like that
> warning it could be configurable for big sites. If you think that's sane
> I might have a go at it - though I mostly work in C++ so the result
> probably won't be too pretty initially.
>
> --
> Craig Ringer
>
--
Rainer Pruy
Geschäftsführer
Acrys Consult GmbH & Co. KG
Untermainkai 29-30, D-60329 Frankfurt
Tel: +49-69-244506-0 - Fax: +49-69-244506-50
Web: http://www.acrys.com - Email: office@acrys.com
Handelsregister: Frankfurt am Main, HRA 31151
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-10 22:00 Simon Riggs <simon@2ndquadrant.com>
parent: Heikki Linnakangas <heikki@enterprisedb.com>
4 siblings, 1 reply; 30+ messages in thread
From: Simon Riggs @ 2008-03-10 22:00 UTC (permalink / raw)
To: Heikki Linnakangas <heikki@enterprisedb.com>; +Cc: Craig Ringer <craig@postnewspapers.com.au>; pgsql-performance
On Mon, 2008-03-10 at 11:01 +0000, Heikki Linnakangas wrote:
> According
> to oprofile, all the time is spent in TransactionIdIsInProgress.
I recently submitted a patch to optimise this. Your comments would be
welcome on the patch.
--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com
PostgreSQL UK 2008 Conference: http://www.postgresql.org.uk
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-11 06:24 Pavan Deolasee <pavan.deolasee@gmail.com>
parent: Heikki Linnakangas <heikki@enterprisedb.com>
4 siblings, 0 replies; 30+ messages in thread
From: Pavan Deolasee @ 2008-03-11 06:24 UTC (permalink / raw)
To: Heikki Linnakangas <heikki@enterprisedb.com>; +Cc: Craig Ringer <craig@postnewspapers.com.au>; pgsql-performance
On Mon, Mar 10, 2008 at 4:31 PM, Heikki Linnakangas
<heikki@enterprisedb.com> wrote:
> According
> to oprofile, all the time is spent in TransactionIdIsInProgress. I think
> it would be pretty straightforward to store the committed subtransaction
> ids in a sorted array, instead of a linked list, and binary search.
Assuming that in most of the cases, there will be many committed and few aborted
subtransactions, how about storing the list of *aborted* subtransactions ?
Thanks,
Pavan
--
Pavan Deolasee
EnterpriseDB http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Very slow (2 tuples/second) sequential scan afterbulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-11 09:58 Heikki Linnakangas <heikki@enterprisedb.com>
parent: Simon Riggs <simon@2ndquadrant.com>
0 siblings, 0 replies; 30+ messages in thread
From: Heikki Linnakangas @ 2008-03-11 09:58 UTC (permalink / raw)
To: Simon Riggs <simon@2ndquadrant.com>; +Cc: Craig Ringer <craig@postnewspapers.com.au>; pgsql-performance
Simon Riggs wrote:
> On Mon, 2008-03-10 at 11:01 +0000, Heikki Linnakangas wrote:
>> According
>> to oprofile, all the time is spent in TransactionIdIsInProgress.
>
> I recently submitted a patch to optimise this. Your comments would be
> welcome on the patch.
You mean this one:
http://archives.postgresql.org/pgsql-patches/2008-02/msg00008.php
? Unfortunately that patch won't help in this case.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: [PERFORM] Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-11 12:34 Heikki Linnakangas <heikki@enterprisedb.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 3 replies; 30+ messages in thread
From: Heikki Linnakangas @ 2008-03-11 12:34 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Craig Ringer <craig@postnewspapers.com.au>; pgsql-patches <pgsql-patches@postgresql.org>
(moved to pgsql-patches, as there's a patch attached)
Tom Lane wrote:
> Getting rid of the linked-list representation would be a win in a couple
> of ways --- we'd not need the bogus "list of XIDs" support in pg_list.h,
> and xactGetCommittedChildren would go away. OTOH AtSubCommit_childXids
> would get more expensive.
I couldn't let this case go, so I wrote a patch. I replaced the linked
list with an array that's enlarged at AtSubCommit_childXids when necessary.
I couldn't measure any performance hit from the extra work of enlarging
and memcpying the array. I tried two test cases:
1. Insert one row per subtransaction, and commit the subtransaction
after each insert. This is like the OP's case.
printf("CREATE TABLE foo (id int4);\n");
printf("BEGIN;\n");
for(i=1; i <= 100000; i++)
{
printf("SAVEPOINT sp%d;\n", i);
printf("INSERT INTO foo VALUES (1);\n");
printf("RELEASE SAVEPOINT sp%d;\n", i);
}
printf("COMMIT;\n");
2. Insert one row per subtransaction, but leave the subtransaction in
not-committed state
printf("CREATE TABLE foo (id int4, t text);\n");
printf("BEGIN;\n");
for(i=1; i <= 100000; i++)
{
printf("SAVEPOINT sp%d;\n", i);
printf("INSERT INTO foo VALUES (1, 'f');\n");
}
printf("COMMIT;\n");
Test case 1 is not bad, because we just keep appending to the parent
array one at a time. Test case 2 might become slower, as the number of
subtransactions increases, as at the commit of each subtransaction you
need to enlarge the parent array and copy all the already-committed
childxids to it. However, you hit the limit on amount of shared mem
required for holding the per-xid locks before that becomes a problem,
and the performance becomes dominated by other things anyway (notably
LockReassignCurrentOwner).
I initially thought that using a single palloc'd array to hold all the
XIDs would introduce a new limit on the number committed
subtransactions, thanks to MaxAllocSize, but that's not the case.
Without patch, we actually allocate an array like that anyway in
xactGetCommittedChildren.
Elsewhere in our codebase where we use arrays that are enlarged as
needed, we keep track of the "allocated" size and the "used" size of the
array separately, and only call repalloc when the array fills up, and
repalloc a larger than necessary array when it does. I chose to just
call repalloc every time instead, as repalloc is smart enough to fall
out quickly if the chunk the allocation was made in is already larger
than the new size. There might be some gain avoiding the repeated
repalloc calls, but I doubt it's worth the code complexity, and calling
repalloc with a larger than necessary size can actually force it to
unnecessarily allocate a new, larger chunk instead of reusing the old
one. Thoughts on that?
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
Attachments:
childXids-binsearch-4.patch (text/x-diff, 11.1K) download
Index: src/backend/access/transam/xact.c
===================================================================
RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/backend/access/transam/xact.c,v
retrieving revision 1.258
diff -c -r1.258 xact.c
*** src/backend/access/transam/xact.c 4 Mar 2008 19:54:06 -0000 1.258
--- src/backend/access/transam/xact.c 11 Mar 2008 12:31:28 -0000
***************
*** 130,136 ****
int gucNestLevel; /* GUC context nesting depth */
MemoryContext curTransactionContext; /* my xact-lifetime context */
ResourceOwner curTransactionOwner; /* my query resources */
! List *childXids; /* subcommitted child XIDs */
Oid prevUser; /* previous CurrentUserId setting */
bool prevSecDefCxt; /* previous SecurityDefinerContext setting */
bool prevXactReadOnly; /* entry-time xact r/o state */
--- 130,137 ----
int gucNestLevel; /* GUC context nesting depth */
MemoryContext curTransactionContext; /* my xact-lifetime context */
ResourceOwner curTransactionOwner; /* my query resources */
! TransactionId *childXids; /* subcommitted child XIDs, in XID order */
! int nChildXids; /* # of subcommitted child XIDs */
Oid prevUser; /* previous CurrentUserId setting */
bool prevSecDefCxt; /* previous SecurityDefinerContext setting */
bool prevXactReadOnly; /* entry-time xact r/o state */
***************
*** 156,162 ****
0, /* GUC context nesting depth */
NULL, /* cur transaction context */
NULL, /* cur transaction resource owner */
! NIL, /* subcommitted child Xids */
InvalidOid, /* previous CurrentUserId setting */
false, /* previous SecurityDefinerContext setting */
false, /* entry-time xact r/o state */
--- 157,164 ----
0, /* GUC context nesting depth */
NULL, /* cur transaction context */
NULL, /* cur transaction resource owner */
! NULL, /* subcommitted child Xids */
! 0, /* # of subcommitted child Xids */
InvalidOid, /* previous CurrentUserId setting */
false, /* previous SecurityDefinerContext setting */
false, /* entry-time xact r/o state */
***************
*** 559,565 ****
*/
for (s = CurrentTransactionState; s != NULL; s = s->parent)
{
! ListCell *cell;
if (s->state == TRANS_ABORT)
continue;
--- 561,567 ----
*/
for (s = CurrentTransactionState; s != NULL; s = s->parent)
{
! int low, high;
if (s->state == TRANS_ABORT)
continue;
***************
*** 567,576 ****
continue; /* it can't have any child XIDs either */
if (TransactionIdEquals(xid, s->transactionId))
return true;
! foreach(cell, s->childXids)
{
! if (TransactionIdEquals(xid, lfirst_xid(cell)))
return true;
}
}
--- 569,594 ----
continue; /* it can't have any child XIDs either */
if (TransactionIdEquals(xid, s->transactionId))
return true;
!
! /*
! * Return true for any committed child of this subtransaction. As
! * the childXids array is ordered, we can binary search.
! */
! low = 0;
! high = s->nChildXids - 1;
! while (low <= high)
{
! int middle;
! TransactionId probe;
!
! middle = low + (high - low) / 2;
! probe = s->childXids[middle];
! if (TransactionIdEquals(probe, xid))
return true;
+ else if (TransactionIdPrecedes(probe, xid))
+ low = middle + 1;
+ else
+ high = middle - 1;
}
}
***************
*** 1068,1073 ****
--- 1086,1093 ----
{
TransactionState s = CurrentTransactionState;
MemoryContext old_cxt;
+ TransactionId *new_childXids;
+ TransactionId new_nChildXids;
Assert(s->parent != NULL);
***************
*** 1078,1098 ****
*/
old_cxt = MemoryContextSwitchTo(TopTransactionContext);
! s->parent->childXids = lappend_xid(s->parent->childXids,
! s->transactionId);
! if (s->childXids != NIL)
! {
! s->parent->childXids = list_concat(s->parent->childXids,
! s->childXids);
! /*
! * list_concat doesn't free the list header for the second list; do so
! * here to avoid memory leakage (kluge)
! */
pfree(s->childXids);
- s->childXids = NIL;
- }
MemoryContextSwitchTo(old_cxt);
}
--- 1098,1132 ----
*/
old_cxt = MemoryContextSwitchTo(TopTransactionContext);
! /* Enlarge the parent array so that it can hold all the new xids */
! new_nChildXids = s->parent->nChildXids + s->nChildXids + 1;
! if (s->parent->childXids == NULL)
! new_childXids = palloc((new_nChildXids + 10) * sizeof(TransactionId));
! else
! new_childXids = repalloc(s->parent->childXids,
! new_nChildXids * sizeof(TransactionId));
! /*
! * Copy all my XIDs to parent's array.
! *
! * Note: We rely on the fact that the XID of a child always follows that
! * of its parent's. By copying the XID of this subtransaction before the
! * XIDs of its children, we ensure that the array stays ordered. Likewise,
! * all XIDs already in the array belong to our parent, or subtransactions
! * started and committed before us, so their XIDs must precede ours.
! */
! new_childXids[s->parent->nChildXids] = s->transactionId;
!
! if (s->nChildXids > 0)
! memcpy(&new_childXids[s->parent->nChildXids + 1],
! s->childXids, s->nChildXids * sizeof(TransactionId));
!
! s->parent->childXids = new_childXids;
! s->parent->nChildXids = new_nChildXids;
!
! if (s->childXids != NULL)
pfree(s->childXids);
MemoryContextSwitchTo(old_cxt);
}
***************
*** 1336,1343 ****
* AtSubCommit_childXids). This means we'd better free the list
* explicitly at abort to avoid leakage.
*/
! list_free(s->childXids);
! s->childXids = NIL;
}
/* ----------------------------------------------------------------
--- 1370,1378 ----
* AtSubCommit_childXids). This means we'd better free the list
* explicitly at abort to avoid leakage.
*/
! if (s->childXids != NULL)
! pfree(s->childXids);
! s->nChildXids = 0;
}
/* ----------------------------------------------------------------
***************
*** 1506,1512 ****
*/
s->nestingLevel = 1;
s->gucNestLevel = 1;
! s->childXids = NIL;
GetUserIdAndContext(&s->prevUser, &s->prevSecDefCxt);
/* SecurityDefinerContext should never be set outside a transaction */
Assert(!s->prevSecDefCxt);
--- 1541,1548 ----
*/
s->nestingLevel = 1;
s->gucNestLevel = 1;
! s->nChildXids = 0;
! s->childXids = NULL;
GetUserIdAndContext(&s->prevUser, &s->prevSecDefCxt);
/* SecurityDefinerContext should never be set outside a transaction */
Assert(!s->prevSecDefCxt);
***************
*** 1702,1708 ****
s->subTransactionId = InvalidSubTransactionId;
s->nestingLevel = 0;
s->gucNestLevel = 0;
! s->childXids = NIL;
/*
* done with commit processing, set current transaction state back to
--- 1738,1745 ----
s->subTransactionId = InvalidSubTransactionId;
s->nestingLevel = 0;
s->gucNestLevel = 0;
! s->nChildXids = 0;
! s->childXids = NULL;
/*
* done with commit processing, set current transaction state back to
***************
*** 1931,1937 ****
s->subTransactionId = InvalidSubTransactionId;
s->nestingLevel = 0;
s->gucNestLevel = 0;
! s->childXids = NIL;
/*
* done with 1st phase commit processing, set current transaction state
--- 1968,1975 ----
s->subTransactionId = InvalidSubTransactionId;
s->nestingLevel = 0;
s->gucNestLevel = 0;
! s->nChildXids = 0;
! s->childXids = NULL;
/*
* done with 1st phase commit processing, set current transaction state
***************
*** 2101,2107 ****
s->subTransactionId = InvalidSubTransactionId;
s->nestingLevel = 0;
s->gucNestLevel = 0;
! s->childXids = NIL;
/*
* done with abort processing, set current transaction state back to
--- 2139,2146 ----
s->subTransactionId = InvalidSubTransactionId;
s->nestingLevel = 0;
s->gucNestLevel = 0;
! s->nChildXids = 0;
! s->childXids = NULL;
/*
* done with abort processing, set current transaction state back to
***************
*** 4051,4056 ****
--- 4090,4108 ----
static void
ShowTransactionStateRec(TransactionState s)
{
+ StringInfoData buf;
+
+ initStringInfo(&buf);
+
+ if (s->nChildXids > 0)
+ {
+ int i;
+
+ appendStringInfo(&buf, "%u", s->childXids[0]);
+ for (i = 1; i < s->nChildXids; i++)
+ appendStringInfo(&buf, " %u", s->childXids[i]);
+ }
+
if (s->parent)
ShowTransactionStateRec(s->parent);
***************
*** 4064,4071 ****
(unsigned int) s->subTransactionId,
(unsigned int) currentCommandId,
currentCommandIdUsed ? " (used)" : "",
! s->nestingLevel,
! nodeToString(s->childXids))));
}
/*
--- 4116,4122 ----
(unsigned int) s->subTransactionId,
(unsigned int) currentCommandId,
currentCommandIdUsed ? " (used)" : "",
! s->nestingLevel, buf.data)));
}
/*
***************
*** 4154,4162 ****
TransactionState s = CurrentTransactionState;
int nchildren;
TransactionId *children;
- ListCell *p;
! nchildren = list_length(s->childXids);
if (nchildren == 0)
{
*ptr = NULL;
--- 4205,4212 ----
TransactionState s = CurrentTransactionState;
int nchildren;
TransactionId *children;
! nchildren = s->nChildXids;
if (nchildren == 0)
{
*ptr = NULL;
***************
*** 4166,4177 ****
children = (TransactionId *) palloc(nchildren * sizeof(TransactionId));
*ptr = children;
! foreach(p, s->childXids)
! {
! TransactionId child = lfirst_xid(p);
!
! *children++ = child;
! }
return nchildren;
}
--- 4216,4222 ----
children = (TransactionId *) palloc(nchildren * sizeof(TransactionId));
*ptr = children;
! memcpy(children, s->childXids, nchildren * sizeof(TransactionId));
return nchildren;
}
Index: src/include/nodes/pg_list.h
===================================================================
RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/include/nodes/pg_list.h,v
retrieving revision 1.57
diff -c -r1.57 pg_list.h
*** src/include/nodes/pg_list.h 1 Jan 2008 19:45:58 -0000 1.57
--- src/include/nodes/pg_list.h 11 Mar 2008 11:56:21 -0000
***************
*** 26,36 ****
* (At the moment, ints and Oids are the same size, but they may not
* always be so; try to be careful to maintain the distinction.)
*
- * There is also limited support for lists of TransactionIds; since these
- * are used in only one or two places, we don't provide a full implementation,
- * but map them onto Oid lists. This effectively assumes that TransactionId
- * is no wider than Oid and both are unsigned types.
- *
*
* Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
--- 26,31 ----
***************
*** 160,171 ****
#define list_make4_oid(x1,x2,x3,x4) lcons_oid(x1, list_make3_oid(x2, x3, x4))
/*
- * Limited support for lists of TransactionIds, mapped onto lists of Oids
- */
- #define lfirst_xid(lc) ((TransactionId) lfirst_oid(lc))
- #define lappend_xid(list, datum) lappend_oid(list, (Oid) (datum))
-
- /*
* foreach -
* a convenience macro which loops through the list
*/
--- 155,160 ----
* Re: [PERFORM] Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-11 13:06 Pavan Deolasee <pavan.deolasee@gmail.com>
parent: Heikki Linnakangas <heikki@enterprisedb.com>
2 siblings, 0 replies; 30+ messages in thread
From: Pavan Deolasee @ 2008-03-11 13:06 UTC (permalink / raw)
To: Heikki Linnakangas <heikki@enterprisedb.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Craig Ringer <craig@postnewspapers.com.au>; pgsql-patches <pgsql-patches@postgresql.org>
On Tue, Mar 11, 2008 at 6:04 PM, Heikki Linnakangas
<heikki@enterprisedb.com> wrote:
> (moved to pgsql-patches, as there's a patch attached)
>
>
> I couldn't let this case go, so I wrote a patch. I replaced the linked
> list with an array that's enlarged at AtSubCommit_childXids when necessary.
>
We can replace the array of xids with an array of flags where i'th flag is
set to true if the corresponding subtransaction is committed. This would
make lookup O(1) operation. Of course, the downside is when the array
is sparse. Or we can switch to flag based representation once the array size
grows beyond a limit.
Thanks,
Pavan
--
Pavan Deolasee
EnterpriseDB http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: [PERFORM] Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-11 13:29 Alvaro Herrera <alvherre@commandprompt.com>
parent: Heikki Linnakangas <heikki@enterprisedb.com>
2 siblings, 0 replies; 30+ messages in thread
From: Alvaro Herrera @ 2008-03-11 13:29 UTC (permalink / raw)
To: Heikki Linnakangas <heikki@enterprisedb.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Craig Ringer <craig@postnewspapers.com.au>; pgsql-patches <pgsql-patches@postgresql.org>
Heikki Linnakangas wrote:
> I couldn't let this case go, so I wrote a patch. I replaced the linked
> list with an array that's enlarged at AtSubCommit_childXids when
> necessary.
Do you still need to palloc the return value from
xactGetCommittedChildren? Perhaps you can save the palloc/memcpy/pfree
and just return the pointer to the array already in memory?
Not that it'll any much of a performance impact, but just for
cleanliness :-)
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: [PERFORM] Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-11 21:06 Tom Lane <tgl@sss.pgh.pa.us>
parent: Heikki Linnakangas <heikki@enterprisedb.com>
2 siblings, 1 reply; 30+ messages in thread
From: Tom Lane @ 2008-03-11 21:06 UTC (permalink / raw)
To: Heikki Linnakangas <heikki@enterprisedb.com>; +Cc: Craig Ringer <craig@postnewspapers.com.au>; pgsql-patches <pgsql-patches@postgresql.org>
"Heikki Linnakangas" <heikki@enterprisedb.com> writes:
> I initially thought that using a single palloc'd array to hold all the
> XIDs would introduce a new limit on the number committed
> subtransactions, thanks to MaxAllocSize, but that's not the case.
> Without patch, we actually allocate an array like that anyway in
> xactGetCommittedChildren.
Right.
> Elsewhere in our codebase where we use arrays that are enlarged as
> needed, we keep track of the "allocated" size and the "used" size of the
> array separately, and only call repalloc when the array fills up, and
> repalloc a larger than necessary array when it does. I chose to just
> call repalloc every time instead, as repalloc is smart enough to fall
> out quickly if the chunk the allocation was made in is already larger
> than the new size. There might be some gain avoiding the repeated
> repalloc calls, but I doubt it's worth the code complexity, and calling
> repalloc with a larger than necessary size can actually force it to
> unnecessarily allocate a new, larger chunk instead of reusing the old
> one. Thoughts on that?
Seems like a pretty bad idea to me, as the behavior you're counting on
only applies to chunks up to 8K or thereabouts. In a situation where
you are subcommitting lots of XIDs one at a time, this is likely to have
quite awful behavior (or at least, you're at the mercy of the local
malloc library as to how bad it is). I'd go with the same
double-it-each-time-needed approach we use elsewhere.
regards, tom lane
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: [PERFORM] Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-12 13:43 Heikki Linnakangas <heikki@enterprisedb.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 3 replies; 30+ messages in thread
From: Heikki Linnakangas @ 2008-03-12 13:43 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Craig Ringer <craig@postnewspapers.com.au>; pgsql-patches <pgsql-patches@postgresql.org>
Tom Lane wrote:
> "Heikki Linnakangas" <heikki@enterprisedb.com> writes:
>> Elsewhere in our codebase where we use arrays that are enlarged as
>> needed, we keep track of the "allocated" size and the "used" size of the
>> array separately, and only call repalloc when the array fills up, and
>> repalloc a larger than necessary array when it does. I chose to just
>> call repalloc every time instead, as repalloc is smart enough to fall
>> out quickly if the chunk the allocation was made in is already larger
>> than the new size. There might be some gain avoiding the repeated
>> repalloc calls, but I doubt it's worth the code complexity, and calling
>> repalloc with a larger than necessary size can actually force it to
>> unnecessarily allocate a new, larger chunk instead of reusing the old
>> one. Thoughts on that?
>
> Seems like a pretty bad idea to me, as the behavior you're counting on
> only applies to chunks up to 8K or thereabouts.
Oh, you're right. Though I'm sure libc realloc has all kinds of smarts
as well, it does seem better to not rely too much on that.
> In a situation where
> you are subcommitting lots of XIDs one at a time, this is likely to have
> quite awful behavior (or at least, you're at the mercy of the local
> malloc library as to how bad it is). I'd go with the same
> double-it-each-time-needed approach we use elsewhere.
Yep, patch attached. I also changed xactGetCommittedChildren to return
the original array instead of copying it, as Alvaro suggested.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
Attachments:
childXids-binsearch-5.patch (text/x-diff, 14.1K) download
Index: src/backend/access/transam/twophase.c
===================================================================
RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/backend/access/transam/twophase.c,v
retrieving revision 1.39
diff -c -r1.39 twophase.c
*** src/backend/access/transam/twophase.c 1 Jan 2008 19:45:48 -0000 1.39
--- src/backend/access/transam/twophase.c 12 Mar 2008 12:45:13 -0000
***************
*** 827,833 ****
save_state_data(children, hdr.nsubxacts * sizeof(TransactionId));
/* While we have the child-xact data, stuff it in the gxact too */
GXactLoadSubxactData(gxact, hdr.nsubxacts, children);
- pfree(children);
}
if (hdr.ncommitrels > 0)
{
--- 827,832 ----
Index: src/backend/access/transam/xact.c
===================================================================
RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/backend/access/transam/xact.c,v
retrieving revision 1.258
diff -c -r1.258 xact.c
*** src/backend/access/transam/xact.c 4 Mar 2008 19:54:06 -0000 1.258
--- src/backend/access/transam/xact.c 12 Mar 2008 13:41:10 -0000
***************
*** 130,136 ****
int gucNestLevel; /* GUC context nesting depth */
MemoryContext curTransactionContext; /* my xact-lifetime context */
ResourceOwner curTransactionOwner; /* my query resources */
! List *childXids; /* subcommitted child XIDs */
Oid prevUser; /* previous CurrentUserId setting */
bool prevSecDefCxt; /* previous SecurityDefinerContext setting */
bool prevXactReadOnly; /* entry-time xact r/o state */
--- 130,138 ----
int gucNestLevel; /* GUC context nesting depth */
MemoryContext curTransactionContext; /* my xact-lifetime context */
ResourceOwner curTransactionOwner; /* my query resources */
! TransactionId *childXids; /* subcommitted child XIDs, in XID order */
! int nChildXids; /* # of subcommitted child XIDs */
! int maxChildXids; /* allocated size of childXids */
Oid prevUser; /* previous CurrentUserId setting */
bool prevSecDefCxt; /* previous SecurityDefinerContext setting */
bool prevXactReadOnly; /* entry-time xact r/o state */
***************
*** 156,162 ****
0, /* GUC context nesting depth */
NULL, /* cur transaction context */
NULL, /* cur transaction resource owner */
! NIL, /* subcommitted child Xids */
InvalidOid, /* previous CurrentUserId setting */
false, /* previous SecurityDefinerContext setting */
false, /* entry-time xact r/o state */
--- 158,166 ----
0, /* GUC context nesting depth */
NULL, /* cur transaction context */
NULL, /* cur transaction resource owner */
! NULL, /* subcommitted child Xids */
! 0, /* # of subcommitted child Xids */
! 0, /* allocated size of childXids */
InvalidOid, /* previous CurrentUserId setting */
false, /* previous SecurityDefinerContext setting */
false, /* entry-time xact r/o state */
***************
*** 559,565 ****
*/
for (s = CurrentTransactionState; s != NULL; s = s->parent)
{
! ListCell *cell;
if (s->state == TRANS_ABORT)
continue;
--- 563,569 ----
*/
for (s = CurrentTransactionState; s != NULL; s = s->parent)
{
! int low, high;
if (s->state == TRANS_ABORT)
continue;
***************
*** 567,576 ****
continue; /* it can't have any child XIDs either */
if (TransactionIdEquals(xid, s->transactionId))
return true;
! foreach(cell, s->childXids)
{
! if (TransactionIdEquals(xid, lfirst_xid(cell)))
return true;
}
}
--- 571,596 ----
continue; /* it can't have any child XIDs either */
if (TransactionIdEquals(xid, s->transactionId))
return true;
!
! /*
! * Return true for any committed child of this subtransaction. As
! * the childXids array is ordered, we can binary search.
! */
! low = 0;
! high = s->nChildXids - 1;
! while (low <= high)
{
! int middle;
! TransactionId probe;
!
! middle = low + (high - low) / 2;
! probe = s->childXids[middle];
! if (TransactionIdEquals(probe, xid))
return true;
+ else if (TransactionIdPrecedes(probe, xid))
+ low = middle + 1;
+ else
+ high = middle - 1;
}
}
***************
*** 985,992 ****
/* Clean up local data */
if (rels)
pfree(rels);
- if (children)
- pfree(children);
return latestXid;
}
--- 1005,1010 ----
***************
*** 1067,1100 ****
AtSubCommit_childXids(void)
{
TransactionState s = CurrentTransactionState;
! MemoryContext old_cxt;
Assert(s->parent != NULL);
/*
! * We keep the child-XID lists in TopTransactionContext; this avoids
! * setting up child-transaction contexts for what might be just a few
! * bytes of grandchild XIDs.
*/
! old_cxt = MemoryContextSwitchTo(TopTransactionContext);
! s->parent->childXids = lappend_xid(s->parent->childXids,
! s->transactionId);
!
! if (s->childXids != NIL)
{
! s->parent->childXids = list_concat(s->parent->childXids,
! s->childXids);
/*
! * list_concat doesn't free the list header for the second list; do so
! * here to avoid memory leakage (kluge)
*/
! pfree(s->childXids);
! s->childXids = NIL;
}
! MemoryContextSwitchTo(old_cxt);
}
/*
--- 1085,1155 ----
AtSubCommit_childXids(void)
{
TransactionState s = CurrentTransactionState;
! TransactionId new_nChildXids;
Assert(s->parent != NULL);
/*
! * The parent childXids array will need to hold all my XID and all my
! * childXids, in addition to XIDs already there.
*/
! new_nChildXids = s->parent->nChildXids + s->nChildXids + 1;
! /* Allocate or enlarge the parent array if necessary */
! if (s->parent->maxChildXids < new_nChildXids)
{
! int new_maxChildXids;
! TransactionId *new_childXids;
/*
! * Make it 2x what's needed right now, to avoid having to enlarge it
! * repeatedly. But we can't go above MaxAllocSize.
*/
! new_maxChildXids = Min(new_nChildXids * 2,
! MaxAllocSize / sizeof(TransactionId));
!
! if (new_maxChildXids < new_nChildXids)
! ereport(ERROR,
! (errcode(ERRCODE_OUT_OF_MEMORY),
! errmsg("maximum number of committed subtransactions (%d) reached",
! new_maxChildXids)));
!
! /*
! * We keep the child-XID lists in TopTransactionContext; this avoids
! * setting up child-transaction contexts for what might be just a few
! * bytes of grandchild XIDs.
! */
! if (s->parent->childXids == NULL)
! new_childXids =
! MemoryContextAlloc(TopTransactionContext,
! new_maxChildXids * sizeof(TransactionId));
! else
! new_childXids = repalloc(s->parent->childXids,
! new_maxChildXids * sizeof(TransactionId));
!
! s->parent->childXids = new_childXids;
! s->parent->maxChildXids = new_maxChildXids;
}
! /*
! * Copy all my XIDs to parent's array.
! *
! * Note: We rely on the fact that the XID of a child always follows that
! * of its parent's. By copying the XID of this subtransaction before the
! * XIDs of its children, we ensure that the array stays ordered. Likewise,
! * all XIDs already in the array belong to our parent, or subtransactions
! * started and committed before us, so their XIDs must precede ours.
! */
! s->parent->childXids[s->parent->nChildXids] = s->transactionId;
!
! if (s->nChildXids > 0)
! memcpy(&s->parent->childXids[s->parent->nChildXids + 1],
! s->childXids, s->nChildXids * sizeof(TransactionId));
!
! s->parent->nChildXids = new_nChildXids;
!
! if (s->childXids != NULL)
! pfree(s->childXids);
}
/*
***************
*** 1259,1266 ****
/* And clean up local data */
if (rels)
pfree(rels);
- if (children)
- pfree(children);
return latestXid;
}
--- 1314,1319 ----
***************
*** 1336,1343 ****
* AtSubCommit_childXids). This means we'd better free the list
* explicitly at abort to avoid leakage.
*/
! list_free(s->childXids);
! s->childXids = NIL;
}
/* ----------------------------------------------------------------
--- 1389,1399 ----
* AtSubCommit_childXids). This means we'd better free the list
* explicitly at abort to avoid leakage.
*/
! if (s->childXids != NULL)
! pfree(s->childXids);
! s->childXids = NULL;
! s->nChildXids = 0;
! s->maxChildXids = 0;
}
/* ----------------------------------------------------------------
***************
*** 1506,1512 ****
*/
s->nestingLevel = 1;
s->gucNestLevel = 1;
! s->childXids = NIL;
GetUserIdAndContext(&s->prevUser, &s->prevSecDefCxt);
/* SecurityDefinerContext should never be set outside a transaction */
Assert(!s->prevSecDefCxt);
--- 1562,1570 ----
*/
s->nestingLevel = 1;
s->gucNestLevel = 1;
! s->childXids = NULL;
! s->nChildXids = 0;
! s->maxChildXids = 0;
GetUserIdAndContext(&s->prevUser, &s->prevSecDefCxt);
/* SecurityDefinerContext should never be set outside a transaction */
Assert(!s->prevSecDefCxt);
***************
*** 1702,1708 ****
s->subTransactionId = InvalidSubTransactionId;
s->nestingLevel = 0;
s->gucNestLevel = 0;
! s->childXids = NIL;
/*
* done with commit processing, set current transaction state back to
--- 1760,1768 ----
s->subTransactionId = InvalidSubTransactionId;
s->nestingLevel = 0;
s->gucNestLevel = 0;
! s->childXids = NULL;
! s->nChildXids = 0;
! s->maxChildXids = 0;
/*
* done with commit processing, set current transaction state back to
***************
*** 1931,1937 ****
s->subTransactionId = InvalidSubTransactionId;
s->nestingLevel = 0;
s->gucNestLevel = 0;
! s->childXids = NIL;
/*
* done with 1st phase commit processing, set current transaction state
--- 1991,1999 ----
s->subTransactionId = InvalidSubTransactionId;
s->nestingLevel = 0;
s->gucNestLevel = 0;
! s->childXids = NULL;
! s->nChildXids = 0;
! s->maxChildXids = 0;
/*
* done with 1st phase commit processing, set current transaction state
***************
*** 2101,2107 ****
s->subTransactionId = InvalidSubTransactionId;
s->nestingLevel = 0;
s->gucNestLevel = 0;
! s->childXids = NIL;
/*
* done with abort processing, set current transaction state back to
--- 2163,2171 ----
s->subTransactionId = InvalidSubTransactionId;
s->nestingLevel = 0;
s->gucNestLevel = 0;
! s->childXids = NULL;
! s->nChildXids = 0;
! s->maxChildXids = 0;
/*
* done with abort processing, set current transaction state back to
***************
*** 4051,4056 ****
--- 4115,4133 ----
static void
ShowTransactionStateRec(TransactionState s)
{
+ StringInfoData buf;
+
+ initStringInfo(&buf);
+
+ if (s->nChildXids > 0)
+ {
+ int i;
+
+ appendStringInfo(&buf, "%u", s->childXids[0]);
+ for (i = 1; i < s->nChildXids; i++)
+ appendStringInfo(&buf, " %u", s->childXids[i]);
+ }
+
if (s->parent)
ShowTransactionStateRec(s->parent);
***************
*** 4064,4071 ****
(unsigned int) s->subTransactionId,
(unsigned int) currentCommandId,
currentCommandIdUsed ? " (used)" : "",
! s->nestingLevel,
! nodeToString(s->childXids))));
}
/*
--- 4141,4147 ----
(unsigned int) s->subTransactionId,
(unsigned int) currentCommandId,
currentCommandIdUsed ? " (used)" : "",
! s->nestingLevel, buf.data)));
}
/*
***************
*** 4144,4179 ****
* xactGetCommittedChildren
*
* Gets the list of committed children of the current transaction. The return
! * value is the number of child transactions. *children is set to point to a
! * palloc'd array of TransactionIds. If there are no subxacts, *children is
! * set to NULL.
*/
int
xactGetCommittedChildren(TransactionId **ptr)
{
TransactionState s = CurrentTransactionState;
- int nchildren;
- TransactionId *children;
- ListCell *p;
! nchildren = list_length(s->childXids);
! if (nchildren == 0)
! {
*ptr = NULL;
! return 0;
! }
!
! children = (TransactionId *) palloc(nchildren * sizeof(TransactionId));
! *ptr = children;
!
! foreach(p, s->childXids)
! {
! TransactionId child = lfirst_xid(p);
!
! *children++ = child;
! }
! return nchildren;
}
/*
--- 4220,4241 ----
* xactGetCommittedChildren
*
* Gets the list of committed children of the current transaction. The return
! * value is the number of child transactions. *children is set to point to an
! * array of TransactionIds. The array is allocated in TopTransactioncontext;
! * the caller should not try to pfree() it. If there are no subxacts,
! * *children is set to NULL.
*/
int
xactGetCommittedChildren(TransactionId **ptr)
{
TransactionState s = CurrentTransactionState;
! if (s->nChildXids == 0)
*ptr = NULL;
! else
! *ptr = s->childXids;
! return s->nChildXids;
}
/*
Index: src/include/nodes/pg_list.h
===================================================================
RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/include/nodes/pg_list.h,v
retrieving revision 1.57
diff -c -r1.57 pg_list.h
*** src/include/nodes/pg_list.h 1 Jan 2008 19:45:58 -0000 1.57
--- src/include/nodes/pg_list.h 12 Mar 2008 10:32:25 -0000
***************
*** 26,36 ****
* (At the moment, ints and Oids are the same size, but they may not
* always be so; try to be careful to maintain the distinction.)
*
- * There is also limited support for lists of TransactionIds; since these
- * are used in only one or two places, we don't provide a full implementation,
- * but map them onto Oid lists. This effectively assumes that TransactionId
- * is no wider than Oid and both are unsigned types.
- *
*
* Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
--- 26,31 ----
***************
*** 160,171 ****
#define list_make4_oid(x1,x2,x3,x4) lcons_oid(x1, list_make3_oid(x2, x3, x4))
/*
- * Limited support for lists of TransactionIds, mapped onto lists of Oids
- */
- #define lfirst_xid(lc) ((TransactionId) lfirst_oid(lc))
- #define lappend_xid(list, datum) lappend_oid(list, (Oid) (datum))
-
- /*
* foreach -
* a convenience macro which loops through the list
*/
--- 155,160 ----
* Re: [PERFORM] Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-12 15:47 Pavan Deolasee <pavan.deolasee@gmail.com>
parent: Heikki Linnakangas <heikki@enterprisedb.com>
2 siblings, 1 reply; 30+ messages in thread
From: Pavan Deolasee @ 2008-03-12 15:47 UTC (permalink / raw)
To: Heikki Linnakangas <heikki@enterprisedb.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Craig Ringer <craig@postnewspapers.com.au>; pgsql-patches <pgsql-patches@postgresql.org>
On Wed, Mar 12, 2008 at 7:13 PM, Heikki Linnakangas
<heikki@enterprisedb.com> wrote:
>
>
> Yep, patch attached. I also changed xactGetCommittedChildren to return
> the original array instead of copying it, as Alvaro suggested.
>
Any comments on the flag based approach I suggested earlier ? Am I
missing some normal scenario where it won't work well ?
Thanks,
Pavan
--
Pavan Deolasee
EnterpriseDB http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: [PERFORM] Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-12 15:57 Tom Lane <tgl@sss.pgh.pa.us>
parent: Pavan Deolasee <pavan.deolasee@gmail.com>
0 siblings, 1 reply; 30+ messages in thread
From: Tom Lane @ 2008-03-12 15:57 UTC (permalink / raw)
To: Pavan Deolasee <pavan.deolasee@gmail.com>; +Cc: Heikki Linnakangas <heikki@enterprisedb.com>; Craig Ringer <craig@postnewspapers.com.au>; pgsql-patches <pgsql-patches@postgresql.org>
"Pavan Deolasee" <pavan.deolasee@gmail.com> writes:
> On Wed, Mar 12, 2008 at 7:13 PM, Heikki Linnakangas
> <heikki@enterprisedb.com> wrote:
>> Yep, patch attached. I also changed xactGetCommittedChildren to return
>> the original array instead of copying it, as Alvaro suggested.
> Any comments on the flag based approach I suggested earlier ?
I didn't like it; it seemed overly complicated (consider dealing with
XID wraparound), and it would have problems with a slow transaction
generating a sparse set of subtransaction XIDs. I think getting rid of
the linear search will be enough to fix the performance problem.
regards, tom lane
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: [PERFORM] Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-12 17:02 Pavan Deolasee <pavan.deolasee@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 30+ messages in thread
From: Pavan Deolasee @ 2008-03-12 17:02 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Heikki Linnakangas <heikki@enterprisedb.com>; Craig Ringer <craig@postnewspapers.com.au>; pgsql-patches <pgsql-patches@postgresql.org>
On Wed, Mar 12, 2008 at 9:27 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> I didn't like it; it seemed overly complicated (consider dealing with
> XID wraparound),
We are talking about subtransactions here. I don't think we support
subtransaction wrap-around, do we ?
> and it would have problems with a slow transaction
> generating a sparse set of subtransaction XIDs.
I agree thats the worst case. But is that common ? Thats what I
was thinking when I proposed the alternate solution. I thought that can
happen only if most of the subtransactions abort, which again I thought
is not a normal case. But frankly I am not sure if my assumption is correct.
> I think getting rid of
> the linear search will be enough to fix the performance problem.
>
I wonder if a skewed binary search would help more ? For example,
if we know that the range of xids stored in the array is 1 to 1000 and
if we are searching for a number closer to 1000, we can break the
array into <large,small> parts instead of equal parts and then
search.
Well, may be I making simple things complicated ;-)
Thanks,
Pavan
--
Pavan Deolasee
EnterpriseDB http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: [PERFORM] Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-12 17:14 Heikki Linnakangas <heikki@enterprisedb.com>
parent: Pavan Deolasee <pavan.deolasee@gmail.com>
1 sibling, 1 reply; 30+ messages in thread
From: Heikki Linnakangas @ 2008-03-12 17:14 UTC (permalink / raw)
To: Pavan Deolasee <pavan.deolasee@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Craig Ringer <craig@postnewspapers.com.au>; pgsql-patches <pgsql-patches@postgresql.org>
Pavan Deolasee wrote:
> On Wed, Mar 12, 2008 at 9:27 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
>> I didn't like it; it seemed overly complicated (consider dealing with
>> XID wraparound),
>
> We are talking about subtransactions here. I don't think we support
> subtransaction wrap-around, do we ?
Imagine that you start a transaction just before transaction
wrap-around, so that the top level XID is 2^31-10. Then you start 20
subtransactions. What XIDs will they get? Now how would you map those to
a bitmap?
It's certainly possible, you could index the bitmap by the index from
top transaction XID for example. But it does get a bit complicated.
>> and it would have problems with a slow transaction
>> generating a sparse set of subtransaction XIDs.
>
> I agree thats the worst case. But is that common ? Thats what I
> was thinking when I proposed the alternate solution. I thought that can
> happen only if most of the subtransactions abort, which again I thought
> is not a normal case. But frankly I am not sure if my assumption is correct.
It's not that common to have hundreds of thousands of subtransactions to
begin with..
>> I think getting rid of
>> the linear search will be enough to fix the performance problem.
>
> I wonder if a skewed binary search would help more ? For example,
> if we know that the range of xids stored in the array is 1 to 1000 and
> if we are searching for a number closer to 1000, we can break the
> array into <large,small> parts instead of equal parts and then
> search.
Possibly, but I doubt it's worth the trouble. The simple binary search
solved the performance problem well enough. In the test case of the OP,
with 300000 subtransactions, with the patch, there was no longer any
measurable difference whether you ran the "SELECT COUNT(*)" in the same
transaction as the INSERTs or after a COMMIT.
> Well, may be I making simple things complicated ;-)
I think so :-).
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: [PERFORM] Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-12 17:22 Tom Lane <tgl@sss.pgh.pa.us>
parent: Pavan Deolasee <pavan.deolasee@gmail.com>
1 sibling, 0 replies; 30+ messages in thread
From: Tom Lane @ 2008-03-12 17:22 UTC (permalink / raw)
To: Pavan Deolasee <pavan.deolasee@gmail.com>; +Cc: Heikki Linnakangas <heikki@enterprisedb.com>; Craig Ringer <craig@postnewspapers.com.au>; pgsql-patches <pgsql-patches@postgresql.org>
"Pavan Deolasee" <pavan.deolasee@gmail.com> writes:
> On Wed, Mar 12, 2008 at 9:27 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> and it would have problems with a slow transaction
>> generating a sparse set of subtransaction XIDs.
> I agree thats the worst case. But is that common ? Thats what I
> was thinking when I proposed the alternate solution. I thought that can
> happen only if most of the subtransactions abort, which again I thought
> is not a normal case.
No, I was thinking of the case where other sessions are chewing up XIDs
while the lots-of-subtransactions transaction runs. If it's slow
enough, there could be very large gaps between the XIDs it acquires for
its subtransactions. So you'd have a situation where the exact same
transaction processing might or might not run out of memory depending
on what else happened meanwhile. Not a very pleasant property.
regards, tom lane
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: [PERFORM] Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-12 17:26 Pavan Deolasee <pavan.deolasee@gmail.com>
parent: Heikki Linnakangas <heikki@enterprisedb.com>
0 siblings, 1 reply; 30+ messages in thread
From: Pavan Deolasee @ 2008-03-12 17:26 UTC (permalink / raw)
To: Heikki Linnakangas <heikki@enterprisedb.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Craig Ringer <craig@postnewspapers.com.au>; pgsql-patches <pgsql-patches@postgresql.org>
On Wed, Mar 12, 2008 at 10:44 PM, Heikki Linnakangas
<heikki@enterprisedb.com> wrote:
>
> Imagine that you start a transaction just before transaction
> wrap-around, so that the top level XID is 2^31-10. Then you start 20
> subtransactions. What XIDs will they get? Now how would you map those to
> a bitmap?
>
Wait. Subtransaction ids are local to a transaction and always start from 1.
See this:
/*
* reinitialize within-transaction counters
*/
s->subTransactionId = TopSubTransactionId;
currentSubTransactionId = TopSubTransactionId;
>
> It's not that common to have hundreds of thousands of subtransactions to
> begin with..
True. But thats the case we are trying to solve here :-)
Thanks,
Pavan
--
Pavan Deolasee
EnterpriseDB http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: [PERFORM] Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-12 17:33 Heikki Linnakangas <heikki@enterprisedb.com>
parent: Pavan Deolasee <pavan.deolasee@gmail.com>
0 siblings, 1 reply; 30+ messages in thread
From: Heikki Linnakangas @ 2008-03-12 17:33 UTC (permalink / raw)
To: Pavan Deolasee <pavan.deolasee@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Craig Ringer <craig@postnewspapers.com.au>; pgsql-patches <pgsql-patches@postgresql.org>
Pavan Deolasee wrote:
> Wait. Subtransaction ids are local to a transaction and always start from 1.
> See this:
>
> /*
> * reinitialize within-transaction counters
> */
> s->subTransactionId = TopSubTransactionId;
> currentSubTransactionId = TopSubTransactionId;
No, we're not talking about SubTransactionIds. We're talking about real
XIDs assigned to subtransactions. Subtransactions are assigned regular
XIDs as well, just like top-level transactions.
I can see where you were coming from with you suggestion now :-).
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: [PERFORM] Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-12 17:48 Pavan Deolasee <pavan.deolasee@gmail.com>
parent: Heikki Linnakangas <heikki@enterprisedb.com>
0 siblings, 0 replies; 30+ messages in thread
From: Pavan Deolasee @ 2008-03-12 17:48 UTC (permalink / raw)
To: Heikki Linnakangas <heikki@enterprisedb.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Craig Ringer <craig@postnewspapers.com.au>; pgsql-patches <pgsql-patches@postgresql.org>
On Wed, Mar 12, 2008 at 11:03 PM, Heikki Linnakangas
<heikki@enterprisedb.com> wrote:
> Subtransactions are assigned regular
> XIDs as well, just like top-level transactions.
>
Ah, got it now. I never noticed this before.
Thanks,
Pavan
--
Pavan Deolasee
EnterpriseDB http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: [PERFORM] Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-17 02:20 Tom Lane <tgl@sss.pgh.pa.us>
parent: Heikki Linnakangas <heikki@enterprisedb.com>
2 siblings, 0 replies; 30+ messages in thread
From: Tom Lane @ 2008-03-17 02:20 UTC (permalink / raw)
To: Heikki Linnakangas <heikki@enterprisedb.com>; +Cc: Craig Ringer <craig@postnewspapers.com.au>; pgsql-patches <pgsql-patches@postgresql.org>
"Heikki Linnakangas" <heikki@enterprisedb.com> writes:
> Yep, patch attached. I also changed xactGetCommittedChildren to return
> the original array instead of copying it, as Alvaro suggested.
Applied with minor corrections (mostly comment fixes, but there were
a couple of real mistakes).
regards, tom lane
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: [PERFORM] Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit
@ 2008-03-17 02:21 Bruce Momjian <bruce@momjian.us>
parent: Heikki Linnakangas <heikki@enterprisedb.com>
2 siblings, 0 replies; 30+ messages in thread
From: Bruce Momjian @ 2008-03-17 02:21 UTC (permalink / raw)
To: Heikki Linnakangas <heikki@enterprisedb.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Craig Ringer <craig@postnewspapers.com.au>; pgsql-patches <pgsql-patches@postgresql.org>
This has been applied by Tom.
---------------------------------------------------------------------------
Heikki Linnakangas wrote:
> Tom Lane wrote:
> > "Heikki Linnakangas" <heikki@enterprisedb.com> writes:
> >> Elsewhere in our codebase where we use arrays that are enlarged as
> >> needed, we keep track of the "allocated" size and the "used" size of the
> >> array separately, and only call repalloc when the array fills up, and
> >> repalloc a larger than necessary array when it does. I chose to just
> >> call repalloc every time instead, as repalloc is smart enough to fall
> >> out quickly if the chunk the allocation was made in is already larger
> >> than the new size. There might be some gain avoiding the repeated
> >> repalloc calls, but I doubt it's worth the code complexity, and calling
> >> repalloc with a larger than necessary size can actually force it to
> >> unnecessarily allocate a new, larger chunk instead of reusing the old
> >> one. Thoughts on that?
> >
> > Seems like a pretty bad idea to me, as the behavior you're counting on
> > only applies to chunks up to 8K or thereabouts.
>
> Oh, you're right. Though I'm sure libc realloc has all kinds of smarts
> as well, it does seem better to not rely too much on that.
>
> > In a situation where
> > you are subcommitting lots of XIDs one at a time, this is likely to have
> > quite awful behavior (or at least, you're at the mercy of the local
> > malloc library as to how bad it is). I'd go with the same
> > double-it-each-time-needed approach we use elsewhere.
>
> Yep, patch attached. I also changed xactGetCommittedChildren to return
> the original array instead of copying it, as Alvaro suggested.
>
> --
> Heikki Linnakangas
> EnterpriseDB http://www.enterprisedb.com
>
> --
> Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-patches
--
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] 30+ messages in thread
end of thread, other threads:[~2008-03-17 02:21 UTC | newest]
Thread overview: 30+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2008-03-10 09:55 Very slow (2 tuples/second) sequential scan after bulk insert; speed returns to ~500 tuples/second after commit Craig Ringer <craig@postnewspapers.com.au>
2008-03-10 11:01 ` Heikki Linnakangas <heikki@enterprisedb.com>
2008-03-10 11:29 ` Craig Ringer <craig@postnewspapers.com.au>
2008-03-10 16:55 ` Rainer Pruy <Rainer.Pruy@Acrys.COM>
2008-03-10 12:16 ` Craig Ringer <craig@postnewspapers.com.au>
2008-03-10 14:33 ` Tom Lane <tgl@sss.pgh.pa.us>
2008-03-10 14:48 ` Craig Ringer <craig@postnewspapers.com.au>
2008-03-10 15:02 ` Tom Lane <tgl@sss.pgh.pa.us>
2008-03-10 15:10 ` Craig Ringer <craig@postnewspapers.com.au>
2008-03-10 15:03 ` Heikki Linnakangas <heikki@enterprisedb.com>
2008-03-10 15:20 ` Tom Lane <tgl@sss.pgh.pa.us>
2008-03-11 12:34 ` Heikki Linnakangas <heikki@enterprisedb.com>
2008-03-11 13:06 ` Pavan Deolasee <pavan.deolasee@gmail.com>
2008-03-11 13:29 ` Alvaro Herrera <alvherre@commandprompt.com>
2008-03-11 21:06 ` Tom Lane <tgl@sss.pgh.pa.us>
2008-03-12 13:43 ` Heikki Linnakangas <heikki@enterprisedb.com>
2008-03-12 15:47 ` Pavan Deolasee <pavan.deolasee@gmail.com>
2008-03-12 15:57 ` Tom Lane <tgl@sss.pgh.pa.us>
2008-03-12 17:02 ` Pavan Deolasee <pavan.deolasee@gmail.com>
2008-03-12 17:14 ` Heikki Linnakangas <heikki@enterprisedb.com>
2008-03-12 17:26 ` Pavan Deolasee <pavan.deolasee@gmail.com>
2008-03-12 17:33 ` Heikki Linnakangas <heikki@enterprisedb.com>
2008-03-12 17:48 ` Pavan Deolasee <pavan.deolasee@gmail.com>
2008-03-12 17:22 ` Tom Lane <tgl@sss.pgh.pa.us>
2008-03-17 02:20 ` Tom Lane <tgl@sss.pgh.pa.us>
2008-03-17 02:21 ` Bruce Momjian <bruce@momjian.us>
2008-03-10 14:53 ` Heikki Linnakangas <heikki@enterprisedb.com>
2008-03-10 22:00 ` Simon Riggs <simon@2ndquadrant.com>
2008-03-11 09:58 ` Re: Very slow (2 tuples/second) sequential scan afterbulk insert; speed returns to ~500 tuples/second after commit Heikki Linnakangas <heikki@enterprisedb.com>
2008-03-11 06:24 ` Pavan Deolasee <pavan.deolasee@gmail.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