Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

Hi, Thank for all the work on this topic - I'd somehow accidentally marked this thread as read when coming back from vacation... Greetings, Andres Freund

Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

On Wed, Sep 1, 2021 at 5:33 PM Dilip Kumar wrote: > > On Wed, Sep 1, 2021 at 5:23 PM Amit Kapila wrote: >> >> On Wed, Sep 1, 2021 at 1:53 PM Dilip Kumar wrote: >> > >> >> The latest patch looks good to me. I have made some changes in the >> comments, see attached. I am planning to push this tomo

Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

On Wed, Sep 1, 2021 at 5:23 PM Amit Kapila wrote: > On Wed, Sep 1, 2021 at 1:53 PM Dilip Kumar wrote: > > > > The latest patch looks good to me. I have made some changes in the > comments, see attached. I am planning to push this tomorrow unless you > or others have any comments on it. > These

Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

On Wed, Sep 1, 2021 at 1:53 PM Dilip Kumar wrote: > The latest patch looks good to me. I have made some changes in the comments, see attached. I am planning to push this tomorrow unless you or others have any comments on it. -- With Regards, Amit Kapila. v8-0001-Optimize-fileset-usage-in-appl

Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

On Tue, Aug 31, 2021 at 3:20 PM Amit Kapila wrote: > > On Fri, Aug 27, 2021 at 12:04 PM Dilip Kumar wrote: > > > > Few comments on v6-0002* > = > 1. > -BufFileDeleteFileSet(FileSet *fileset, const char *name) > +BufFileDeleteFileSet(FileSet *fileset, const char *name, bool

Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

On Fri, Aug 27, 2021 at 12:04 PM Dilip Kumar wrote: > Few comments on v6-0002* = 1. -BufFileDeleteFileSet(FileSet *fileset, const char *name) +BufFileDeleteFileSet(FileSet *fileset, const char *name, bool missing_ok) { char segment_name[MAXPGPATH]; int segment = 0; @@

Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

On Tue, Aug 31, 2021 at 7:39 AM houzj.f...@fujitsu.com < houzj.f...@fujitsu.com> wrote: > From Mon, Aug 30, 2021 2:15 PM Masahiko Sawada > wrote: > > On Fri, Aug 27, 2021 at 3:34 PM Dilip Kumar > wrote: > > > > > > On Fri, Aug 27, 2021 at 10:56 AM houzj.f...@fujitsu.com < > houzj.f...@fujitsu.co

RE: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

From Mon, Aug 30, 2021 2:15 PM Masahiko Sawada wrote: > On Fri, Aug 27, 2021 at 3:34 PM Dilip Kumar wrote: > > > > On Fri, Aug 27, 2021 at 10:56 AM houzj.f...@fujitsu.com > > wrote: > > > > > > On Thu, Aug 26, 2021 2:18 PM Dilip Kumar wrote: > > > > > > > The patch looks good to me, I have reb

Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

On Mon, Aug 30, 2021 at 6:42 AM Masahiko Sawada wrote: > > On Fri, Aug 27, 2021 at 2:25 PM Dilip Kumar wrote: > > > > On Thu, Aug 26, 2021 at 2:10 PM Amit Kapila wrote: > >> > >> > >> Thanks, Sawada-San and Dilip for confirmation. I would like to commit > >> this and the second patch (the second

Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

On Fri, Aug 27, 2021 at 3:34 PM Dilip Kumar wrote: > > On Fri, Aug 27, 2021 at 10:56 AM houzj.f...@fujitsu.com > wrote: > > > > On Thu, Aug 26, 2021 2:18 PM Dilip Kumar wrote: > > > > > The patch looks good to me, I have rebased 0002 atop > > > this patch and also done some cosmetic fixes in 000

Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

On Fri, Aug 27, 2021 at 2:25 PM Dilip Kumar wrote: > > On Thu, Aug 26, 2021 at 2:10 PM Amit Kapila wrote: >> >> >> Thanks, Sawada-San and Dilip for confirmation. I would like to commit >> this and the second patch (the second one still needs some more >> testing and review) for PG-15 as there is

Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

On Fri, Aug 27, 2021 at 10:56 AM houzj.f...@fujitsu.com wrote: > > On Thu, Aug 26, 2021 2:18 PM Dilip Kumar wrote: > > > The patch looks good to me, I have rebased 0002 atop > > this patch and also done some cosmetic fixes in 0002. > > Here are some comments for the 0002 patch. > > 1) > > - * top

RE: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

On Fri, Aug 27, 2021 1:25 PM Dilip Kumar wrote: > On Thu, Aug 26, 2021 at 2:10 PM Amit Kapila > wrote: > >> Thanks, Sawada-San and Dilip for confirmation. I would like to commit >> this and the second patch (the second one still needs some more >> testing and re

RE: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

On Thu, Aug 26, 2021 2:18 PM Dilip Kumar wrote: > The patch looks good to me, I have rebased 0002 atop > this patch and also done some cosmetic fixes in 0002.  Here are some comments for the 0002 patch. 1) - * toplevel transaction. Each subscription has a separate set of files. + * toplevel tr

Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

On Thu, Aug 26, 2021 at 2:10 PM Amit Kapila wrote: > > Thanks, Sawada-San and Dilip for confirmation. I would like to commit > this and the second patch (the second one still needs some more > testing and review) for PG-15 as there is no bug per-se related to > this work in PG-14 but I see an arg

Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

On Thu, Aug 26, 2021 at 11:47 AM Dilip Kumar wrote: > > On Wed, Aug 25, 2021 at 5:49 PM Amit Kapila wrote: >> >> On Tue, Aug 24, 2021 at 3:55 PM Dilip Kumar wrote: >> > >> > On Tue, Aug 24, 2021 at 12:26 PM Amit Kapila >> > wrote: >> > >> >> The first patch looks good to me. I have made minor

Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

On Wed, Aug 25, 2021 at 5:49 PM Amit Kapila wrote: > On Tue, Aug 24, 2021 at 3:55 PM Dilip Kumar wrote: > > > > On Tue, Aug 24, 2021 at 12:26 PM Amit Kapila > wrote: > > > > The first patch looks good to me. I have made minor changes to the > attached patch. The changes include: fixing compilat

Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

On Wed, Aug 25, 2021 at 9:19 PM Amit Kapila wrote: > > On Tue, Aug 24, 2021 at 3:55 PM Dilip Kumar wrote: > > > > On Tue, Aug 24, 2021 at 12:26 PM Amit Kapila > > wrote: > > > > The first patch looks good to me. I have made minor changes to the > attached patch. The changes include: fixing comp

Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

On Tue, Aug 24, 2021 at 3:55 PM Dilip Kumar wrote: > > On Tue, Aug 24, 2021 at 12:26 PM Amit Kapila wrote: > The first patch looks good to me. I have made minor changes to the attached patch. The changes include: fixing compilation warning, made some comment changes, ran pgindent, and few other

RE: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

On Tuesday, August 24, 2021 6:25 PM Dilip Kumar wrote: > On Tue, Aug 24, 2021 at 12:26 PM Amit Kapila wrote: > > > > I was also thinking about the same, does it make sense to name it > > > just ""%s/%s%lu.%u.fileset"? > > Done > > > I think it is reasonable to use .fileset as proposed by you.

Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

On Tue, Aug 24, 2021 at 12:26 PM Amit Kapila wrote: > > I was also thinking about the same, does it make sense to name it just > > ""%s/%s%lu.%u.fileset"? Done > I think it is reasonable to use .fileset as proposed by you. > > Few other comments: > = > 1. > + /* > + * Register b

Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

On Mon, Aug 23, 2021 at 3:13 PM Dilip Kumar wrote: > > On Mon, Aug 23, 2021 at 1:43 PM Amit Kapila wrote: > > Note: merge comments from multiple mails > > > > I think we should handle that in worker.c itself, by adding a > > > before_dsm_detach function before_shmem_exit right? > > > > > > > Yeah

Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

On Mon, Aug 23, 2021 at 1:43 PM Amit Kapila wrote: Note: merge comments from multiple mails > > I think we should handle that in worker.c itself, by adding a > > before_dsm_detach function before_shmem_exit right? > > > > Yeah, I thought of handling it in worker.c similar to what you've in 0002

Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

On Mon, Aug 23, 2021 at 12:52 PM Dilip Kumar wrote: > > On Mon, Aug 23, 2021 at 11:43 AM Amit Kapila wrote: > > > > On Mon, Aug 23, 2021 at 9:53 AM Dilip Kumar wrote: > > > > > > On Mon, Aug 23, 2021 at 9:11 AM houzj.f...@fujitsu.com > > > wrote: > > > > > > > 4) > > > > -extern File SharedFile

Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

On Sat, Aug 21, 2021 at 9:38 PM Dilip Kumar wrote: > > On Wed, Aug 18, 2021 at 3:45 PM Dilip Kumar wrote: > > > > On Wed, Aug 18, 2021 at 11:24 AM Amit Kapila > > wrote: > > > > > > On Tue, Aug 17, 2021 at 4:34 PM Andres Freund wrote: > > > > > > > > On 2021-08-17 10:54:30 +0530, Amit Kapila w

Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

On Mon, Aug 23, 2021 at 11:43 AM Amit Kapila wrote: > > On Mon, Aug 23, 2021 at 9:53 AM Dilip Kumar wrote: > > > > On Mon, Aug 23, 2021 at 9:11 AM houzj.f...@fujitsu.com > > wrote: > > > > > 4) > > > -extern File SharedFileSetCreate(SharedFileSet *fileset, const char > > > *name); > > > -extern

Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

