Horton

Thread

  1. [PATCH] Support Int64 GUCs

    Aleksander Alekseev <aleksander@timescale.com> — 2024-09-12T11:08:15Z

    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() ?

Secondly, DefineCustomInt64Variable() is not test-covered. Turned out
it was not even defined (although declared) in the original patch.
This was fixed in the attached version. Maybe one of the test modules
could use it even if it makes little sense for this particular module?
For instance, test/modules/worker_spi/ could use it for
worker_spi.naptime.

Last but not least, large values like 12345678912345 could be
difficult to read. Perhaps we should also support 12_345_678_912_345
syntax? This is not implemented in the attached patch and arguably
could be discussed separately when and if we merge it.

Thoughts?

[1]: https://www.postgresql.org/message-id/flat/CAJ7c6TP9Ce021ebJ%3D5zOhMjiG3Wqg4hO6Mg0WsccErvAD9vZYA%40mail.gmail.com

--
Best regards,
Aleksander Alekseev

  2. Re: [PATCH] Support Int64 GUCs

    Pavel Borisov <pashkin.elfe@gmail.com> — 2024-09-12T11:29:14Z

    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 be non-atomic on older
architectures.


> Secondly, DefineCustomInt64Variable() is not test-covered. Turned out
> it was not even defined (although declared) in the original patch.
> This was fixed in the attached version. Maybe one of the test modules
> could use it even if it makes little sense for this particular module?
> For instance, test/modules/worker_spi/ could use it for
> worker_spi.naptime.
>
> Last but not least, large values like 12345678912345 could be
> difficult to read. Perhaps we should also support 12_345_678_912_345
> syntax? This is not implemented in the attached patch and arguably
> could be discussed separately when and if we merge it.
>

I think 12345678912345 is good enough. Underscore dividers make reading
little bit easier but look weird overall. I can't remember other places
where we output long numbers with dividers.

Regards,
Pavel Borisov
Supabase

  3. Re: [PATCH] Support Int64 GUCs

    Aleksander Alekseev <aleksander@timescale.com> — 2024-09-12T11:34:40Z

    Hi Pavel,

> 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.

Thanks for the feedback.

> I think 12345678912345 is good enough. Underscore dividers make reading little bit easier but look weird overall. I can't remember other places where we output long numbers with dividers.

We already support this in SQL:

psql (18devel)
=# SELECT 123_456;
 ?column?
----------
   123456

-- 
Best regards,
Aleksander Alekseev

  4. Re: [PATCH] Support Int64 GUCs

    Nathan Bossart <nathandbossart@gmail.com> — 2024-09-12T13:46:15Z

    On Thu, Sep 12, 2024 at 02:08:15PM +0300, Aleksander Alekseev wrote:
> 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 don't understand why we would want to make these GUCs 64-bit.  All of the
allowed values fit in an int32, so AFAICT this would only serve to mislead
users into thinking they could set these much higher than they can/should.

TBH I'm quite skeptical that this would even be particularly useful for
extension authors.  In what cases would a floating point value not suffice?
I'm not totally opposed to the idea of 64-bit GUCs, but I'd like more
information about the motivation.

-- 
nathan

  5. Re: [PATCH] Support Int64 GUCs

    Alexander Korotkov <aekorotkov@gmail.com> — 2024-09-20T12:13:29Z

    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 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

  6. Re: [PATCH] Support Int64 GUCs

    Alexander Korotkov <aekorotkov@gmail.com> — 2024-09-20T12:33:21Z

    Hi, Aleksander!

Thank you for your work on this subject.

On Thu, Sep 12, 2024 at 2:08 PM Aleksander Alekseev
<aleksander@timescale.com> wrote:
> 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() ?

It doesn't look like these *_age GUCs could benefit from being 64-bit,
before 64-bit transaction ids get in.  However, I think there are some
better candidates.

autovacuum_vacuum_threshold
autovacuum_vacuum_insert_threshold
autovacuum_analyze_threshold

This GUCs specify number of tuples before vacuum/analyze.  That could
be more than 2^31.  With large tables of small tuples, I can't even
say this is always impractical to have values greater than 2^31.

