Re: Patch to address creation of PgStat* contexts with null parent context

Hi, On 9/17/22 6:10 PM, Andres Freund wrote: Hi, On 2022-09-07 11:11:11 +0200, Drouvot, Bertrand wrote: On 9/6/22 7:53 AM, Kyotaro Horiguchi wrote: At Mon, 5 Sep 2022 14:46:55 +0200, "Drouvot, Bertrand" wrote in Looks like that both approaches have their pros and cons. I'm tempted to vote

Re: Patch to address creation of PgStat* contexts with null parent context

Hi, On 2022-09-07 11:11:11 +0200, Drouvot, Bertrand wrote: > On 9/6/22 7:53 AM, Kyotaro Horiguchi wrote: > > At Mon, 5 Sep 2022 14:46:55 +0200, "Drouvot, Bertrand" > > wrote in > > > Looks like that both approaches have their pros and cons. I'm tempted > > > to vote +1 on "just changing" the

Re: Patch to address creation of PgStat* contexts with null parent context

At Wed, 7 Sep 2022 11:11:11 +0200, "Drouvot, Bertrand" wrote in > On 9/6/22 7:53 AM, Kyotaro Horiguchi wrote: > > So, I'm fine with just replacing the parent context at the three > > places. Looks good to me. To make sure, I ran make check-world with adding an assertion check that all

Re: Patch to address creation of PgStat* contexts with null parent context

(It seems to me I overlooked some mails.. sorry.) At Mon, 5 Sep 2022 15:47:37 -0700, Andres Freund wrote in > On 2022-09-05 17:32:20 +0900, Kyotaro Horiguchi wrote: > > The rationale of creating them at pgstat_attach_shmem is that anyway once > > pgstat_attach_shmem is called, the process

Re: Patch to address creation of PgStat* contexts with null parent context

Hi, On 2022-09-05 17:32:20 +0900, Kyotaro Horiguchi wrote: > The rationale of creating them at pgstat_attach_shmem is that anyway once > pgstat_attach_shmem is called, the process fainally creates the contexts at > the end of the process, and (I think) it's simpler that we don't do if() > check

Re: Patch to address creation of PgStat* contexts with null parent context

At Mon, 5 Sep 2022 08:52:44 +0200, "Drouvot, Bertrand" wrote in > Could using TopMemoryContext like in the attach be an option? (aka > changing CacheMemoryContext by TopMemoryContext in the 3 places of > interest): that would ensure the 3 pgStat* contexts to have a non NULL > parent context.

Re: Patch to address creation of PgStat* contexts with null parent context

Hi, On 2022-08-08 15:12:08 +0900, Kyotaro Horiguchi wrote: > At Sat, 6 Aug 2022 19:19:39 -0700, Andres Freund wrote in > > Hi, > > > > On 2022-08-05 17:22:38 +0900, Kyotaro Horiguchi wrote: > > > I think it a bit different. Previously that memory (but for a bit > > > different use, precisely)

Re: Patch to address creation of PgStat* contexts with null parent context

At Sat, 6 Aug 2022 19:19:39 -0700, Andres Freund wrote in > Hi, > > On 2022-08-05 17:22:38 +0900, Kyotaro Horiguchi wrote: > > I think it a bit different. Previously that memory (but for a bit > > different use, precisely) was required only when stats data is read so > > almost all server

Re: Patch to address creation of PgStat* contexts with null parent context

Hi, On 2022-08-05 17:22:38 +0900, Kyotaro Horiguchi wrote: > I think it a bit different. Previously that memory (but for a bit > different use, precisely) was required only when stats data is read so > almost all server processes didn't need it. Now, every server process > that uses pgstats

Re: Patch to address creation of PgStat* contexts with null parent context

At Fri, 05 Aug 2022 17:22:38 +0900 (JST), Kyotaro Horiguchi wrote in > Thus I thought that we may let pgstat_initialize() promptly allocate > the memory. > > Does it make sense? > > > About the patch, I had something like the attached in my mind. I haven't fully checked, but this change

Re: Patch to address creation of PgStat* contexts with null parent context

At Thu, 04 Aug 2022 13:12:32 -0400, Reid Thompson wrote in > On Fri, 2022-07-29 at 11:53 +0900, Kyotaro Horiguchi wrote: > > > > That makes the memorycontext-tree structure unstable because > > CacheMemoryContext can be created on-the-fly. > > > > Honestly I don't like to call

Re: Patch to address creation of PgStat* contexts with null parent context

On Fri, 2022-07-29 at 11:53 +0900, Kyotaro Horiguchi wrote: > > That makes the memorycontext-tree structure unstable because > CacheMemoryContext can be created on-the-fly. > > Honestly I don't like to call CreateCacheMemoryContext in the two > functions on-the-fly.  Since every process that

Re: Patch to address creation of PgStat* contexts with null parent context

At Thu, 28 Jul 2022 22:03:13 +0800, Zhang Mingli wrote in > Hi, > > On Jul 28, 2022, 21:30 +0800, Reid Thompson , > wrote: > > Attached is a patch to address this. Good Catch! > Codes seem good, my question is: > > Do auto vacuum processes need CacheMemoryContext? pgstat_report_vacuum

Re: Patch to address creation of PgStat* contexts with null parent context

Hi, On Jul 28, 2022, 21:30 +0800, Reid Thompson , wrote: > Hi, > > There are instances where pgstat_setup_memcxt() and > pgstat_prep_pending_entry() are invoked before the CacheMemoryContext > has been created.  This results in PgStat* contexts being created > without a parent context.  Most

Patch to address creation of PgStat* contexts with null parent context

Hi, There are instances where pgstat_setup_memcxt() and pgstat_prep_pending_entry() are invoked before the CacheMemoryContext has been created.  This results in PgStat* contexts being created without a parent context.  Most easily reproduced/seen in autovacuum worker via pgstat_setup_memcxt().