Re: 16-bit page checksums for 9.2

On Mon, Feb 20, 2012 at 4:18 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> What straightforward implementation is that?? This *is* the straightforward one.
>
> God knows what else we'd break if we drop the lock, reacquire as an
> exclusive, then drop it again and reacquire in shared mode. Code tends
> to assume that when you take a lock you hold it until you release;
> doing otherwise would allow all manner of race conditions to emerge.
>
> And do you really think that would be faster?

I don't know, but neither do you, because you apparently haven't tried
it. Games where we drop the shared lock and get an exclusive lock
are used in numerous places in the source code; see, for example,
heap_update(), so I don't believe that there is any reason to reject
that a priori. Indeed, I can think of at least one pretty good reason
to do it exactly that way: it's the way we've handled all previous
problems of this type, and in general it's better to make new code
look like existing code than to invent something new, especially when
you haven't made any effort to quantify the problem or the extent to
which the proposed solution mitigates it.

> If you've got some rational objections, please raise them.

Look, I think that the objections I have been raising are rational.
YMMV, and obviously does. The bottom line here is that I can't stop
you from committing this patch, and we both know that. And, really,
at the end of the day, I have no desire to keep you from committing
this patch, even if I had the power to do so, because I agree that the
feature has merit. But I can object to it and, as the patch stands
now, I do object to it, for the following specific reasons:

1. You haven't posted any meaningful performance test results, of
either normal cases or worst cases.
2. You're fiddling with the buffer locking in a way that I'm very
doubtful about without having tried any of the alternatives.
3. You're rearranging the page header in a way that I find ugly and baroque.

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