On Tue, Aug 25, 2020 at 6:27 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Tue, Aug 25, 2020 at 10:41 AM Dilip Kumar <dilipbal...@gmail.com> wrote: > > > > On Tue, Aug 25, 2020 at 9:31 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > > > > > I think the existing design is superior as it allows the flexibility > > > to create transaction files in different temp_tablespaces which is > > > quite important to consider as we know the files will be created only > > > for large transactions. Once we fix the sharedfileset for a worker all > > > the files will be created in the temp_tablespaces chosen for the first > > > time apply worker creates it even if it got changed at some later > > > point of time (user can change its value and then do reload config > > > which I think will impact the worker settings as well). This all can > > > happen because we set the tablespaces at the time of > > > SharedFileSetInit. > > > > Yeah, I agree with this point, that if we use the single shared > > fileset then it will always use the same tablespace for all the > > streaming transactions. And, we might get the benefit of concurrent > > I/O if we use different tablespaces as we are not immediately flushing > > the files to the disk. > > > > Okay, so let's retain the original approach then. I have made a few > cosmetic modifications in the first two patches which include updating > docs, comments, slightly modify the commit message, and change the > code to match the nearby code. One change which you might have a > different opinion is below: > > + case WAIT_EVENT_LOGICAL_CHANGES_READ: > + event_name = "ReorderLogicalChangesRead"; > + break; > + case WAIT_EVENT_LOGICAL_CHANGES_WRITE: > + event_name = "ReorderLogicalChangesWrite"; > + break; > + case WAIT_EVENT_LOGICAL_SUBXACT_READ: > + event_name = "ReorderLogicalSubxactRead"; > + break; > + case WAIT_EVENT_LOGICAL_SUBXACT_WRITE: > + event_name = "ReorderLogicalSubxactWrite"; > + break; > > Why do we want to name these events starting with name as Reorder*? I > think these are used in subscriber-side, so no need to use the word > Reorder, so I have removed it from the attached patch. I am planning > to push the first patch (v53-0001-Extend-the-BufFile-interface) in > this series tomorrow unless you have any comments on the same.
Your changes in 0001 and 0002, looks fine to me. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
