On Thursday, January 12, 2023 7:08 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Thu, Jan 12, 2023 at 4:21 PM shveta malik <shveta.ma...@gmail.com> wrote: > > > > On Thu, Jan 12, 2023 at 10:34 AM Amit Kapila <amit.kapil...@gmail.com> > wrote: > > > > > > On Thu, Jan 12, 2023 at 9:54 AM Peter Smith <smithpb2...@gmail.com> > wrote: > > > > > > > > > > > > doc/src/sgml/monitoring.sgml > > > > > > > > 5. pg_stat_subscription > > > > > > > > @@ -3198,11 +3198,22 @@ SELECT pid, wait_event_type, wait_event > > > > FROM pg_stat_activity WHERE wait_event i > > > > > > > > <row> > > > > <entry role="catalog_table_entry"><para > > > > role="column_definition"> > > > > + <structfield>apply_leader_pid</structfield> > <type>integer</type> > > > > + </para> > > > > + <para> > > > > + Process ID of the leader apply worker, if this process is a > > > > apply > > > > + parallel worker. NULL if this process is a leader apply worker > > > > or a > > > > + synchronization worker. > > > > + </para></entry> > > > > + </row> > > > > + > > > > + <row> > > > > + <entry role="catalog_table_entry"><para > > > > + role="column_definition"> > > > > <structfield>relid</structfield> <type>oid</type> > > > > </para> > > > > <para> > > > > OID of the relation that the worker is synchronizing; null for > > > > the > > > > - main apply worker > > > > + main apply worker and the parallel apply worker > > > > </para></entry> > > > > </row> > > > > > > > > 5a. > > > > > > > > (Same as general comment #1 about terminology) > > > > > > > > "apply_leader_pid" --> "leader_apply_pid" > > > > > > > > > > How about naming this as just leader_pid? I think it could be > > > helpful in the future if we decide to parallelize initial sync (aka > > > parallel > > > copy) because then we could use this for the leader PID of parallel > > > sync workers as well. > > > > > > -- > > > > I still prefer leader_apply_pid. > > leader_pid does not tell which 'operation' it belongs to. 'apply' > > gives the clarity that it is apply related process. > > > > But then do you suggest that tomorrow if we allow parallel sync workers then > we have a separate column leader_sync_pid? I think that doesn't sound like a > good idea and moreover one can refer to docs for clarification.
I agree that leader_pid would be better not only for future parallel copy sync feature, but also it's more consistent with the leader_pid column in pg_stat_activity. And here is the version patch which addressed Peter's comments and renamed all the related stuff to leader_pid. Best Regards, Hou zj
v79-0004-Retry-to-apply-streaming-xact-only-in-apply-work.patch
Description: v79-0004-Retry-to-apply-streaming-xact-only-in-apply-work.patch
v79-0001-Add-leader_pid-to-pg_stat_subscription.patch
Description: v79-0001-Add-leader_pid-to-pg_stat_subscription.patch
v79-0002-Stop-extra-worker-if-GUC-was-changed.patch
Description: v79-0002-Stop-extra-worker-if-GUC-was-changed.patch
v79-0003-Add-GUC-stream_serialize_threshold-and-test-seri.patch
Description: v79-0003-Add-GUC-stream_serialize_threshold-and-test-seri.patch
