Thanks for working on this. At Mon, 23 Mar 2020 20:56:59 +0000, Teja Mupparti <tejesw...@hotmail.com> wrote in > This is my *first* attempt to submit a Postgres patch, please let me know if > I missed any process or format of the patch
Welcome! The format looks fine to me. It would be better if it had a commit message that explains what the patch does. (in the format that git format-patch emits.) > The original bug reporting-email and the relevant discussion is here ... > The crux of the fix is, in the current code, engine drops the buffer and then > truncates the file, but a crash before the truncate and after the buffer-drop > is causing the corruption. Patch reverses the order i.e. truncate the file > and drop the buffer later. BufferAlloc doesn't wait for the BM_IO_IN_PROGRESS for a valid buffer. I'm not sure it's acceptable to remember all to-be-deleted buffers while truncation. + /*START_CRIT_SECTION();*/ Is this a point of argument? It is not needed if we choose the strategy (c) in [1], since the protocol is aiming to allow server to continue running after truncation failure. [1]: https://www.postgresql.org/message-id/20191207001232.klidxnm756wqxvwx%40alap3.anarazel.de However, note that md truncates a "file" a non-atomic way. mdtruncate truncates multiple files from the last segment toward the beginning. If mdtruncate successfully truncated the first several segments then failed, retaining all buffers triggers assertion failure in mdwrite while buffer flush. Some typos found: + * a backround task might flush them to the disk right after we s/backround/background/ + * saved list of buffers that were marked as BM_IO_IN_PRGRESS just s/BM_IO_IN_PRGRESS/BM_IO_IN_PROGRESS/ + * as BM_IO_IN_PROGRES. Though the buffers are marked for IO, they s/BM_IO_IN_PROGRES/BM_IO_IN_PROGRESS/ + * being dicarded). s/dicarded/discarded/ regards. -- Kyotaro Horiguchi NTT Open Source Software Center