On Mon, Aug 23, 2021 at 9:53 AM Dilip Kumar wrote: > > On Mon, Aug 23, 2021 at 9:11 AM houzj.f...@fujitsu.com > wrote: > > > 4) > > -extern File SharedFileSetCreate(SharedFileSet *fileset, const char *name); > > -extern File SharedFileSetOpen(SharedFileSet *fileset, const char *name, > > -

Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

On Mon, Aug 23, 2021 at 9:11 AM houzj.f...@fujitsu.com wrote: > 4) > -extern File SharedFileSetCreate(SharedFileSet *fileset, const char *name); > -extern File SharedFileSetOpen(SharedFileSet *fileset, const char *name, > - int mode); > -ext

RE: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

On Sat, Aug 21, 2021 8:38 PM Dilip Kumar wrote: > On Wed, Aug 18, 2021 at 3:45 PM Dilip Kumar wrote: > > I was looking into this, so if we want to do that I think the outline > > will look like this > > > > - There will be a fileset.c and fileset.h files, and we will expose a > > new structure Fi

Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

On Wed, Aug 18, 2021 at 3:45 PM Dilip Kumar wrote: > > On Wed, Aug 18, 2021 at 11:24 AM Amit Kapila wrote: > > > > On Tue, Aug 17, 2021 at 4:34 PM Andres Freund wrote: > > > > > > On 2021-08-17 10:54:30 +0530, Amit Kapila wrote: > > > > 5. How can we provide a strict mechanism to not allow to us

Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

