Re: [PATCH] Transaction traceability - txid_status(bigint)

From: Craig Ringer <craig(at)2ndquadrant(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Petr Jelinek <petr(at)2ndquadrant(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>, Álvaro Herrera <alvaro(dot)herrera(at)2ndquadrant(dot)com>
Subject: Re: [PATCH] Transaction traceability - txid_status(bigint)
Date: 2017-01-23 06:32:13
Message-ID: CAMsr+YHODEdUUA5vw1_rjPS_ASSvEMeJN_34rqd3pzHf5OFdJg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On 28 December 2016 at 18:00, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:
> On 23 December 2016 at 18:00, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:
>
>> I'll have to follow up with a patch soon, as it's Toddler o'Clock.
>
> Here we go.
>
> This patch advances oldestXid, under XidGenLock, _before_ truncating clog.
>
> txid_status() holds XidGenLock from when it tests oldestXid until it's
> done looking up clog, thus eliminating the race.
>
> CLOG_TRUNCATE records now contain the oldestXid, so they can advance
> oldestXid on a standby, or when we've truncated clog since the most
> recent checkpoint on the master during recovery. It's advanced under
> XidGenLock during redo to protect against this race on standby.
>
> As outlined in my prior mail I think this is the right approach. I
> don't like taking XidGenLock twice, but we don't advance datfrozenxid
> much so it's not a big concern. While a separate ClogTruncationLock
> could be added like in my earlier patch, oldestXid is currently under
> XidGenLock and I'd rather not change that.
>
> The biggest change here is that oldestXid is advanced separately to
> the vac limits in the rest of ShmemVariableCache. As far as I can tell
> we don't prevent two manual VACUUMs on different DBs from trying to
> concurrently run vac_truncate_clog, so this has to be safe against two
> invocations racing each other. Rather than try to lock out such
> concurrency, the patch ensures that oldestXid can never go backwards.
> It doesn't really matter if the vac limits go backwards, it's no worse
> than what can already happen in the current code.
>
> We cannot advance the vacuum limits before we truncate the clog away,
> in case someone tries to access a very new xid (if we're near
> wraparound)
>
> I'm pretty sure that commit timestamps suffer from the same flaw as
> Robert identified upthread with clog. This patch fixes the clog race,
> but not the similar one in commit timestamps. Unlike the clog race
> with txid_status(), the commit timestamps one is already potentially
> user-visible since we allow arbitrary xids to be looked up for commit
> timestamps. I'll address that separately.

Rebased patch attached. I've split the clog changes out from
txid_status() its self.

There is relevant discussion on the commit timestamp truncation fix
thread where the similar fix for commit_ts got committed.

https://www.postgresql.org/message-id/flat/979ff13d-0b8e-4937-01e8-2925c0adc306%402ndquadrant(dot)com#979ff13d-0b8e-4937-01e8-2925c0adc306(at)2ndquadrant(dot)com

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachment Content-Type Size
0001-Fix-race-between-clog-truncation-and-lookup.patch text/x-patch 12.5 KB
0002-Introduce-txid_status-bigint-to-get-status-of-an-xac.patch text/x-patch 19.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Davis 2017-01-23 06:32:48 Re: Review: GIN non-intrusive vacuum of posting tree
Previous Message Amit Kapila 2017-01-23 06:07:34 Re: Parallel Index Scans