On Fri, Jan 24, 2020 at 8:20 AM Fujii Masao <masao.fu...@oss.nttdata.com> wrote: > > On 2020/01/24 15:38, Arthur Zakirov wrote: > > On 2020/01/24 14:56, Michael Paquier wrote: > >> On Fri, Jan 24, 2020 at 01:28:29PM +0900, Arthur Zakirov wrote: > >>> It is compiled and passes the tests. There is the documentation and > >>> it is > >>> built too without an error. > >>> > >>> It seems that consensus about the returned type was reached and I > >>> marked the > >>> patch as "Ready for Commiter". > >> > >> + fsync_fname_ext(filename, S_ISDIR(fst.st_mode), false, ERROR); > >> One comment here: should we warn better users in the docs that a fsync > >> failule will not trigger a PANIC here? Here, fsync failure on heap > >> file => ERROR => potential data corruption. > > > > Ah, true. It is possible to add couple sentences that pg_file_sync() > > doesn't depend on data_sync_retry GUC and doesn't raise a PANIC even for > > database files. > > Thanks all for the review! > > So, what about the attached patch? > In the patch, I added the following note to the doc. > > -------------------- > Note that > <xref linkend="guc-data-sync-retry"/> has no effect on this function, > and therefore a PANIC-level error will not be raised even on failure to > flush database files. > --------------------
We should explicitly mention that this can cause corruption. How about: -------------------- Note that <xref linkend="guc-data-sync-retry"/> has no effect on this function, and therefore a PANIC-level error will not be raised even on failure to flush database files. If that happens, the underlying database objects may be corrupted. --------------------
