Re: archive modules
Nathan Bossart <bossartn@amazon.com>
From: "Bossart, Nathan" <bossartn@amazon.com>
To: David Steele <david@pgmasters.net>, Fujii Masao <masao.fujii@oss.nttdata.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2021-11-10T18:22:40Z
Lists: pgsql-hackers
On 11/10/21, 8:10 AM, "David Steele" <david@pgmasters.net> wrote: > On 11/7/21 1:04 AM, Fujii Masao wrote: >> It's helpful if you share how much this approach reduces >> the amount of overhead. > > FWIW we have a test for this in pgBackRest. Running > `archive_command=pgbackrest archive-push ...` 1000 times via system() > yields an average of 3ms per execution. pgBackRest reports ~1ms of time > here so the system() overhead is ~2ms. These times are on my very fast > workstation and in my experience servers are quite a bit slower. In the previous thread [0], I noted a 50% speedup for a basic archiving strategy that involved copying the file to a different directory. > I would prefer this module to be in core as our standard implementation > and load by default in a vanilla install. Hm. I think I disagree with putting it in contrib and with making it the default archive library. The first reason is backward compatibility. There has already been quite a bit of discussion about this, and I don't see how we can get away with anything except for maintaining the existing behavior for now. Maybe we could move to a better default down the road, but I'm hesitant to press that issue too much at the moment. The second reason is that the basic_archive module has a couple of deficiencies. For example, it doesn't handle inconvenient server crashes well (e.g., after archiving but before we've renamed the .ready file). A way to fix this might be to compare the archive file with the to-be-archived file and to succeed if they are exactly the same. Another problem is that there is no handling for multiple servers using basic_archive to write WAL to the same location. This is because basic_archive first copies data to a temporary file that is always named "archtemp." This might be fixed by appending a random string to the temporary file or by locking it somehow, but there are still a few things left to figure out. I think it'd be awesome to eventually fix all these issues in basic_archive and to recommend it as a proper archiving strategy, but I'm worried that this will introduce a significant amount of complexity to this patch. I really only intended for basic_archive to be used for testing and to demonstrate that it's possible use the archive module infrastructure to do something useful. If folks really want it in contrib, I'd at least add a big warning about the aforementioned problems in its docs. Nathan [0] https://postgr.es/m/E9035E94-EC76-436E-B6C9-1C03FBD8EF54%40amazon.com
Commits
-
Disallow setting archive_library and archive_command at the same time
- d627ce3b706d 16.0 landed
-
Restore archive_command documentation
- ba50834551f9 16.0 landed
- 5f56933ea5d5 15.0 landed
-
Doc: improve explanation of when custom GUCs appear in pg_settings.
- 44a51dc990cc 15.0 landed
- 857808a4116b 16.0 landed
-
doc: Add note about re-archiving of same WAL files in docs.
- 3cabe45a819f 16.0 cited
-
In basic_archive tests, insist on wal_level='replica'.
- 00c360a89c1a 15.0 landed
-
Allow archiving via loadable modules.
- 5ef1eefd76f4 15.0 landed
-
Move the code to archive files via the shell to a separate file.
- aeb4cc9ea07a 15.0 landed