Re: WAL usage calculation patch

From: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
To: Julien Rouhaud <rjuju123(at)gmail(dot)com>, Kirill Bychik <kirill(dot)bychik(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
Subject: Re: WAL usage calculation patch
Date: 2020-03-19 12:03:02
Message-ID: 5b71ad52-68c6-08cb-6408-31307e9e6f10@oss.nttdata.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2020/03/19 2:19, Julien Rouhaud wrote:
> On Wed, Mar 18, 2020 at 09:02:58AM +0300, Kirill Bychik wrote:
>>
>> There is a higher-level Instrumentation API that can be used with
>> INSTRUMENT_WAL flag to collect the wal usage information. I believe
>> the instrumentation is widely used in the executor code, so it should
>> not be a problem to colelct instrumentation information on autovacuum
>> worker level.
>>
>> Just a recommendation/chat, though. I am happy with the way the data
>> is collected now. If you commit this variant, please add a TODO to
>> rework wal usage to common instr API.
>
>
> The instrumentation is somewhat intended to be used with executor nodes, not
> backend commands. I don't see real technical reason that would prevent that,
> but I prefer to keep things as-is for now, as it sound less controversial.
> This is for the 3rd patch, which may not even be considered for this CF anyway.
>
>
>>>> As for the tests, please get somebody else to review this. I strongly
>>>> believe checking full page writes here could be a source of
>>>> instability.
>>>
>>>
>>> I'm also a little bit dubious about it. The initial checkpoint should make
>>> things stable (of course unless full_page_writes is disabled), and Cfbot also
>>> seems happy about it. At least keeping it for the temporary tables test
>>> shouldn't be a problem.
>>
>> Temp tables should show zero FPI WAL records, true :)
>>
>> I have no objections to the patch.
>
>
> I'm attaching a v5 with fp records only for temp tables, so there's no risk of
> instability. As I previously said I'm fine with your two patches, so unless
> you have objections on the fpi test for temp tables or the documentation
> changes, I believe those should be ready for committer.

You added the columns into pg_stat_database, but seem to forget to
update the document for pg_stat_database.

Is it really reasonable to add the columns for vacuum's WAL usage into
pg_stat_database? I'm not sure how much the information about
the amount of WAL generated by vacuum per database is useful.
Isn't it better to make VACUUM VERBOSE and autovacuum log include
that information, instead, to see how much each vacuum activity
generates the WAL? Sorry if this discussion has already been done
upthread.

Regards,

--
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2020-03-19 12:33:09 Re: Internal key management system
Previous Message Kyotaro Horiguchi 2020-03-19 11:30:04 Re: shared-memory based stats collector