On 11/27/2016 07:25 PM, Petr Jelinek wrote:
On 15/11/16 01:44, Tomas Vondra wrote:
Attached is v6 of the patch series, fixing most of the points:
* common bits (valgrind/randomization/wipe) moved to memdebug.h/c
Instead of introducing a new header file, I've added the prototypes to
memdebug.h (which was already used for the valgrind stuff anyway), and
the implementations to a new memdebug.c file. Not sure what you meant by
"static inlines" though.
I think Andres wanted to put the implementation to the static inline
functions directly in the header (see parts of pg_list or how atomics
work), but I guess you way works too.
I see. Well turning that into static inlines just like in pg_list is
possible. I guess the main reason is performance - for pg_list that
probably makes sense, but the memory randomization/valgrind stuff is
only ever used for debugging, which already does a lot of expensive
stuff anyway.
I've however kept SlabContext->freelist as an array, because there may
be many blocks with the same number of free chunks, in which case moving
the block in the list would be expensive. This way it's simply
dlist_delete + dlist_push.
+1
I get mxact isolation test failures in test_decoding with this version
of patch:
step s0w: INSERT INTO do_write DEFAULT VALUES;
+ WARNING: problem in slab TXN: number of free chunks 33 in block
0x22beba0 does not match bitmap 34
step s0start: SELECT data FROM
pg_logical_slot_get_changes('isolation_slot', NULL, NULL,
'include-xids', 'false');
data
and
step s0alter: ALTER TABLE do_write ADD column ts timestamptz;
step s0w: INSERT INTO do_write DEFAULT VALUES;
+ WARNING: problem in slab TXN: number of free chunks 33 in block
0x227c3f0 does not match bitmap 34
step s0start: SELECT data FROM
pg_logical_slot_get_changes('isolation_slot', NULL, NULL,
'include-xids', 'false');
data
D'oh! I believe this is a simple thinko in SlabCheck, which iterates
over chunks like this:
for (j = 0; j <= slab->chunksPerBlock; j++)
...
which is of course off-by-one error (and the 33 vs. 34 error message is
consistent with this theory).
Also, let's just rename the Gen to Generation.
OK.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
