Re: [HACKERS] Setting pd_lower in GIN metapage

2017-11-05 Thread Amit Kapila
On Sat, Nov 4, 2017 at 2:03 AM, Tom Lane wrote: > Michael Paquier writes: >> On Fri, Nov 3, 2017 at 1:10 AM, Amit Kapila wrote: >>> On Fri, Nov 3, 2017 at 2:54 AM, Tom Lane wrote: I've marked the CF entry closed. However, I'm not sure if we're quite done with this thread. Weren't we

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-11-05 Thread Amit Langote
On 2017/11/03 6:24, Tom Lane wrote: > Amit Langote writes: >> On 2017/09/26 16:30, Michael Paquier wrote: >>> Cool, let's switch it back to a ready for committer status then. > >> Sure, thanks. > > Pushed with some cosmetic adjustments --- mostly, making the comments more > explicit about why we

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-11-03 Thread Tom Lane
Michael Paquier writes: > On Fri, Nov 3, 2017 at 1:10 AM, Amit Kapila wrote: >> On Fri, Nov 3, 2017 at 2:54 AM, Tom Lane wrote: >>> I've marked the CF entry closed. However, I'm not sure if we're quite >>> done with this thread. Weren't we going to adjust nbtree and hash to >>> be more aggress

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-11-02 Thread Michael Paquier
On Fri, Nov 3, 2017 at 1:10 AM, Amit Kapila wrote: > On Fri, Nov 3, 2017 at 2:54 AM, Tom Lane wrote: >> I've marked the CF entry closed. However, I'm not sure if we're quite >> done with this thread. Weren't we going to adjust nbtree and hash to >> be more aggressive about labeling their metapa

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-11-02 Thread Amit Kapila
On Fri, Nov 3, 2017 at 2:54 AM, Tom Lane wrote: > Amit Langote writes: >> On 2017/09/26 16:30, Michael Paquier wrote: >>> Cool, let's switch it back to a ready for committer status then. > >> Sure, thanks. > > Pushed with some cosmetic adjustments --- mostly, making the comments more > explicit a

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-11-02 Thread Tom Lane
Amit Langote writes: > On 2017/09/26 16:30, Michael Paquier wrote: >> Cool, let's switch it back to a ready for committer status then. > Sure, thanks. Pushed with some cosmetic adjustments --- mostly, making the comments more explicit about why we need the apparently-redundant assignments to pd_

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-26 Thread Amit Langote
On 2017/09/26 16:30, Michael Paquier wrote: > On Tue, Sep 26, 2017 at 4:22 PM, Amit Langote > wrote: >>> Except that small thing, the patches do their duty. >> >> Thanks, revised patches attached. > > Cool, let's switch it back to a ready for committer status then. Sure, thanks. Regards, Amit

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-26 Thread Michael Paquier
On Tue, Sep 26, 2017 at 4:22 PM, Amit Langote wrote: >> Except that small thing, the patches do their duty. > > Thanks, revised patches attached. Cool, let's switch it back to a ready for committer status then. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) T

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-26 Thread Amit Langote
On 2017/09/26 12:17, Michael Paquier wrote: > On Mon, Sep 25, 2017 at 3:48 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, re

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-26 Thread Amit Langote
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, regar

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-25 Thread Michael Paquier
On Mon, Sep 25, 2017 at 3:48 PM, Amit Langote wrote: > Trying to catch up. Glad to see you back. > Just to remind, I didn't actually start this thread [1] to address the > issue that the FPWs of meta pages written to WAL are not getting > compressed. An external backup tool relies on pd_lower t

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-25 Thread Amit Kapila
On Mon, Sep 25, 2017 at 12:18 PM, Amit Langote wrote: > Hi. > > Trying to catch up. > > On 2017/09/25 13:43, Michael Paquier wrote: >> On Sun, Sep 24, 2017 at 2:25 PM, Amit Kapila wrote: >>> Added and updated the comments for both btree and hash index patches. >> >> I don't have real complaints a

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-24 Thread Amit Langote
Hi. Trying to catch up. On 2017/09/25 13:43, Michael Paquier wrote: > On Sun, Sep 24, 2017 at 2:25 PM, Amit Kapila wrote: >> Added and updated the comments for both btree and hash index patches. > > I don't have real complaints about this patch, this looks fine to me. > > +* Currently, the

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-24 Thread Michael Paquier
On Mon, Sep 25, 2017 at 2:26 PM, Amit Kapila wrote: > I understand that and I think there are always multiple ways to write > same information. It might be better to pass this patch series to > committer if you don't see any mistake because he might anyway change > some comments before committing

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-24 Thread Amit Kapila
On Mon, Sep 25, 2017 at 10:13 AM, Michael Paquier wrote: > On Sun, Sep 24, 2017 at 2:25 PM, Amit Kapila wrote: >> Added and updated the comments for both btree and hash index patches. > > I don't have real complaints about this patch, this looks fine to me. > > +* Currently, the advantage of

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-24 Thread Michael Paquier
On Sun, Sep 24, 2017 at 2:25 PM, Amit Kapila wrote: > Added and updated the comments for both btree and hash index patches. I don't have real complaints about this patch, this looks fine to me. +* Currently, the advantage of setting pd_lower is in limited cases like +* during wal_consist

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-23 Thread Amit Kapila
On Wed, Sep 20, 2017 at 9:19 AM, Amit Kapila wrote: > On Wed, Sep 20, 2017 at 4:25 AM, Michael Paquier > wrote: >> On Wed, Sep 20, 2017 at 12:47 AM, Tom Lane wrote: >>> Amit Kapila writes: On Tue, Sep 19, 2017 at 9:27 AM, Michael Paquier wrote: > I am not saying that no index AMs

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-19 Thread Michael Paquier
On Wed, Sep 20, 2017 at 12:49 PM, Amit Kapila wrote: > On Wed, Sep 20, 2017 at 4:25 AM, Michael Paquier > wrote: >> Also, _hash_init() would need some extra work to >> generate FPWs, but I don't think that it is necessary per its handling >> of a per-record meta data either. So REGBUF_STANDARD co

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-19 Thread Amit Kapila
On Wed, Sep 20, 2017 at 4:25 AM, Michael Paquier wrote: > On Wed, Sep 20, 2017 at 12:47 AM, Tom Lane wrote: >> Amit Kapila writes: >>> On Tue, Sep 19, 2017 at 9:27 AM, Michael Paquier >>> wrote: I am not saying that no index AMs take advantage FPW compressibility for their meta pages.

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-19 Thread Michael Paquier
On Wed, Sep 20, 2017 at 12:47 AM, Tom Lane wrote: > Amit Kapila writes: >> On Tue, Sep 19, 2017 at 9:27 AM, Michael Paquier >> wrote: >>> I am not saying that no index AMs take advantage FPW compressibility >>> for their meta pages. There are cases like this one, as well as one >>> code path in

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-19 Thread Tom Lane
Amit Kapila writes: > On Tue, Sep 19, 2017 at 9:27 AM, Michael Paquier > wrote: >> I am not saying that no index AMs take advantage FPW compressibility >> for their meta pages. There are cases like this one, as well as one >> code path in BRIN where this is useful, and it is useful as well when >

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-19 Thread Amit Kapila
On Tue, Sep 19, 2017 at 9:33 AM, Michael Paquier wrote: > On Tue, Sep 19, 2017 at 12:57 PM, Michael Paquier > wrote: >> I'd think about adjusting the comments the proper way for each AM so >> as one can read those comments and catch any limitation immediately. >> The fact this has not been pointe

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-19 Thread Amit Kapila
On Tue, Sep 19, 2017 at 9:27 AM, Michael Paquier wrote: > On Tue, Sep 19, 2017 at 12:40 PM, Amit Kapila wrote: >> On Mon, Sep 18, 2017 at 4:03 PM, Michael Paquier >> wrote: >>> On Mon, Sep 18, 2017 at 11:16 AM, Amit Kapila >>> wrote: On Sat, Sep 16, 2017 at 7:15 PM, Michael Paquier

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-18 Thread Michael Paquier
On Tue, Sep 19, 2017 at 12:57 PM, Michael Paquier wrote: > I'd think about adjusting the comments the proper way for each AM so > as one can read those comments and catch any limitation immediately. > The fact this has not been pointed out on this thread before any > checks and the many mails exch

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-18 Thread Michael Paquier
On Tue, Sep 19, 2017 at 12:40 PM, Amit Kapila wrote: > On Mon, Sep 18, 2017 at 4:03 PM, Michael Paquier > wrote: >> On Mon, Sep 18, 2017 at 11:16 AM, Amit Kapila >> wrote: >>> On Sat, Sep 16, 2017 at 7:15 PM, Michael Paquier >>> wrote: >>> >>> You have already noticed above that it will h

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-18 Thread Amit Kapila
On Mon, Sep 18, 2017 at 4:03 PM, Michael Paquier wrote: > On Mon, Sep 18, 2017 at 11:16 AM, Amit Kapila wrote: >> On Sat, Sep 16, 2017 at 7:15 PM, Michael Paquier >> wrote: >>> >> >> You have already noticed above that it will help when >> wal_checking_consistency is used and that was the main m

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-18 Thread Michael Paquier
On Mon, Sep 18, 2017 at 11:16 AM, Amit Kapila wrote: > On Sat, Sep 16, 2017 at 7:15 PM, Michael Paquier > wrote: >> On Fri, Sep 15, 2017 at 4:22 PM, Amit Langote >> wrote: >>> On 2017/09/14 16:00, Michael Paquier wrote: On Wed, Sep 13, 2017 at 4:43 PM, Amit Langote wrote: > Sure,

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-17 Thread Amit Kapila
On Sat, Sep 16, 2017 at 7:15 PM, Michael Paquier wrote: > On Fri, Sep 15, 2017 at 4:22 PM, Amit Langote > wrote: >> On 2017/09/14 16:00, Michael Paquier wrote: >>> On Wed, Sep 13, 2017 at 4:43 PM, Amit Langote >>> wrote: Sure, no problem. >>> >>> OK, I took a closer look at all patches, but

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-16 Thread Michael Paquier
On Fri, Sep 15, 2017 at 4:22 PM, Amit Langote wrote: > On 2017/09/14 16:00, Michael Paquier wrote: >> On Wed, Sep 13, 2017 at 4:43 PM, Amit Langote >> wrote: >>> Sure, no problem. >> >> OK, I took a closer look at all patches, but did not run any manual >> tests to test the compression except som

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-15 Thread Amit Langote
On 2017/09/14 16:00, Michael Paquier wrote: > On Wed, Sep 13, 2017 at 4:43 PM, Amit Langote > wrote: >> Sure, no problem. > > OK, I took a closer look at all patches, but did not run any manual > tests to test the compression except some stuff with > wal_consistency_checking. Thanks for the revi

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-14 Thread Michael Paquier
On Thu, Sep 14, 2017 at 6:36 PM, Amit Kapila wrote: > Why do we need to change metapage at every place for btree ... I have been hunting for some time places where meta buffers were marked as dirtied and logged. So in the effort, I think that my hands and mind got hotter, forgetting that pd_lower

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-14 Thread Amit Kapila
On Thu, Sep 14, 2017 at 12:30 PM, Michael Paquier wrote: > On Wed, Sep 13, 2017 at 4:43 PM, Amit Langote > wrote: >> Sure, no problem. > > > +* > +* This won't be of any help unless we use option like REGBUF_STANDARD > +* while registering the buffer for metapage as otherwise, it won

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-14 Thread Michael Paquier
On Wed, Sep 13, 2017 at 4:43 PM, Amit Langote wrote: > Sure, no problem. OK, I took a closer look at all patches, but did not run any manual tests to test the compression except some stuff with wal_consistency_checking. + if (opaque->flags & GIN_DELETED) + mask_page_content(page); + el

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-13 Thread Amit Langote
On 2017/09/13 16:20, Michael Paquier wrote: > On Wed, Sep 13, 2017 at 2:48 PM, Amit Langote > wrote: >> I updated the patches so that the metapage's pd_lower is set to the >> correct value just before *every* point where we are about to insert a >> full page image of the metapage into WAL. That's

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-13 Thread Michael Paquier
On Wed, Sep 13, 2017 at 2:48 PM, Amit Langote wrote: > I updated the patches so that the metapage's pd_lower is set to the > correct value just before *every* point where we are about to insert a > full page image of the metapage into WAL. That's in addition to doing the > same in various metapag

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-12 Thread Amit Langote
On 2017/09/13 13:05, Tom Lane wrote: > Amit Langote writes: >> On 2017/09/12 23:27, Amit Kapila wrote: >>> I think one point which might be missed is that the patch needs to >>> modify pd_lower for all usages of metapage, not only when it is first >>> time initialized. > >> Maybe I'm missing some

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-12 Thread Tom Lane
Amit Langote writes: > On 2017/09/12 23:27, Amit Kapila wrote: >> I think one point which might be missed is that the patch needs to >> modify pd_lower for all usages of metapage, not only when it is first >> time initialized. > Maybe I'm missing something, but isn't the metadata size fixed and h

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-12 Thread Amit Langote
Thanks for the review. On 2017/09/12 23:27, Amit Kapila wrote: > On Tue, Sep 12, 2017 at 3:51 PM, Amit Langote wrote: >> I updated the patches for GIN, BRIN, and SP-GiST to include the following >> changes: >> >> 1. Pass REGBUF_STNADARD flag when registering the metapage buffer >> > > I have look

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-12 Thread Amit Kapila
On Tue, Sep 12, 2017 at 3:51 PM, Amit Langote wrote: > On 2017/09/11 18:13, Michael Paquier wrote: >> On Mon, Sep 11, 2017 at 5:40 PM, Amit Langote wrote: >>> On 2017/09/10 15:22, Michael Paquier wrote: Coordinating efforts here would be nice. If you, Amit K, are taking care of a patch f

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-12 Thread Amit Langote
On 2017/09/11 18:13, Michael Paquier wrote: > On Mon, Sep 11, 2017 at 5:40 PM, Amit Langote wrote: >> On 2017/09/10 15:22, Michael Paquier wrote: >>> Coordinating efforts here would be nice. If you, Amit K, are taking >>> care of a patch for btree and hash, would you, Amit L, write the part >>> for

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-11 Thread Amit Kapila
On Mon, Sep 11, 2017 at 12:43 PM, Michael Paquier wrote: > On Mon, Sep 11, 2017 at 4:07 PM, Amit Kapila wrote: >> I have prepared separate patches for hash and btree index. I think >> for another type of indexes, it is better to first fix the pd_lower >> issue. > > Just wondering (sorry I have n

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-11 Thread Michael Paquier
On Mon, Sep 11, 2017 at 5:40 PM, Amit Langote wrote: > On 2017/09/10 15:22, Michael Paquier wrote: >> On Sun, Sep 10, 2017 at 3:15 PM, Amit Kapila wrote: >>> On Sat, Sep 9, 2017 at 9:00 PM, Tom Lane wrote: Michael Paquier writes: > On Fri, Sep 8, 2017 at 5:24 AM, Tom Lane wrote: >

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-11 Thread Amit Langote
On 2017/09/10 15:22, Michael Paquier wrote: > On Sun, Sep 10, 2017 at 3:15 PM, Amit Kapila wrote: >> On Sat, Sep 9, 2017 at 9:00 PM, Tom Lane wrote: >>> Michael Paquier writes: On Fri, Sep 8, 2017 at 5:24 AM, Tom Lane wrote: > In short, this patch needs a significant rewrite, and more

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-11 Thread Michael Paquier
On Mon, Sep 11, 2017 at 4:07 PM, Amit Kapila wrote: > I have prepared separate patches for hash and btree index. I think > for another type of indexes, it is better to first fix the pd_lower > issue. Just wondering (sorry I have not looked at your patch in details)... Have you tested the compres

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-11 Thread Amit Kapila
On Mon, Sep 11, 2017 at 7:18 AM, Amit Kapila wrote: > On Sun, Sep 10, 2017 at 9:56 PM, Tom Lane wrote: >> Amit Kapila writes: >>> On Sun, Sep 10, 2017 at 11:52 AM, Michael Paquier >>> wrote: Coordinating efforts here would be nice. If you, Amit K, are taking care of a patch for btree

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-10 Thread Amit Kapila
On Sun, Sep 10, 2017 at 9:56 PM, Tom Lane wrote: > Amit Kapila writes: >> On Sun, Sep 10, 2017 at 11:52 AM, Michael Paquier >> wrote: >>> Coordinating efforts here would be nice. If you, Amit K, are taking >>> care of a patch for btree and hash > >> I think here we should first agree on what we

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-10 Thread Michael Paquier
On Mon, Sep 11, 2017 at 1:26 AM, Tom Lane wrote: > FWIW, now that we've noticed the discrepancy, I'm for using > REGBUF_STANDARD or equivalent for all metapage calls. Even if it > saves no space, inconsistency is bad because it's confusing. OK, I don't mind having a more aggressive approach, but

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-10 Thread Tom Lane
Amit Kapila writes: > On Sun, Sep 10, 2017 at 11:52 AM, Michael Paquier > wrote: >> Coordinating efforts here would be nice. If you, Amit K, are taking >> care of a patch for btree and hash > I think here we should first agree on what we want to do. Based on > Tom's comment, I was thinking of c

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-10 Thread Michael Paquier
On Sun, Sep 10, 2017 at 3:28 PM, Amit Kapila wrote: > I think here we should first agree on what we want to do. Based on > Tom's comment, I was thinking of changing comments in btree/hash part > and additionally for hash indexes, I can see if we can pass > REGBUF_STANDARD for all usages of metapa

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-09 Thread Amit Kapila
On Sun, Sep 10, 2017 at 11:52 AM, Michael Paquier wrote: > On Sun, Sep 10, 2017 at 3:15 PM, Amit Kapila wrote: >> On Sat, Sep 9, 2017 at 9:00 PM, Tom Lane wrote: >>> Michael Paquier writes: On Fri, Sep 8, 2017 at 5:24 AM, Tom Lane wrote: > In short, this patch needs a significant rewr

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-09 Thread Michael Paquier
On Sun, Sep 10, 2017 at 3:15 PM, Amit Kapila wrote: > On Sat, Sep 9, 2017 at 9:00 PM, Tom Lane wrote: >> Michael Paquier writes: >>> On Fri, Sep 8, 2017 at 5:24 AM, Tom Lane wrote: In short, this patch needs a significant rewrite, and more analysis than you've done so far on whether t

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-09 Thread Amit Kapila
On Sat, Sep 9, 2017 at 9:00 PM, Tom Lane wrote: > Michael Paquier writes: >> On Fri, Sep 8, 2017 at 5:24 AM, Tom Lane wrote: >>> In short, this patch needs a significant rewrite, and more analysis than >>> you've done so far on whether there's actually any benefit to be gained. >>> It might not

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-09 Thread Amit Kapila
On Fri, Sep 8, 2017 at 1:54 AM, Tom Lane wrote: > Michael Paquier writes: >> On Thu, Sep 7, 2017 at 2:40 PM, Amit Langote >> wrote: >>> On 2017/09/07 13:09, Michael Paquier wrote: pd_lower should remain at 0 on pre-10 servers. > >>> Doesn't PageInit(), which is where any page gets initializ

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-09 Thread Tom Lane
Michael Paquier writes: > On Fri, Sep 8, 2017 at 5:24 AM, Tom Lane wrote: >> In short, this patch needs a significant rewrite, and more analysis than >> you've done so far on whether there's actually any benefit to be gained. >> It might not be worth messing with. > I did some measurements of th

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-09 Thread Michael Paquier
On Fri, Sep 8, 2017 at 5:24 AM, Tom Lane wrote: > This means that the premise of this patch is wrong. Adjusting pd_lower, > by itself, would accomplish precisely zip for WAL compression, because > we'd still not be telling the WAL code to compress out the hole. > > To get any benefit, I think we'

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-07 Thread Tom Lane
Michael Paquier writes: > On Thu, Sep 7, 2017 at 2:40 PM, Amit Langote > wrote: >> On 2017/09/07 13:09, Michael Paquier wrote: >>> pd_lower should remain at 0 on pre-10 servers. >> Doesn't PageInit(), which is where any page gets initialized, has always >> set pd_lower to SizeOfPageHeaderData?

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-07 Thread Robert Haas
On Wed, Sep 6, 2017 at 9:42 PM, Amit Langote wrote: > I too tend to think that any users who use this masking facility would > know to expect to get these failures on upgraded clusters with invalid > pd_lower in meta pages. I strongly disagree. -- Robert Haas EnterpriseDB: http://www.enterprise

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-06 Thread Michael Paquier
On Thu, Sep 7, 2017 at 2:40 PM, Amit Langote wrote: > On 2017/09/07 13:09, Michael Paquier wrote: >> pd_lower should remain at 0 on pre-10 servers. > > Doesn't PageInit(), which is where any page gets initialized, has always > set pd_lower to SizeOfPageHeaderData? Yes, sorry. I had a mind slipper

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-06 Thread Amit Langote
On 2017/09/07 13:09, Michael Paquier wrote: > On Thu, Sep 7, 2017 at 12:59 PM, Tom Lane wrote: >> The idea I'd had was to apply the masking only if pd_lower >= >> SizeOfPageHeaderData, or if you wanted to be stricter, only if >> pd_lower != 0. > > If putting a check, it seems to me that the stric

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-06 Thread Michael Paquier
On Thu, Sep 7, 2017 at 12:59 PM, Tom Lane wrote: > The idea I'd had was to apply the masking only if pd_lower >= > SizeOfPageHeaderData, or if you wanted to be stricter, only if > pd_lower != 0. If putting a check, it seems to me that the stricter one makes the most sense. pd_lower should remain

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-06 Thread Tom Lane
Michael Paquier writes: > On Thu, Sep 7, 2017 at 5:49 AM, Tom Lane wrote: >> I looked briefly at these patches. I'm not sure that it's safe for the >> mask functions to assume that meta pages always have valid pd_lower. >> What happens when replaying log data concerning an old index that doesn't

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-06 Thread Michael Paquier
On Thu, Sep 7, 2017 at 10:42 AM, Amit Langote wrote: > I too tend to think that any users who use this masking facility would > know to expect to get these failures on upgraded clusters with invalid > pd_lower in meta pages. Yes, I don't think that an optimization reducing WAL that impacts all us

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-06 Thread Amit Langote
On 2017/09/07 8:51, Michael Paquier wrote: > On Thu, Sep 7, 2017 at 5:49 AM, Tom Lane wrote: >> Amit Langote writes: >>> On 2017/08/22 9:39, Michael Paquier wrote: On Tue, Jun 27, 2017 at 4:56 PM, Amit Langote wrote: > I updated brin_mask() and spg_mask() in the attached updated pa

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-06 Thread Michael Paquier
On Thu, Sep 7, 2017 at 5:49 AM, Tom Lane wrote: > Amit Langote writes: >> On 2017/08/22 9:39, Michael Paquier wrote: >>> On Tue, Jun 27, 2017 at 4:56 PM, Amit Langote >>> wrote: I updated brin_mask() and spg_mask() in the attached updated patches so that they consider meta pages as con

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-09-06 Thread Tom Lane
Amit Langote writes: > On 2017/08/22 9:39, Michael Paquier wrote: >> On Tue, Jun 27, 2017 at 4:56 PM, Amit Langote >> wrote: >>> I updated brin_mask() and spg_mask() in the attached updated patches so >>> that they consider meta pages as containing unused space. I looked briefly at these patches

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-08-21 Thread Amit Langote
On 2017/08/22 9:39, Michael Paquier wrote: > On Tue, Jun 27, 2017 at 4:56 PM, Amit Langote > wrote: >> On 2017/06/27 10:22, Michael Paquier wrote: >>> On Mon, Jun 26, 2017 at 4:11 PM, Masahiko Sawada >>> wrote: Thank you for the patches! I checked additional patches for brin and spgist

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-08-21 Thread Michael Paquier
On Tue, Jun 27, 2017 at 4:56 PM, Amit Langote wrote: > On 2017/06/27 10:22, Michael Paquier wrote: >> On Mon, Jun 26, 2017 at 4:11 PM, Masahiko Sawada >> wrote: >>> Thank you for the patches! I checked additional patches for brin and >>> spgist. They look good to me. >> >> Last versions are stil

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-06-27 Thread Amit Langote
On 2017/06/27 10:22, Michael Paquier wrote: > On Mon, Jun 26, 2017 at 4:11 PM, Masahiko Sawada > wrote: >> Thank you for the patches! I checked additional patches for brin and >> spgist. They look good to me. > > Last versions are still missing something: brin_mask() and spg_mask() > can be upda

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-06-26 Thread Michael Paquier
On Mon, Jun 26, 2017 at 4:11 PM, Masahiko Sawada wrote: > Thank you for the patches! I checked additional patches for brin and > spgist. They look good to me. Last versions are still missing something: brin_mask() and spg_mask() can be updated so as mask_unused_space() is called for meta pages. E

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-06-26 Thread Masahiko Sawada
On Mon, Jun 26, 2017 at 3:54 PM, Amit Langote wrote: > On 2017/06/26 10:54, Michael Paquier wrote: >> On Fri, Jun 23, 2017 at 11:17 AM, Amit Langote >> wrote: >>> That was it, thanks for the pointer. >> >> GinInitMetabuffer() sets up pd_lower and pd_upper anyway using >> PageInit so the check of

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-06-25 Thread Amit Langote
On 2017/06/26 10:54, Michael Paquier wrote: > On Fri, Jun 23, 2017 at 11:17 AM, Amit Langote > wrote: >> That was it, thanks for the pointer. > > GinInitMetabuffer() sets up pd_lower and pd_upper anyway using > PageInit so the check of PageIsVerified is guaranteed to work in any > case. Upgraded

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-06-25 Thread Masahiko Sawada
On Mon, Jun 26, 2017 at 10:54 AM, Michael Paquier wrote: > On Fri, Jun 23, 2017 at 11:17 AM, Amit Langote > wrote: >> That was it, thanks for the pointer. > > GinInitMetabuffer() sets up pd_lower and pd_upper anyway using > PageInit so the check of PageIsVerified is guaranteed to work in any > ca

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-06-25 Thread Masahiko Sawada
On Sat, Jun 24, 2017 at 1:35 AM, Alvaro Herrera wrote: > Masahiko Sawada wrote: >> On Fri, Jun 23, 2017 at 3:31 PM, Michael Paquier >> wrote: >> > On Fri, Jun 23, 2017 at 3:17 PM, Amit Langote >> > wrote: >> >> Initially, I had naively set wal_consistency_check = all before running >> >> make in

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-06-25 Thread Michael Paquier
On Fri, Jun 23, 2017 at 11:17 AM, Amit Langote wrote: > That was it, thanks for the pointer. GinInitMetabuffer() sets up pd_lower and pd_upper anyway using PageInit so the check of PageIsVerified is guaranteed to work in any case. Upgraded pages will still have their pd_lower set to the previous

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-06-23 Thread Alvaro Herrera
Masahiko Sawada wrote: > On Fri, Jun 23, 2017 at 3:31 PM, Michael Paquier > wrote: > > On Fri, Jun 23, 2017 at 3:17 PM, Amit Langote > > wrote: > >> Initially, I had naively set wal_consistency_check = all before running > >> make installcheck and then had to wait for a long time to confirm that

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-06-23 Thread Masahiko Sawada
On Fri, Jun 23, 2017 at 3:31 PM, Michael Paquier wrote: > On Fri, Jun 23, 2017 at 3:17 PM, Amit Langote > wrote: >> Initially, I had naively set wal_consistency_check = all before running >> make installcheck and then had to wait for a long time to confirm that WAL >> generated by the gin test in

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-06-22 Thread Michael Paquier
On Fri, Jun 23, 2017 at 3:17 PM, Amit Langote wrote: > Initially, I had naively set wal_consistency_check = all before running > make installcheck and then had to wait for a long time to confirm that WAL > generated by the gin test indeed caused consistency check failure on the > standby with the

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-06-22 Thread Amit Langote
On 2017/06/23 15:07, Michael Paquier wrote: > On Fri, Jun 23, 2017 at 2:56 PM, Masahiko Sawada > wrote: >> Thank you for updating the patch. It looks good to me. >> BTW I'm inclined to have a regression test case where doing 'make >> check' to the streaming replication environment with >> wal_con

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-06-22 Thread Michael Paquier
On Fri, Jun 23, 2017 at 2:56 PM, Masahiko Sawada wrote: > Thank you for updating the patch. It looks good to me. > BTW I'm inclined to have a regression test case where doing 'make > check' to the streaming replication environment with > wal_consistency_check on standby server so that we can detec

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-06-22 Thread Masahiko Sawada
On Fri, Jun 23, 2017 at 11:17 AM, Amit Langote wrote: > On 2017/06/23 10:22, Masahiko Sawada wrote: >> On Thu, Jun 22, 2017 at 6:55 PM, Amit Langote >> wrote: >>> On 2017/06/22 16:56, Michael Paquier wrote: Did you check this patch with wal_consistency_checking? I am getting failures so

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-06-22 Thread Amit Langote
On 2017/06/23 10:22, Masahiko Sawada wrote: > On Thu, Jun 22, 2017 at 6:55 PM, Amit Langote > wrote: >> On 2017/06/22 16:56, Michael Paquier wrote: >>> Did you check this patch with wal_consistency_checking? I am getting >>> failures so your patch does not have the masking of GIN pages >>> complet

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-06-22 Thread Masahiko Sawada
On Thu, Jun 22, 2017 at 6:55 PM, Amit Langote wrote: > On 2017/06/22 16:56, Michael Paquier wrote: >> On Wed, Jun 21, 2017 at 9:42 AM, Amit Langote >> wrote: >>> On 2017/06/20 20:37, Amit Kapila wrote: On Tue, Jun 20, 2017 at 1:50 PM, Amit Langote wrote: > On 2017/06/19 23:31, Tom

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-06-22 Thread Amit Langote
On 2017/06/22 16:56, Michael Paquier wrote: > On Wed, Jun 21, 2017 at 9:42 AM, Amit Langote > wrote: >> On 2017/06/20 20:37, Amit Kapila wrote: >>> On Tue, Jun 20, 2017 at 1:50 PM, Amit Langote >>> wrote: On 2017/06/19 23:31, Tom Lane wrote: > I'd suggest a rule like "if pd_lower is smal

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-06-22 Thread Michael Paquier
On Wed, Jun 21, 2017 at 9:42 AM, Amit Langote wrote: > On 2017/06/20 20:37, Amit Kapila wrote: >> On Tue, Jun 20, 2017 at 1:50 PM, Amit Langote >> wrote: >>> On 2017/06/19 23:31, Tom Lane wrote: I'd suggest a rule like "if pd_lower is smaller than SizeOfPageHeaderData then don't trust i

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-06-20 Thread Amit Langote
On 2017/06/20 20:37, Amit Kapila wrote: > On Tue, Jun 20, 2017 at 1:50 PM, Amit Langote > wrote: >> On 2017/06/19 23:31, Tom Lane wrote: >>> I'd suggest a rule like "if pd_lower is smaller than SizeOfPageHeaderData >>> then don't trust it, but assume all of the page is valid data". >> >> Actually,

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-06-20 Thread Amit Kapila
On Tue, Jun 20, 2017 at 1:50 PM, Amit Langote wrote: > On 2017/06/19 23:31, Tom Lane wrote: >> Amit Kapila writes: >>> On Mon, Jun 19, 2017 at 11:37 AM, Amit Langote >>> wrote: What are some arguments against setting pd_lower in the GIN metapage as follows? >> >>> Actually, hash index

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-06-20 Thread Amit Langote
On 2017/06/19 23:31, Tom Lane wrote: > Amit Kapila writes: >> On Mon, Jun 19, 2017 at 11:37 AM, Amit Langote >> wrote: >>> What are some arguments against setting pd_lower in the GIN metapage as >>> follows? > >> Actually, hash index also has similar code (See _hash_init_metabuffer) >> and I see

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-06-20 Thread Amit Langote
On 2017/06/19 22:59, Amit Kapila wrote: > On Mon, Jun 19, 2017 at 11:37 AM, Amit Langote > wrote: >> What are some arguments against setting pd_lower in the GIN metapage as >> follows? >> >> GinMetaPageData *metad = GinPageGetMeta(page); >> >> ((PageHeader) page)->pd_lower = >> ((char *) metad

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-06-19 Thread Tom Lane
Amit Kapila writes: > On Mon, Jun 19, 2017 at 11:37 AM, Amit Langote > wrote: >> What are some arguments against setting pd_lower in the GIN metapage as >> follows? > Actually, hash index also has similar code (See _hash_init_metabuffer) > and I see no harm in doing this at similar other places.

Re: [HACKERS] Setting pd_lower in GIN metapage

2017-06-19 Thread Amit Kapila
On Mon, Jun 19, 2017 at 11:37 AM, Amit Langote wrote: > What are some arguments against setting pd_lower in the GIN metapage as > follows? > > GinMetaPageData *metad = GinPageGetMeta(page); > > ((PageHeader) page)->pd_lower = > ((char *) metad + sizeof(GinMetaPageData)) - (char *) page; > > I