On Sun, Aug 7, 2022 at 9:47 AM Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> On 2022-08-07 09:24:40 +0530, Dilip Kumar wrote:
> > On Sat, Aug 6, 2022 at 9:36 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > >
> > > Dilip Kumar <dilipbalaut@gmail.com> writes:
> > > > On Fri, Aug 5, 2022 at 10:43 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > > >> Yeah maybe it is not necessary to close as these unowned smgr will
> > > >> automatically get closed on the transaction end.
> > >
> > > I do not think this is a great idea for the per-relation smgrs created
> > > during RelationCopyStorageUsingBuffer. Yeah, they'll be mopped up at
> > > transaction end, but that doesn't mean that creating possibly tens of
> > > thousands of transient smgrs isn't going to cause performance issues.
>
> I was assuming that the files would get reopened at the end of the transaction
> anyway, but it looks like that's not the case, unless wal_level=minimal.
>
> Hm. CreateAndCopyRelationData() calls RelationCreateStorage() with
> register_delete = false, which is ok because createdb_failure_callback will
> clean things up. But that's another thing that's not great for a routine with
> a general name...
>
>
> > Okay, so for that we can simply call smgrcloserellocator(rlocator);
> > before exiting the RelationCopyStorageUsingBuffer() right?
>
> Yea, I think so.
Done, along with that, I have also got the hunk of smgropen and
smgrclose in ScanSourceDatabasePgClass() which I had in v1 patch[1].
Because here we do not want to reuse the smgr of the pg_class again so
instead of closing at the end with smgrcloserellocator() we can just
keep the smgr reference and close immediately after getting the number
of blocks. Whereas in CreateAndCopyRelationData and
RelationCopyStorageUsingBuffer() we are using the smgr of the source
and dest relation multiple time so it make sense to not close it
immediately and we can close while exiting the function with
smgrcloserellocator().
[1]
+ smgr = smgropen(rlocator, InvalidBackendId);
+ nblocks = smgrnblocks(smgr, MAIN_FORKNUM);
+ smgrclose(smgr);
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