On Wed, Aug 18, 2021 at 3:45 PM Dilip Kumar wrote: > > On Wed, Aug 18, 2021 at 11:24 AM Amit Kapila wrote: > > > > On Tue, Aug 17, 2021 at 4:34 PM Andres Freund wrote: > > > > > > On 2021-08-17 10:54:30 +0530, Amit Kapila wrote: > > > > 5. How can we provide a strict mechanism to not allow to us

Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

On Wed, Aug 18, 2021 at 11:24 AM Amit Kapila wrote: > > On Tue, Aug 17, 2021 at 4:34 PM Andres Freund wrote: > > > > On 2021-08-17 10:54:30 +0530, Amit Kapila wrote: > > > 5. How can we provide a strict mechanism to not allow to use dsm APIs > > > for non-dsm FileSet? One idea could be that we ca

Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

On Tue, Aug 17, 2021 at 4:34 PM Andres Freund wrote: > > On 2021-08-17 10:54:30 +0530, Amit Kapila wrote: > > 5. How can we provide a strict mechanism to not allow to use dsm APIs > > for non-dsm FileSet? One idea could be that we can have a variable > > (probably bool) in SharedFileSet structure

Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

On Wed, Aug 18, 2021 at 9:30 AM Amit Kapila wrote: > > On Wed, Aug 18, 2021 at 8:23 AM houzj.f...@fujitsu.com > wrote: > > > > On Wed, Aug 18, 2021 9:17 AM houzj.f...@fujitsu.com wrote: > > > Hi, > > > > > > I took a quick look at the v2 patch and noticed a typo. > > > > > > + * backends and rend

Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

On Wed, Aug 18, 2021 at 8:23 AM houzj.f...@fujitsu.com wrote: > > On Wed, Aug 18, 2021 9:17 AM houzj.f...@fujitsu.com wrote: > > Hi, > > > > I took a quick look at the v2 patch and noticed a typo. > > > > + * backends and render it read-only. If missing_ok is true then it will > > return > > + *

RE: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

On Wed, Aug 18, 2021 9:17 AM houzj.f...@fujitsu.com wrote: > Hi, > > I took a quick look at the v2 patch and noticed a typo. > > + * backends and render it read-only. If missing_ok is true then it will > return > + * NULL if file doesn not exist otherwise error. > */ > doesn not=> doesn't >

RE: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

Hi, I took a quick look at the v2 patch and noticed a typo. + * backends and render it read-only. If missing_ok is true then it will return + * NULL if file doesn not exist otherwise error. */ doesn not=> doesn't Best regards, Houzj

Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

Hi, On 2021-08-17 10:54:30 +0530, Amit Kapila wrote: > 5. How can we provide a strict mechanism to not allow to use dsm APIs > for non-dsm FileSet? One idea could be that we can have a variable > (probably bool) in SharedFileSet structure which will be initialized > in SharedFileSetInit based on w

Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