> Secondly, DefineCustomInt64Variable() is not test-covered. Turned out
> it was not even defined (although declared) in the original patch.
> This was fixed in the attached version. Maybe one of the test modules
> could use it even if it makes little sense for this particular module?
> For instance, test/modules/worker_spi/ could use it for
> worker_spi.naptime.

I don't think there are good candidates among existing extension GUCs.
I think we could add something for pure testing purposes somewhere in
src/test/modules.

> Last but not least, large values like 12345678912345 could be
> difficult to read. Perhaps we should also support 12_345_678_912_345
> syntax? This is not implemented in the attached patch and arguably
> could be discussed separately when and if we merge it.

I also think we're good with 12345678912345 so far.

------
Regards,
Alexander Korotkov
Supabase

  7. Re: [PATCH] Support Int64 GUCs

    Aleksander Alekseev <aleksander@timescale.com> — 2024-09-24T09:27:20Z

    Hi, Alexander!

> Thank you for your work on this subject.

Thanks for your feedback.

> It doesn't look like these *_age GUCs could benefit from being 64-bit,
> before 64-bit transaction ids get in.  However, I think there are some
> better candidates.
>
> autovacuum_vacuum_threshold
> autovacuum_vacuum_insert_threshold
> autovacuum_analyze_threshold
>
> This GUCs specify number of tuples before vacuum/analyze.  That could
> be more than 2^31.  With large tables of small tuples, I can't even
> say this is always impractical to have values greater than 2^31.

Sounds good to me. Fixed.

> > Secondly, DefineCustomInt64Variable() is not test-covered. Turned out
> > it was not even defined (although declared) in the original patch.
> > This was fixed in the attached version. Maybe one of the test modules
> > could use it even if it makes little sense for this particular module?
> > For instance, test/modules/worker_spi/ could use it for
> > worker_spi.naptime.
>
> I don't think there are good candidates among existing extension GUCs.
> I think we could add something for pure testing purposes somewhere in
> src/test/modules.

I found a great candidate in src/test/modules/delay_execution:

```
    DefineCustomIntVariable("delay_execution.post_planning_lock_id",
                            "Sets the advisory lock ID to be
locked/unlocked after planning.",
```

Advisory lock IDs are bigints [1]. I modified the module to use Int64's.

I guess it may also answer Nathan's question.

> > Last but not least, large values like 12345678912345 could be
> > difficult to read. Perhaps we should also support 12_345_678_912_345
> > syntax? This is not implemented in the attached patch and arguably
> > could be discussed separately when and if we merge it.
>
> I also think we're good with 12345678912345 so far.

Fair enough.

PFA the updated patch.

[1]: https://www.postgresql.org/docs/current/functions-admin.html#FUNCTIONS-ADVISORY-LOCKS

--
Best regards,
Aleksander Alekseev

  8. Re: [PATCH] Support Int64 GUCs

    Aleksander Alekseev <aleksander@timescale.com> — 2024-09-24T09:35:03Z

    Hi,

> PFA the updated patch.

It is worth mentioning that v2 should not be merged as is.
Particularly although it changes the following GUCs:

> autovacuum_vacuum_threshold
> autovacuum_vacuum_insert_threshold
> autovacuum_analyze_threshold

... it doesn't affect the code that uses these GUCs which results in
casting int64s to ints.

I would appreciate a bit more feedback on v2. If the community is fine
with modifying these GUCs I will correct the patch in this respect.

-- 
Best regards,
Aleksander Alekseev

  9. Re: [PATCH] Support Int64 GUCs

    Japin Li <japinli@hotmail.com> — 2024-09-24T10:55:16Z

    Hi, Aleksander Alekseev

Thanks for updating the patch.

