public inbox for [email protected]  
help / color / mirror / Atom feed
From: Amit Kapila <[email protected]>
To: Andres Freund <[email protected]>
Cc: Zhijie Hou (Fujitsu) <[email protected]>
Cc: Aleksander Alekseev <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: shveta malik <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Jan Wieck <[email protected]>
Subject: Re: Commit Timestamp and LSN Inversion issue
Date: Mon, 11 Nov 2024 13:49:58 +0530
Message-ID: <CAA4eK1+ZdL=sC4sdUOQz+X9=7Z8Uxhkd0VqwcB9rgzXjvu=6yA@mail.gmail.com> (raw)
In-Reply-To: <xhcapr7lur35itqrbk4qdqn7rzfaqr3g6dvyxzfx6jeyknhvbv@h3wkgixuuuon>
References: <CAJpy0uBxEJnabEp3JS=n9X19Vx2ZK3k5AR7N0h-cSMtOwYV3fA@mail.gmail.com>
	<CAJ7c6TNZY3idBDoGOaoS_fF6vJuBMjnDhZC4i5V0J58s2SJ3AA@mail.gmail.com>
	<CAA4eK1LOmtMpmJAxsmN0yXJb-Q9vi6KThDYXdpVGL_WZW5ZgvQ@mail.gmail.com>
	<CAJ7c6TOcQPcydC7HW=xPEDN6_Ec7iSTP9j=1VTAeG=haguOQHw@mail.gmail.com>
	<CAA4eK1KhAxe6=VTxa6xEbc22_VVBLsVuvRH8Wxws8i50gH=eqw@mail.gmail.com>
	<[email protected]>
	<gkbrjxwyjbqckjbrkep3uy2mvrcq2mrfzprfmegbeiqxidtmbu@3cgs5iocjykk>
	<OS0PR01MB57162A227EC357482FEB4470945D2@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<xhcapr7lur35itqrbk4qdqn7rzfaqr3g6dvyxzfx6jeyknhvbv@h3wkgixuuuon>

On Fri, Nov 8, 2024 at 8:23 PM Andres Freund <[email protected]> wrote:
>
> On 2024-11-08 09:08:55 +0000, Zhijie Hou (Fujitsu) wrote:
> > >
> > > I think it's *completely* unacceptable to call a hook inside
> > > ReserveXLogInsertLocation, with the spinlock held, no less. That's the most
> > > contended section of code in postgres.
> >
> > I understand your concern and appreciate the feedback. I've made some
> > adjustments to the patch by directly placing the code to adjust the commit
> > timestamp within the spinlock, aiming to keep it as efficient as possible. The
> > changes have resulted in just a few extra lines. Would this approach be
> > acceptable to you ?
>
> No, not at all. I think it's basically not acceptable to add any nontrivial
> instruction to the locked portion of ReserveXLogInsertLocation().
>
> Before long we're going to have to replace that spinlocked instruction with
> atomics, which will make it impossible to just block while holding the lock
> anyway.
>
> IMO nothing that touches core xlog insert infrastructure is acceptable for
> this.
>

I can't think of a solution other than the current proposal where we
do both the operations (reserve WAL space for commit and adjust
commit_timestamp, if required) together to resolve LSN<->Timestamp
invertion issue. Please let us know if you have any other ideas for
solving this. Even, if we want to change ReserveXLogInsertLocation to
make the locked portion an atomic operation, we can still do it for
records other than commit. Now, if we don't have any other solution
for LSN<->Timestamp inversion issue, changing the entire locked
portion to atomic will close the door to solving this problem forever
unless we have some other way to solve it which can make it difficult
to rely on commit_timestamps for certain scenarios.

-- 
With Regards,
Amit Kapila.






view thread (33+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Commit Timestamp and LSN Inversion issue
  In-Reply-To: <CAA4eK1+ZdL=sC4sdUOQz+X9=7Z8Uxhkd0VqwcB9rgzXjvu=6yA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox