Re: pgsql: walreceiver uses a temporary replication slot by default
On 2020/02/12 7:53, Jehan-Guillaume de Rorthais wrote: Hello, On Mon, 10 Feb 2020 16:37:53 +0100 Peter Eisentraut wrote: On 2020-01-23 21:49, Robert Haas wrote: On Tue, Jan 14, 2020 at 8:57 AM Peter Eisentraut wrote: walreceiver uses a temporary replication slot by default If no permanent replication slot is configured using primary_slot_name, the walreceiver now creates and uses a temporary replication slot. A new setting wal_receiver_create_temp_slot can be used to disable this behavior, for example, if the remote instance is out of replication slots. Reviewed-by: Masahiko Sawada Discussion: https://www.postgresql.org/message-id/CA%2Bfd4k4dM0iEPLxyVyme2RAFsn8SUgrNtBJOu81YqTY4V%2BnqZA%40mail.gmail.com Neither the commit message for this patch nor any of the comments in the patch seem to explain why this is a desirable change. I assume that's probably discussed on the thread that is linked here, but you shouldn't have to dig through the discussion thread to figure out what the benefits of a change like this are. You are right, this has gotten a bit lost in the big thread. The rationale is basically the same as why client-side tools like pg_basebackup use a temporary slot: So that the WAL data that they are interested in doesn't disappear while they are connected. In my humble opinion, I prefer the previous behavior, streaming without temporary slot, for one reason: primary availability. +1 Should the standby lag far behind the primary (no matter the root cause), the standby was disconnected because of missing WAL. Worst case scenario, we must rebuild it, hopefully from backups. Best case scenario, it fetches WALs from PITR backup. As soon as the later is possible in the stack, I consider slot like a burden from the operability point of view. If standbys can not fetch archived WAL from PITR, then we can consider slots. With temp slot created by default, if one standby lag far behind, it can make the primary unavailable. We have nothing yet to forbid a slot to fill the pg_wal partition. How new users creating their first cluster would react in such situation? I suppose the original discussion was mostly targeting them? Recovering from this is way more scary than building a standby. So the default behavior might not be desirable and maybe wal_receiver_create_temp_slot might be off by default? Note that Kyotaro HORIGUCHI is working on a patch to restricting maximum keep segments by repslots: https://www.postgresql.org/message-id/flat/20190627162256.4f4872b8%40firost#6cba1177f766e7ffa5237789e748da38 Yeah, I think it's better to disable this option until something like Horiguchi-san's proposal will have been committed, i.e., until the upper limit on the number (or size) of WAL files that remain for slots become configurable. Regards, -- Fujii Masao NTT DATA CORPORATION Advanced Platform Technology Group Research and Development Headquarters
pgsql: Doc: fix old oversights in GRANT/REVOKE documentation.
Doc: fix old oversights in GRANT/REVOKE documentation. The GRANTED BY clause in GRANT/REVOKE ROLE has been there since 2005 but was never documented. I'm not sure now whether that was just an oversight or was intentional (given the limited capability of the option). But seeing that pg_dumpall does emit code that uses this option, it seems like not documenting it at all is a bad idea. Also, when we upgraded the syntax to allow CURRENT_USER/SESSION_USER as the privilege recipient, the role form of GRANT was incorrectly not modified to show that, and REVOKE's docs weren't touched at all. Although I'm not that excited about GRANTED BY, the other oversight seems serious enough to justify a back-patch. Discussion: https://postgr.es/m/[email protected] Branch -- REL_12_STABLE Details --- https://git.postgresql.org/pg/commitdiff/2efefd28add2b91da87739b52d04522a61676fc9 Modified Files -- doc/src/sgml/ref/grant.sgml | 24 doc/src/sgml/ref/revoke.sgml | 42 +++--- 2 files changed, 47 insertions(+), 19 deletions(-)
pgsql: Doc: fix old oversights in GRANT/REVOKE documentation.
Doc: fix old oversights in GRANT/REVOKE documentation. The GRANTED BY clause in GRANT/REVOKE ROLE has been there since 2005 but was never documented. I'm not sure now whether that was just an oversight or was intentional (given the limited capability of the option). But seeing that pg_dumpall does emit code that uses this option, it seems like not documenting it at all is a bad idea. Also, when we upgraded the syntax to allow CURRENT_USER/SESSION_USER as the privilege recipient, the role form of GRANT was incorrectly not modified to show that, and REVOKE's docs weren't touched at all. Although I'm not that excited about GRANTED BY, the other oversight seems serious enough to justify a back-patch. Discussion: https://postgr.es/m/[email protected] Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/dce988145f7e455f224310afec6b06bcf3eed24e Modified Files -- doc/src/sgml/ref/grant.sgml | 24 doc/src/sgml/ref/revoke.sgml | 42 +++--- 2 files changed, 47 insertions(+), 19 deletions(-)
pgsql: Doc: fix old oversights in GRANT/REVOKE documentation.
Doc: fix old oversights in GRANT/REVOKE documentation. The GRANTED BY clause in GRANT/REVOKE ROLE has been there since 2005 but was never documented. I'm not sure now whether that was just an oversight or was intentional (given the limited capability of the option). But seeing that pg_dumpall does emit code that uses this option, it seems like not documenting it at all is a bad idea. Also, when we upgraded the syntax to allow CURRENT_USER/SESSION_USER as the privilege recipient, the role form of GRANT was incorrectly not modified to show that, and REVOKE's docs weren't touched at all. Although I'm not that excited about GRANTED BY, the other oversight seems serious enough to justify a back-patch. Discussion: https://postgr.es/m/[email protected] Branch -- REL9_6_STABLE Details --- https://git.postgresql.org/pg/commitdiff/804a650e4b0cddca546ddb37c7785c93b0752d07 Modified Files -- doc/src/sgml/ref/grant.sgml | 24 +++ doc/src/sgml/ref/revoke.sgml | 46 2 files changed, 49 insertions(+), 21 deletions(-)
pgsql: Doc: fix old oversights in GRANT/REVOKE documentation.
Doc: fix old oversights in GRANT/REVOKE documentation. The GRANTED BY clause in GRANT/REVOKE ROLE has been there since 2005 but was never documented. I'm not sure now whether that was just an oversight or was intentional (given the limited capability of the option). But seeing that pg_dumpall does emit code that uses this option, it seems like not documenting it at all is a bad idea. Also, when we upgraded the syntax to allow CURRENT_USER/SESSION_USER as the privilege recipient, the role form of GRANT was incorrectly not modified to show that, and REVOKE's docs weren't touched at all. Although I'm not that excited about GRANTED BY, the other oversight seems serious enough to justify a back-patch. Discussion: https://postgr.es/m/[email protected] Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/a56c495873c8a507534c3a98217664d2684497a5 Modified Files -- doc/src/sgml/ref/grant.sgml | 24 +++ doc/src/sgml/ref/revoke.sgml | 46 2 files changed, 49 insertions(+), 21 deletions(-)
pgsql: Doc: fix old oversights in GRANT/REVOKE documentation.
Doc: fix old oversights in GRANT/REVOKE documentation. The GRANTED BY clause in GRANT/REVOKE ROLE has been there since 2005 but was never documented. I'm not sure now whether that was just an oversight or was intentional (given the limited capability of the option). But seeing that pg_dumpall does emit code that uses this option, it seems like not documenting it at all is a bad idea. Also, when we upgraded the syntax to allow CURRENT_USER/SESSION_USER as the privilege recipient, the role form of GRANT was incorrectly not modified to show that, and REVOKE's docs weren't touched at all. Although I'm not that excited about GRANTED BY, the other oversight seems serious enough to justify a back-patch. Discussion: https://postgr.es/m/[email protected] Branch -- REL9_5_STABLE Details --- https://git.postgresql.org/pg/commitdiff/2b9d4ec1d3c0da473232a9c1b065b976e3a2d13e Modified Files -- doc/src/sgml/ref/grant.sgml | 24 +++ doc/src/sgml/ref/revoke.sgml | 46 2 files changed, 49 insertions(+), 21 deletions(-)
pgsql: Doc: fix old oversights in GRANT/REVOKE documentation.
Doc: fix old oversights in GRANT/REVOKE documentation. The GRANTED BY clause in GRANT/REVOKE ROLE has been there since 2005 but was never documented. I'm not sure now whether that was just an oversight or was intentional (given the limited capability of the option). But seeing that pg_dumpall does emit code that uses this option, it seems like not documenting it at all is a bad idea. Also, when we upgraded the syntax to allow CURRENT_USER/SESSION_USER as the privilege recipient, the role form of GRANT was incorrectly not modified to show that, and REVOKE's docs weren't touched at all. Although I'm not that excited about GRANTED BY, the other oversight seems serious enough to justify a back-patch. Discussion: https://postgr.es/m/[email protected] Branch -- REL_11_STABLE Details --- https://git.postgresql.org/pg/commitdiff/736ba917f4849915b70794f291f85dedd890d2f8 Modified Files -- doc/src/sgml/ref/grant.sgml | 24 doc/src/sgml/ref/revoke.sgml | 42 +++--- 2 files changed, 47 insertions(+), 19 deletions(-)
pgsql: Remove long-dead comments.
Remove long-dead comments. These should've been dropped by a8bb8eb58, but evidently were missed. Not important enough to back-patch. Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/0973f5602c349ad99fae6f57cdcc26754a03ba83 Modified Files -- src/backend/commands/user.c | 4 1 file changed, 4 deletions(-)
pgsql: Doc: Restructure B-Tree support routine docs.
Doc: Restructure B-Tree support routine docs. Use a top-level "variablelist", with one item per B-Tree support function. This structure matches the structure used by various "Extensibility" sections in other documentation chapters for other index access methods. An explicit list makes it much clearer where each item begins and ends. This wasn't really a problem before now, but an upcoming patch that adds deduplication to nbtree will need to have its own new B-Tree support function. Ease the burden of translators by tidying up btree.sgml ahead of committing the deduplication patch. Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/caba0910afa124b8c6c61208b487846eea6b1970 Modified Files -- doc/src/sgml/btree.sgml | 420 ++-- 1 file changed, 225 insertions(+), 195 deletions(-)
Re: pgsql: walreceiver uses a temporary replication slot by default
On Wed, Feb 12, 2020 at 06:11:06PM +0900, Fujii Masao wrote: > On 2020/02/12 7:53, Jehan-Guillaume de Rorthais wrote: >> In my humble opinion, I prefer the previous behavior, streaming without >> temporary slot, for one reason: primary availability. > > +1 > >> With temp slot created by default, if one standby lag far behind, it can make >> the primary unavailable. We have nothing yet to forbid a slot to fill the >> pg_wal partition. How new users creating their first cluster would react in >> such >> situation? I suppose the original discussion was mostly targeting them? >> Recovering from this is way more scary than building a standby. >> >> So the default behavior might not be desirable and maybe >> wal_receiver_create_temp_slot might be off by default? >> >> Note that Kyotaro HORIGUCHI is working on a patch to restricting maximum keep >> segments by repslots: >> >> https://www.postgresql.org/message-id/flat/20190627162256.4f4872b8%40firost#6cba1177f766e7ffa5237789e748da38 > > Yeah, I think it's better to disable this option until something like > Horiguchi-san's proposal will have been committed, i.e., until > the upper limit on the number (or size) of WAL files that remain > for slots become configurable. Even with that, are we sure this extra feature would be a reason sufficient to change the default value of this option to be enabled? I am not sure about that either. My opinion is that this option is useful to have and that it is not really a problem if you have slot monitoring on the primary (or a standby for cascading). And I'd like to believe that it is a common practice lately for base backups, archivers based on pg_receivewal or even logical decoding, but it could be surprising for some users who do not do that yet. So Jehan-Guillaume's arguments sound also sensible to me (he also maintains an automatic failover solution called PAF). From what I can see nobody really likes the current state of things for this option, and that does not come down only to its default value. The default GUC value and the way the parameter is loaded by the WAL sender are problematic, still easy enough to fix. How do we move on from here? I could post a patch based on what Sergei Kornilov has sent around [1], but that's Peter's feature. Any opinions? [1]: https://www.postgresql.org/message-id/[email protected] -- Michael signature.asc Description: PGP signature
