On Mon, Jun 21, 2021 at 9:21 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Mon, Jun 21, 2021 at 12:09 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Mon, Jun 21, 2021 at 7:56 AM Mark Dilger > > <mark.dil...@enterprisedb.com> wrote: > > > > > > > On Jun 20, 2021, at 7:17 PM, Masahiko Sawada <sawada.m...@gmail.com> > > > > wrote: > > > > > > > > I will submit the patch. > > > > > > Great, thanks! > > > > > > > There was a discussion that the skipping transaction patch would also > > > > need to have a feature that tells users the details of the last > > > > failure transaction such as its XID, timestamp, action etc. In that > > > > sense, those two patches might need the common infrastructure that the > > > > apply workers leave the error details somewhere so that the users can > > > > see it. > > > > > > Right. Subscription on error triggers would need that, too, if we wrote > > > them. > > > > > > > Is it really useful to write only error message to the system catalog? > > > > Even if we see the error message like "duplicate key value violates > > > > unique constraint “test_tab_pkey”” on the system catalog, we will end > > > > up needing to check the server log for details to properly resolve the > > > > conflict. If the user wants to know whether the subscription is > > > > disabled manually or automatically, the error message on the system > > > > catalog might not necessarily be necessary. > > > > > > > > I think the two key points are (a) to define exactly what all > > information is required to be logged on error, > > When it comes to the patch for skipping transactions, it would > somewhat depend on how users specify transactions to skip. On the > other hand, for this patch, the minimal information would be whether > the subscription is disabled automatically by the server. >
True, but still there will be some information related to ERROR which we wanted the user to see unless we ask them to refer to logs for that. > > (b) where do we want to > > store the information based on requirements. I see that for (b) Mark > > is inclined to use the existing catalog table. I feel that is worth > > considering but not sure if that is the best way to deal with it. For > > example, if we store that information in the catalog, we might need to > > consider storing it both in pg_subscription and pg_subscription_rel, > > otherwise, we might overwrite the errors as I think what is happening > > in the currently proposed patch. The other possibilities could be to > > define a new catalog table to capture the error information or log the > > required information via stats collector and then the user can see > > that info via some stats view. > > This point is also related to the point whether or not that > information needs to last after the server crash (and restart). When > it comes to the patch for skipping transactions, there was a > discussion that we don’t necessarily need it since the tools will be > used in rare cases. But for this proposed patch, I guess it would be > useful if it does. It might be worth considering doing a different way > for each patch. For example, we send the details of last failure > transaction to the stats collector while updating subenabled to > something like “automatically-disabled” instead of to just “false” (or > using another column to show the subscriber is disabled automatically > by the server). > I agree that it is worth considering to have subenabled to have a tri-state (enable, disabled, automatically-disabled) value instead of just a boolean. But in this case, if the stats collector missed updating the information, the user may have to manually update the subscription and let the error happen again to see it. -- With Regards, Amit Kapila.
