On Mon, Apr 18, 2022 at 9:29 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Thu, Apr 14, 2022 at 5:52 PM Euler Taveira <eu...@eulerto.com> wrote: > > > > On Wed, Apr 13, 2022, at 7:45 AM, Amit Kapila wrote: > > > > Sawada-San, Euler, do you have any opinion on this approach? I > > personally still prefer the approach implemented in v10 [1] especially > > due to the latest finding by Wang-San that we can't update the > > lag-tracker apart from when it is invoked at the transaction end. > > However, I am fine if we like this approach more. > > > > It seems v15 is simpler and less error prone than v10. v10 has a mix of > > OutputPluginUpdateProgress() and the new function update_progress(). The v10 > > also calls update_progress() for every change action in pgoutput_change(). > > It > > is not a good approach for maintainability -- new changes like sequences > > need > > extra calls. > > > > Okay, let's use the v15 approach as Sawada-San also seems to have a > preference for that. > > > However, as you mentioned there should handle the track lag case. > > > > Both patches change the OutputPluginUpdateProgress() so it cannot be > > backpatched. Are you planning to backpatch it? If so, the boolean variable > > (last_write or end_xacts depending of which version you are considering) > > could > > be added to LogicalDecodingContext. > > > > If we add it to LogicalDecodingContext then I think we have to always > reset the variable after its use which will make it look ugly and > error-prone. I was not thinking to backpatch it because of the API > change but I guess if we want to backpatch then we can add it to > LogicalDecodingContext for back-branches. I am not sure if that will > look committable but surely we can try. >
Even, if we want to add the variable in the struct in back-branches, we need to ensure not to change the size of the struct as it is exposed, see email [1] for a similar mistake we made in another case. [1] - https://www.postgresql.org/message-id/2358496.1649168259%40sss.pgh.pa.us -- With Regards, Amit Kapila.
