Re: Design of pg_stat_subscription_workers vs pgstats

public inbox for [email protected]  
help / color / mirror / Atom feed

From: David G. Johnston <[email protected]>
To: Amit Kapila <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: Design of pg_stat_subscription_workers vs pgstats
Date: Sat, 19 Feb 2022 06:21:43 -0700
Message-ID: <CAKFQuwZzHcMQKDH_i9LPsk89Omc4ew69X0_khKxPkNeJrx_3oA@mail.gmail.com> (raw)
In-Reply-To: <CAA4eK1L5qyPngMU9MFPvGzQvopUS4dpn03sH==ndva3F119SRw@mail.gmail.com>
References: <[email protected]>
	<CAA4eK1+MDngbOQfMcAMsrf__s2a-MMMHaCR0zwde3GVeEi-bbQ@mail.gmail.com>
	<CAKFQuwaGo9zOhr_E3eYm+Pevvzi=MCGYo5nkzvkuX35wiyysYw@mail.gmail.com>
	<CAA4eK1K12FFF0wNV0k6UtLK9BP=S6PawWvepAAVbx7g+hb245Q@mail.gmail.com>
	<CAD21AoCWZDmW01BVpr_h=4uJERTBo3qFO6n_hzE0vynAH+=Gpw@mail.gmail.com>
	<CAA4eK1K36U=cTpt704p_iiz+riF9TXd1xBEpOni9RsZgjQZFbA@mail.gmail.com>
	<CAKFQuwb8yaWxxH-gSt4NG9HhVnmKK_GnCEotVtjG1JQohOb0Qw@mail.gmail.com>
	<CAA4eK1JDQfM8ChVo=9e=+4cawSOSpi678+TyWTd3cw76Tvq8mg@mail.gmail.com>
	<CAKFQuwYHFkW8fP_a62wk-YBb4o+n9UXG4Ji3E4O9DwZrv0jgQQ@mail.gmail.com>
	<CAD21AoCKxcVB9xh5o_Zm8-q0qukuQncNfBD6LVkY=my8ZJbqkQ@mail.gmail.com>
	<[email protected]>
	<CAA4eK1Jwn5hhgHJWZ9n+wYyQEk+brfR1zkuqLjh=waji8RYWAw@mail.gmail.com>
	<CAD21AoAqEt=TbUDY96Uhf9pNuh-NDR_3-CByd+xR=tFEq==1NA@mail.gmail.com>
	<CAD21AoA-MVZcMA1iYwbtvTCOHx1SwG8amQUTT8j2ssh2zuc6OQ@mail.gmail.com>
	<CAKFQuwZk-Trf78QW7-_q9Pr2EGBhtg=wSA2hRiOW-K845Zv2cQ@mail.gmail.com>
	<CAA4eK1L5qyPngMU9MFPvGzQvopUS4dpn03sH==ndva3F119SRw@mail.gmail.com>

On Saturday, February 19, 2022, Amit Kapila <[email protected]> wrote:

> On Sat, Feb 19, 2022 at 1:17 AM David G. Johnston
> <[email protected]> wrote:
> >
> > On Fri, Feb 18, 2022 at 1:26 AM Masahiko Sawada <[email protected]>
> wrote:
> >>
> >>
> >> Here is the summary of the discussion, changes, and plan.
> >>
> >> 1. Move some error information such as the error message to a new
> >> system catalog, pg_subscription_error. The pg_subscription_error table
> >> would have the following columns:
> >>
> >> * sesubid : subscription Oid.
> >> * serelid : relation Oid (NULL for apply worker).
> >> * seerrlsn : commit-LSN or the error transaction.
> >> * seerrcmd : command (INSERT, UPDATE, etc.) of the error transaction.
> >> * seerrmsg : error message
> >
> >
> > Not a fan of the "se" prefix but overall yes. We should include a
> timestamp.
> >
>
> How about naming it pg_subscription_worker_error as Peter E. has
> suggested in one of his emails? I find pg_subscription_error slightly
> odd as one could imagine that even the errors related to subscription
> commands like Alter Subscription.
>
>
Adding worker makes sense.


> >>
> >> The tuple is inserted or updated when an apply worker or a tablesync
> >> worker raises an error. If the same error occurs in a row, the update
> >> is skipped.
> >
>
> Are you going to query table to check if it is same error?


I don’t get the question, the quoted text is your which I disagree with.
But the error message is being captured in any case.

>
> >
> > I disagree with this - I would treat every new instance of an error to
> be important and insert on conflict (sesubid, serelid) the new entry,
> updating lsn/cmd/msg with the new values.
> >
>
> I don't think that will be a problem but what advantage are you
> envisioning with updating the same info except for timestamp?


Omission of timestamp (or any other non-key field we have) from the update
is an oversight.


> >> The tuple is removed in the following cases:
> >>
> >> * the subscription is dropped.
> >> * the table is dropped.
> >>
> >> * the table is removed from the subscription.
> >> * the worker successfully committed a non-empty transaction.
> >
> >
> > Correct.  This handles the "end of sync worker" just fine since its
> final action should be a successful commit of a non-empty transaction.
> >>
>
> I think for tablesync workers, we may need slightly different handling
> as there could probably be no transactions to apply apart from the
> initial copy. Now, I think for tablesync worker, we can't postpone it
> till after we update the rel state as SUBREL_STATE_SYNCDONE because if
> we do it after that and there is some error updating/deleting the
> tuple, the tablesync worker won't be launched again and that entry
> will remain in the system for a longer duration.
>

Not sure…but I don’t see how you can not have a non-empty transaction while
still having an error.

Are subscriptions “dropped” upon a reboot?  If not, that needs its own case
for row removal.

David J.

view thread (23+ 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]
  Subject: Re: Design of pg_stat_subscription_workers vs pgstats
  In-Reply-To: <CAKFQuwZzHcMQKDH_i9LPsk89Omc4ew69X0_khKxPkNeJrx_3oA@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