Re: Minimal logical decoding on standbys

On Mon, Apr 3, 2023 at 4:26 AM Jeff Davis wrote: > > On Fri, 2023-03-31 at 02:50 -0700, Jeff Davis wrote: > > But if the ConditionVariableEventSleep() API is added, then I think > > we > > should change the non-recovery case to use a CV as well for > > consistency, and it would avoid the need for

Re: Minimal logical decoding on standbys

On Mon, Apr 3, 2023 at 1:31 AM Jeff Davis wrote: > > On Sun, 2023-04-02 at 10:11 +0200, Drouvot, Bertrand wrote: > > I was thinking that, if a new LogicalWalSndWakeup() replaces > > "ConditionVariableBroadcast(>replayedCV);" > > in ApplyWalRecord() then, it could be possible that some

Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode

On Sat, 1 Apr 2023 at 13:24, Melanie Plageman wrote: > Your diff LGTM. > > Earlier upthread in [1], Bharath had mentioned in a review comment about > removing the global variables that he would have expected the analogous > global in analyze.c to also be removed (vac_strategy [and analyze.c also

Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)

Hello, I found that the 'standalone backend' backend type is not documented right now. Adding something like (from commit message) would be helpful: Both the bootstrap backend and single user mode backends will have backend_type STANDALONE_BACKEND. -- Pavel Luzanov Postgres Professional:

Re: Support logical replication of DDLs

On Sun, Apr 2, 2023 at 3:25 PM Phil Florent wrote: > > As an end-user, I am highly interested in the patch > https://commitfest.postgresql.org/42/3595/ but I don't fully get its main > goal in its first version. > It's "for all tables" that will be implemented ? > If one needs a complete

Re: Is RecoveryConflictInterrupt() entirely safe in a signal handler?

On Sat, Jan 14, 2023 at 3:23 PM Thomas Munro wrote: > On Thu, Jan 5, 2023 at 2:14 PM Tom Lane wrote: > > The rcancelrequested API is something that I devised out of whole cloth > > awhile ago. It's not in Tcl's copy of the code, which AFAIK is the > > only other project using this regex engine.

Fix a comment in basic_archive about NO_INSTALLCHECK

Hi, It looks like comments in make file and meson file about not running basic_archive tests in NO_INSTALLCHECK mode are wrong. The comments say the module needs to be loaded via shared_preload_libraries=basic_archive, but it actually doesn't. The custom file needs archive related parameters and

Re: running logical replication as the subscription owner

On Mon, Mar 27, 2023 at 04:47:22PM -0400, Robert Haas wrote: > So that's a grey area, at least IMHO. The patch could be revised in > some way, and the permissions requirements downgraded. However, if we > do that, I think we're going to have to document that although in > theory table owners can

Re: Should vacuum process config file reload more often

On Sat, Apr 1, 2023 at 4:09 AM Melanie Plageman wrote: > > On Fri, Mar 31, 2023 at 10:31 AM Melanie Plageman > wrote: > > > > On Thu, Mar 30, 2023 at 3:26 PM Daniel Gustafsson wrote: > > > > > > > On 30 Mar 2023, at 04:57, Masahiko Sawada wrote: > > > > > > > As another idea, why don't we use

Re: [PATCH] Support % wildcard in extension upgrade filenames

I want to chime in on the issue of lower-number releases that are released after higher-number releases. The way I see this particular problem is that we always put upgrade SQL files in release "packages," and they obviously become static resources. While I [intentionally] overlook some details

Re: Infinite Interval

On Sun, Apr 2, 2023 at 6:54 PM Tom Lane wrote: > >Joseph Koshakow writes: >>> I've added an errcontext to all the errors of the form "X out of >>> range". > >Please note the style guidelines [1]: > >errcontext(const char *msg, ...) is not normally called directly from >

Re: Pass heaprel to GlobalVisTestFor() in vacuumRedirectAndPlaceholder()

Hi, On 2023-04-02 15:52:14 -0700, Peter Geoghegan wrote: > On Sun, Apr 2, 2023 at 3:30 PM Andres Freund wrote: > > > Actually, I suppose that isn't quite true, since you'd still need to > > > find a way to pass the heap relation down to nbtree VACUUM. Say by > > > adding it to IndexVacuumInfo. >

Re: Minimal logical decoding on standbys

> From 56a9559555918a99c202a0924f7b2ede9de4e75d Mon Sep 17 00:00:00 2001 > From: bdrouvotAWS > Date: Tue, 7 Feb 2023 08:59:47 + > Subject: [PATCH v52 3/6] Allow logical decoding on standby. > > Allow a logical slot to be created on standby. Restrict its usage > or its creation if wal_level

Re: Minimal logical decoding on standbys

On Sun, 2023-04-02 at 15:29 -0700, Andres Freund wrote: > I agree that the *wait* has to go through condition_variable.c, but > it doesn't > seem right that creation of the WES needs to go through > condition_variable.c. The kind of WES required by a CV is an implementation detail, so I was

Re: Minimal logical decoding on standbys

On Fri, 2023-03-31 at 02:50 -0700, Jeff Davis wrote: > But if the ConditionVariableEventSleep() API is added, then I think > we > should change the non-recovery case to use a CV as well for > consistency, and it would avoid the need for WalSndWakeup(). It seems like what we ultimately want is for

Re: Infinite Interval

Joseph Koshakow writes: >> I've added an errcontext to all the errors of the form "X out of >> range". Please note the style guidelines [1]: errcontext(const char *msg, ...) is not normally called directly from an ereport message site; rather it is used in error_context_stack

Re: Pass heaprel to GlobalVisTestFor() in vacuumRedirectAndPlaceholder()

On Sun, Apr 2, 2023 at 3:30 PM Andres Freund wrote: > > Actually, I suppose that isn't quite true, since you'd still need to > > find a way to pass the heap relation down to nbtree VACUUM. Say by > > adding it to IndexVacuumInfo. > > It has been added to that already, so it should really be as

Re: hio.c does visibilitymap_pin()/IO while holding buffer lock

Hi, On 2023-03-28 19:17:21 -0700, Andres Freund wrote: > On 2023-03-28 18:21:02 -0700, Andres Freund wrote: > > Here's a draft patch. > > Attached is v2, with a stupid bug fixed and a bit of comment / pgindent > polish. I'd welcome some review (Tomas?), but otherwise I'm planning to push ahead

Re: Infinite Interval

>On Sun, Apr 2, 2023 at 5:36 PM Tom Lane wrote: > >Joseph Koshakow writes: >> I've attached a patch with these changes that is meant to be applied >> over the previous three patches. Let me know what you think. > >Does not really seem like an improvement to me --- I think it's >

Re: Pass heaprel to GlobalVisTestFor() in vacuumRedirectAndPlaceholder()

Hi, On 2023-04-02 10:22:18 -0700, Peter Geoghegan wrote: > On Sun, Apr 2, 2023 at 10:18 AM Peter Geoghegan wrote: > > Making nbtree page deletion more efficient when logical decoding is in > > use seems well worthwhile. There is an "XXX" comment about this issue, > > similar to the SP-GiST one.

Re: Minimal logical decoding on standbys

Hi, On 2023-04-02 15:15:44 -0700, Jeff Davis wrote: > On Sun, 2023-04-02 at 14:35 -0700, Andres Freund wrote: > > Why not offer a function to add a CV to a WES? It seems somehow odd > > to require > > going through condition_variable.c to create a WES. > > I agree that it's a bit odd, but

Re: Minimal logical decoding on standbys

On Sun, 2023-04-02 at 14:35 -0700, Andres Freund wrote: > Why not offer a function to add a CV to a WES? It seems somehow odd > to require > going through condition_variable.c to create a WES. I agree that it's a bit odd, but remember that after waiting on a CV's latch, it needs to re-insert

Re: Making background psql nicer to use in tap tests

Hi, On 2023-04-02 22:24:16 +0200, Daniel Gustafsson wrote: > And a v5 to fix a test failure in recovery tests. Thanks for workin gon this! There's this XXX that I added: > @@ -57,11 +51,10 @@ sub test_streaming > COMMIT; > }); > > - $in .= q{ > - COMMIT; > - \q > -

Re: Infinite Interval

Joseph Koshakow writes: > I've attached a patch with these changes that is meant to be applied > over the previous three patches. Let me know what you think. Does not really seem like an improvement to me --- I think it's adding more complexity than it removes. The changes in CONTEXT messages

Re: Minimal logical decoding on standbys

Hi, On 2023-03-31 02:44:33 -0700, Jeff Davis wrote: > From 2f05cab9012950ae9290fccbb6366d50fc01553e Mon Sep 17 00:00:00 2001 > From: Jeff Davis > Date: Wed, 1 Mar 2023 20:02:42 -0800 > Subject: [PATCH v2] Introduce ConditionVariableEventSleep(). > > The new API takes a WaitEventSet which can

Re: O(n) tasks cause lengthy startups and checkpoints

On Sun, Apr 02, 2023 at 04:37:38PM -0400, Tom Lane wrote: > Nathan Bossart writes: >> It's been a little while since I dug into this, but I do see your point >> that the wraparound risk could be higher in some cases. For example, if >> you have a billion temp files to clean up, the custodian

Re: Infinite Interval

> > This code is duplicated in timestamp_pl_interval(). We could create a function > > to encode the infinity handling rules and then call it in these two places. The > > argument types are different, Timestamp and TimestampTz viz. which map to in64, > > so shouldn't be a problem. But it will be

Re: Minimal logical decoding on standbys

On Fri, 2023-03-31 at 02:44 -0700, Jeff Davis wrote: > Thank you, done. I think the nearby line was also wrong, returning > true > when there was no timeout. I combined the lines and got rid of the > early return so it can check the list and timeout condition like > normal. Attached. On second

Re: O(n) tasks cause lengthy startups and checkpoints

On Sun, Apr 02, 2023 at 04:23:05PM -0400, Tom Lane wrote: > Nathan Bossart writes: >> On Sun, Apr 02, 2023 at 01:40:05PM -0400, Tom Lane wrote: >>> * Why does LookupCustodianFunctions think it needs to search the >>> constant array? > >> The order of the tasks in the array isn't guaranteed to

Re: O(n) tasks cause lengthy startups and checkpoints

Nathan Bossart writes: > It's been a little while since I dug into this, but I do see your point > that the wraparound risk could be higher in some cases. For example, if > you have a billion temp files to clean up, the custodian could be stuck on > that task for a long time. I will give this

Re: [PATCH] Add `verify-system` sslmode to use system CA pool for server cert

> On 31 Mar 2023, at 19:59, Jacob Champion wrote: >> + # Let tests differentiate between vanilla OpenSSL and LibreSSL. >> + AC_CHECK_DECLS([LIBRESSL_VERSION_NUMBER], [], [], [#include >> ]) >> We have a check for SSL_CTX_set_cert_cb which is specifically done since it's >> not present in

Re: Making background psql nicer to use in tap tests

> On 31 Mar 2023, at 22:33, Daniel Gustafsson wrote: > > The attached v4 fixes some incorrect documentation (added by me in v3), and > fixes that background_psql() didn't honor on_error_stop and extraparams passed > by the user. I've also added a commit which implements the \password test >

Re: O(n) tasks cause lengthy startups and checkpoints

Nathan Bossart writes: > On Sun, Apr 02, 2023 at 01:40:05PM -0400, Tom Lane wrote: >> * Why does LookupCustodianFunctions think it needs to search the >> constant array? > The order of the tasks in the array isn't guaranteed to match the order in > the CustodianTask enum. Why not? It's a

Re: [EXTERNAL] Re: [PATCH] Report the query string that caused a memory error under Valgrind

Onur Tirtir writes: > Thank you for reviewing the patch and for your feedback. I believe the v2 > patch should be able to handle other protocol messages too. I like the concept here, but the reporting that the v2 patch provides would be seriously horrid: it's trying to print stuff that isn't

Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode

On Sat, Apr 1, 2023 at 1:57 PM Justin Pryzby wrote: > > On Sat, Apr 01, 2023 at 01:29:13PM -0400, Melanie Plageman wrote: > > Hi, > > > > I was just doing some cleanup on the main patch in this set and realized > > that it was missing a few things. One of which is forbidding the > >

Re: Minimal logical decoding on standbys

Hi, Btw, most of the patches have some things that pgindent will change (and some that my editor will highlight). It wouldn't hurt to run pgindent for the later patches... Pushed the WAL format change. On 2023-04-02 10:27:45 +0200, Drouvot, Bertrand wrote: > During WAL replay on standby, when

Re: Minimal logical decoding on standbys

On Sun, 2023-04-02 at 10:11 +0200, Drouvot, Bertrand wrote: > I was thinking that, if a new LogicalWalSndWakeup() replaces > "ConditionVariableBroadcast(>replayedCV);" > in ApplyWalRecord() then, it could be possible that some walsender(s) > are requested to wake up while they are actually doing

Re: O(n) tasks cause lengthy startups and checkpoints

On Sun, Apr 02, 2023 at 11:42:26AM -0700, Andres Freund wrote: > Just want to note that I've repeatedly objected to 0002 and 0003, i.e. moving > serialized logical decoding snapshots and mapping files, to custodian, and > still do. Without further work it increases wraparound risks (the filenames

Re: GUC for temporarily disabling event triggers

On Mon, Mar 06, 2023 at 01:24:56PM +0100, Daniel Gustafsson wrote: > > On 27 Jan 2023, at 15:00, Mikhail Gribkov wrote: > > > There is a complete framework for disabling various types of the event > > triggers separately, but, the list of valid GUC values only include 'all' > > and 'none'. Why

Re: O(n) tasks cause lengthy startups and checkpoints

On Sun, Apr 02, 2023 at 01:40:05PM -0400, Tom Lane wrote: > I took a brief look through v20, and generally liked what I saw, > but there are a few things troubling me: Thanks for taking a look. > * The comments for CustodianEnqueueTask claim that it won't enqueue an > already-queued task, but I

Re: GUC for temporarily disabling event triggers

> On 7 Mar 2023, at 16:02, Mikhail Gribkov wrote: > * The patch does what it intends to do; > * The implementation way is clear; > * All test are passed; > * No additional problems catched - at least by my eyes; > > I think it can be marked as Ready for Committer This patch has been RFC for

Re: O(n) tasks cause lengthy startups and checkpoints

Hi, On 2023-04-02 13:40:05 -0400, Tom Lane wrote: > Nathan Bossart writes: > > another rebase for cfbot > > I took a brief look through v20, and generally liked what I saw, > but there are a few things troubling me: Just want to note that I've repeatedly objected to 0002 and 0003, i.e. moving

Re: regression coverage gaps for gist and hash indexes

Hi, On 2023-04-02 13:03:51 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2023-04-02 12:38:32 -0400, Tom Lane wrote: > >> If they have to run serially then that means that their runtime > >> contributes 1-for-1 to the total runtime of the core regression tests, > >> which is not nice at

Re: O(n) tasks cause lengthy startups and checkpoints

Nathan Bossart writes: > another rebase for cfbot I took a brief look through v20, and generally liked what I saw, but there are a few things troubling me: * The comments for CustodianEnqueueTask claim that it won't enqueue an already-queued task, but I don't think I believe that, because it

Re: Add "host" to startup packet

> On 2 Apr 2023, at 18:33, Tom Lane wrote: > > Greg Stark writes: >> My question is a bit different. How does this interact with TLS SNI. >> Can you just use the SNI name given in the TLS handshake? Should the >> server require them to match? Is there any value to having a separate >> source

Re: Pass heaprel to GlobalVisTestFor() in vacuumRedirectAndPlaceholder()

On Sun, Apr 2, 2023 at 10:18 AM Peter Geoghegan wrote: > Making nbtree page deletion more efficient when logical decoding is in > use seems well worthwhile. There is an "XXX" comment about this issue, > similar to the SP-GiST one. It looks like you already have everything > you need to make this

Re: Pass heaprel to GlobalVisTestFor() in vacuumRedirectAndPlaceholder()

On Sun, Apr 2, 2023 at 1:25 AM Drouvot, Bertrand wrote: > now that the heap relation is passed down to vacuumRedirectAndPlaceholder() > thanks to 61b313e47e, we can also pass it down to GlobalVisTestFor() too (to > allow more pruning). What about BTPageIsRecyclable() and

Re: regression coverage gaps for gist and hash indexes

Andres Freund writes: > On 2023-04-02 12:38:32 -0400, Tom Lane wrote: >> If they have to run serially then that means that their runtime >> contributes 1-for-1 to the total runtime of the core regression tests, >> which is not nice at all. > Agreed, it's not nice. At least reasonably quick (74ms

Re: regression coverage gaps for gist and hash indexes

Hi, On 2023-04-02 12:38:32 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2023-04-01 06:02:47 +0200, Drouvot, Bertrand wrote: > >> +1 to put them in gist.sql and hash_index.sql. > > > Unfortunately it turns out that running them in a parallel group reliably > > prevents cleanup of the

Re: regression coverage gaps for gist and hash indexes

Andres Freund writes: > On 2023-04-01 06:02:47 +0200, Drouvot, Bertrand wrote: >> +1 to put them in gist.sql and hash_index.sql. > Unfortunately it turns out that running them in a parallel group reliably > prevents cleanup of the dead rows, at least on my machine. Thereby preventing > any

Re: Add "host" to startup packet

Greg Stark writes: > My question is a bit different. How does this interact with TLS SNI. > Can you just use the SNI name given in the TLS handshake? Should the > server require them to match? Is there any value to having a separate > source for this info? Is something similar available in GSSAPI

Re: regression coverage gaps for gist and hash indexes

Hi, On 2023-04-01 06:02:47 +0200, Drouvot, Bertrand wrote: > On 4/1/23 1:13 AM, Andres Freund wrote: > > On 2023-03-31 17:00:00 +0300, Alexander Lakhin wrote: > > > 31.03.2023 15:55, Tom Lane wrote: > > > > See also the thread about bug #16329 [1]. Alexander promised to look > > > > into

Re: Add "host" to startup packet

On Sun, 2 Apr 2023 at 11:38, Tom Lane wrote: > > Even if all that infrastructure sprang into existence, is this really any > more useful than basing your switching on the host's resolved IP address? > I'm doubtful that there's enough win there to justify pushing this rock > to the top of the

Re: Add "host" to startup packet

Lev Kokotov writes: > Patch attached below. TLDR, I'd like to add "host" to the startup packet. I don't think this is of any use at all in isolation. What is the server going to do with it? What's your plan for persuading clients other than libpq to supply it? How are poolers supposed to

Re: Add "host" to startup packet

Hi Lev,02.04.2023, 14:43, "Lev Kokotov" :Patch attached below. TLDR, I'd like to add "host" to the startup packet.I'm trying to run multiple Postgres servers in a multi-tenant environment behind a pooler. Currently, the only way to differentiate Postgres databases is with the user/dbname

Add "host" to startup packet

Hello, Patch attached below. TLDR, I'd like to add "host" to the startup packet. I'm trying to run multiple Postgres servers in a multi-tenant environment behind a pooler . Currently, the only way to differentiate Postgres databases is with the user/dbname

Pass heaprel to GlobalVisTestFor() in vacuumRedirectAndPlaceholder()

hi hackers, now that the heap relation is passed down to vacuumRedirectAndPlaceholder() thanks to 61b313e47e, we can also pass it down to GlobalVisTestFor() too (to allow more pruning). Please find attached a tiny patch to do so. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS

Re: Minimal logical decoding on standbys

Hi, On 4/1/23 6:50 AM, Amit Kapila wrote: On Fri, Mar 31, 2023 at 7:14 PM Drouvot, Bertrand wrote: That sounds like a good idea. We could imagine creating a LogicalWalSndWakeup() doing the Walsender(s) triage based on a new variable (as you suggest). But, it looks to me that we: - would