On Thu, Aug 13, 2020 at 6:47 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Thu, Aug 13, 2020 at 12:08 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Fri, Aug 7, 2020 at 2:04 PM Dilip Kumar <dilipbal...@gmail.com> wrote: > > > > > > On Thu, Aug 6, 2020 at 2:43 PM Amit Kapila <amit.kapil...@gmail.com> > > > wrote: > > > > > > .. > > > > This patch's functionality can be independently verified by SQL APIs > > > > > > Your changes look fine to me. > > > > > > > I have pushed that patch last week and attached are the remaining > > patches. I have made a few changes in the next patch > > 0001-Extend-the-BufFile-interface.patch and have some comments on it > > which are as below: > > > > Few more comments on the latest patches: > v48-0002-Add-support-for-streaming-to-built-in-replicatio > 1. It appears to me that we don't remove the temporary folders created > by the apply worker. So, we have folders like > pgsql_tmp15324.0.sharedfileset in base/pgsql_tmp directory even when > the apply worker exits. I think we can remove these by calling > PathNameDeleteTemporaryDir in SharedFileSetUnregister while removing > the fileset from registered filesetlist.
I think we need to call SharedFileSetDeleteAll(input_fileset), from SharedFileSetUnregister, so that all the directories created for this fileset are removed > 2. > +typedef struct SubXactInfo > +{ > + TransactionId xid; /* XID of the subxact */ > + int fileno; /* file number in the buffile */ > + off_t offset; /* offset in the file */ > +} SubXactInfo; > + > +static uint32 nsubxacts = 0; > +static uint32 nsubxacts_max = 0; > +static SubXactInfo *subxacts = NULL; > +static TransactionId subxact_last = InvalidTransactionId; > > Will it be better if we move all the subxact related variables (like > nsubxacts, nsubxacts_max and subxact_last) inside SubXactInfo struct > as all the information anyway is related to sub-transactions? I have moved them all to a structure. > 3. > + /* > + * If there is no subtransaction then nothing to do, but if already have > + * subxact file then delete that. > + */ > > extra space before 'but' in the above sentence is not required. Fixed > v48-0001-Extend-the-BufFile-interface > 4. > - * SharedFileSets can also be used by backends when the temporary files need > - * to be opened/closed multiple times and the underlying files need to > survive > + * 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. > * > > No need of 'also' in the above sentence. Fixed -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com