public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tomas Vondra <[email protected]>
To: Amit Kapila <[email protected]>
To: [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]>
Cc: Dilip Kumar <[email protected]>
Subject: Re: Perform streaming logical transactions by background workers and parallel apply
Date: Sun, 15 Jan 2023 18:09:00 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAA4eK1KK7Xty7SaCReY_b5moPURowMGOsfvhOb6xQ1EAaJAh6A@mail.gmail.com>
References: <CAA4eK1LGmZsevrqJra0V4O8oBU_eKyzm2VMpSAYQaDgC6n4fkA@mail.gmail.com>
<CAFiTN-uhnJkrUsQByBHMK268T-GEx_D8DQ6b2T+aW6RiU75pbQ@mail.gmail.com>
<OS0PR01MB571621ED532C2D7C3E01625894FB9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<CAFiTN-t5+_zTf86EzgCwObs1ED-P_5Ab1KMw340d3oPY_+Cwpw@mail.gmail.com>
<OS3PR01MB5718AE486227CE3ACB844C5E94F89@OS3PR01MB5718.jpnprd01.prod.outlook.com>
<CAFiTN-sgQ-K9BnMWKLR7Hm36QNjS7ZOBF6hsXJmQz5ChC0Desw@mail.gmail.com>
<CAA4eK1+P9WOFBupaTZCzYtranSH=8TSUYV=kk5xQJ-VNKnvgmQ@mail.gmail.com>
<OS0PR01MB571685A645E509267AC5F30894F99@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<OS0PR01MB57168A5A1C31BC316CB63D3C94F99@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<CAA4eK1KK7Xty7SaCReY_b5moPURowMGOsfvhOb6xQ1EAaJAh6A@mail.gmail.com>
Hi,
I think there's a bug in how get_transaction_apply_action() interacts
with handle_streamed_transaction() to decide whether the transaction is
streamed or not. Originally, the code was simply:
/* not in streaming mode */
if (!in_streamed_transaction)
return false;
But now this decision was moved to get_transaction_apply_action(), which
does this:
if (am_parallel_apply_worker())
{
return TRANS_PARALLEL_APPLY;
}
else if (in_remote_transaction)
{
return TRANS_LEADER_APPLY;
}
and handle_streamed_transaction() then uses the result like this:
/* not in streaming mode */
if (apply_action == TRANS_LEADER_APPLY)
return false;
Notice this is not equal to the original behavior, because the two flags
(in_remote_transaction and in_streamed_transaction) are not inverse.
That is,
in_remote_transaction=false
does not imply we're processing streamed transaction. It's allowed both
flags are false, i.e. a change may be "non-transactional" and not
streamed, though the only example of such thing in the protocol are
logical messages. Which are however ignored in the apply worker, so I'm
not surprised no existing test failed on this.
So I think get_transaction_apply_action() should do this:
if (am_parallel_apply_worker())
{
return TRANS_PARALLEL_APPLY;
}
else if (!in_streamed_transaction)
{
return TRANS_LEADER_APPLY;
}
FWIW I've noticed this after rebasing the sequence decoding patch, which
adds another type of protocol message with the transactional vs.
non-transactional behavior, similar to "logical messages" except that in
this case the worker does not ignore that.
Also, I think get_transaction_apply_action() would deserve better
comments explaining how/why it makes the decisions.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
view thread (105+ 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], [email protected]
Subject: Re: Perform streaming logical transactions by background workers and parallel apply
In-Reply-To: <[email protected]>
* 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