On 2021/04/23 16:30, Fujii Masao wrote:
>
>
> 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.
Thanks for your comments.
OK, I separate two patches.
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")
Second one has the changes for the type of the BufferUsage's and WalUsage's
members. I change the type from long to int64. Is it better to make new thread?
("v6-0002-change-the-data-type-of-XXXUsage-from-long-to-int64.patch")
Regards,
--
Masahiro Ikeda
NTT DATA CORPORATION
