Hello, thank you for the comment. At Sat, 12 Nov 2016 10:28:56 +0530, Amit Kapila <amit.kapil...@gmail.com> wrote in <caa4ek1k0ggqtbxcykqi6qnqowgzeovvphcgpj_rkobolpej...@mail.gmail.com> > On Tue, Nov 8, 2016 at 5:18 PM, Kyotaro HORIGUCHI > <horiguchi.kyot...@lab.ntt.co.jp> wrote: > > Hello, > > > >> on something else than LW_EXCLUSIVE compared to now. To keep things > >> more simple I' would still favor using WALInsertLocks for this patch, > >> that looks more consistent, and also because there is no noticeable > >> difference. > > > > Ok, the patch looks fine. So there's nothing for me but to accept > > the current shape since the discussion about performance seems > > not to be settled with out performance measurement with machines > > with many cores. > > > > 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)? > > > @@ -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. 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. But rethinking about the above, the namging of "XLOG_NO_PROGRESS" might be inappropriate. "XLOG_NO_CKPT_TRIGGER" or any sainer name would be needed. regards, -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers