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