Re: wal stats questions
От | Masahiro Ikeda |
---|---|
Тема | Re: wal stats questions |
Дата | |
Msg-id | 5a22c695-04f8-b1e8-f83a-8f76e4a7a9ac@oss.nttdata.com обсуждение исходный текст |
Ответ на | Re: wal stats questions (Masahiro Ikeda <ikedamsh@oss.nttdata.com>) |
Ответы |
Re: wal stats questions
(Kyotaro Horiguchi <horikyota.ntt@gmail.com>)
|
Список | pgsql-hackers |
I update the patch since there were my misunderstanding points. On 2021/03/26 16:20, Masahiro Ikeda wrote: > Thanks for many your suggestions! > I made the patch to handle the issues. > >> 1) What is the motivation to have both prevWalUsage and pgWalUsage, >> instead of just accumulating the stats since the last submission? >> There doesn't seem to be any comment explaining it? Computing >> differences to previous values, and copying to prev*, isn't free. I >> assume this is out of a fear that the stats could get reset before >> they're used for instrument.c purposes - but who knows? > > I removed the unnecessary code copying pgWalUsage and just reset the > pgWalUsage after reporting the stats in pgstat_report_wal(). I didn't change this. >> 2) Why is there both pgstat_send_wal() and pgstat_report_wal()? With the >> former being used by wal writer, the latter by most other processes? >> There again don't seem to be comments explaining this. > > I added the comments why two functions are separated. > (But is it better to merge them?) I updated the comments for following reasons. >> 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. I thought to implement if the WAL stats counters were not updated, skip to send all statistics including the counters for databases and so on. But I think it's not good because it may take more time to be reflected the generated stats by read-only transaction. > Although I added the condition which the write/sync counters are updated or > not, I haven't understood the following case yet...Sorry. I checked related > code and tested to insert large object, but I couldn't. I'll investigate more > deeply, but if you already know the function which leads the following case, > please let me know. I understood the above case (Fujii-san, thanks for your advice in person). When to flush buffers, for example, to select buffer replacement victim, it requires a WAL flush if the buffer is dirty. So, to check the WAL stats counters are updated or not, I check the number of generated wal record and the counter of syncing in pgstat_report_wal(). The reason why not to check the counter of writing is that if to write is happened, to sync is happened too in the above case. I added the comments in the patch. Regards, -- Masahiro Ikeda NTT DATA CORPORATION
Вложения
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Tomas VondraДата:
Сообщение: Re: Merging statistics from children instead of re-sampling everything
Следующее
От: Tomas VondraДата:
Сообщение: Re: Merging statistics from children instead of re-sampling everything