> On Sep 24, 2024, at 17:27, Aleksander Alekseev <aleksander@timescale.com> wrote:
> 
> Hi, Alexander!
> 
>> Thank you for your work on this subject.
> 
> Thanks for your feedback.
> 
>> It doesn't look like these *_age GUCs could benefit from being 64-bit,
>> before 64-bit transaction ids get in.  However, I think there are some
>> better candidates.
>> 
>> autovacuum_vacuum_threshold
>> autovacuum_vacuum_insert_threshold
>> autovacuum_analyze_threshold
>> 
>> This GUCs specify number of tuples before vacuum/analyze.  That could
>> be more than 2^31.  With large tables of small tuples, I can't even
>> say this is always impractical to have values greater than 2^31.
> 
> Sounds good to me. Fixed.

I found the autovacuum_vacuum_threshold, autovacuum_vacuum_insert_threshold
and autovacuum_analyze_threshold is change to int64 for relation option,
however the GUCs are still integers.

```
postgres=# select * from pg_settings where name = 'autovacuum_vacuum_threshold' \gx
-[ RECORD 1 ]---+------------------------------------------------------------
name            | autovacuum_vacuum_threshold
setting         | 50
unit            |
category        | Autovacuum
short_desc      | Minimum number of tuple updates or deletes prior to vacuum.
extra_desc      |
context         | sighup
vartype         | integer
source          | default
min_val         | 0
max_val         | 2147483647
enumvals        |
boot_val        | 50
reset_val       | 50
sourcefile      |
sourceline      |
pending_restart | f
```

Is there something I missed?

> 
>>> Secondly, DefineCustomInt64Variable() is not test-covered. Turned out
>>> it was not even defined (although declared) in the original patch.
>>> This was fixed in the attached version. Maybe one of the test modules
>>> could use it even if it makes little sense for this particular module?
>>> For instance, test/modules/worker_spi/ could use it for
>>> worker_spi.naptime.
>> 
>> I don't think there are good candidates among existing extension GUCs.
>> I think we could add something for pure testing purposes somewhere in
>> src/test/modules.
> 
> I found a great candidate in src/test/modules/delay_execution:
> 
> ```
>    DefineCustomIntVariable("delay_execution.post_planning_lock_id",
>                            "Sets the advisory lock ID to be
> locked/unlocked after planning.",
> ```
> 
> Advisory lock IDs are bigints [1]. I modified the module to use Int64's.

I check the delay_execution.post_planning_lock_id parameter, and it’s varitype
is int64, maybe bigint is better, see [1].

```
postgres=# select * from pg_settings where name = 'delay_execution.post_planning_lock_id' \gx
-[ RECORD 1 ]---+----------------------------------------------------------------
name            | delay_execution.post_planning_lock_id
setting         | 0
unit            |
category        | Customized Options
short_desc      | Sets the advisory lock ID to be locked/unlocked after planning.
extra_desc      | Zero disables the delay.
context         | user
vartype         | int64
source          | default
min_val         | 0
max_val         | 9223372036854775807
enumvals        |
boot_val        | 0
reset_val       | 0
sourcefile      |
sourceline      |
pending_restart | f
```

[1] https://www.postgresql.org/docs/current/datatype-numeric.html


--
Regrads,
Japin Li

  10. Re: [PATCH] Support Int64 GUCs

    Nathan Bossart <nathandbossart@gmail.com> — 2024-09-24T14:48:04Z

    On Tue, Sep 24, 2024 at 12:27:20PM +0300, Aleksander Alekseev wrote:
>> It doesn't look like these *_age GUCs could benefit from being 64-bit,
>> before 64-bit transaction ids get in.  However, I think there are some
>> better candidates.
>>
>> autovacuum_vacuum_threshold
>> autovacuum_vacuum_insert_threshold
>> autovacuum_analyze_threshold
>>
>> This GUCs specify number of tuples before vacuum/analyze.  That could
>> be more than 2^31.  With large tables of small tuples, I can't even
>> say this is always impractical to have values greater than 2^31.
>
> [...]
> 
> I found a great candidate in src/test/modules/delay_execution:
> 
> ```
>     DefineCustomIntVariable("delay_execution.post_planning_lock_id",
>                             "Sets the advisory lock ID to be
> locked/unlocked after planning.",
> ```
> 
> Advisory lock IDs are bigints [1]. I modified the module to use Int64's.
> 
> I guess it may also answer Nathan's question.

