Feedback-ID: id4a34324:Fastmail
Date: Tue, 12 Nov 2024 12:45:35 -0500
From: Andres Freund <andres@anarazel.de>
To: Jan Wieck <jan@wi3ck.info>
Cc: Amit Kapila <amit.kapila16@gmail.com>, Tomas Vondra <tomas@vondra.me>, 
	"Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>, Aleksander Alekseev <aleksander@timescale.com>, 
	pgsql-hackers <pgsql-hackers@postgresql.org>, shveta malik <shveta.malik@gmail.com>
Subject: Re: Commit Timestamp and LSN Inversion issue
Message-ID: <xniptxjmqddyofohc7d5o376ygjoz4dkdtgpcqddznu6pw4xtl@ww2kyxgrr3na>
References: <xhcapr7lur35itqrbk4qdqn7rzfaqr3g6dvyxzfx6jeyknhvbv@h3wkgixuuuon>
 <CAA4eK1+ZdL=sC4sdUOQz+X9=7Z8Uxhkd0VqwcB9rgzXjvu=6yA@mail.gmail.com>
 <11ee3489-40ba-45d6-ba6a-7c3def725e84@vondra.me>
 <CAA4eK1KQ6H4u5=_5Xq44HhX3RFXW4HW-z0F4T4J1nf2xVCo04w@mail.gmail.com>
 <CAA4eK1L6Vpf9EmL2bpfD3kdvM6_2XQqwJ9GY6uY-hgcot76cqA@mail.gmail.com>
 <00f7cfb2-e3a6-4bd7-b19c-fd75d3838049@wi3ck.info>
 <lyydpy6xly7gefcswkimur733wcbke4bdjwjedvluyo2ebfdr7@xz5k4mbjgivm>
 <296b2c56-c688-41a0-a752-b242268691fd@wi3ck.info>
 <wpitjnzyno7pmhnaudsehw5v6lw644cz52wx4qqd2wvfazlxgq@gsogjmyhn3xw>
 <94fc0a8b-5d18-4a82-8082-b76a0e725788@wi3ck.info>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <94fc0a8b-5d18-4a82-8082-b76a0e725788@wi3ck.info>
Archived-At: <https://www.postgresql.org/message-id/xniptxjmqddyofohc7d5o376ygjoz4dkdtgpcqddznu6pw4xtl%40ww2kyxgrr3na>
Precedence: bulk

Hi,

On 2024-11-12 11:40:39 -0500, Jan Wieck wrote:
> On 11/12/24 10:34, Andres Freund wrote:
> > I have working code - pretty ugly at this state, but mostly needs a fair bit
> > of elbow grease not divine inspiration...  It's not a trivial change, but
> > entirely doable.
> > 
> > The short summary of how it works is that it uses a single 64bit atomic that
> > is internally subdivided into a ringbuffer position in N high bits and an
> > offset from a base LSN in the remaining bits.  The insertion sequence is
> > 
> > ...
> > 
> > This leaves you with a single xadd to contended cacheline as the contention
> > point (scales far better than cmpxchg and far far better than
> > cmpxchg16b). There's a bit of contention for the ringbuffer[].oldpos being set
> > and read, but it's only by two backends, not all of them.
> 
> That sounds rather promising.
> 
> Would it be reasonable to have both implementations available at least at
> compile time, if not at runtime?

No, not reasonably.


> Is it possible that we need to do that anyway for some time or are those
> atomic operations available on all supported CPU architectures?

We have a fallback atomics implementation for the uncommon architectures
without 64bit atomics.


> In any case, thanks for the input. Looks like in the long run we need to
> come up with a different way to solve the inversion problem.

IMO there's absolutely no way the changes proposed in this thread so far
should get merged.

Greetings,

Andres Freund