On Wed, Mar 23, 2022 at 5:54 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Wed, Mar 23, 2022 at 4:42 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > Okay this is an interesting point. So one option is that in case of
> > failure while using the wal log strategy we do not remove the database
> > directory, because an abort transaction will take care of removing the
> > relation file. But then in failure case we will leave the orphaned
> > database directory with version file and the relmap file. Another
> > option is to do the redundant cleanup as we are doing now. Any other
> > options?
>
> I think our overriding goal should be to get everything using one
> mechanism. It doesn't look straightforward to get everything to go
> through the PendingRelDelete mechanism, because as you say, it can't
> handle non-relation files or directories. However, what if we opt out
> of that mechanism? We could do that either by not using
> RelationCreateStorage() in the first place and directly calling
> smgrcreate(), or by using RelationPreserveStorage() afterwards to yank
> the file back out of the list.
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.
[1]
+ /*
+ * Create and copy all forks of the relation. We are not using
+ * RelationCreateStorage() as it is registering the cleanup for the
+ * underlying relation storage on the transaction abort. But during create
+ * database failure, we have a separate cleanup mechanism for the whole
+ * database directory. Therefore, we don't need to register cleanup for
+ * each individual relation storage.
+ */
+ smgrcreate(RelationGetSmgr(dst_rel), MAIN_FORKNUM, false);
+ if (permanent)
+ log_smgrcreate(&dst_rnode, MAIN_FORKNUM);
+
+ /* copy main fork. */
+ RelationCopyStorageUsingBuffer(src_rel, dst_rel, MAIN_FORKNUM, permanent);
+
+ /* copy those extra forks that exist */
+ for (ForkNumber forkNum = MAIN_FORKNUM + 1;
+ forkNum <= MAX_FORKNUM; forkNum++)
+ {
+ if (smgrexists(RelationGetSmgr(src_rel), forkNum))
+ {
+ smgrcreate(RelationGetSmgr(dst_rel), forkNum, false);
+
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
