From: | Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> |
---|---|
To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
Cc: | Jeff Janes <jeff(dot)janes(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: segfault in hot standby for hash indexes |
Date: | 2017-03-23 10:56:05 |
Message-ID: | CAE9k0PnqW7wNY7w-K4vDO1xUNYjXkkas1PFXZSah_mGQP=xb6A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Mar 23, 2017 at 9:17 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Thu, Mar 23, 2017 at 8:43 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > On Wed, Mar 22, 2017 at 3:39 PM, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> wrote:
> >> Hi,
> >>
> >> On Wed, Mar 22, 2017 at 8:41 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >>
> >> To fix this, I think we should pass 'REGBUF_KEEP_DATA' while
> >> registering the buffer. Something like this,
> >>
> >> - XLogRegisterBuffer(0, buf, REGBUF_STANDARD);
> >> + XLogRegisterBuffer(0, buf, REGBUF_STANDARD |
> >> REGBUF_KEEP_DATA);
> >>
> >> Attached is the patch that fixes this issue.
> >>
> >
> > I think this will work, but not sure if there is a merit to deviate
> > from what btree does to handle this case. One thing I find slightly
> > awkward in hash_xlog_vacuum_get_latestRemovedXid() is that you are
> > using a number of tuples registered as part of fixed data
> > (xl_hash_vacuum_one_page) to traverse the data registered as buf data.
> > I think it will be better if we register offsets also in fixed part of
> > data as we are doing btree case.
Agreed. I have made the changes accordingly. Please check attached v2 patch.
>
> >
> >
>
> Also another small point in this regard, do we need two separate
> variables to track number of deleted items in below code? I think one
> variable is sufficient.
>
> _hash_vacuum_one_page()
> {
> ..
> deletable[ndeletable++] = offnum;
> tuples_removed += 1;--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com
> ..
> }
>
Yes, I think 'ndeletable' alone should be fine.
--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com
Attachment | Content-Type | Size |
---|---|---|
fixes_in_hash_xlog_vacuum_get_latestRemovedXid_v2.patch | application/x-patch | 4.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Khandekar | 2017-03-23 10:56:26 | Re: Parallel Append implementation |
Previous Message | Andreas Karlsson | 2017-03-23 10:45:12 | Re: Refactor handling of database attributes between pg_dump and pg_dumpall |