RE: Perform streaming logical transactions by background workers and parallel apply
Wei Wang (Fujitsu) <wangw.fnst@fujitsu.com>
Attachments
- v14-0001-Perform-streaming-logical-transactions-by-backgr.patch (application/octet-stream) patch v14-0001
- v14-0002-Test-streaming-parallel-option-in-tap-test.patch (application/octet-stream) patch v14-0002
- v14-0003-Add-some-checks-before-using-apply-background-wo.patch (application/octet-stream) patch v14-0003
- v14-0004-Retry-to-apply-streaming-xact-only-in-apply-work.patch (application/octet-stream) patch v14-0004
On Thu, Jun 23, 2022 at 16:44 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Thu, Jun 23, 2022 at 12:51 PM wangw.fnst@fujitsu.com
> <wangw.fnst@fujitsu.com> wrote:
> >
> > On Mon, Jun 20, 2022 at 11:00 AM Amit Kapila <amit.kapila16@gmail.com>
> wrote:
> > > I have improved the comments in this and other related sections of the
> > > patch. See attached.
> > Thanks for your comments and patch!
> > Improved the comments as you suggested.
> >
> > > > > 3.
> > > > > +
> > > > > + <para>
> > > > > + Setting streaming mode to <literal>apply</literal> could export invalid
> > > LSN
> > > > > + as finish LSN of failed transaction. Changing the streaming mode and
> > > making
> > > > > + the same conflict writes the finish LSN of the failed transaction in the
> > > > > + server log if required.
> > > > > + </para>
> > > > >
> > > > > How will the user identify that this is an invalid LSN value and she
> > > > > shouldn't use it to SKIP the transaction? Can we change the second
> > > > > sentence to: "User should change the streaming mode to 'on' if they
> > > > > would instead wish to see the finish LSN on error. Users can use
> > > > > finish LSN to SKIP applying the transaction." I think we can give
> > > > > reference to docs where the SKIP feature is explained.
> > > > Improved the sentence as suggested.
> > > >
> > >
> > > You haven't answered first part of the comment: "How will the user
> > > identify that this is an invalid LSN value and she shouldn't use it to
> > > SKIP the transaction?". Have you checked what value it displays? For
> > > example, in one of the case in apply_error_callback as shown in below
> > > code, we don't even display finish LSN if it is invalid.
> > > else if (XLogRecPtrIsInvalid(errarg->finish_lsn))
> > > errcontext("processing remote data for replication origin \"%s\"
> > > during \"%s\" in transaction %u",
> > > errarg->origin_name,
> > > logicalrep_message_type(errarg->command),
> > > errarg->remote_xid);
> > I am sorry that I missed something in my previous reply.
> > The invalid LSN value here is to say InvalidXLogRecPtr (0/0).
> > Here is an example :
> > ```
> > 2022-06-23 14:30:11.343 CST [822333] logical replication worker CONTEXT:
> processing remote data for replication origin "pg_16389" during "INSERT" for
> replication target relation "public.tab" in transaction 727 finished at 0/0
> > ```
> >
>
> I don't think it is a good idea to display invalid values. We can mask
> this as we are doing in other cases in function apply_error_callback.
> The ideal way is that we provide a view/system table for users to
> check these errors but that is a matter of another patch. So users
> probably need to check Logs to see if the error is from a background
> apply worker to decide whether or not to switch streaming mode.
Thanks for your comments.
I improved it as you suggested. I mask the LSN if it is invalid LSN(0/0).
Also, I improved the related pg-doc as following:
```
When the streaming mode is <literal>parallel</literal>, the finish LSN of
failed transactions may not be logged. In that case, it may be necessary to
change the streaming mode to <literal>on</literal> and cause the same
conflicts again so the finish LSN of the failed transaction will be written
to the server log. For the usage of finish LSN, please refer to <link
linkend="sql-altersubscription"><command>ALTER SUBSCRIPTION ...
SKIP</command></link>.
```
After improving this (mask invalid LSN), I found that this improvement and
parallel apply patch do not seem to have a strong correlation. Would it be
better to improve and commit in another separate patch?
I also improved patches as suggested by Peter-san in [1] and [2].
Thanks for Shi Yu to improve the patches by addressing the comments in [2].
Attach the new patches.
[1] - https://www.postgresql.org/message-id/CAHut%2BPtu_eWOVWAKrwkUFdTAh_r-RZsbDFkFmKwEAmxws%3DSh5w%40mail.gmail.com
[2] - https://www.postgresql.org/message-id/CAHut%2BPsDzRu6PD1uSRkftRXef-KwrOoYrcq7Cm0v4otisi5M%2Bg%40mail.gmail.com
Regards,
Wang wei
Commits
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Fix invalid memory access during the shutdown of the parallel apply worker.
- 3d144c6c8602 16.0 landed
-
Fix assertion failure in apply worker.
- de63f8dadee4 16.0 landed
-
Use elog to report unexpected action in handle_streamed_transaction().
- 781ac42d43ab 16.0 landed
-
Use appropriate wait event when sending data in the apply worker.
- d9d7fe68d35e 16.0 landed
-
Allow the logical_replication_mode to be used on the subscriber.
- 9f2213a7c575 16.0 landed
-
Rename GUC logical_decoding_mode to logical_replication_mode.
- 1e8b61735cfb 16.0 landed
-
Display the leader apply worker's PID for parallel apply workers.
- d540a02a724b 16.0 landed
-
Improve the code to decide and process the apply action.
- c981d9145dea 16.0 landed
-
Document the newly added wait events added by commit 216a784829.
- cd06ccd78fcf 16.0 landed
-
Perform apply of large transactions by parallel workers.
- 216a784829c2 16.0 landed
-
Wake up a subscription's replication worker processes after DDL.
- c6e1f62e2cee 16.0 cited
-
Add copyright notices to meson files
- 8284cf5f746f 16.0 cited
-
Better document logical replication parameters
- a8500750ca0a 16.0 cited
-
Add a common function to generate the origin name.
- 776e1c8a5d14 16.0 cited
-
Harmonize parameter names in storage and AM code.
- bfcf1b34805f 16.0 cited
-
Avoid using list_length() to test for empty list.
- efd0c16becbf 16.0 cited
-
Improve two comments related to a boolean DefElem's value
- 8445f5a21d40 16.0 cited
-
Fix partition table's REPLICA IDENTITY checking on the subscriber.
- 26b3455afaa9 15.0 cited
-
Fix data inconsistency between publisher and subscriber.
- b7658c24c7c1 15.0 cited
-
Fix cache look-up failures while applying changes in logical replication.
- 5a97b1325453 15.0 cited