Hm.  I'm not sure I find any of these to be particularly convincing
examples of why we need int64 GUCs.  Yes, the GUCs in question could
potentially be set to higher values, but I've yet to hear of this being a
problem in practice.  We might not want to encourage such high values,
either.

-- 
nathan

  11. Re: [PATCH] Support Int64 GUCs

    Aleksander Alekseev <aleksander@timescale.com> — 2024-09-25T11:03:12Z

    Hi,

> I found the autovacuum_vacuum_threshold, autovacuum_vacuum_insert_threshold
> and autovacuum_analyze_threshold is change to int64 for relation option,
> however the GUCs are still integers.
>
> ```
> postgres=# select * from pg_settings where name = 'autovacuum_vacuum_threshold' \gx
> -[ RECORD 1 ]---+------------------------------------------------------------
> name            | autovacuum_vacuum_threshold
> setting         | 50
> unit            |
> category        | Autovacuum
> short_desc      | Minimum number of tuple updates or deletes prior to vacuum.
> extra_desc      |
> context         | sighup
> vartype         | integer
> source          | default
> min_val         | 0
> max_val         | 2147483647
> enumvals        |
> boot_val        | 50
> reset_val       | 50
> sourcefile      |
> sourceline      |
> pending_restart | f
> ```
>
> Is there something I missed?

No, you found a bug. The patch didn't change ConfigureNamesInt64[]
thus these GUCs were still treated as int32s.

Here is the corrected patch v3. Thanks!

=# select * from pg_settings where name = 'autovacuum_vacuum_threshold';
-[ RECORD 1 ]---+------------------------------------------------------------
name            | autovacuum_vacuum_threshold
setting         | 1234605616436508552
unit            |
category        | Autovacuum
short_desc      | Minimum number of tuple updates or deletes prior to vacuum.
extra_desc      |
context         | sighup
vartype         | int64
source          | configuration file
min_val         | 0
max_val         | 9223372036854775807
enumvals        |
boot_val        | 50
reset_val       | 1234605616436508552
sourcefile      | /Users/eax/pginstall/data-master/postgresql.conf
sourceline      | 664
pending_restart | f


-- 
Best regards,
Aleksander Alekseev

  12. Re: [PATCH] Support Int64 GUCs

    Japin Li <japinli@hotmail.com> — 2024-09-25T14:38:46Z

    On Sep 25, 2024, at 19:03, Aleksander Alekseev <aleksander@timescale.com> wrote:

Hi,

I found the autovacuum_vacuum_threshold, autovacuum_vacuum_insert_threshold
and autovacuum_analyze_threshold is change to int64 for relation option,
however the GUCs are still integers.

```
postgres=# select * from pg_settings where name = 'autovacuum_vacuum_threshold' \gx
-[ RECORD 1 ]---+------------------------------------------------------------
name            | autovacuum_vacuum_threshold
setting         | 50
unit            |
category        | Autovacuum
short_desc      | Minimum number of tuple updates or deletes prior to vacuum.
extra_desc      |
context         | sighup
vartype         | integer
source          | default
min_val         | 0
max_val         | 2147483647
enumvals        |
boot_val        | 50
reset_val       | 50
sourcefile      |
sourceline      |
pending_restart | f
```

Is there something I missed?

No, you found a bug. The patch didn't change ConfigureNamesInt64[]
thus these GUCs were still treated as int32s.

Here is the corrected patch v3. Thanks!

=# select * from pg_settings where name = 'autovacuum_vacuum_threshold';
-[ RECORD 1 ]---+------------------------------------------------------------
name            | autovacuum_vacuum_threshold
setting         | 1234605616436508552
unit            |
category        | Autovacuum
short_desc      | Minimum number of tuple updates or deletes prior to vacuum.
extra_desc      |
context         | sighup
vartype         | int64
source          | configuration file
min_val         | 0
max_val         | 9223372036854775807
enumvals        |
boot_val        | 50
reset_val       | 1234605616436508552
sourcefile      | /Users/eax/pginstall/data-master/postgresql.conf
sourceline      | 664
pending_restart | f


