|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)|
|Views:||Raw Message | Whole Thread | Download mbox|
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
> 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.
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
|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|