Date: Tue, 21 Jun 2016 12:46:26 -0700
From: Andres Freund <andres@anarazel.de>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>,
	Thomas Munro <thomas.munro@enterprisedb.com>,
	Alvaro Herrera <alvherre@2ndquadrant.com>,
	Stephen Frost <sfrost@snowman.net>,
	Michael Paquier <michael.paquier@gmail.com>,
	Masahiko Sawada <sawada.mshk@gmail.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: Reviewing freeze map code
Message-ID: <20160621194626.o7jfjjp4wfawgx46@alap3.anarazel.de>
References: 
 <CAEepm=2at9oWWqt-eMxz3rw3stSa+m6LRzeM+J+iXKBkUJx9FQ@mail.gmail.com>
 <CA+TgmoZ-1TXCsxWcp3i-KMo5DNB-6p-odqw7c-N_q3pzZY1TJg@mail.gmail.com>
 <20160620193302.s4dbul6japgar5eg@alap3.anarazel.de>
 <CAA4eK1Kop7jfQFUfeuSdDgZAKEm-z51Ya5zHuJXEXG7PGG6Taw@mail.gmail.com>
 <20160621035138.rwbbqcnaoqgnd5iv@alap3.anarazel.de>
 <CA+TgmoZdsd5GQU2a0wwX1dyY+UjuUKbG6iUX6kcoEXsmUJTuEA@mail.gmail.com>
 <20160621165415.2khpeffceio4gxs3@alap3.anarazel.de>
 <CA+TgmoZkDPB+KAo2qVez_P_jnZ+UT00aWe4wFq1GQh52ZQX7ow@mail.gmail.com>
 <20160621174953.wmxqf5sxmo7lv2if@alap3.anarazel.de>
 <CA+TgmoYxOBMnvsqt1wvOoBd3wQvaVMMSuhbx-03EH0Wuqr+yPw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CA+TgmoYxOBMnvsqt1wvOoBd3wQvaVMMSuhbx-03EH0Wuqr+yPw@mail.gmail.com>
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

On 2016-06-21 15:38:25 -0400, Robert Haas wrote:
> On Tue, Jun 21, 2016 at 1:49 PM, Andres Freund <andres@anarazel.de> wrote:
> >> I'm also a bit dubious that LockAcquire is safe to call in general
> >> with interrupts held.
> >
> > Looks like we could just acquire the tuple-lock *before* doing the
> > toast_insert_or_update/RelationGetBufferForTuple, but after releasing
> > the buffer lock. That'd allow us to do avoid doing the nested locking,
> > should make the recovery just a goto l2;, ...
> 
> Why isn't that racey?  Somebody else can grab the tuple lock after we
> release the buffer content lock and before we acquire the tuple lock.

Sure, but by the time the tuple lock is released, they'd have updated
xmax. So once we acquired that we can just do
		if (xmax_infomask_changed(oldtup.t_data->t_infomask,
								  infomask) ||
			!TransactionIdEquals(HeapTupleHeaderGetRawXmax(oldtup.t_data),
								 xwait))
			goto l2;
which is fine, because we've not yet done the toasting.


I'm not sure wether this approach is better than deleting potentially
toasted data though. It's probably faster, but will likely touch more
places in the code, and eat up a infomask bit (infomask & HEAP_MOVED
== HEAP_MOVED in my prototype).


Andres


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