Re: pgsql: Allow using syncfs() in frontend utilities.

[Robert Haas]

On Wed, Mar 27, 2024 at 11:25 AM Nathan Bossart
 wrote:
> Committed.  Again, I apologize this took so long.

No worries from my side; I only noticed recently. I guess Peter's been
waiting a while, though. Thanks for committing.

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

Re: pgsql: Allow using syncfs() in frontend utilities.

[Nathan Bossart]

On Wed, Mar 27, 2024 at 10:41:42AM -0400, Robert Haas wrote:
> On Tue, Mar 26, 2024 at 12:34 PM Nathan Bossart
>  wrote:
>> Here's a first attempt at a patch based on Robert's suggestion from
>> upthread.
> 
> WFM.

Committed.  Again, I apologize this took so long.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

Re: pgsql: Allow using syncfs() in frontend utilities.

[Robert Haas]

On Tue, Mar 26, 2024 at 12:34 PM Nathan Bossart
 wrote:
> Here's a first attempt at a patch based on Robert's suggestion from
> upthread.

WFM.

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

Re: pgsql: Allow using syncfs() in frontend utilities.

[Nathan Bossart]

On Tue, Mar 26, 2024 at 10:11:31AM -0500, Nathan Bossart wrote:
> On Tue, Mar 26, 2024 at 11:18:57AM +0100, Peter Eisentraut wrote:
>> On 22.03.24 17:52, Robert Haas wrote:
>>> I'd like to complain about this commit's addition of a new appendix.
>> 
>> I already complained about that at 
>> 
>> and some follow-up was announced but didn't happen.  It was on my list to
>> look into cleaning up during beta.
> 
> Sorry about this, I lost track of it.

Here's a first attempt at a patch based on Robert's suggestion from
upthread.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
>From c51703257821079016456b152e4a9f41169dad96 Mon Sep 17 00:00:00 2001
From: Nathan Bossart 
Date: Tue, 26 Mar 2024 11:26:56 -0500
Subject: [PATCH v1 1/1] Adjust documentation for syncfs().

Commit 8c16ad3b43 created a new appendix for syncfs(), which is
excessive for the small amount of content it contains.  This commit
moves the description of the caveats to be aware of when using
syncfs() back to the documentation for recovery_init_sync_method.
The documentation for the other utilities with syncfs() support now
directs readers to recovery_init_sync_method for information about
these caveats.

Reported-by: Peter Eisentraut
Suggested-by: Robert Haas
Discussion: https://postgr.es/m/42804669-7063-1320-ed37-3226d5f1067d%40eisentraut.org
Discussion: https://postgr.es/m/CA%2BTgmobUiqKr%2BZMCLc5Qap-sXBnjfGUU%2BZBmzYEjUuWyjsGr1g%40mail.gmail.com
---
 doc/src/sgml/config.sgml   | 12 ++---
 doc/src/sgml/filelist.sgml |  1 -
 doc/src/sgml/postgres.sgml |  1 -
 doc/src/sgml/ref/initdb.sgml   |  4 +--
 doc/src/sgml/ref/pg_basebackup.sgml|  4 +--
 doc/src/sgml/ref/pg_checksums.sgml |  4 +--
 doc/src/sgml/ref/pg_combinebackup.sgml |  4 +--
 doc/src/sgml/ref/pg_dump.sgml  |  5 ++--
 doc/src/sgml/ref/pg_rewind.sgml|  4 +--
 doc/src/sgml/ref/pgupgrade.sgml|  4 +--
 doc/src/sgml/syncfs.sgml   | 36 --
 11 files changed, 24 insertions(+), 55 deletions(-)
 delete mode 100644 doc/src/sgml/syncfs.sgml

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 65a6e6c408..5468637e2e 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -10870,9 +10870,15 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
 On Linux, syncfs may be used instead, to ask the
 operating system to synchronize the file systems that contain the
 data directory, the WAL files and each tablespace (but not any other
-file systems that may be reachable through symbolic links).  See
- for more information about using
-syncfs().
+file systems that may be reachable through symbolic links).  This may
+be a lot faster than the fsync setting, because it
+doesn't need to open each file one by one.  On the other hand, it may
+be slower if a file system is shared by other applications that
+modify a lot of files, since those files will also be written to disk.
+Furthermore, on versions of Linux before 5.8, I/O errors encountered
+while writing data to disk may not be reported to
+PostgreSQL, and relevant error messages may
+appear only in kernel logs.


 This parameter can only be set in the
diff --git a/doc/src/sgml/filelist.sgml b/doc/src/sgml/filelist.sgml
index e0dca81cb2..c92a16c7ac 100644
--- a/doc/src/sgml/filelist.sgml
+++ b/doc/src/sgml/filelist.sgml
@@ -182,7 +182,6 @@
 
 
 
-
 
 
 
diff --git a/doc/src/sgml/postgres.sgml b/doc/src/sgml/postgres.sgml
index 2c107199d3..381af69be2 100644
--- a/doc/src/sgml/postgres.sgml
+++ b/doc/src/sgml/postgres.sgml
@@ -289,7 +289,6 @@ break is not needed in a wider output rendering.
   
   
   
-  
   
 
  
