Re: wal stats questions
От | Masahiro Ikeda |
---|---|
Тема | Re: wal stats questions |
Дата | |
Msg-id | 3a1339bc-df6a-925d-b194-907480ff9383@oss.nttdata.com обсуждение исходный текст |
Ответ на | Re: wal stats questions (Fujii Masao <masao.fujii@oss.nttdata.com>) |
Ответы |
Re: wal stats questions
(torikoshia <torikoshia@oss.nttdata.com>)
|
Список | pgsql-hackers |
On 2021/05/12 19:19, Fujii Masao wrote: > > > On 2021/05/11 18:46, Masahiro Ikeda wrote: >> >> >> On 2021/05/11 16:44, Fujii Masao wrote: >>> >>> >>> On 2021/04/28 9:10, Masahiro Ikeda wrote: >>>> >>>> >>>> On 2021/04/27 21:56, Fujii Masao wrote: >>>>> >>>>> >>>>> On 2021/04/26 10:11, Masahiro Ikeda wrote: >>>>>> >>>>>> First patch has only the changes for pg_stat_wal view. >>>>>> ("v6-0001-performance-improvements-of-reporting-wal-stats-without-introducing-a-new-variable.patch") >>>>>> >>>>>> >>>>>> >>>>> >>>>> + pgWalUsage.wal_records == prevWalUsage.wal_records && >>>>> + walStats.wal_write == 0 && walStats.wal_sync == 0 && >>>>>> WalStats.m_wal_write should be checked here instead of walStats.wal_write? >>>> >>>> Thanks! Yes, I'll fix it. >>> >>> Thanks! >> >> Thanks for your comments! >> I fixed them. > > Thanks for updating the patch! > > if ((pgStatTabList == NULL || pgStatTabList->tsa_used == 0) && > pgStatXactCommit == 0 && pgStatXactRollback == 0 && > + pgWalUsage.wal_records == prevWalUsage.wal_records && > + WalStats.m_wal_write == 0 && WalStats.m_wal_sync == 0 && > > I'm just wondering if the above WAL activity counters need to be checked. > Maybe it's not necessary because "pgStatXactCommit == 0 && pgStatXactRollback > == 0" > is checked? IOW, is there really the case where WAL activity counters are updated > even when both pgStatXactCommit and pgStatXactRollback are zero? Thanks for checking. I haven't found the case yet. But, I added the condition because there is a discussion that it's safer. (It seems the mail thread chain is broken, Sorry...) I copy the discussion at the time below. https://www.postgresql.org/message-id/20210330.172843.267174731834281075.horikyota.ntt%40gmail.com >>>> 3) Doing if (memcmp(&WalStats, &all_zeroes, sizeof(PgStat_MsgWal)) == 0) >>>> just to figure out if there's been any changes isn't all that >>>> cheap. This is regularly exercised in read-only workloads too. Seems >>>> adding a boolean WalStats.have_pending = true or such would be >>>> better. >>>> 4) For plain backends pgstat_report_wal() is called by >>>> pgstat_report_stat() - but it is not checked as part of the "Don't >>>> expend a clock check if nothing to do" check at the top. It's >>>> probably rare to have pending wal stats without also passing one of >>>> the other conditions, but ... >>> >>> I added the logic to check if the stats counters are updated or not in >>> pgstat_report_stat() using not only generated wal record but also write/sync >>> counters, and it can skip to call reporting function. >> >> I removed the checking code whether the wal stats counters were updated or not >> in pgstat_report_stat() since I couldn't understand the valid case yet. >> pgstat_report_stat() is called by backends when the transaction is finished. >> This means that the condition seems always pass. > > Doesn't the same holds for all other counters? If you are saying that > "wal counters should be zero if all other stats counters are zero", we > need an assertion to check that and a comment to explain that. > > I personally find it safer to add the WAL-stats condition to the > fast-return check, rather than addin such assertion. > + if (pgWalUsage.wal_records != prevWalUsage.wal_records) > + { > + WalUsage walusage; > + > + /* > + * Calculate how much WAL usage counters were increased by substracting > + * the previous counters from the current ones. Fill the results in > + * WAL stats message. > + */ > + MemSet(&walusage, 0, sizeof(WalUsage)); > + WalUsageAccumDiff(&walusage, &pgWalUsage, &prevWalUsage); > > Isn't it better to move the code "prevWalUsage = pgWalUsage" into here? > Because it's necessary only when pgWalUsage.wal_records != > prevWalUsage.wal_records. Yes, I fixed it. Regards, -- Masahiro Ikeda NTT DATA CORPORATION
Вложения
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Alvaro HerreraДата:
Сообщение: Re: Enhanced error message to include hint messages for redundant options error