Обсуждение: Re: Add log_autovacuum_{vacuum|analyze}_min_duration

Поиск
Список
Период
Сортировка

Re: Add log_autovacuum_{vacuum|analyze}_min_duration

От
Michael Banck
Дата:
Hi,

On Tue, Jun 03, 2025 at 03:35:20PM +0900, Shinya Kato wrote:
> I am proposing the introduction of two new GUC parameters,
> log_autovacuum_{vacuum|analyze}_min_duration, to replace the existing
> log_autovacuum_min_duration.

How about adding log_autoanalyze_min_duration instead? That would still
slightly retcon the log_autovacuum_min_duration meaning/semantics by no
longer logging autoanalyze unless the new GUC is set, but at least not
rename the GUC and make both shorter while still being comprehensible
IMO. Not sure what others think?


Michael





Re: Add log_autovacuum_{vacuum|analyze}_min_duration

От
Shinya Kato
Дата:
Thank you for the comment!

On Tue, Jun 3, 2025 at 4:42 PM Michael Banck <mbanck@gmx.net> wrote:
>
> Hi,
>
> On Tue, Jun 03, 2025 at 03:35:20PM +0900, Shinya Kato wrote:
> > I am proposing the introduction of two new GUC parameters,
> > log_autovacuum_{vacuum|analyze}_min_duration, to replace the existing
> > log_autovacuum_min_duration.
>
> How about adding log_autoanalyze_min_duration instead? That would still
> slightly retcon the log_autovacuum_min_duration meaning/semantics by no
> longer logging autoanalyze unless the new GUC is set, but at least not
> rename the GUC and make both shorter while still being comprehensible
> IMO. Not sure what others think?

I surely think adding log_autoanalyze_min_duration is simpler and
shorter, but the reason I chose this GUC name is for consistency with
other autovacuum parameters. Existing autovacuum parameters that have
separate settings for vacuum and analyze operations follow the pattern
autovacuum_{vacuum|analyze}_*.
https://www.postgresql.org/docs/devel/runtime-config-vacuum.html#RUNTIME-CONFIG-AUTOVACUUM

Shinya Kato
NTT OSS Center



Re: Add log_autovacuum_{vacuum|analyze}_min_duration

От
Michael Banck
Дата:
Hi,

On Tue, Jun 03, 2025 at 05:25:40PM +0900, Shinya Kato wrote:
> On Tue, Jun 3, 2025 at 4:42 PM Michael Banck <mbanck@gmx.net> wrote:
> > On Tue, Jun 03, 2025 at 03:35:20PM +0900, Shinya Kato wrote:
> > > I am proposing the introduction of two new GUC parameters,
> > > log_autovacuum_{vacuum|analyze}_min_duration, to replace the existing
> > > log_autovacuum_min_duration.
> >
> > How about adding log_autoanalyze_min_duration instead? That would still
> > slightly retcon the log_autovacuum_min_duration meaning/semantics by no
> > longer logging autoanalyze unless the new GUC is set, but at least not
> > rename the GUC and make both shorter while still being comprehensible
> > IMO. Not sure what others think?
> 
> I surely think adding log_autoanalyze_min_duration is simpler and
> shorter, but the reason I chose this GUC name is for consistency with
> other autovacuum parameters. Existing autovacuum parameters that have
> separate settings for vacuum and analyze operations follow the pattern
> autovacuum_{vacuum|analyze}_*.
> https://www.postgresql.org/docs/devel/runtime-config-vacuum.html#RUNTIME-CONFIG-AUTOVACUUM

Right, but the GUCs that directly affect either vacuum or autovacuum
behaviour need the qualification (and then vacuum/analyze on top of it).
I think we have less constraints with the logging GUC and do not need to
mirror the behaviorial GUCs at all costs. But again, that is just my two
cents.

Unless we want to have 4 logging GUCs
(log_{auto,}vacuum_{vacuum,analyze}_min_duration) which I think would
be overkill?


Michael



Re: Add log_autovacuum_{vacuum|analyze}_min_duration

