Thomas Munro wrote:
> On 22 August 2014 23:02, Alvaro Herrera <alvhe...@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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to