Re: [HACKERS] Setting pd_lower in GIN metapage - Mailing list pgsql-hackers
From | Amit Langote |
---|---|
Subject | Re: [HACKERS] Setting pd_lower in GIN metapage |
Date | |
Msg-id | 90ccb153-6338-27c8-8b56-e08c64534264@lab.ntt.co.jp Whole thread Raw |
In response to | Re: [HACKERS] Setting pd_lower in GIN metapage (Amit Kapila <amit.kapila16@gmail.com>) |
List | pgsql-hackers |
On 2017/09/26 11:34, Amit Kapila wrote: > On Mon, Sep 25, 2017 at 12:18 PM, Amit Langote wrote: >> So, ISTM, comments that the patches add should all say that setting the >> meta pages' pd_lower to the correct value helps to pass those pages to >> xlog.c as compressible standard layout pages, regardless of whether they >> are actually passed that way. Even if the patches do take care of the >> latter as well. >> >> Did I miss something? >> >> Looking at Amit K's updated patches for btree and hash, it seems that he >> updated the comment to say that setting pd_lower to the correct value is >> *essential*, because those pages are passed as REGBUF_STANDARD pages and >> hence will be compressed. That seems to contradict what I wrote above, >> but I think that's not wrong, too. >> > > I think you are missing that there are many cases where we use only > REGBUF_STANDARD for meta-pages (cf. hash index). For btree, where the > advantage is in fewer cases, I have explicitly stated those cases. I do see that there are some places that use only REGBUF_STANDARD. I also understand that specifying this flag is necessary condition for XLogRecordAssemble() to perform the hole compression, if it is to be performed at all. ISTM, the hole is compressed only if we write the FP image. However, reasons for why FP image needs to be written, AFAICS, are independent of whether the hole is (should be) compressed or not. Whether compression should occur or not depends on whether the REGBUF_STANDARD flag is passed, that is, whether a caller is sure that the page is of standard layout. The last part is only true if page's pd_lower is always set correctly. Perhaps, we should focus on just writing exactly why setting pd_lower to the correct value is essential. I think that's a good change, so I adopted it from your patch. The *only* reason why it's essential to set pd_lower to the correct value, as I see it, is that xloginsert.c *might* compress the hole in the page and its pd_lower better be correct if we want compression to preserve the metadata content (in meta pages). OTOH, it's not clear to me why we should write in that comment *why* the compression would occur or why the FP image would be written in the first place. Whether or not FP image is written has nothing to do with whether we should set pd_lower correctly, does it? Also, since we want to repeat the same comment in multiple places where we now set pd_lower (gin, brin, spgist patches have many more new places where meta page's pd_lower is set), keeping it to the point would be good. But as you said a few times, we should leave it to the committer to decide the exact text to write in these comment, which I think is fine. > Do you still have confusion? This is an area of PG code I don't have much experience with and is also a bit complex, so sorry if I'm saying things that are not quite right. Thanks, Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
pgsql-hackers by date: