Re: crash-safe visibility map, take five

On Tue, May 10, 2011 at 1:00 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Tue, May 10, 2011 at 11:34 AM, Jesper Krogh <jesper(at)krogh(dot)cc> wrote:
>>> Or what is the downside for keeping it across IO? Will it block other
>>> processes trying to read it?
>
>> Heikki might be in a better position to comment on that than I am,
>> since he wrote the existing code.  But I think that's basically the
>> issue.  When one process has an exclusive buffer lock, nobody else can
>> scan the tuples in that block - so a sequential scan, for example,
>> that reached that block, or an index scan that needed to probe into
>> it, would pile up behind the read of the visibility map page.
>
> Right, it's the loss of potential concurrency that's annoying here.
>
> On the other hand, the concurrency loss might be entirely theoretical
> --- in particular, if other potential readers of the heap page would
> probably also need to wait for the visibility page to come in, then
> nothing is gained by letting them acquire the heap page lock sooner.
>
> I've not read this thread in any detail yet, but if we're going to be
> jumping through extremely complex hoops to avoid that scenario, it might
> be better to KISS ... especially in the first iteration.

I wouldn't describe the hoops as extremely complex; I don't feel any
inclination to simplify the patch beyond what it is right now. Of
course, we'll see what the feedback looks like after more people have
read the patch, but my feeling is that the patch strikes a reasonable
balance between performance and keeping it simple. There are some
more complicated shenanigans that I started to experiment with and
ripped out as premature optimization, but this part I think is OK.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company