diff --git a/doc/src/sgml/ref/initdb.sgml b/doc/src/sgml/ref/initdb.sgml
index 377c3cb20a..dc9011b40e 100644
--- a/doc/src/sgml/ref/initdb.sgml
+++ b/doc/src/sgml/ref/initdb.sgml
@@ -394,8 +394,8 @@ PostgreSQL documentation
 On Linux, syncfs may be used instead to ask the
 operating system to synchronize the whole file systems that contain the
 data directory, the WAL files, and each tablespace.  See
- for more information about using
-syncfs().
+ for information about
+the caveats to be aware of when using syncfs.


 This option has no effect when --no-sync is used.
diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml
index 208530f393..82d0c8e008 100644
--- a/doc/src/sgml/ref/pg_basebackup.sgml
+++ b/doc/src/sgml/ref/pg_basebackup.sgml
@@ -642,8 +642,8 @@ PostgreSQL documentation
 backup directory.  When the plain format is used,
 pg_basebackup will also synchronize the file systems
 that contain the 

Re: pgsql: Allow using syncfs() in frontend utilities.

[Nathan Bossart]

On Tue, Mar 26, 2024 at 11:18:57AM +0100, Peter Eisentraut wrote:
> On 22.03.24 17:52, Robert Haas wrote:
>> I'd like to complain about this commit's addition of a new appendix.
> 
> I already complained about that at 
> 
> and some follow-up was announced but didn't happen.  It was on my list to
> look into cleaning up during beta.

Sorry about this, I lost track of it.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

Re: pgsql: Allow using syncfs() in frontend utilities.

[Nathan Bossart]

On Fri, Mar 22, 2024 at 12:52:15PM -0400, Robert Haas wrote:
> I'd like to complain about this commit's addition of a new appendix. I
> do understand the temptation to document caveats like this centrally
> instead of in multiple places, but as I've been complaining about over
> in the "documentation structure" thread, our top-level documentation
> index is too big, and I feel strongly that we need to de-clutter it
> rather than cluttering it further. This added a new chapter which is
> just 5 sentences long. I understand that this was done because the
> same issue applies to a bunch of different utilities and we didn't
> want to duplicate this text in all of those places, but I feel like
> this approach just doesn't scale. If we did this in every place where
> we have this much text that we want to avoid duplicating, we'd soon
> have hundreds of appendixes.

Sorry I missed this.  I explored a couple of options last year but the
discussion trailed off [0].

> What I would suggest we do instead is pick one of the places where
> this comes up and document it there, perhaps the
> recovery_init_sync_method GUC. And then make the documentation for the
> other say something like, you know those issues we documented for
> recovery_init_sync_method? Well they also apply to this.

WFM.  I'll put together a patch.

[0] https://postgr.es/m/20231009204823.GA659480%40nathanxps13

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

Re: pgsql: Allow using syncfs() in frontend utilities.

[Peter Eisentraut]

On 22.03.24 17:52, Robert Haas wrote:

On Wed, Sep 6, 2023 at 7:28 PM Nathan Bossart  wrote:

Allow using syncfs() in frontend utilities.

This commit allows specifying a --sync-method in several frontend
utilities that must synchronize many files to disk (initdb,
pg_basebackup, pg_checksums, pg_dump, pg_rewind, and pg_upgrade).
On Linux, users can specify "syncfs" to synchronize the relevant
file systems instead of calling fsync() for every single file.  In
many cases, using syncfs() is much faster.

As with recovery_init_sync_method, this new option comes with some
caveats.  The descriptions of these caveats have been moved to a
new appendix section in the documentation.


Hi,

I'd like to complain about this commit's addition of a new appendix.


I already complained about that at 
 
and some follow-up was announced but didn't happen.  It was on my list 
to look into cleaning up during beta.

Re: pgsql: Allow using syncfs() in frontend utilities.

[Robert Haas]

On Wed, Sep 6, 2023 at 7:28 PM Nathan Bossart  wrote:
> Allow using syncfs() in frontend utilities.
>
> This commit allows specifying a --sync-method in several frontend
> utilities that must synchronize many files to disk (initdb,
> pg_basebackup, pg_checksums, pg_dump, pg_rewind, and pg_upgrade).
> On Linux, users can specify "syncfs" to synchronize the relevant
> file systems instead of calling fsync() for every single file.  In
> many cases, using syncfs() is much faster.
>
> As with recovery_init_sync_method, this new option comes with some
> caveats.  The descriptions of these caveats have been moved to a
> new appendix section in the documentation.

Hi,

I'd like to complain about this commit's addition of a new appendix. I
do understand the temptation to document caveats like this centrally
instead of in multiple places, but as I've been complaining about over
in the "documentation structure" thread, our top-level documentation
index is too big, and I feel strongly that we need to de-clutter it
rather than cluttering it further. This added a new chapter which is
just 5 sentences long. I understand that this was done because the
same issue applies to a bunch of different utilities and we didn't
want to duplicate this text in all of those places, but I feel like
this approach just doesn't scale. If we did this in every place where
we have this much text that we want to avoid duplicating, we'd soon
have hundreds of appendixes.

What I would suggest we do instead is pick one of the places where
this comes up and document it there, perhaps the
recovery_init_sync_method GUC. And then make the documentation for the
other say something like, you know those issues we documented for
recovery_init_sync_method? Well they also apply to this.

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