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. 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? 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. 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. -- With Regards, Amit Kapila.