От
Nathan Bossart
Дата:
On Tue, Jun 03, 2025 at 10:57:11AM +0200, Michael Banck wrote:
> On Tue, Jun 03, 2025 at 05:25:40PM +0900, Shinya Kato wrote:
>> I surely think adding log_autoanalyze_min_duration is simpler and
>> shorter, but the reason I chose this GUC name is for consistency with
>> other autovacuum parameters. Existing autovacuum parameters that have
>> separate settings for vacuum and analyze operations follow the pattern
>> autovacuum_{vacuum|analyze}_*.
>> https://www.postgresql.org/docs/devel/runtime-config-vacuum.html#RUNTIME-CONFIG-AUTOVACUUM
> 
> Right, but the GUCs that directly affect either vacuum or autovacuum
> behaviour need the qualification (and then vacuum/analyze on top of it).
> I think we have less constraints with the logging GUC and do not need to
> mirror the behaviorial GUCs at all costs. But again, that is just my two
> cents.

I lean towards log_autovacuum_{vacuum|analyze}_min_duration.  If
log_autovacuum_min_duration didn't exist, that's probably the naming scheme
we'd go with.  However, I'm not sure we can get away with renaming
log_autovacuum_min_duration.  Presumably we'd need to at least keep it
around as a backward-compatibility GUC, and its behavior would probably
change, too (e.g., to only logging vacuums).  Maybe that's acceptable if we
buy the assertion that autoanalyze is typically much faster than autovacuum
(and so autoanalyzes weren't getting logged, anyway).

-- 
nathan



Re: Add log_autovacuum_{vacuum|analyze}_min_duration

От
Sami Imseih
Дата:
> On Tue, Jun 03, 2025 at 10:57:11AM +0200, Michael Banck wrote:
> > On Tue, Jun 03, 2025 at 05:25:40PM +0900, Shinya Kato wrote:
> >> I surely think adding log_autoanalyze_min_duration is simpler and
> >> shorter, but the reason I chose this GUC name is for consistency with
> >> other autovacuum parameters. Existing autovacuum parameters that have
> >> separate settings for vacuum and analyze operations follow the pattern
> >> autovacuum_{vacuum|analyze}_*.
> >> https://www.postgresql.org/docs/devel/runtime-config-vacuum.html#RUNTIME-CONFIG-AUTOVACUUM
> >
> > Right, but the GUCs that directly affect either vacuum or autovacuum
> > behaviour need the qualification (and then vacuum/analyze on top of it).
> > I think we have less constraints with the logging GUC and do not need to
> > mirror the behaviorial GUCs at all costs. But again, that is just my two
> > cents.
>
> I lean towards log_autovacuum_{vacuum|analyze}_min_duration.  If
> log_autovacuum_min_duration didn't exist, that's probably the naming scheme
> we'd go with.  However, I'm not sure we can get away with renaming
> log_autovacuum_min_duration.  Presumably we'd need to at least keep it
> around as a backward-compatibility GUC, and its behavior would probably
> change, too

I think deprecating a GUC like log_autovacuum_min_duration would be quite
difficult. May I suggest we keep its current behavior, which is to control
logging for both autoanalyze and autovacuum, and instead introduce only one
new GUC, log_autovacuum_analyze_min_duration, which defaults to -1? For
workloads that require different logging for autoanalyze, this new setting
can be enabled.

--
Sami Imseih
Amazon Web Services (AWS)



Re: Add log_autovacuum_{vacuum|analyze}_min_duration

От
Fujii Masao
Дата:

On 2025/06/04 4:32, Sami Imseih wrote:
>> On Tue, Jun 03, 2025 at 10:57:11AM +0200, Michael Banck wrote:
>>> On Tue, Jun 03, 2025 at 05:25:40PM +0900, Shinya Kato wrote:
>>>> I surely think adding log_autoanalyze_min_duration is simpler and
>>>> shorter, but the reason I chose this GUC name is for consistency with
>>>> other autovacuum parameters. Existing autovacuum parameters that have
>>>> separate settings for vacuum and analyze operations follow the pattern
>>>> autovacuum_{vacuum|analyze}_*.
>>>> https://www.postgresql.org/docs/devel/runtime-config-vacuum.html#RUNTIME-CONFIG-AUTOVACUUM
>>>
>>> Right, but the GUCs that directly affect either vacuum or autovacuum
>>> behaviour need the qualification (and then vacuum/analyze on top of it).
>>> I think we have less constraints with the logging GUC and do not need to
>>> mirror the behaviorial GUCs at all costs. But again, that is just my two
>>> cents.
>>
>> I lean towards log_autovacuum_{vacuum|analyze}_min_duration.  If
>> log_autovacuum_min_duration didn't exist, that's probably the naming scheme
>> we'd go with.  However, I'm not sure we can get away with renaming
>> log_autovacuum_min_duration.  Presumably we'd need to at least keep it
>> around as a backward-compatibility GUC, and its behavior would probably
>> change, too
> 
> I think deprecating a GUC like log_autovacuum_min_duration would be quite
> difficult.

Also deprecating the log_autovacuum_min_duration reloption might be tricky.
If we remove support for it in v19, how should pg_dump handle tables with
this option set from older versions? Should it translate it into both
log_autovacuum_vacuum_min_duration and log_autovacuum_analyze_min_duration
during dump? Would pg_upgrade run into the same issue?

Regards,

-- 
Fujii Masao
NTT DATA Japan Corporation




Re: Add log_autovacuum_{vacuum|analyze}_min_duration

От
wenhui qiu
Дата:
HI 
I vote log_autovacuum_{vacuum|analyze}_min_duration. Then don't remove log_autovacuum_min_duration so easily!

On Wed, Jun 4, 2025 at 7:16 AM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:


On 2025/06/04 4:32, Sami Imseih wrote:
>> On Tue, Jun 03, 2025 at 10:57:11AM +0200, Michael Banck wrote:
>>> On Tue, Jun 03, 2025 at 05:25:40PM +0900, Shinya Kato wrote:
>>>> I surely think adding log_autoanalyze_min_duration is simpler and
>>>> shorter, but the reason I chose this GUC name is for consistency with
>>>> other autovacuum parameters. Existing autovacuum parameters that have
>>>> separate settings for vacuum and analyze operations follow the pattern
>>>> autovacuum_{vacuum|analyze}_*.
>>>> https://www.postgresql.org/docs/devel/runtime-config-vacuum.html#RUNTIME-CONFIG-AUTOVACUUM
>>>
>>> Right, but the GUCs that directly affect either vacuum or autovacuum
>>> behaviour need the qualification (and then vacuum/analyze on top of it).
>>> I think we have less constraints with the logging GUC and do not need to
>>> mirror the behaviorial GUCs at all costs. But again, that is just my two
>>> cents.
>>
>> I lean towards log_autovacuum_{vacuum|analyze}_min_duration.  If
>> log_autovacuum_min_duration didn't exist, that's probably the naming scheme
>> we'd go with.  However, I'm not sure we can get away with renaming
>> log_autovacuum_min_duration.  Presumably we'd need to at least keep it
>> around as a backward-compatibility GUC, and its behavior would probably
>> change, too
>
> I think deprecating a GUC like log_autovacuum_min_duration would be quite
> difficult.

Also deprecating the log_autovacuum_min_duration reloption might be tricky.
If we remove support for it in v19, how should pg_dump handle tables with
this option set from older versions? Should it translate it into both
log_autovacuum_vacuum_min_duration and log_autovacuum_analyze_min_duration
during dump? Would pg_upgrade run into the same issue?

Regards,

--
Fujii Masao
NTT DATA Japan Corporation



Re: Add log_autovacuum_{vacuum|analyze}_min_duration

От
Shinya Kato
Дата:
Thank you all for the comments!

On Wed, Jun 4, 2025 at 10:30 AM wenhui qiu <qiuwenhuifx@gmail.com> wrote:
>
> HI
> I vote log_autovacuum_{vacuum|analyze}_min_duration. Then don't remove log_autovacuum_min_duration so easily!
>
> On Wed, Jun 4, 2025 at 7:16 AM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>>
>>
>>
>> On 2025/06/04 4:32, Sami Imseih wrote:
>> >> On Tue, Jun 03, 2025 at 10:57:11AM +0200, Michael Banck wrote:
>> >>> On Tue, Jun 03, 2025 at 05:25:40PM +0900, Shinya Kato wrote:
>> >>>> I surely think adding log_autoanalyze_min_duration is simpler and
>> >>>> shorter, but the reason I chose this GUC name is for consistency with
>> >>>> other autovacuum parameters. Existing autovacuum parameters that have
>> >>>> separate settings for vacuum and analyze operations follow the pattern
>> >>>> autovacuum_{vacuum|analyze}_*.
>> >>>> https://www.postgresql.org/docs/devel/runtime-config-vacuum.html#RUNTIME-CONFIG-AUTOVACUUM
>> >>>
>> >>> Right, but the GUCs that directly affect either vacuum or autovacuum
>> >>> behaviour need the qualification (and then vacuum/analyze on top of it).
>> >>> I think we have less constraints with the logging GUC and do not need to
>> >>> mirror the behaviorial GUCs at all costs. But again, that is just my two
>> >>> cents.
>> >>
>> >> I lean towards log_autovacuum_{vacuum|analyze}_min_duration.  If
>> >> log_autovacuum_min_duration didn't exist, that's probably the naming scheme
>> >> we'd go with.  However, I'm not sure we can get away with renaming
>> >> log_autovacuum_min_duration.  Presumably we'd need to at least keep it
>> >> around as a backward-compatibility GUC, and its behavior would probably
>> >> change, too
>> >
>> > I think deprecating a GUC like log_autovacuum_min_duration would be quite
>> > difficult.
>>
>> Also deprecating the log_autovacuum_min_duration reloption might be tricky.
>> If we remove support for it in v19, how should pg_dump handle tables with
>> this option set from older versions? Should it translate it into both
>> log_autovacuum_vacuum_min_duration and log_autovacuum_analyze_min_duration
>> during dump? Would pg_upgrade run into the same issue?
>>

I understand it's hard to deprecate log_autovacuum_min_duration. I
think there are three approaches that reflect your comments.

Approach 1:
- log_autovacuum_min_duration: Same behavior, which controls
autovacuum and autoanalyze logging.
- log_autoanalyze_min_duration: New parameter, which controls
autoanalyze logging.

Approach 2:
- log_autovacuum_min_duration: Changed behavior, which controls only
autovacuum logging.
- log_autoanalyze_min_duration: New parameter, which controls
autoanalyze logging.

Approach 3:
- log_autovacuum_min_duration: Retained for backward compatibility.
- log_autovacuum_{vacuum,analyze}_min_duration: New parameter.

Thoughts?


And I added a new entry for the next commitfest.
https://commitfest.postgresql.org/patch/5797/

--
Best regards,
Shinya Kato
NTT OSS Center



Re: Add log_autovacuum_{vacuum|analyze}_min_duration

От
Sami Imseih
Дата:
> Approach 2:
> - log_autovacuum_min_duration: Changed behavior, which controls only
> autovacuum logging.
> - log_autoanalyze_min_duration: New parameter, which controls
> autoanalyze logging.

My vote is for this approach. It is probably OK to change the behavior of
log_autovacuum_min_duration, as the new GUC will have the same default
value.

log_autoanalyze_min_duration makes sense, especially since
"autoanalyze" is the term we already use in system views (e.g.,
pg_stat_all_tables.last_autoanalyze). I do not think we need to worry
about consistency with other autovacuum parameters (e.g.,
autovacuum_[vacuum|analyze]_threshold, etc.), because in this case we are
only talking about logging, so we have more flexibility in naming.

Initially, I was not sure if there is a use case in which someone would want
to turn off autovacuum logging but keep autoanalyze logging (or vice versa),
but there may be, and this will be more flexible.

--

Sami Imseih

Re: Add log_autovacuum_{vacuum|analyze}_min_duration

От
Shinya Kato
Дата:
On Thu, Jun 5, 2025 at 3:53 AM Sami Imseih <samimseih@gmail.com> wrote:
> Approach 2:
> - log_autovacuum_min_duration: Changed behavior, which controls only
> autovacuum logging.
> - log_autoanalyze_min_duration: New parameter, which controls
> autoanalyze logging.

My vote is for this approach. It is probably OK to change the behavior of
log_autovacuum_min_duration, as the new GUC will have the same default
value.


Thank you for voting. I also think this approach is reasonable to implement.

log_autoanalyze_min_duration makes sense, especially since
"autoanalyze" is the term we already use in system views (e.g.,
pg_stat_all_tables.last_autoanalyze). I do not think we need to worry
about consistency with other autovacuum parameters (e.g.,
autovacuum_[vacuum|analyze]_threshold, etc.), because in this case we are
only talking about logging, so we have more flexibility in naming.


+1. 
 

Initially, I was not sure if there is a use case in which someone would want
to turn off autovacuum logging but keep autoanalyze logging (or vice versa),
but there may be, and this will be more flexible.


My concern is less about turning autovacuum and autoanalyze logs on or off individually, and more about the fact that setting a large value for log_autovacuum_min_duration prevents autoanalyze logs from being recorded.

--
Best regards,
Shinya Kato
NTT OSS Center

Re: Add log_autovacuum_{vacuum|analyze}_min_duration

От
Shinya Kato
Дата:
On Wed, Jun 11, 2025 at 1:49 PM Shinya Kato <shinya11.kato@gmail.com> wrote:
On Thu, Jun 5, 2025 at 3:53 AM Sami Imseih <samimseih@gmail.com> wrote:
> Approach 2:
> - log_autovacuum_min_duration: Changed behavior, which controls only
> autovacuum logging.
> - log_autoanalyze_min_duration: New parameter, which controls
> autoanalyze logging.

My vote is for this approach. It is probably OK to change the behavior of
log_autovacuum_min_duration, as the new GUC will have the same default
value.


Thank you for voting. I also think this approach is reasonable to implement.

A new patch is attached. 
Thoughts?

--
Best regards,
Shinya Kato
NTT OSS Center
Вложения

Re: Add log_autovacuum_{vacuum|analyze}_min_duration

От
kasaharatt
Дата:
Hi,

2025-07-02 15:37 に Shinya Kato wrote:
> On Mon, Jun 23, 2025 at 4:24 PM Shinya Kato <shinya11.kato@gmail.com>
> wrote:
>>
>> On Wed, Jun 11, 2025 at 1:49 PM Shinya Kato <shinya11.kato@gmail.com>
>> wrote:
>>>
>>> On Thu, Jun 5, 2025 at 3:53 AM Sami Imseih <samimseih@gmail.com>
>>> wrote:
>>>>
>>>> > Approach 2:
>>>> > - log_autovacuum_min_duration: Changed behavior, which controls only
>>>> > autovacuum logging.
>>>> > - log_autoanalyze_min_duration: New parameter, which controls
>>>> > autoanalyze logging.
>>>>
>>>> My vote is for this approach. It is probably OK to change the
>>>> behavior of
>>>> log_autovacuum_min_duration, as the new GUC will have the same
>>>> default
>>>> value.
>>>
>>>
>>> Thank you for voting. I also think this approach is reasonable to
>>> implement.
>>
>>
>> A new patch is attached.
>> Thoughts?
I reviewed this patch.
I also have no particular objections to the Approach 2.

> +    <term><literal>log_autoanalyze_min_duration</literal>,
> <literal>toast.log_autoanalyze_min_duration</literal>(<type>integer</type>)
(snip)
> +       "toast.log_autoanalyze_min_duration",
This patch adds the log_autoanalyze_min_duration parameter fot TOAST
tables.
However since PostgreSQL currently does not support ANALYZE on TOAST
tables,
isn't this parameter unnecessary?

Best regards,
--
Kasahara Tatsuhito
NTT DATA Japan Corporation



Re: Add log_autovacuum_{vacuum|analyze}_min_duration

От
Shinya Kato
Дата:
Hi,

On Wed, Aug 13, 2025 at 5:44 PM kasaharatt <kasaharatt@oss.nttdata.com> wrote:

> >>>> > Approach 2:
> >>>> > - log_autovacuum_min_duration: Changed behavior, which controls only
> >>>> > autovacuum logging.
> >>>> > - log_autoanalyze_min_duration: New parameter, which controls
> >>>> > autoanalyze logging.
> >>>>
> >>>> My vote is for this approach. It is probably OK to change the
> >>>> behavior of
> >>>> log_autovacuum_min_duration, as the new GUC will have the same
> >>>> default
> >>>> value.
> >>>
> >>>
> >>> Thank you for voting. I also think this approach is reasonable to
> >>> implement.
> >>
> >>
> >> A new patch is attached.
> >> Thoughts?
> I reviewed this patch.
> I also have no particular objections to the Approach 2.

Thank you for the review!

> > +    <term><literal>log_autoanalyze_min_duration</literal>,
> > <literal>toast.log_autoanalyze_min_duration</literal>(<type>integer</type>)
> (snip)
> > +       "toast.log_autoanalyze_min_duration",
> This patch adds the log_autoanalyze_min_duration parameter fot TOAST
> tables.
> However since PostgreSQL currently does not support ANALYZE on TOAST
> tables,
> isn't this parameter unnecessary?

You're right; that was a mistake. I've fixed it in the v4 patch.

+log_autoanalyze_min_duration = 0

Additionally, I removed the above setting from the test files in
src/test/modules/xid_wraparound/t/ (001_emergency_vacuum.pl,
002_limits.pl, 003_wraparounds.pl). The reason is that these tests
check for autovacuum logs, not autoanalyze logs. You can run the test
with the following command:
make check -C src/test/modules/xid_wraparound PG_TEST_EXTRA='xid_wraparound'


--
Best regards,
Shinya Kato
NTT OSS Center

Вложения

Re: Add log_autovacuum_{vacuum|analyze}_min_duration

От
kasaharatt
Дата:
Hi,

2025-08-14 13:26 に Shinya Kato wrote:
> Hi,
>
> On Wed, Aug 13, 2025 at 5:44 PM kasaharatt <kasaharatt@oss.nttdata.com>
> wrote:
>
>> >>>> > Approach 2:
>> >>>> > - log_autovacuum_min_duration: Changed behavior, which controls only
>> >>>> > autovacuum logging.
>> >>>> > - log_autoanalyze_min_duration: New parameter, which controls
>> >>>> > autoanalyze logging.
>> >>>>
>> >>>> My vote is for this approach. It is probably OK to change the
>> >>>> behavior of
>> >>>> log_autovacuum_min_duration, as the new GUC will have the same
>> >>>> default
>> >>>> value.
>> >>>
>> >>>
>> >>> Thank you for voting. I also think this approach is reasonable to
>> >>> implement.
>> >>
>> >>
>> >> A new patch is attached.
>> >> Thoughts?
>> I reviewed this patch.
>> I also have no particular objections to the Approach 2.
>
> Thank you for the review!
>
>> > +    <term><literal>log_autoanalyze_min_duration</literal>,
>> > <literal>toast.log_autoanalyze_min_duration</literal>(<type>integer</type>)
>> (snip)
>> > +       "toast.log_autoanalyze_min_duration",
>> This patch adds the log_autoanalyze_min_duration parameter fot TOAST
>> tables.
>> However since PostgreSQL currently does not support ANALYZE on TOAST
>> tables,
>> isn't this parameter unnecessary?
>
> You're right; that was a mistake. I've fixed it in the v4 patch.
Thanks for the patch updating.

> Additionally, I removed the above setting from the test files in
> src/test/modules/xid_wraparound/t/ (001_emergency_vacuum.pl,
> 002_limits.pl, 003_wraparounds.pl). The reason is that these tests
> check for autovacuum logs, not autoanalyze logs. You can run the test
> with the following command:
> make check -C src/test/modules/xid_wraparound
> PG_TEST_EXTRA='xid_wraparound'
Yeah, I agree that.

I confirmed this patch could apply to HEAD and all make check tests were
passed.
I've manually tested it in a few different ways, and it's working as
expected so far.

Unless there are any objections from others, I think I can mark this
patch as "Ready for Committer".

Best regards,
--
Kasahara Tatsuhito
NTT DATA Japan Corporation



Re: Add log_autovacuum_{vacuum|analyze}_min_duration

От
kasaharatt
Дата:
2025-08-14 17:56 に kasaharatt wrote:
> Hi,
> 
> 2025-08-14 13:26 に Shinya Kato wrote:
>> Hi,
>> 
>> On Wed, Aug 13, 2025 at 5:44 PM kasaharatt 
>> <kasaharatt@oss.nttdata.com> wrote:
>> 
>>> >>>> > Approach 2:
>>> >>>> > - log_autovacuum_min_duration: Changed behavior, which controls only
>>> >>>> > autovacuum logging.
>>> >>>> > - log_autoanalyze_min_duration: New parameter, which controls
>>> >>>> > autoanalyze logging.
>>> >>>>
>>> >>>> My vote is for this approach. It is probably OK to change the
>>> >>>> behavior of
>>> >>>> log_autovacuum_min_duration, as the new GUC will have the same
>>> >>>> default
>>> >>>> value.
>>> >>>
>>> >>>
>>> >>> Thank you for voting. I also think this approach is reasonable to
>>> >>> implement.
>>> >>
>>> >>
>>> >> A new patch is attached.
>>> >> Thoughts?
>>> I reviewed this patch.
>>> I also have no particular objections to the Approach 2.
>> 
>> Thank you for the review!
>> 
>>> > +    <term><literal>log_autoanalyze_min_duration</literal>,
>>> > <literal>toast.log_autoanalyze_min_duration</literal>(<type>integer</type>)
>>> (snip)
>>> > +       "toast.log_autoanalyze_min_duration",
>>> This patch adds the log_autoanalyze_min_duration parameter fot TOAST
>>> tables.
>>> However since PostgreSQL currently does not support ANALYZE on TOAST
>>> tables,
>>> isn't this parameter unnecessary?
>> 
>> You're right; that was a mistake. I've fixed it in the v4 patch.
> Thanks for the patch updating.
> 
>> Additionally, I removed the above setting from the test files in
>> src/test/modules/xid_wraparound/t/ (001_emergency_vacuum.pl,
>> 002_limits.pl, 003_wraparounds.pl). The reason is that these tests
>> check for autovacuum logs, not autoanalyze logs. You can run the test
>> with the following command:
>> make check -C src/test/modules/xid_wraparound 
>> PG_TEST_EXTRA='xid_wraparound'
> Yeah, I agree that.
> 
> I confirmed this patch could apply to HEAD and all make check tests 
> were passed.
> I've manually tested it in a few different ways, and it's working as
> expected so far.
> 
> Unless there are any objections from others, I think I can mark this
> patch as "Ready for Committer".

Since there were no particular comments, I've marked it as Ready for 
Committer.

Best regards,
-- 
Kasahara Tatsuhito
NTT DATA Japan Corporation



Re: Add log_autovacuum_{vacuum|analyze}_min_duration

От
kasaharatt
Дата:
Hi,

2025-08-21 16:39 に kasaharatt wrote:
> 2025-08-14 17:56 に kasaharatt wrote:
>> Hi,
>>
>> 2025-08-14 13:26 に Shinya Kato wrote:
>>> Hi,
>>>
>>> On Wed, Aug 13, 2025 at 5:44 PM kasaharatt
>>> <kasaharatt@oss.nttdata.com> wrote:
>>>
>>>> >>>> > Approach 2:
>>>> >>>> > - log_autovacuum_min_duration: Changed behavior, which controls only
>>>> >>>> > autovacuum logging.
>>>> >>>> > - log_autoanalyze_min_duration: New parameter, which controls
>>>> >>>> > autoanalyze logging.
>>>> >>>>
>>>> >>>> My vote is for this approach. It is probably OK to change the
>>>> >>>> behavior of
>>>> >>>> log_autovacuum_min_duration, as the new GUC will have the same
>>>> >>>> default
>>>> >>>> value.
>>>> >>>
>>>> >>>
>>>> >>> Thank you for voting. I also think this approach is reasonable to
>>>> >>> implement.
>>>> >>
>>>> >>
>>>> >> A new patch is attached.
>>>> >> Thoughts?
>>>> I reviewed this patch.
>>>> I also have no particular objections to the Approach 2.
>>>
>>> Thank you for the review!
>>>
>>>> > +    <term><literal>log_autoanalyze_min_duration</literal>,
>>>> > <literal>toast.log_autoanalyze_min_duration</literal>(<type>integer</type>)
>>>> (snip)
>>>> > +       "toast.log_autoanalyze_min_duration",
>>>> This patch adds the log_autoanalyze_min_duration parameter fot TOAST
>>>> tables.
>>>> However since PostgreSQL currently does not support ANALYZE on TOAST
>>>> tables,
>>>> isn't this parameter unnecessary?
>>>
>>> You're right; that was a mistake. I've fixed it in the v4 patch.
>> Thanks for the patch updating.
>>
>>> Additionally, I removed the above setting from the test files in
>>> src/test/modules/xid_wraparound/t/ (001_emergency_vacuum.pl,
>>> 002_limits.pl, 003_wraparounds.pl). The reason is that these tests
>>> check for autovacuum logs, not autoanalyze logs. You can run the test
>>> with the following command:
>>> make check -C src/test/modules/xid_wraparound
>>> PG_TEST_EXTRA='xid_wraparound'
>> Yeah, I agree that.
>>
>> I confirmed this patch could apply to HEAD and all make check tests
>> were passed.
>> I've manually tested it in a few different ways, and it's working as
>> expected so far.
>>
>> Unless there are any objections from others, I think I can mark this
>> patch as "Ready for Committer".
>
> Since there were no particular comments, I've marked it as Ready for
> Committer.

The changes(*) to guc_tables.c have been pushed into HEAD,
so you may need to fix the patch.

(*)
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=63599896545c7869f7dd28cd593e8b548983d613

Best regards,
--
Kasahara Tatsuhito
NTT DATA Japan Corporation



Re: Add log_autovacuum_{vacuum|analyze}_min_duration

От
Shinya Kato
Дата:
Hi,

On Thu, Sep 4, 2025 at 11:31 AM kasaharatt <kasaharatt@oss.nttdata.com> wrote:
> The changes(*) to guc_tables.c have been pushed into HEAD,
> so you may need to fix the patch.
>
> (*)
> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=63599896545c7869f7dd28cd593e8b548983d613

Thank you for reporting and I fixed the patch.

--
Best regards,
Shinya Kato
NTT OSS Center

Вложения

Re: Add log_autovacuum_{vacuum|analyze}_min_duration

От
kasaharatt
Дата:
2025-09-04 13:13 に Shinya Kato wrote:
> Hi,
> 
> On Thu, Sep 4, 2025 at 11:31 AM kasaharatt <kasaharatt@oss.nttdata.com> 
> wrote:
>> The changes(*) to guc_tables.c have been pushed into HEAD,
>> so you may need to fix the patch.
>> 
>> (*)
>> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=63599896545c7869f7dd28cd593e8b548983d613
> 
> Thank you for reporting and I fixed the patch.
Thank you for the quick fix.

I've confirmed that it can be applied to HEAD and that all regression 
tests pass.
I've run a few quick local checks and it works as expected.
ISTM there are no particular issues with the changes.

The status has been changed to Ready for Committer again.

Best regards,
-- 
Kasahara Tatsuhito
NTT DATA Japan Corporation