On Tue, Aug 17, 2021 at 1:30 PM Dilip Kumar wrote: > > On Tue, Aug 17, 2021 at 10:54 AM Amit Kapila wrote: > > > > > 5. How can we provide a strict mechanism to not allow to use dsm APIs > > for non-dsm FileSet? One idea could be that we can have a variable > > (probably bool) in SharedFileSet st

Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

On Tue, Aug 17, 2021 at 10:54 AM Amit Kapila wrote: > > On Mon, Aug 16, 2021 at 8:18 PM Dilip Kumar wrote: > > > > On Fri, Aug 13, 2021 at 9:29 PM Andres Freund wrote: > > > > > > I think we can extend this API but I guess it is better to then do it > > > > for dsm-based as well so that these ge

Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

On Tue, Aug 17, 2021 at 12:06 PM Amit Kapila wrote: > One more comment: > @@ -2976,39 +2952,17 @@ subxact_info_write(Oid subid, TransactionId xid) > .. > + /* Try to open the subxact file, if it doesn't exist then create it */ > + fd = BufFileOpenShared(xidfileset, path, O_RDWR, true); > + if (fd

Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

On Tue, Aug 17, 2021 at 10:54 AM Amit Kapila wrote: > > On Mon, Aug 16, 2021 at 8:18 PM Dilip Kumar wrote: > > > > On Fri, Aug 13, 2021 at 9:29 PM Andres Freund wrote: > > > > > > I think we can extend this API but I guess it is better to then do it > > > > for dsm-based as well so that these ge

Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

On Mon, Aug 16, 2021 at 8:18 PM Dilip Kumar wrote: > > On Fri, Aug 13, 2021 at 9:29 PM Andres Freund wrote: > > > > I think we can extend this API but I guess it is better to then do it > > > for dsm-based as well so that these get tracked via resowner. > > > > DSM segments are resowner managed a

Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

On Fri, Aug 13, 2021 at 9:29 PM Andres Freund wrote: > > I think we can extend this API but I guess it is better to then do it > > for dsm-based as well so that these get tracked via resowner. > > DSM segments are resowner managed already, so it's not obvious that that'd buy > us much? Although I

Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

Hi, (dropping -committers to avoid moderation stalls due xposting to multiple lists - I find that more annoying than helpful) On 2021-08-13 14:38:37 +0530, Amit Kapila wrote: > > What I'm wondering is why it is a good idea to have a SharedFileSet specific > > cleanup mechanism. One that only ope

Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

On Thu, Aug 12, 2021 at 6:24 PM Andres Freund wrote: > > On 2021-08-12 05:48:19 -0700, Andres Freund wrote: > > I think SharedFileSetInit() needs a comment explaining that it needs to be > > called in a process-lifetime memory context if used without dsm > > segments. Because otherwise SharedFileS

Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

On Thu, Aug 12, 2021 at 6:18 PM Andres Freund wrote: > > Hi, > > On 2021-08-12 15:06:23 +0530, Amit Kapila wrote: > > On Thu, Aug 12, 2021 at 1:52 PM Andres Freund wrote: > > > I'm not so sure. Why does sharedfileset have its own proc exit hook in the > > > first place? ISTM that this should be d

Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

Hi, On 2021-08-12 05:48:19 -0700, Andres Freund wrote: > I think SharedFileSetInit() needs a comment explaining that it needs to be > called in a process-lifetime memory context if used without dsm > segments. Because otherwise SharedFileSetDeleteOnProcExit() will access > already freed memory (bo

Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

Hi, On 2021-08-12 15:06:23 +0530, Amit Kapila wrote: > On Thu, Aug 12, 2021 at 1:52 PM Andres Freund wrote: > > I'm not so sure. Why does sharedfileset have its own proc exit hook in the > > first place? ISTM that this should be dealt with using resowners, rathers > > than > > a sharedfileset sp

Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

On Thu, Aug 12, 2021 at 1:52 PM Andres Freund wrote: > > On 2021-08-12 11:46:09 +0530, Amit Kapila wrote: > > On Thu, Aug 12, 2021 at 11:38 AM Dilip Kumar wrote: > > > On Thu, Aug 12, 2021 at 7:39 AM Masahiko Sawada > > > wrote: > > > > It seems to me that moving the shared fileset cleanup to >

Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

Hi, On 2021-08-12 11:46:09 +0530, Amit Kapila wrote: > On Thu, Aug 12, 2021 at 11:38 AM Dilip Kumar wrote: > > On Thu, Aug 12, 2021 at 7:39 AM Masahiko Sawada > > wrote: > > > It seems to me that moving the shared fileset cleanup to > > > before_shmem_exit() is the right approach to fix this pr