Re: wrong fds used for refilenodes after pg_upgrade relfilenode changes Reply-To:

On Tue, Feb 22, 2022 at 4:40 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
> On 2022-02-22 01:11:21 -0800, Andres Freund wrote:
> > I've started to work on a few debugging aids to find problem like
> > these. Attached are two WIP patches:
>
> Forgot to attach. Also importantly includes a tap test for several of these
> issues

Hi,

Just a few very preliminary comments:

- I am having some trouble understanding clearly what 0001 is doing.
I'll try to study it further.

- 0002 seems like it's generally a good idea. I haven't yet dug into
why the call sites for AssertFileNotDeleted() are where they are, or
whether that's a complete set of places to check.

- In general, 0003 makes a lot of sense to me.

+ /*
+ * Finally tell the kernel to write the data to
storage. Don't smgr if
+ * previously closed, otherwise we could end up evading fd-reuse
+ * protection.
+ */

- I think the above hunk is missing a word, because it uses smgr as a
verb. I also think that it's easy to imagine this explanation being
insufficient for some future hacker to understand the issue.

- While 0004 seems useful for testing, it's an awfully big hammer. I'm
not sure we should be thinking about committing something like that,
or at least not as a default part of the build. But ... maybe?

--
Robert Haas
EDB: http://www.enterprisedb.com