Hi,

On 2017-02-24 14:10:38 -0800, Andres Freund wrote:
> I've not yet looked a lot at the next type of context - I want to get
> this much committed first...
>
> I plan to give this another pass sometime this weekend and then push
> soon.

Before committing I wanted to make sure that
http://archives.postgresql.org/message-id/32354.1487977458%40sss.pgh.pa.us
isn't a sufficient fix.

With the test of N=1000000 from this thread I measured both runtime and
memory usage (note that's peak virtual memory which includes 2GB of
shared_buffers and such), in assert enabled builds.

master:                         doesn't finish reasonably
master+doubly linked list fix:  9390.805 ms     VmPeak: 10,969,424 kb
master+this thread:             6500.293 ms     VmPeak:  2,969,528 kB

So the doubly-linked-list fix is great (and much more backpatchable),
but the patches in this thread are both better runtime *and* peak memory
usage wise.  So that seems like a clear call.

I've not yet reviewed the generational allocator yet, but during these
measurements I get:
postgres[3970][1]=# select count(*) FROM pg_logical_slot_get_changes('ttt', 
NULL, NULL);
WARNING:  01000: problem in Generation Tuples: number of free chunks 0 in block 
0x55d011ef10f0 exceeds 7234 allocated
LOCATION:  GenerationCheck, generation.c:693
WARNING:  01000: problem in Generation Tuples: number of free chunks 0 in block 
0x55d01023eba0 exceeds 65532 allocated
LOCATION:  GenerationCheck, generation.c:693
WARNING:  01000: problem in Generation Tuples: number of free chunks 0 in block 
0x55d00d7fb870 exceeds 65532 allocated
LOCATION:  GenerationCheck, generation.c:693
WARNING:  01000: problem in Generation Tuples: number of free chunks 0 in block 
0x55d00cde17b0 exceeds 65531 allocated
LOCATION:  GenerationCheck, generation.c:693

that seems to occur when there's currently in-progress transactions when
finishing decoding:

#0  GenerationCheck (context=0x5629129407c8)
    at /home/andres/src/postgresql/src/backend/utils/mmgr/generation.c:692
#1  0x00005629105d92db in GenerationReset (context=0x5629129407c8)
    at /home/andres/src/postgresql/src/backend/utils/mmgr/generation.c:255
#2  0x00005629105d93c6 in GenerationDelete (context=0x5629129407c8)
    at /home/andres/src/postgresql/src/backend/utils/mmgr/generation.c:287
#3  0x00005629105e1e12 in MemoryContextDelete (context=0x5629129407c8)
    at /home/andres/src/postgresql/src/backend/utils/mmgr/mcxt.c:225
#4  0x00005629105e1ee3 in MemoryContextDeleteChildren (context=0x562912940008)
    at /home/andres/src/postgresql/src/backend/utils/mmgr/mcxt.c:245
#5  0x00005629105e1de0 in MemoryContextDelete (context=0x562912940008)
    at /home/andres/src/postgresql/src/backend/utils/mmgr/mcxt.c:208
#6  0x00005629103d5451 in ReorderBufferFree (rb=0x562912906320)
    at 
/home/andres/src/postgresql/src/backend/replication/logical/reorderbuffer.c:278
#7  0x00005629103cea4f in FreeDecodingContext (ctx=0x562912904310)
    at /home/andres/src/postgresql/src/backend/replication/logical/logical.c:462
#8  0x00005629103d03f0 in pg_logical_slot_get_changes_guts 
(fcinfo=0x7fffc2042e50, confirm=0 '\000', 

could it be that the test's condition is inverted?

I'll work on getting slab committed first, and then review / edit /
commit generation.c later.  One first note there is that I'm wondering
if generation.c is a too generic filename.

- Andres


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

Reply via email to