Re: Fix checkpoint skip logic on idle systems by tracking LSN progress
От | Amit Kapila |
---|---|
Тема | Re: Fix checkpoint skip logic on idle systems by tracking LSN progress |
Дата | |
Msg-id | CAA4eK1KJAXA3PdxH4T1QJKBNOvyUK8UKm_GCvTuT+FC5jpjmjg@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: Fix checkpoint skip logic on idle systems by tracking LSN progress (Michael Paquier <michael.paquier@gmail.com>) |
Ответы |
Re: Fix checkpoint skip logic on idle systems by
tracking LSN progress
|
Список | pgsql-hackers |
On Mon, Nov 14, 2016 at 9:33 AM, Michael Paquier <michael.paquier@gmail.com> wrote: > On Mon, Nov 14, 2016 at 12:49 PM, Kyotaro HORIGUCHI > <horiguchi.kyotaro@lab.ntt.co.jp> wrote: >> At Sat, 12 Nov 2016 10:28:56 +0530, Amit Kapila <amit.kapila16@gmail.com> wrote in <CAA4eK1K0gGQTBxCyKqi6QnqOWGzEoVVPHCgPJ_RkOBoLPejCTA@mail.gmail.com> >>> I think it is good to check the performance impact of this patch on >>> many core m/c. Is it possible for you to once check with Alexander >>> Korotkov to see if he can provide you access to his powerful m/c which >>> has 70 cores (if I remember correctly)? > > I heard about a number like that, and there is no reason to not do > tests to be sure. With that many cores we are more likely going to see > the limitation of the number of XLOG insert slots popping up as a > bottleneck, but that's just an assumption without any numbers. > Alexander (added in CC), could it be possible to get an access to this > machine? > >>> @@ -1035,6 +1038,7 @@ LogAccessExclusiveLocks(int nlocks, >>> xl_standby_lock *locks) >>> XLogBeginInsert(); >>> XLogRegisterData((char *) &xlrec, offsetof(xl_standby_locks, locks)); >>> XLogRegisterData((char *) locks, nlocks * sizeof(xl_standby_lock)); >>> + XLogSetFlags(XLOG_NO_PROGRESS); >>> >>> >>> Is it right to set XLOG_NO_PROGRESS flag in LogAccessExclusiveLocks? >>> This function is called not only in LogStandbySnapshot(), but during >>> DDL operations as well where lockmode >= AccessExclusiveLock. >> >> This does not remove any record from WAL. So theoretically any >> kind of record can be NO_PROGRESS, but practically as long as >> checkpoints are not unreasonably suppressed. Any explicit >> database operation must be accompanied with at least commit >> record that triggers checkpoint. NO_PROGRESSing there doesn't >> seem to me to harm database durability for this reason. >> By this theory, you can even mark the insert record as no progress which is not good. >> The objective of this patch is skipping WALs on completely-idle >> state and the NO_PROGRESSing is necessary to do its work. Of >> course we can distinguish exclusive lock with PROGRESS and >> without PROGRESS but it is unnecessary complexity. > > The point that applies here is that logging the exclusive lock > information is necessary for the *standby* recovery conflicts, not the > primary which is why it should not influence the checkpoint activity > that is happening on the primary. So marking this record with > NO_PROGRESS is actually fine to me. > The progress parameter is used not only for checkpoint activity but by bgwriter as well for logging standby snapshot. If you want to keep this record under no_progress category (which I don't endorse), then it might be better to add a comment, so that it will be easier for the readers of this code to understand the reason. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
В списке pgsql-hackers по дате отправления: