From: | Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
Cc: | Ashwin Agrawal <aagrawal(at)pivotal(dot)io>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Remove HeapTuple and Buffer dependency for predicate locking functions |
Date: | 2019-08-01 06:01:32 |
Message-ID: | CAGz5QCKwYx+mbGH28w77iqUWXdiirDQkaz8M9vos5Or1uFcoQw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Aug 1, 2019 at 2:36 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
> > > >
> > > > 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.
>
> Looking at the code as of master, we currently have:
>
> - PredicateLockTuple() calls SubTransGetTopmostTransaction() to figure
> out a whether the tuple has been locked by the current
> transaction. That check afaict just should be
> TransactionIdIsCurrentTransactionId(), without all the other
> stuff that's done today.
>
Yeah. this is the only part where predicate locking uses the subxids.
Since, predicate locking always use the top xid, IMHO, it'll be good
to make this api independent of subxids.
> TransactionIdIsCurrentTransactionId() imo ought to be optimized to
> always check for the top level transactionid first - that's a good bet
> today, but even moreso for the upcoming AMs that won't have separate
> xids for subtransactions. Alternatively we shouldn't make that a
> binary search for each subtrans level, but just have a small
> simplehash hashtable for xids.
A check for top transaction id first and usage of simple sound like
good optimizations. But, I'm not sure whether these changes should be
part of this patch or a separate one.
--
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2019-08-01 06:01:33 | Re: concerns around pg_lsn |
Previous Message | Jeevan Ladhe | 2019-08-01 05:59:54 | Re: concerns around pg_lsn |