Thanks for updating the patch.

After testing the v3 patch, I found it cannot correctly handle the number with underscore.

See:

```
postgres=# alter system set autovacuum_vacuum_threshold to 2_147_483_648;
ERROR:  invalid value for parameter "autovacuum_vacuum_threshold": "2_147_483_648"
postgres=# alter system set autovacuum_vacuum_threshold to 2_147_483_647;
ALTER SYSTEM
```

IIRC, the lexer only supports integers but not int64.

--
Best regards,
Japin Li

  13. Re: [PATCH] Support Int64 GUCs

    Aleksander Alekseev <aleksander@timescale.com> — 2024-09-25T14:44:28Z

    Hi,

> ```
> postgres=# alter system set autovacuum_vacuum_threshold to 2_147_483_648;
> ERROR:  invalid value for parameter "autovacuum_vacuum_threshold": "2_147_483_648"
> postgres=# alter system set autovacuum_vacuum_threshold to 2_147_483_647;
> ALTER SYSTEM
> ```
>
> IIRC, the lexer only supports integers but not int64.

Right. Supporting underscores for GUCs was discussed above but not
implemented in the patch. As Alexander rightly pointed out this is not
a priority and can be discussed separately.

-- 
Best regards,
Aleksander Alekseev

  14. Re: [PATCH] Support Int64 GUCs

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-09-25T15:08:02Z

    FWIW, I agree with the upthread opinions that we shouldn't do this
(invent int64 GUCs).  I don't think we need the added code bloat
and risk of breaking user code that isn't expecting this new GUC
type.  We invented the notion of GUC units in part to ensure that
int32 GUCs could be adapted to handle potentially-large numbers.
And there's always the fallback position of using a float8 GUC
if you really feel you need a wider range.

			regards, tom lane

  15. Re: [PATCH] Support Int64 GUCs

    Alexander Korotkov <aekorotkov@gmail.com> — 2024-09-25T18:04:36Z

    Hi, Tom!

On Wed, Sep 25, 2024 at 6:08 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> FWIW, I agree with the upthread opinions that we shouldn't do this
> (invent int64 GUCs).  I don't think we need the added code bloat
> and risk of breaking user code that isn't expecting this new GUC
> type.  We invented the notion of GUC units in part to ensure that
> int32 GUCs could be adapted to handle potentially-large numbers.
> And there's always the fallback position of using a float8 GUC
> if you really feel you need a wider range.

Thank you for your feedback.
Do you think we don't need int64 GUCs just now, when 64-bit
transaction ids are far from committable shape?  Or do you think we
don't need int64 GUCs even if we have 64-bit transaction ids?  If yes,
what do you think we should use for *_age variables with 64-bit
transaction ids?

------
Regards,
Alexander Korotkov
Supabase

  16. Re: [PATCH] Support Int64 GUCs

    wenhui qiu <qiuwenhuifx@gmail.com> — 2024-09-26T09:30:47Z

     Hi Alexander
      I think we need int64 GUCs, due to these  parameters（
autovacuum_freeze_table_age, autovacuum_freeze_max_age,When a table age is
greater than any of these parameters an aggressive vacuum will be
performed, When we implementing xid64, is it still necessary to be in the
int range? btw, I have a suggestion to record a warning in the log when the
table age exceeds the int maximum. These default values we can set a
reasonable values ,for example autovacuum_freeze_max_age=4294967295 or
8589934592.


Thanks

Alexander Korotkov <aekorotkov@gmail.com> 于2024年9月26日周四 02:05写道：

