On Thu, Sep 12, 2024 at 2:29 PM Pavel Borisov <pashkin.elfe@gmail.com> wrote:
> Hi, Alexander!
> Thank you for working on this!
>
> On Thu, 12 Sept 2024 at 15:08, Aleksander Alekseev <aleksander@timescale.com> wrote:
>>
>> Hi,
>>
>> Attached is a self-sufficient patch extracted from a larger patchset
>> [1]. The entire patchset probably will not proceed further in the
>> nearest future. Since there was interest in this particular patch it
>> deserves being discussed in a separate thread.
>>
>> Currently we support 32-bit integer values in GUCs, but don't support
>> 64-bit ones. The proposed patch adds this support.
>>
>> Firstly, it adds DefineCustomInt64Variable() which can be used by the
>> extension authors.
>>
>> Secondly, the following core GUCs are made 64-bit:
>>
>> ```
>> autovacuum_freeze_min_age
>> autovacuum_freeze_max_age
>> autovacuum_freeze_table_age
>> autovacuum_multixact_freeze_min_age
>> autovacuum_multixact_freeze_max_age
>> autovacuum_multixact_freeze_table_age
>> ```
>>
>> I see several open questions with the patch in its current state.
>>
>> Firstly, I'm not sure if it is beneficial to affect the named GUCs out
>> of the context of the larger patchset. Perhaps we have better GUCs
>> that could benefit from being 64-bit? Or should we just leave alone
>> the core GUCs and focus on providing DefineCustomInt64Variable() ?
>
> I think the direction is good and delivering 64-bit GUCs is very much worth committing.
> The patch itself looks good, but we could need to add locks against concurrently modifying 64-bit values, which could
benon-atomic on older architectures.
GUCs are located in the local memory. No concurrent read/writes of
them are possible. It might happen that SIGHUP comes during
read/write of the GUC variable. But, that's protected the other way:
SignalHandlerForConfigReload() just sets the ConfigReloadPending flag,
which is processed during CHECK_FOR_INTERRUPTS(). So, I don't see a
need to locks here.
------
Regards,
Alexander Korotkov
Supabase
