On Wed, Mar 23, 2022 at 2:14 AM Robert Haas <robertmhaas@gmail.com> wrote:
>
> So I talked to Andres and Thomas about this and they told me that I
> was right to worry about this problem. Over on the thread about "wrong
> fds used for refilenodes after pg_upgrade relfilenode changes
> Reply-To:" there is a plan to make use ProcSignalBarrier to make smgr
> objects disappear, and ProcSignalBarrier can be processed at any
> CHECK_FOR_INTERRUPTS(), so then we'd have a problem here. Commit
> f10f0ae420ee62400876ab34dca2c09c20dcd030 established a policy that you
> should always re-fetch the smgr object instead of reusing one you've
> already got, and even before that it was known to be unsafe to keep
> them around for any period of time, because anything that opened a
> relation, including a syscache lookup, could potentially accept
> invalidations. So most of our code is already hardened against the
> possibility of smgr objects disappearing. I have a feeling there may
> be some that isn't, but it would be good if this patch didn't
> introduce more such code at the same time that patch is trying to
> introduce more ways to get rid of smgr objects. It was suggested to me
> that what this patch ought to be doing is calling
> CreateFakeRelcacheEntry() and then using RelationGetSmgr(fakerel)
> every time we need the SmgrRelation, without ever keeping it around
> for any amount of code. That way, if the smgr relation gets closed out
> from under us at a CHECK_FOR_INTERRUPTS(), we'll just recreate it at
> the next RelationGetSmgr() call.
Okay, I have changed this in my latest version of the patch.
> Andres also noted that he thinks the patch performs redundant cleanup,
> because of the fact that it uses RelationCreateStorage. That will
> arrange to remove files on abort, but createdb() also has its own
> mechanism for that. It doesn't seem like a thing to do twice in two
> different ways.
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?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com