> Hi, Tom!
>
> On Wed, Sep 25, 2024 at 6:08 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > FWIW, I agree with the upthread opinions that we shouldn't do this
> > (invent int64 GUCs).  I don't think we need the added code bloat
> > and risk of breaking user code that isn't expecting this new GUC
> > type.  We invented the notion of GUC units in part to ensure that
> > int32 GUCs could be adapted to handle potentially-large numbers.
> > And there's always the fallback position of using a float8 GUC
> > if you really feel you need a wider range.
>
> Thank you for your feedback.
> Do you think we don't need int64 GUCs just now, when 64-bit
> transaction ids are far from committable shape?  Or do you think we
> don't need int64 GUCs even if we have 64-bit transaction ids?  If yes,
> what do you think we should use for *_age variables with 64-bit
> transaction ids?
>
> ------
> Regards,
> Alexander Korotkov
> Supabase
>
>
>

  17. Re: [PATCH] Support Int64 GUCs

    Alexander Korotkov <aekorotkov@gmail.com> — 2024-09-26T15:55:07Z

    On Thu, Sep 26, 2024 at 12:30 PM wenhui qiu <qiuwenhuifx@gmail.com> wrote:
>       I think we need int64 GUCs, due to these  parameters（ autovacuum_freeze_table_age, autovacuum_freeze_max_age,When a table age is greater than any of these parameters an aggressive vacuum will be performed, When we implementing xid64, is it still necessary to be in the int range? btw, I have a suggestion to record a warning in the log when the table age exceeds the int maximum. These default values we can set a reasonable values ,for example autovacuum_freeze_max_age=4294967295 or 8589934592.

In principle, even with 64-bit transaction ids we could specify *_age
GUCs as int32 with bigger units or as float8.  That feels a bit
awkward for me.  This is why I queried more about Tom's opinion in
more details: did he propose to wait with int64 GUCs before we have
64-bit transaction ids, or give up about them completely?

Links.
1. https://www.postgresql.org/message-id/3649727.1727276882%40sss.pgh.pa.us

------
Regards,
Alexander Korotkov
Supabase

  18. Re: [PATCH] Support Int64 GUCs

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-09-26T16:39:10Z

    Alexander Korotkov <aekorotkov@gmail.com> writes:
> Do you think we don't need int64 GUCs just now, when 64-bit
> transaction ids are far from committable shape?  Or do you think we
> don't need int64 GUCs even if we have 64-bit transaction ids?  If yes,
> what do you think we should use for *_age variables with 64-bit
> transaction ids?

I seriously doubt that _age values exceeding INT32_MAX would be
useful, even in the still-extremely-doubtful situation that we
get to true 64-bit XIDs.  But if you think we must have that,
we could still use float8 GUCs for them.  float8 is exact up
to 2^53 (given IEEE math), and you certainly aren't going to
convince me that anyone needs _age values exceeding that.
For that matter, an imprecise representation of such an age
limit would still be all right wouldn't it?

			regards, tom lane

  19. Re: [PATCH] Support Int64 GUCs

    Aleksander Alekseev <aleksander@timescale.com> — 2024-09-29T21:31:33Z

    Hi,

> I seriously doubt that _age values exceeding INT32_MAX would be
> useful, even in the still-extremely-doubtful situation that we
> get to true 64-bit XIDs.  But if you think we must have that,
> we could still use float8 GUCs for them.  float8 is exact up
> to 2^53 (given IEEE math), and you certainly aren't going to
> convince me that anyone needs _age values exceeding that.
> For that matter, an imprecise representation of such an age
> limit would still be all right wouldn't it?

Considering the recent feedback. I'm marking the corresponding CF
entry as "Rejected".

Thanks to everyone involved!

-- 
Best regards,
Aleksander Alekseev

  20. Re: [PATCH] Support Int64 GUCs

    Evgeny <evorop@gmail.com> — 2024-12-08T14:11:42Z

    Hi hackers!

Upgraded the "Int64 GUC" patch in order to conform to f3f06b13308e3 
updates. Rebased and tested upon the current master (3f9b9621766). The 
patch is still needed to be up to date as a part of the xid64 solution.

Best regards,
Evgeny Voropaev,
TantorLabs, LLC.
<evorop@gmail.com>
<evgeny.voropaev@tantorlabs.ru>

  21. Re: [PATCH] Support Int64 GUCs

    Evgeny Voropaev <evorop.wiki@gmail.com> — 2024-12-08T14:16:25Z

    Hi hackers!

