public inbox for [email protected]  

help / color / mirror / Atom feed

From: shveta malik <[email protected]>
To: Zhijie Hou (Fujitsu) <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: vignesh C <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: Nisha Moond <[email protected]>
Cc: Hayato Kuroda (Fujitsu) <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: shveta malik <[email protected]>
Subject: Re: Conflict detection for update_deleted in logical replication
Date: Tue, 27 May 2025 15:59:45 +0530
Message-ID: <CAJpy0uBSsRuVOeuo-i8R_aO0CMiORHTnEBZ9z-TDq941WqhyLA@mail.gmail.com> (raw)
In-Reply-To: <OS0PR01MB57161C2C4345AAFB3EA2E5309465A@OS0PR01MB5716.jpnprd01.prod.outlook.com>
References: <CAA4eK1+r9V6DpH9gYRa2xOx167FapbuKdc4gESr8Etxpx2zrqw@mail.gmail.com>
	<CAA4eK1+VNaGi-GU6awgFKmTgidLTHo2HDuzV1+aT8sjn8QtPxg@mail.gmail.com>
	<OS0PR01MB5716B7983231B9DF3713EF2A9499A@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<CAA4eK1J+dmsB=od4pkGBjZkeurpWGuRXOeXBbNRbh3hVUr7DxQ@mail.gmail.com>
	<OS0PR01MB57161C2C4345AAFB3EA2E5309465A@OS0PR01MB5716.jpnprd01.prod.outlook.com>

On Mon, May 26, 2025 at 12:46 PM Zhijie Hou (Fujitsu)
<[email protected]> wrote:
>
> Attaching the V32 patch set which addressed comments in [1]~[5].

Thanks for the patch, I am still reviewing the patches, please find
few trivial comments for patch001:

1)

+ FullTransactionId last_phase_at; /* publisher transaction ID that must
+ * be awaited to complete before
+ * entering the final phase
+ * (RCI_WAIT_FOR_LOCAL_FLUSH) */

'last_phase_at' seems like we are talking about the phase in the past.
(similar to 'last' in last_recv_time).
Perhaps we should name it as 'final_phase_at'


2)
RetainConflictInfoData data = {0};

We can change this name as well to rci_data.

3)
+ /*
+ * Compute FullTransactionId for the oldest running transaction ID. This
+ * handles the case where transaction ID wraparound has occurred.
+ */
+ full_xid = FullTransactionIdFromAllowableAt(next_full_xid,
oldest_running_xid);

Shall we name it to full_oldest_xid for better clarity?


4)
+ /*
+ * Update and check the remote flush position if we are applying changes
+ * in a loop. This is done at most once per WalWriterDelay to avoid
+ * performing costy operations in get_flush_position() too frequently
+ * during change application.
+ */
+ if (last_flushpos < rci_data->remote_lsn && rci_data->last_recv_time &&
+ TimestampDifferenceExceeds(rci_data->flushpos_update_time,
+    rci_data->last_recv_time, WalWriterDelay))

a) costy --> costly

thanks
Shveta

view thread (375+ 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: Conflict detection for update_deleted in logical replication
  In-Reply-To: <CAJpy0uBSsRuVOeuo-i8R_aO0CMiORHTnEBZ9z-TDq941WqhyLA@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