On 11/13/24 03:56, Amit Kapila wrote:
the key point Andres
is raising is that we won't be able to convert the operation in
ReserveXLogInsertLocation() to use atomics after such a solution. Now,
even, if the change is only in the *commit* code path, we may or may
not be able to maintain two code
On Tue, Nov 12, 2024 at 5:30 PM Tomas Vondra wrote:
>
> On 11/12/24 04:05, Amit Kapila wrote:
> >
> > Right, and the patch sent by Hou-San [1] (based on the original patch
> > by Jan) is somewhat on these lines. The idea you have shared or
> > implemented by the patch is a logical clock stored in
Dear Andres,
> I don't think the suggested workload is useful here.
> pg_logical_emit_message(transactional = false)
> does insert the WAL without the commit, i.e., xlcommitrecis always NULL.
> This means backends won't go through added codes in
> ReserveXLogInsertLocation().
Just in case I want
Dear Andres,
Thanks for giving comments for my test!
> This is not a useful measurement for overhead introduced in
> ReserveXLogInsertLocation(). What you're measuring here is the number of
> commits/second, not the WAL insertion rate. The number of commits/second is
> largely determined by your
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 summar
Hello,
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
Hi,
On 2024-11-12 10:12:40 -0500, Jan Wieck wrote:
> On 11/12/24 08:55, Andres Freund wrote:
> > Hi,
> >
> > On 2024-11-12 08:51:49 -0500, Jan Wieck wrote:
> > > On 11/11/24 23:21, Amit Kapila wrote:
> > > > As the inversion issue can mainly hamper logical replication-based
> > > > solutions we ca
Hello,
On 11/12/24 08:55, Andres Freund wrote:
Hi,
On 2024-11-12 08:51:49 -0500, Jan Wieck wrote:
On 11/11/24 23:21, Amit Kapila wrote:
> As the inversion issue can mainly hamper logical replication-based
> solutions we can do any of this additional work under spinlock only
> when the current
On 11/11/24 23:22, Amit Kapila wrote:
On Mon, Nov 11, 2024 at 9:05 PM Tomas Vondra wrote:
Alternatively, we could simply stop relying on the timestamps recorded
in the commit, and instead derive "monotonic" commit timestamps after
the fact. For example, we could track timestamps for some subse
Hi,
On 2024-11-11 09:49:19 +, Hayato Kuroda (Fujitsu) wrote:
> I've done performance tests compared with master vs. v2 patch.
> It showed that for small transactions cases, the performance difference was
> 0-2%,
> which was almost the same of the run-by-run variation.
>
> We may completely c
Hi,
On 2024-11-12 08:51:49 -0500, Jan Wieck wrote:
> On 11/11/24 23:21, Amit Kapila wrote:
> > As the inversion issue can mainly hamper logical replication-based
> > solutions we can do any of this additional work under spinlock only
> > when the current record is a commit record (which the curren
On 11/11/24 23:21, Amit Kapila wrote:
As the inversion issue can mainly hamper logical replication-based
solutions we can do any of this additional work under spinlock only
when the current record is a commit record (which the currently
proposed patch is already doing) and "wal_level = logical" a
On 11/12/24 04:05, Amit Kapila wrote:
> On Mon, Nov 11, 2024 at 9:05 PM Tomas Vondra wrote:
>>
>> On 11/11/24 09:19, Amit Kapila wrote:
>>>
>>> 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_timest
On Mon, Nov 11, 2024 at 9:05 PM Tomas Vondra wrote:
>
> Alternatively, we could simply stop relying on the timestamps recorded
> in the commit, and instead derive "monotonic" commit timestamps after
> the fact. For example, we could track timestamps for some subset of
> commits, and then approxima
On Tue, Nov 12, 2024 at 8:35 AM Amit Kapila wrote:
>
> On Mon, Nov 11, 2024 at 9:05 PM Tomas Vondra wrote:
> >
> > On 11/11/24 09:19, Amit Kapila wrote:
> > >
> > > I can't think of a solution other than the current proposal where we
> > > do both the operations (reserve WAL space for commit and
On Mon, Nov 11, 2024 at 9:05 PM Tomas Vondra wrote:
>
> On 11/11/24 09:19, Amit Kapila wrote:
> >
> > 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<
On 11/11/24 09:19, Amit Kapila wrote:
> On Fri, Nov 8, 2024 at 8:23 PM Andres Freund wrote:
>>
>> On 2024-11-08 09:08:55 +, Zhijie Hou (Fujitsu) wrote:
I think it's *completely* unacceptable to call a hook inside
ReserveXLogInsertLocation, with the spinlock held, no less. That's
Dear hackers,
> 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. Wou
On Fri, Nov 8, 2024 at 8:23 PM Andres Freund wrote:
>
> On 2024-11-08 09:08:55 +, 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 sectio
Hi,
On 2024-11-08 09:08:55 +, Zhijie Hou (Fujitsu) wrote:
> On Friday, November 8, 2024 2:20 AM Andres Freund wrote:
> > On 2024-11-05 08:58:36 -0500, Jan Wieck wrote:
> > > The attached solution is minimally invasive because it doesn't move
> > > the timestamp generation (clock_gettime() cal
On Thu, Nov 7, 2024 at 9:39 PM Jan Wieck wrote:
>
> On 11/6/24 21:30, Zhijie Hou (Fujitsu) wrote:
> >
> > Thanks for the patch! I am reading the patch and noticed few minor things.
> >
> > 1.
> > + /*
> > + * This is a local transaction. Make sure that the xact_time
> > + * higher th
On Friday, November 8, 2024 2:20 AM Andres Freund wrote:
Hi,
> On 2024-11-05 08:58:36 -0500, Jan Wieck wrote:
> > The attached solution is minimally invasive because it doesn't move
> > the timestamp generation (clock_gettime() call) into the critical
> > section of
> > ReserveXLogInsertLocation
Hi,
On 2024-11-07 15:05:31 -0500, Jan Wieck wrote:
> On 11/7/24 13:19, Andres Freund 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.
>
> Aside from yo
On 11/7/24 13:19, Andres Freund 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.
Aside from your technical concerns, can we at least agree that the
commit-ts vs
Hi,
On 2024-11-05 08:58:36 -0500, Jan Wieck wrote:
> The attached solution is minimally invasive because it doesn't move the
> timestamp generation (clock_gettime() call) into the critical section of
> ReserveXLogInsertLocation() that is protected by a spinlock. Instead it
> keeps track of the las
On 11/6/24 21:30, Zhijie Hou (Fujitsu) wrote:
Thanks for the patch! I am reading the patch and noticed few minor things.
1.
+ /*
+* This is a local transaction. Make sure that the xact_time
+* higher than any timestamp we have seen thus far.
+*
+* TODO: Thi
On Tuesday, November 5, 2024 9:59 PM Jan Wieck wrote:
>
> Hi Hackers,
>
> On 9/5/24 01:39, Amit Kapila wrote:
> >
> > We can't forget CDR completely as this could only be a potential
> > problem in that context. Right now, we don't have any built-in
> > resolution strategies, so this can't impac
On 11/6/24 06:23, Amit Kapila wrote:
I think we avoid calling hook/callback functions after holding a lock
(spinlock or LWLock) as the user may do an expensive operation or
acquire some other locks in those functions which could lead to
deadlocks or impact the concurrency. So, it would be better
On Tue, Nov 5, 2024 at 7:28 PM Jan Wieck wrote:
>
> >
> > We can't forget CDR completely as this could only be a potential
> > problem in that context. Right now, we don't have any built-in
> > resolution strategies, so this can't impact but if this is a problem
> > then we need to have a solution
the system clock for conflict
> resolution but that is not the specific point of this thread. As
> mentioned in the subject of this thread, the problem is "Commit
> Timestamp and LSN Inversion issue". The LSN value and timestamp for a
> commit are not generated atomically, so
On Wed, Sep 4, 2024 at 12:23 PM shveta malik wrote:
>
> Hello hackers,
> (Cc people involved in the earlier discussion)
>
> I would like to discuss the $Subject.
>
> While discussing Logical Replication's Conflict Detection and
> Resolution (CDR) design in [1] , it came to our notice that the
> c
the system clock for conflict
> > resolution but that is not the specific point of this thread. As
> > mentioned in the subject of this thread, the problem is "Commit
> > Timestamp and LSN Inversion issue". The LSN value and timestamp for a
> > commit are not generat
As
> mentioned in the subject of this thread, the problem is "Commit
> Timestamp and LSN Inversion issue". The LSN value and timestamp for a
> commit are not generated atomically, so two different transactions can
> have them in different order.
Hm Then I'm having difficul
le that we can't rely on the system clock for conflict
resolution but that is not the specific point of this thread. As
mentioned in the subject of this thread, the problem is "Commit
Timestamp and LSN Inversion issue". The LSN value and timestamp for a
commit are not generated atomi
Hi Shveta,
> While discussing Logical Replication's Conflict Detection and
> Resolution (CDR) design in [1] , it came to our notice that the
> commit LSN and timestamp may not correlate perfectly i.e. commits may
> happen with LSN1 < LSN2 but with Ts1 > Ts2. This issue may arise
> because, during
Hello hackers,
(Cc people involved in the earlier discussion)
I would like to discuss the $Subject.
While discussing Logical Replication's Conflict Detection and
Resolution (CDR) design in [1] , it came to our notice that the
commit LSN and timestamp may not correlate perfectly i.e. commits may
36 matches
Mail list logo
