Date: Mon, 12 Jun 2017 18:49:25 -0400
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Michael Paquier <michael.paquier@gmail.com>
Cc: wangchuanting <wangchuanting@huawei.com>,
	PostgreSQL mailing lists <pgsql-bugs@postgresql.org>
Subject: Re: Re: BUG #14680: startup process on standby encounter a
 deadlock of TwoPhaseStateLock when redo 2PC xlog
Message-ID: <20170612224925.h3tiogbfj4cjeobu@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CAB7nPqRRuR6ig5c5JWNKEdifHY9-SrJ4eWwNy7DC9sAkGzwJzA@mail.gmail.com>
User-Agent: NeoMutt/20170306-137-4415bd-dirty (1.8.0)
Precedence: bulk
Sender: pgsql-bugs-owner@postgresql.org

Michael Paquier wrote:
> On Sun, Jun 11, 2017 at 12:24 PM, Alvaro Herrera
> <alvherre@2ndquadrant.com> wrote:

> I have reworked the comment as follows:
>     /*
> -    * Don't need a lock in the recovery phase.
> +    * It is fine to access TwoPhaseState without a lock here: recovery is
> +    * finished (so if we were a standby, there's no master that can prepare
> +    * transactions anymore), and we haven't yet set WAL as open for writes,
> +    * so local existing backends, if any, cannot do so either.  We could use a
> +    * coding pattern similar to restoreTwoPhaseData, i.e., run the whole loop
> +    * with the lock held; but this loop is far more complex, so instead only
> +    * grab the lock while calling the low-level functions working directly on
> +    * manipulating the two-phase state data.  Functions working directly on
> +    * PGPROC entries linked with the two-phase transaction work with other
> +    * types of locks but we don't want to complicate that more than necessary.
>      */

Hmm.  Honestly I don't like the final sentence you added.  I find it
more confusing than useful, because it doesn't explain what these "other
types of locks" are, or why we care.

However, I found out that this rationale is likely not true, because the
checkpointer may be running concurrently with this code from startup
process, and checkpointer does process 2PC data.  Maybe there are other
reasons why there's no live bug here, but it looks wrong (I didn't try
to reproduce a problem).

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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