public inbox for [email protected]  

help / color / mirror / Atom feed

From: [email protected] <[email protected]>
To: [email protected] <[email protected]>
To: Amit Kapila <[email protected]>
To: [email protected] <[email protected]>
Cc: Dilip Kumar <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: RE: Perform streaming logical transactions by background workers and parallel apply
Date: Fri, 5 Aug 2022 10:00:31 +0000
Message-ID: <OSZPR01MB6310351611A90936165952CDFD9E9@OSZPR01MB6310.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <OS3PR01MB6275D64BE7726B0221B15F389E9F9@OS3PR01MB6275.jpnprd01.prod.outlook.com>
References: <OS3PR01MB62758DBE8FA12BA72A43AC819EB89@OS3PR01MB6275.jpnprd01.prod.outlook.com>
	<CAHut+Pu5ah+NhYMfb7KTbkYTxnuGY2j-7vT0G1MZZsS4SKiHew@mail.gmail.com>
	<OS3PR01MB62755C6C9A75EB09F7218B589E839@OS3PR01MB6275.jpnprd01.prod.outlook.com>
	<OS3PR01MB627594D75E870BBB45E2E80A9E839@OS3PR01MB6275.jpnprd01.prod.outlook.com>
	<CAHut+PvN7fwtUE=bidzrsOUXSt+JpnkJztZ-Jn5t86moofaZ6g@mail.gmail.com>
	<OS3PR01MB62758A6AAED27B3A848CEB7A9E8F9@OS3PR01MB6275.jpnprd01.prod.outlook.com>
	<OS3PR01MB62752B29CEC2592E18236A9A9E909@OS3PR01MB6275.jpnprd01.prod.outlook.com>
	<CAFiTN-uq5Mp+o+_u0iDc8OQV-q7CWf5W=dPgw2ORwxsi6yjVfw@mail.gmail.com>
	<CAA4eK1LB1s=B+krpn9mGbZq46pVei0FD7WjhJORq4bD2GU4fng@mail.gmail.com>
	<CAFiTN-sK3bVppVAioPfxGjZqY9sHKVM8NooK9Wuex2OCTK25wg@mail.gmail.com>
	<OS0PR01MB571651D89C744F84E20AAFE394979@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<CAA4eK1K0zkBJQtF1ZNhSvcwh_+pdCaUTf8+bBZR95yP92c0CuQ@mail.gmail.com>
	<OS3PR01MB6275D64BE7726B0221B15F389E9F9@OS3PR01MB6275.jpnprd01.prod.outlook.com>

On Thu, Aug 4, 2022 2:36 PM Wang, Wei/王 威 <[email protected]> wrote:
> 
> I also did some other improvements based on the suggestions posted in this
> thread. Attach the new patches.
> 

Thanks for updating the patch. Here are some comments on v20-0001 patch.

1.
+typedef struct ApplyBgworkerShared
+{
+	slock_t	mutex;
+
+	/* Status of apply background worker. */
+	ApplyBgworkerStatus	status;
+
+	/* proto version of publisher. */
+	uint32	proto_version;
+
+	TransactionId	stream_xid;
+
+	/* id of apply background worker */
+	uint32	worker_id;
+} ApplyBgworkerShared;

Would it be better to modify the comment of "proto_version" to "Logical protocol
version"?

2. commnent of handle_streamed_transaction()

+ * Exception: When the main apply worker is applying streaming transactions in
+ * parallel mode (e.g. when addressing LOGICAL_REP_MSG_RELATION or
+ * LOGICAL_REP_MSG_TYPE changes), then return false.

This comment doesn't look very clear, could we change it to:

Exception: In SUBSTREAM_PARALLEL mode, if the message type is
LOGICAL_REP_MSG_RELATION or LOGICAL_REP_MSG_TYPE, return false even if this is
the main apply worker.

3.
+/*
+ * There are three fields in message: start_lsn, end_lsn and send_time. Because
+ * we have updated these statistics in apply worker, we could ignore these
+ * fields in apply background worker. (see function LogicalRepApplyLoop)
+ */
+#define SIZE_STATS_MESSAGE (3 * sizeof(uint64))

updated these statistics in apply worker
->
updated these statistics in main apply worker

4.
+static void
+LogicalApplyBgwLoop(shm_mq_handle *mqh, volatile ApplyBgworkerShared *shared)
+{
+	shm_mq_result shmq_res;
+	PGPROC	   *registrant;
+	ErrorContextCallback errcallback;

I think we can define "shmq_res" in the for loop.

5.
+		/*
+		 * We use first byte of message for additional communication between
+		 * main Logical replication worker and apply background workers, so if
+		 * it differs from 'w', then process it first.
+		 */

between main Logical replication worker and apply background workers
->
between main apply worker and apply background workers

Regards,
Shi yu

view thread (625+ 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], [email protected]
  Subject: RE: Perform streaming logical transactions by background workers and parallel apply
  In-Reply-To: <OSZPR01MB6310351611A90936165952CDFD9E9@OSZPR01MB6310.jpnprd01.prod.outlook.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