Horton
Menu

RE: Perform streaming logical transactions by background workers and parallel apply

Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>

View thread
From: "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>
To: "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>, Amit Kapila <amit.kapila16@gmail.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>, "wangw.fnst@fujitsu.com" <wangw.fnst@fujitsu.com>, Peter Smith <smithpb2250@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-11-08T03:56:43Z
Lists: pgsql-hackers

Attachments

On Monday, November 7, 2022 9:19 PM houzj.fnst@fujitsu.com <houzj.fnst@fujitsu.com> wrote:
> 
> On Friday, November 4, 2022 7:45 PM Amit Kapila <amit.kapila16@gmail.com>
> wrote:
> >
> > On Fri, Nov 4, 2022 at 1:36 PM Amit Kapila <amit.kapila16@gmail.com>
> wrote:
> > >
> > > On Thu, Nov 3, 2022 at 6:36 PM houzj.fnst@fujitsu.com
> > > <houzj.fnst@fujitsu.com> wrote:
> > > >
> > > > Thanks for the analysis and summary !
> > > >
> > > > I tried to implement the above idea and here is the patch set.
> > > >
> > >
> > > Few comments on v42-0001
> > > ===========================
> > >
> 
> Thanks for the comments.
> 
> > Few more comments on v42-0001
> > ===============================
> > 1. In parallel_apply_send_data(), it seems winfo->serialize_changes
> > and switching_to_serialize are set to indicate that we have changed
> > parallel to serialize mode. Isn't using just the
> > switching_to_serialize sufficient? Also, it would be better to name
> > switching_to_serialize as parallel_to_serialize or something like
> > that.
> 
> I slightly change the logic to let serialize the message directly when timeout
> instead of invoking apply_dispatch again so that we don't need the
> switching_to_serialize.
> 
> >
> > 2. In parallel_apply_send_data(), the patch has already initialized
> > the fileset, and then again in apply_handle_stream_start(), it will do
> > the same if we fail while sending stream_start message to the parallel
> > worker. It seems we don't need to initialize fileset again for
> > TRANS_LEADER_PARTIAL_SERIALIZE state in apply_handle_stream_start()
> > unless I am missing something.
> 
> Fixed.
> 
> > 3.
> > apply_handle_stream_start(StringInfo s) { ...
> > + if (!first_segment)
> > + {
> > + /*
> > + * Unlock the shared object lock so that parallel apply worker
> > + * can continue to receive and apply changes.
> > + */
> > + parallel_apply_unlock(winfo->shared->stream_lock_id);
> > ...
> > }
> >
> > Can we have an assert before this unlock call that the lock must be
> > held? Similarly, if there are other places then we can have assert
> > there as well.
> 
> It seems we don't have a standard API can be used without a transaction.
> Maybe we can use the list ParallelApplyLockids to check that ?
> 
> > 4. It is not very clear to me how maintaining ParallelApplyLockids
> > list is helpful.
> 
> I will think about this and remove this in next version list if possible.
> 
> >
> > 5.
> > /*
> > + * Handle STREAM START message when the transaction was spilled to disk.
> > + *
> > + * Inintialize fileset if not yet and open the file.
> > + */
> > +void
> > +serialize_stream_start(TransactionId xid, bool first_segment) {
> > + /*
> > + * Start a transaction on stream start,
> >
> > This function's name and comments seem to indicate that it is to
> > handle stream_start message. Is that really the case? It is being
> > called from parallel_apply_send_data() which made me think it can be
> > used from other places as well.
> 
> Adjusted the comment.
> 
> Here is the new version patch set which addressed comments as of last Friday.
> I also added some comments for the newly introduced codes in this version.
>

Sorry, I posted the wrong patch for V43 which lack some changes.
Attach the correct patch set here.

Best regards,
Hou zj

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix invalid memory access during the shutdown of the parallel apply worker.

  2. Fix assertion failure in apply worker.

  3. Use elog to report unexpected action in handle_streamed_transaction().

  4. Use appropriate wait event when sending data in the apply worker.

  5. Allow the logical_replication_mode to be used on the subscriber.

  6. Rename GUC logical_decoding_mode to logical_replication_mode.

  7. Display the leader apply worker's PID for parallel apply workers.

  8. Improve the code to decide and process the apply action.

  9. Document the newly added wait events added by commit 216a784829.

  10. Perform apply of large transactions by parallel workers.

  11. Wake up a subscription's replication worker processes after DDL.

  12. Add copyright notices to meson files

  13. Better document logical replication parameters

  14. Add a common function to generate the origin name.

  15. Harmonize parameter names in storage and AM code.

  16. Avoid using list_length() to test for empty list.

  17. Improve two comments related to a boolean DefElem's value

  18. Fix partition table's REPLICA IDENTITY checking on the subscriber.

  19. Fix data inconsistency between publisher and subscriber.

  20. Fix cache look-up failures while applying changes in logical replication.