From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
---|---|
To: | John Naylor <john(dot)naylor(at)2ndquadrant(dot)com> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Amit Kapila <akapila(at)postgresql(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: pgsql: Avoid creation of the free space map for small heap relations, t |
Date: | 2019-02-28 00:24:11 |
Message-ID: | CAA4eK1+8wuqYp==VfbCkeuV9AevE_8sQt=wAok4RArhxc9vEtg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers pgsql-hackers |
On Wed, Feb 27, 2019 at 11:07 AM John Naylor
<john(dot)naylor(at)2ndquadrant(dot)com> wrote:
>
> On Wed, Feb 27, 2019 at 5:06 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > I have tried this test many times (more than 1000 times) by varying
> > thread count, but couldn't reproduce it. My colleague, Kuntal has
> > tried a similar test overnight, but the issue didn't reproduce which
> > is not surprising to me seeing the nature of the problem. As I could
> > reproduce it via the debugger, I think we can go ahead with the fix.
> > I have improved the comments in the attached patch and I have also
> > tried to address Tom's concern w.r.t comments by adding additional
> > comments atop of data-structure used to maintain the local map.
>
> The flaw in my thinking was treating extension too similarly too
> finding an existing block. With your patch clearing the local map in
> the correct place, it seems the call at hio.c:682 is now superfluous?
>
What if get some valid block from the first call to
GetPageWithFreeSpace via local map which has required space? I think
in that case we will need the call at hio.c:682. Am I missing
something?
> It wasn't sufficient before, so should this call be removed so as not
> to mislead? Or maybe keep it to be safe, with a comment like "This
> should already be cleared by now, but make sure it is."
>
> + * To maintain good performance, we only mark every other page as available
> + * in this map.
>
> I think this implementation detail is not needed to comment on the
> struct. I think the pointer to the README is sufficient.
>
Fair enough, I will remove that part of a comment once we agree on the
above point.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2019-02-28 00:41:37 | pgsql: Fix SCRAM authentication via SSL when mixing versions of OpenSSL |
Previous Message | Peter Eisentraut | 2019-02-27 23:30:49 | pgsql: Remove unused macro |
From | Date | Subject | |
---|---|---|---|
Next Message | Joe Conway | 2019-02-28 00:45:11 | Re: get_controlfile() can leak fds in the backend |
Previous Message | Paul Ramsey | 2019-02-27 23:50:50 | Re: Allowing extensions to supply operator-/function-specific info |