Re: wal stats questions

From: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>, Masahiro Ikeda <ikedamsh(at)oss(dot)nttdata(dot)com>
Cc: torikoshia <torikoshia(at)oss(dot)nttdata(dot)com>, PostgreSQL Development <pgsql-hackers(at)postgresql(dot)org>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
Subject: Re: wal stats questions
Date: 2021-04-23 07:30:27
Message-ID: b6433a7d-0326-43ce-aac5-3fed293835bc@oss.nttdata.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2021/04/23 10:25, Andres Freund wrote:
> Hi,
>
> On 2021-04-23 09:26:17 +0900, Masahiro Ikeda wrote:
>> On 2021/04/23 0:36, Andres Freund wrote:
>>> On Thu, Apr 22, 2021, at 06:42, Fujii Masao wrote:
>>>> On 2021/04/21 18:31, Masahiro Ikeda wrote:
>>>>>> BTW, is it better to change PgStat_Counter from int64 to uint64 because> there aren't any counters with negative number?
>>>> On second thought, it's ok even if the counters like wal_records get overflowed.
>>>> Because they are always used to calculate the diff between the previous and
>>>> current counters. Even when the current counters are overflowed and
>>>> the previous ones are not, WalUsageAccumDiff() seems to return the right
>>>> diff of them. If this understanding is right, I'd withdraw my comment and
>>>> it's ok to use "long" type for those counters. Thought?
>>>
>>> Why long? It's of a platform dependent size, which doesn't seem useful here?
>>
>> I think it's ok to unify uint64. Although it's better to use small size for
>> cache, the idea works well for only some platform which use 4bytes for "long".
>>
>>
>> (Although I'm not familiar with the standardization...)
>> It seems that we need to adopt unsinged long if use "long", which may be 4bytes.
>>
>> I though WalUsageAccumDiff() seems to return the right value too. But, I
>> researched deeply and found that ISO/IEC 9899:1999 defines unsinged integer
>> never overflow(2.6.5 Types 9th section) although it doesn't define overflow of
>> signed integer type.
>>
>> If my understanding is right, the definition only guarantee
>> WalUsageAccumDiff() returns the right value only if the type is unsigned
>> integer. So, it's safe to change "long" to "unsigned long".
>
> I think this should just use 64bit counters. Emitting more than 4
> billion records in one transaction isn't actually particularly rare. And

Right. I agree to change the types of the counters to int64.

I think that it's better to make this change as a separate patch from
the changes for pg_stat_wal view.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Joel Jacobson 2021-04-23 07:30:47 [PATCH] Re: pg_identify_object_as_address() doesn't support pg_event_trigger oids
Previous Message Fujii Masao 2021-04-23 07:10:37 Re: Feature improvement: can we add queryId for pg_catalog.pg_stat_activity view?