On 11/22/13 15:04, Michael Paquier wrote:
2) post recovery cleanup:
- OK, so roughly the soul of this patch is to change the update
mechanism for a left child gin page so as the parent split is always
done first before any new data is inserted in this child. And this
ensures that we can remove the xlog cleanup mechanism for gin page
splits in the same fashion as gist... xlog redo mechanism is then
adapted according to that.
- I did some tests with the patch:
-- Index creation time
vanilla: 3266.834
with the two patches: 3412.473 ms
Hmm. I didn't expect any change in index creation time. Is that
repeatable, or within the margin of error?
2-1) In ginFindParents, is the case where the stack has no parent
possible (aka the stack is the root itself)? Shouldn't this code path
check if root is NULL or not?
No. A root split doesn't need to find the parent (because there isn't
any), so we never call ginFindParents with a stack containing just the
root. If you look at the only caller of ginFindParents, it's quite clear
that stack->parent always exists.
2-2) Not sure that this structure is in-line with the project policy:
struct
{
BlockNumber left;
BlockNumber right;
} children;
Why not adding a complementary structure in gin_private.h doing that?
It could be used as well in ginxlogSplit to specify a left/right
family of block numbers.
I don't think we have a project policy against in-line structs. Might be
better style to not do it, anyway, though.
Come to think of it, that children variable mustn't be declared inside
the if-statement; it's no longer in scope when the XLogInsert was done,
so the &children was pointing to potentially uninitialized piece of
stack. Valgrind would've complained.
I ended up using BlockIdData[2] for that.
Committed, thanks for the review!
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
