On Wed, Mar 23, 2022 at 7:03 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Wed, Mar 23, 2022 at 9:19 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > I think directly using smgrcreate() is a better idea instead of first
> > registering and then unregistering it. I have made that change in
> > the attached patch. After this change now we can merge creating the
> > MAIN_FORKNUM also in the loop below where we are creating other
> > fork[1] with one extra condition but I think current code is in more
> > sync with the other code where we are doing the similar things so I
> > have not merged it in the loop. Please let me know if you think
> > otherwise.
>
> Generally I think our practice is that we do the main fork
> unconditionally (because it should always be there) and the others
> only if they exist. I suggest that you make this consistent with that,
> but you could do it like if (forkNum != MAIN_FORKNUM &&
> !smgrexists(...)) continue if that seems nicer.
Maybe we can do that.
> Do you think that this version handles pending syncs correctly? I
> think perhaps that is overlooked.
Yeah I missed that. So options are either we go to the other approach
and call RelationPreserveStorage() after
RelationCreateStorage(), or we expose the AddPendingSync() function
from the storage layer and then conditionally use it. I think if we
are planning to expose this api then we better rename it to
RelationAddPendingSync(). Honestly, I do not have any specific
preference here. I can try both the approaches and send both if you
or anyone else do not have any preference here?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
