Thomas Munro wrote:
> On 22 August 2014 23:02, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> Forgive me if I have misunderstood but it looks like your incremental
> patch included a couple of unrelated changes, namely
> s/0/InvalidCommandId/ and a reversion of ConditionalMultiXactIdWait.
Yeah, sorry about those, will push separately.
> Undoing those gave me skip-locked-v12-b.patch, attached for reference,
> and I've included those changes in a new full patch
> skip-locked-v13.patch (also rebased).
>
> +1 for the change from if-then-else to switch statements.
>
> I was less sure about the 'goto failed' change, but I couldn't measure
> any change in concurrent throughput in my simple benchmark, so I guess
> that extra buffer lock/unlock doesn't matter and I can see why you
> prefer that control flow.
I was also thinking in reducing the lock level acquired to shared rather
than exclusive in all the paths that "goto failed". Since the lock is
only used to read a couple of fields from the tuple, shared is enough
and should give slightly better concurrency. Per buffer access rules in
src/backend/storage/buffer/README:
: 1. To scan a page for tuples, one must hold a pin and either shared or
: exclusive content lock. To examine the commit status (XIDs and status bits)
: of a tuple in a shared buffer, one must likewise hold a pin and either shared
: or exclusive lock.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
