From: | Ashwin Agrawal <aagrawal(at)pivotal(dot)io> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
Cc: | Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>, Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com> |
Subject: | Re: Remove HeapTuple and Buffer dependency for predicate locking functions |
Date: | 2019-07-31 20:59:24 |
Message-ID: | CALfoeivUiin5Kx89FzhOBRU+TTM4LUd=7PbErAj4=TgfpfCHoQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Jul 31, 2019 at 12:37 PM Ashwin Agrawal <aagrawal(at)pivotal(dot)io> wrote:
>
> On Wed, Jul 31, 2019 at 10:55 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
>
>> Hi,
>>
>> On 2019-07-31 10:42:50 -0700, Ashwin Agrawal wrote:
>> > On Tue, Jul 30, 2019 at 2:58 PM Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
>> wrote:
>> >
>> > > On Tue, Jun 25, 2019 at 6:02 AM Andres Freund <andres(at)anarazel(dot)de>
>> wrote:
>> > > > > - CheckForSerializableConflictOut() no more takes HeapTuple nor
>> > > > > buffer, instead just takes xid. Push heap specific parts from
>> > > > > CheckForSerializableConflictOut() into its own function
>> > > > > HeapCheckForSerializableConflictOut() which calls
>> > > > > CheckForSerializableConflictOut(). The alternative option could
>> be
>> > > > > CheckForSerializableConflictOut() take callback function and
>> > > > > callback arguments, which gets called if required after
>> performing
>> > > > > prechecks. Though currently I fell AM having its own wrapper to
>> > > > > perform AM specific task and then calling
>> > > > > CheckForSerializableConflictOut() is fine.
>> > > >
>> > > > I think it's right to move the xid handling out of
>> > > > CheckForSerializableConflictOut(). But I think we also ought to
>> move the
>> > > > subtransaction handling out of the function - e.g. zheap doesn't
>> > > > want/need that.
>> > >
>> > > Thoughts on this Ashwin?
>> > >
>> >
>> > I think the only part its doing for sub-transaction is
>> > SubTransGetTopmostTransaction(xid). If xid passed to this function is
>> > already top most transaction which is case for zheap and zedstore, then
>> > there is no downside to keeping that code here in common place.
>>
>> Well, it's far from a cheap function. It'll do unnecessary on-disk
>> lookups in many cases. I'd call that quite a downside.
>>
>
> Okay, agree, its costly function and better to avoid the call if possible.
>
> Instead of moving the handling out of the function, how do feel about
> adding boolean isTopTransactionId argument to function
> CheckForSerializableConflictOut(). The AMs, which implicitly know, only
> pass top transaction Id to this function, can pass true and avoid the
> function call to SubTransGetTopmostTransaction(xid). With this
> subtransaction code remains in generic place and AMs intending to use it
> continue to leverage the common code, plus explicitly clarifies the
> behavior as well.
>
Added argument to function to make the subtransaction handling optional in
attached v2 of patch.
Attachment | Content-Type | Size |
---|---|---|
v2-0001-Remove-HeapTuple-dependency-for-predicate-locking.patch | text/x-patch | 25.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2019-07-31 21:06:30 | Re: Remove HeapTuple and Buffer dependency for predicate locking functions |
Previous Message | Tom Lane | 2019-07-31 20:55:31 | Re: Unused header file inclusion |