On Wed, Feb 27, 2019 at 5:06 AM Amit Kapila <amit.kapila16@gmail.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?
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.
--
John Naylor https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services