On Tue, Feb 22, 2022 at 4:40 AM Andres Freund <andres@anarazel.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