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

On Sat, Aug 21, 2021 8:38 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> On Wed, Aug 18, 2021 at 3:45 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> 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 FileSet, which will be the same as SharedFileSet, except
> > mutext and refcount. The fileset.c will expose FileSetInit(),
> > FileSetCreate(), FileSetOpen(), FileSetDelete() and FileSetDeleteAll()
> > interfaces.
> >
> > - sharefileset.c will internally call the fileset.c's interfaces. The
> > SharedFileSet structure will also contain FileSet and other members
> > i.e. mutex and refcount.
> >
> > - the buffile.c's interfaces which are ending with Shared e.g.
> > BufFileCreateShared, BufFileOpenShared, should be converted to
> > BufFileCreate and BufFileOpen respectively. And the input to these
> > interfaces can be converted to FileSet instead of SharedFileSet.
>
> Here is the first draft based on the idea we discussed, 0001, splits
> sharedfileset.c in sharedfileset.c and fileset.c and 0002 is same patch I
> submitted earlier(use single fileset throughout the worker), just it is rebased on
> top of 0001. Please let me know your thoughts.

Hi,

Here are some comments for the new version patches.

1)
+ TempTablespacePath(tempdirpath, tablespace);
+ snprintf(path, MAXPGPATH, "%s/%s%lu.%u.sharedfileset",
+ tempdirpath, PG_TEMP_FILE_PREFIX,
+ (unsigned long) fileset->creator_pid, fileset->number);

do we need to use different filename for shared and un-shared fileset ?

2)
I think we can remove or adjust the following comments in sharedfileset.c.

----
* SharedFileSets can be used by backends when the temporary files need to be
* opened/closed multiple times and the underlying files need to survive across
* transactions.
----
* We can also use this interface if the temporary files are used only by
* single backend but the files need to be opened and closed multiple times
* and also the underlying files need to survive across transactions. For
----

3)
The 0002 patch still used the word "shared fileset" in some places, I think we
should change it to "fileset".

4)
-extern File SharedFileSetCreate(SharedFileSet *fileset, const char *name);
-extern File SharedFileSetOpen(SharedFileSet *fileset, const char *name,
- int mode);
-extern bool SharedFileSetDelete(SharedFileSet *fileset, const char *name,
- bool error_on_failure);
extern void SharedFileSetDeleteAll(SharedFileSet *fileset);
-extern void SharedFileSetUnregister(SharedFileSet *input_fileset);

I noticed the patch delete part of public api, is it better to keep the old api and
let them invoke new api internally ? Having said that, I didn’t find any open source
extension use these old api, so it might be fine to delete them.

Best regards,
Hou zj