On Thu, Sep 12, 2024 at 2:29 PM Pavel Borisov <pashkin.e...@gmail.com> wrote: > Hi, Alexander! > Thank you for working on this! > > On Thu, 12 Sept 2024 at 15:08, Aleksander Alekseev <aleksan...@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 be non-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
