Re: Logical replication timeout problem
От | Amit Kapila |
---|---|
Тема | Re: Logical replication timeout problem |
Дата | |
Msg-id | CAA4eK1L8jDBGqdZU0Fk6LDSpdqE+3wg2AwkgdxwiCqmXHc6aoQ@mail.gmail.com обсуждение исходный текст |
Ответ на | RE: Logical replication timeout problem ("wangw.fnst@fujitsu.com" <wangw.fnst@fujitsu.com>) |
Ответы |
RE: Logical replication timeout problem
("wangw.fnst@fujitsu.com" <wangw.fnst@fujitsu.com>)
RE: Logical replication timeout problem ("wangw.fnst@fujitsu.com" <wangw.fnst@fujitsu.com>) |
Список | pgsql-hackers |
On Wed, Apr 6, 2022 at 6:30 PM wangw.fnst@fujitsu.com <wangw.fnst@fujitsu.com> wrote: > > On Wed, Apr 6, 2022 at 1:59 AM Amit Kapila <amit.kapila16@gmail.com> wrote: > On Wed, Apr 6, 2022 at 4:32 AM Amit Kapila <amit.kapila16@gmail.com> wrote: > > > Thanks for your comments. > > > typedef void (*LogicalOutputPluginWriterUpdateProgress) (struct > > LogicalDecodingContext *lr, > > XLogRecPtr Ptr, > > TransactionId xid, > > - bool skipped_xact > > + bool skipped_xact, > > + bool last_write > > > > In this approach, I don't think we need an additional parameter last_write. Let's > > do the work related to keepalive without a parameter, do you see any problem > > with that? > I agree with you. Modify this point. > > > I think this patch doesn't take into account that we call > > OutputPluginUpdateProgress() from APIs like pgoutput_commit_txn(). I > > think we should always call the new function update_progress from > > those existing call sites and arrange the function such that when > > called from xact end APIs like pgoutput_commit_txn(), it always call > > OutputPluginUpdateProgress and make changes_count as 0. > Improve it. > Add two new input to function update_progress.(skipped_xact and end_xact). > Modify the function invoke from OutputPluginUpdateProgress to update_progress. > > > Also, let's try to evaluate how it impacts lag functionality for large transactions? > I think this patch will not affect lag functionality. It will updates the lag > field of view pg_stat_replication more frequently. > IIUC, when invoking function WalSndUpdateProgress, it will store the lsn of > change and invoking time in lag_tracker. Then when invoking function > ProcessStandbyReplyMessage, it will calculate the lag field according to the > message from subscriber and the information in lag_tracker. This patch does > not modify this logic, but only increases the frequency of invoking. > Please let me know if I understand wrong. > No, your understanding seems correct to me. But what I want to check is if calling the progress function more often has any impact on lag-related fields in pg_stat_replication? I think you need to check the impact of large transaction replay. One comment: +static void +update_progress(LogicalDecodingContext *ctx, bool skipped_xact, bool end_xact) +{ + static int changes_count = 0; + + if (end_xact) + { + /* Update progress tracking at xact end. */ + OutputPluginUpdateProgress(ctx, skipped_xact); + changes_count = 0; + } + /* + * After continuously processing CHANGES_THRESHOLD changes, update progress + * which will also try to send a keepalive message if required. I think you can simply return after making changes_count = 0. There should be an empty line before starting the next comment. -- With Regards, Amit Kapila.
В списке pgsql-hackers по дате отправления:
Предыдущее
От: David RowleyДата:
Сообщение: Re: Add proper planner support for ORDER BY / DISTINCT aggregates
Следующее
От: Michael PaquierДата:
Сообщение: Re: suboverflowed subtransactions concurrency performance optimize