public inbox for [email protected]
help / color / mirror / Atom feedFrom: Dilip Kumar <[email protected]>
To: Amit Kapila <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: [email protected] <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Perform streaming logical transactions by background workers and parallel apply
Date: Fri, 6 Jan 2023 12:59:22 +0530
Message-ID: <CAFiTN-uhnJkrUsQByBHMK268T-GEx_D8DQ6b2T+aW6RiU75pbQ@mail.gmail.com> (raw)
In-Reply-To: <CAA4eK1LGmZsevrqJra0V4O8oBU_eKyzm2VMpSAYQaDgC6n4fkA@mail.gmail.com>
References: <CAA4eK1KdVRffYEYiv9=4tVWhP=Jkfh1h6FM0A1R0NEe8cLT8+g@mail.gmail.com>
<CAA4eK1JU4J3-Z=fcMcyQw-iG-DNr=u6w_g-iM_4xwg1pKupkFQ@mail.gmail.com>
<OS0PR01MB57163693482C5470C41914AF94EA9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<CAHut+PuYJz-26QWMSRN_HwiJNfYG4ZxEV3RGE-VyZtXXeFm+cw@mail.gmail.com>
<OS0PR01MB5716C1C1D8244BA86311193894EB9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<CAA4eK1LrADDNXKRVYox4Q+zSFQvtiQu-7piyd_PCmVjg1gsN2w@mail.gmail.com>
<OS0PR01MB57169EF567017861EACBD10D94E99@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<OS0PR01MB571623EFD8903C93424EFADC94E99@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<OS0PR01MB57166C4E79559C9A5570838C94EC9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<CAA4eK1+NV9BDPjh5XB9PnZYNdnQht8+2JGRcerJpSUXtYPtQsg@mail.gmail.com>
<OS3PR01MB6275B61076717E4CE9E079D19EED9@OS3PR01MB6275.jpnprd01.prod.outlook.com>
<CAA4eK1LXch0EDB9RCDiqqmM3S9_cam=svMMK08i8Fmspms=6Xg@mail.gmail.com>
<OS3PR01MB62756CC6FFBDD431F2878ACE9EF29@OS3PR01MB6275.jpnprd01.prod.outlook.com>
<CAA4eK1Konv5gm2bZhej_CZqotr=qhUHnhhiJixKLRmCMsg+Egw@mail.gmail.com>
<OS3PR01MB627586DA752A9E07F02FB7609EF09@OS3PR01MB6275.jpnprd01.prod.outlook.com>
<CAA4eK1+63gaq1ruaUTvhM6-GRWi0waZT7X+TJL2tx+dWzisveQ@mail.gmail.com>
<OS3PR01MB6275D4B22266412FEEBDB3AE9EF49@OS3PR01MB6275.jpnprd01.prod.outlook.com>
<CAD21AoC=PCkeURevhZdNjcKHYws=paPcotgG1fAnq9g8GdwGXw@mail.gmail.com>
<OS0PR01MB5716087C537FA8311F208B6094F59@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<CAFiTN-vy5aD2qEe2g8ssh+n=iyusFm84FxPsUyWOMeQdvbN3ow@mail.gmail.com>
<CAA4eK1LdtevYSW_Lk5MJJeCMWqQtUgKCtPkX4FNKVS-ptEBGxw@mail.gmail.com>
<CAFiTN-u_hFNEB6iQTtY9sLV_0fYX87OGH5BWbjTALYgfoHnxhw@mail.gmail.com>
<OS0PR01MB5716C2CC59810717EC9601EE94FA9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<CAFiTN-sTYk=h75Jn1a7ee+5hOcdQFjKGDvF_0NWQQXmoBv4A+A@mail.gmail.com>
<OS0PR01MB57162495C97CAEA88AC8E9DB94FA9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<CAFiTN-srQ2qZsx2uK6egT_jz9weBGm-Ff+gE8ObMu3sWYNKpFQ@mail.gmail.com>
<OS0PR01MB5716BA08EABE25F07B61EB6D94FB9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<CAFiTN-sjpOm50q8SfoywuKQAqJAaE9FtjvwHBWHvi+pn39OWQQ@mail.gmail.com>
<CAA4eK1LGmZsevrqJra0V4O8oBU_eKyzm2VMpSAYQaDgC6n4fkA@mail.gmail.com>
On Fri, Jan 6, 2023 at 12:05 PM Amit Kapila <[email protected]> wrote:
>
>
> Yeah, I also don't think sending extra eight bytes with stream_start
> message is worth it. But it is fine to mention the same in the
> comments.
Right.
> > 2.
> >
> > + * XXX Additionally, we also stop the worker if the leader apply worker
> > + * serialize part of the transaction data due to a send timeout. This is
> > + * because the message could be partially written to the queue and there
> > + * is no way to clean the queue other than resending the message until it
> > + * succeeds. Instead of trying to send the data which anyway would have
> > + * been serialized and then letting the parallel apply worker deal with
> > + * the spurious message, we stop the worker.
> > + */
> > + if (winfo->serialize_changes ||
> > + list_length(ParallelApplyWorkerPool) >
> > + (max_parallel_apply_workers_per_subscription / 2))
> >
> > IMHO this reason (XXX Additionally, we also stop the worker if the
> > leader apply worker serialize part of the transaction data due to a
> > send timeout) for stopping the worker looks a bit hackish to me. It
> > may be a rare case so I am not talking about the performance but the
> > reasoning behind stopping is not good. Ideally we should be able to
> > clean up the message queue and reuse the worker.
> >
>
> TBH, I don't know what is the better way to deal with this with the
> current infrastructure. I thought we can do this as a separate
> enhancement in the future.
Okay.
> > 3.
> > + else if (shmq_res == SHM_MQ_WOULD_BLOCK)
> > + {
> > + /* Replay the changes from the file, if any. */
> > + if (pa_has_spooled_message_pending())
> > + {
> > + pa_spooled_messages();
> > + }
> >
> > I think we do not need this pa_has_spooled_message_pending() function.
> > Because this function is just calling pa_get_fileset_state() which is
> > acquiring mutex and getting filestate then if the filestate is not
> > FS_EMPTY then we call pa_spooled_messages() that will again call
> > pa_get_fileset_state() which will again acquire mutex. I think when
> > the state is FS_SERIALIZE_IN_PROGRESS it will frequently call
> > pa_get_fileset_state() consecutively 2 times, and I think we can
> > easily achieve the same behavior with just one call.
> >
>
> This is just to keep the code easy to follow. As this would be a rare
> case, so thought of giving preference to code clarity.
I think the code will be simpler with just one function no? I mean
instead of calling pa_has_spooled_message_pending() in if condition
what if we directly call pa_spooled_messages();, this is anyway
fetching the file_state and if the filestate is EMPTY then it can
return false, and if it returns false we can execute the code which is
there in else condition. We might need to change the name of the
function though.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
view thread (6+ 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: <CAFiTN-uhnJkrUsQByBHMK268T-GEx_D8DQ6b2T+aW6RiU75pbQ@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