Upgraded the "Int64 GUC" patch in order to conform to f3f06b13308e3 
updates. Rebased and tested upon the current master (3f9b9621766). The 
patch is still needed to be up to date as a part of the xid64 solution.

Best regards,
Evgeny Voropaev,
TantorLabs, LLC.
<evorop@gmail.com>
<evgeny.voropaev@tantorlabs.ru>

  22. Re: [PATCH] Support Int64 GUCs

    wenhui qiu <qiuwenhuifx@gmail.com> — 2024-12-10T13:32:17Z

    Hi aleksander
   Didn't you mark this patch as rejected last time? Do we still need to
continue this path?


Thanks

On Sun, Dec 8, 2024 at 10:16 PM Evgeny Voropaev <evorop.wiki@gmail.com>
wrote:

> Hi hackers!
>
> Upgraded the "Int64 GUC" patch in order to conform to f3f06b13308e3
> updates. Rebased and tested upon the current master (3f9b9621766). The
> patch is still needed to be up to date as a part of the xid64 solution.
>
> Best regards,
> Evgeny Voropaev,
> TantorLabs, LLC.
> <evorop@gmail.com>
> <evgeny.voropaev@tantorlabs.ru>

  23. Re: [PATCH] Support Int64 GUCs

    Pavel Borisov <pashkin.elfe@gmail.com> — 2024-12-10T13:43:14Z

    Hi, Wenhui!

On Tue, 10 Dec 2024 at 17:32, wenhui qiu <qiuwenhuifx@gmail.com> wrote:

> Hi aleksander
>    Didn't you mark this patch as rejected last time? Do we still need to
> continue this path?
>
>
> Thanks
>
> On Sun, Dec 8, 2024 at 10:16 PM Evgeny Voropaev <evorop.wiki@gmail.com>
> wrote:
>
>> Hi hackers!
>>
>> Upgraded the "Int64 GUC" patch in order to conform to f3f06b13308e3
>> updates. Rebased and tested upon the current master (3f9b9621766). The
>> patch is still needed to be up to date as a part of the xid64 solution.
>>
>> Best regards,
>> Evgeny Voropaev,
>> TantorLabs, LLC.
>> <evorop@gmail.com>
>> <evgeny.voropaev@tantorlabs.ru>
>
>
I see Aleksander worked on this patch (with many other hackers) and marked
it as rejected. And now Evgeniy, a new developer wants to continue and
rebase this patch and also the patches in another 64-xid thread
disregarding the fact that the patch is rejected. It's not a crime. It's
rejected, true.

Regards,
Pavel Borisov

  24. Re: [PATCH] Support Int64 GUCs

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-12-10T15:13:37Z

    Pavel Borisov <pashkin.elfe@gmail.com> writes:
> I see Aleksander worked on this patch (with many other hackers) and marked
> it as rejected. And now Evgeniy, a new developer wants to continue and
> rebase this patch and also the patches in another 64-xid thread
> disregarding the fact that the patch is rejected. It's not a crime. It's
> rejected, true.

No, but it's a waste of time.  Just rebasing the patch does nothing to
counter the arguments that made us reject it before.

			regards, tom lane

  25. Re: [PATCH] Support Int64 GUCs

    Pavel Borisov <pashkin.elfe@gmail.com> — 2024-12-10T15:22:29Z

    On Tue, 10 Dec 2024 at 19:13, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Pavel Borisov <pashkin.elfe@gmail.com> writes:
> > I see Aleksander worked on this patch (with many other hackers) and
> marked
> > it as rejected. And now Evgeniy, a new developer wants to continue and
> > rebase this patch and also the patches in another 64-xid thread
> > disregarding the fact that the patch is rejected. It's not a crime. It's
> > rejected, true.
>
> No, but it's a waste of time.  Just rebasing the patch does nothing to
> counter the arguments that made us reject it before.
>

Tom, I agree! Just wrote this because of probable confusion of names by
Wenhui above.

Regards,
Pavel