Re: walsender performance regression due to logical decoding on standby changes

2023-05-10 Thread Drouvot, Bertrand
Hi, On 5/9/23 11:00 PM, Andres Freund wrote: Hi, On 2023-05-09 13:38:24 -0700, Jeff Davis wrote: On Tue, 2023-05-09 at 12:02 -0700, Andres Freund wrote: I don't think the approach of not having any sort of "registry" of whether anybody is waiting for the replay position to be updated is

Re: walsender performance regression due to logical decoding on standby changes

2023-05-10 Thread Bharath Rupireddy
On Wed, May 10, 2023 at 12:33 AM Andres Freund wrote: > > Unfortunately I have found the following commit to have caused a performance > regression: > > commit e101dfac3a53c20bfbf1ca85d30a368c2954facf > > The problem is that, on a standby, after the change - as needed to for the > approach to

Re: pg_upgrade and logical replication

2023-05-10 Thread Peter Smith
Here are some review comments for the v5-0001 patch code. == General 1. ALTER SUBSCRIPTION name ADD TABLE (relid = XYZ, state = 'x' [, lsn = 'X/Y']) I was a bit confused by this relation 'state' mentioned in multiple places. IIUC the pg_upgrade logic is going to reject anything with a

Re: pg_upgrade and logical replication

2023-05-10 Thread Peter Smith
On Mon, Apr 24, 2023 at 4:19 PM Julien Rouhaud wrote: > > Hi, > > On Thu, Apr 13, 2023 at 03:26:56PM +1000, Peter Smith wrote: > > > > 1. > > All the comments look alike, so it is hard to know what is going on. > > If each of the main test parts could be highlighted then the test code > > would

Re: [PATCH] Use RelationClose rather than table_close in heap_create_with_catalog

2023-05-10 Thread tender wang
Bharath Rupireddy 于2023年5月10日周三 22:17写道: > On Sat, Mar 18, 2023 at 12:34 PM Xiaoran Wang wrote: > > > > Hi hackers, > > > > In heap_create_with_catalog, the Relation new_rel_desc is created > > by RelationBuildLocalRelation, not table_open. So it's better to > > call RelationClose to release

Re: benchmark results comparing versions 15.2 and 16

2023-05-10 Thread Alexander Lakhin
08.05.2023 16:00, Alexander Lakhin wrote: ... Having compared 15.3 (56e869a09) with master (58f5edf84) I haven't seen significant regressions except a few minor ones. First regression observed with a simple pgbench test: Another noticeable, but not critical performance degradation is revealed

Re: Tables getting stuck at 's' state during logical replication

2023-05-10 Thread Bharath Rupireddy
On Tue, May 9, 2023 at 4:38 PM Amit Kapila wrote: > > On Fri, May 5, 2023 at 7:27 PM Padmavathi G wrote: > > > > Some background on the setup on which I am trying to carry out the upgrade: > > > > We have a pod in a kubernetes cluster which contains the postgres 11 image. > > We are following

Re: smgrzeroextend clarification

2023-05-10 Thread Bharath Rupireddy
On Wed, May 10, 2023 at 3:20 PM Peter Eisentraut wrote: > > I was looking at the new smgrzeroextend() function in the smgr API. The > documentation isn't very extensive: > > /* > * smgrzeroextend() -- Add new zeroed out blocks to a file. > * > * Similar to smgrextend(), except the

Re: [PATCH] Add native windows on arm64 support

2023-05-10 Thread Andrew Dunstan
On 2023-05-08 Mo 15:58, Tom Lane wrote: Andres Freund writes: I don't really have feelings either way - but haven't we gone further and even backpatched things like spinlock support for new arches in the past? Mmmm ... don't really think those cases were comparable. We weren't adding

Re: [PATCH] Use RelationClose rather than table_close in heap_create_with_catalog

2023-05-10 Thread Xiaoran Wang
The routine table_close​ takes 2 params: Relation​ and LOCKMODE​, it first calls RelationClose to decrease the relation cache reference count, then deals with the lock on the table based on LOCKMOD​ param. In heap_create_with_catalog, the Relation new_rel_desc is only a local relation cache,

Re: [PATCH] Use RelationClose rather than table_close in heap_create_with_catalog

2023-05-10 Thread Bharath Rupireddy
On Sat, Mar 18, 2023 at 12:34 PM Xiaoran Wang wrote: > > Hi hackers, > > In heap_create_with_catalog, the Relation new_rel_desc is created > by RelationBuildLocalRelation, not table_open. So it's better to > call RelationClose to release it. > > What's more, the comment for it seems useless,

Re: Allow pg_archivecleanup to remove backup history files

2023-05-10 Thread Bharath Rupireddy
On Tue, May 9, 2023 at 7:03 PM torikoshia wrote: > > Attached a patch with documentation and regression tests. Thanks. I think pg_archivecleanup cleaning up history files makes it a complete feature as there's no need to write custom code/scripts over and above what pg_archivecleanup provides.

Re: MERGE lacks ruleutils.c decompiling support!?

2023-05-10 Thread Alvaro Herrera
BTW, I spent some time adding a cross-check to see if the code was at least approximately correct for all the queries in the MERGE regression tests, and couldn't find any failures. I then extended the test to the other optimizable commands, and couldn't find any problems there either. My

Re: Feature: Add reloption support for table access method

2023-05-10 Thread Jelte Fennema
I'm definitely in favor of this general idea of supporting custom rel options, we could use that for Citus its columnar TableAM. There have been at least two other discussions on this topic: 1.

RE: walsender performance regression due to logical decoding on standby changes

2023-05-10 Thread Zhijie Hou (Fujitsu)
On Wednesday, May 10, 2023 2:36 PM Bharath Rupireddy wrote: > > On Wed, May 10, 2023 at 12:33 AM Andres Freund > wrote: > > > > Unfortunately I have found the following commit to have caused a > performance > > regression: > > > > commit e101dfac3a53c20bfbf1ca85d30a368c2954facf > > > > The

Re: Subscription statistics are not dropped at DROP SUBSCRIPTION in some cases

2023-05-10 Thread Melih Mutlu
Hi, Masahiko Sawada , 8 May 2023 Pzt, 10:24 tarihinde şunu yazdı: > I've attached the patch. Feedback is very welcome. > Thanks for the patch, nice catch. I can confirm that the issue exists on HEAD and gets resolved by this patch. Also it looks like stats are really not affected if transaction

Re: WAL Insertion Lock Improvements

2023-05-10 Thread Michael Paquier
On Mon, May 08, 2023 at 05:57:09PM +0530, Bharath Rupireddy wrote: > Note that I've used pg_logical_emit_message() for ease of > understanding about the txns generating various amounts of WAL, but > the pattern is the same if txns are generating various amounts of WAL > say with inserts. Sounds

Re: Memory leak from ExecutorState context?

2023-05-10 Thread Jehan-Guillaume de Rorthais
Thank you for your review! On Mon, 8 May 2023 11:56:48 -0400 Melanie Plageman wrote: > ... > > 4. accessor->read_buffer is currently allocated in accessor->context as > > well. > > > >This buffer holds tuple read from the fileset. This is still a buffer, > > but not related to any file

smgrzeroextend clarification

2023-05-10 Thread Peter Eisentraut
I was looking at the new smgrzeroextend() function in the smgr API. The documentation isn't very extensive: /* * smgrzeroextend() -- Add new zeroed out blocks to a file. * * Similar to smgrextend(), except the relation can be extended by * multiple blocks at once and the added

Re: walsender performance regression due to logical decoding on standby changes

2023-05-10 Thread Drouvot, Bertrand
Hi, On 5/10/23 8:36 AM, Bharath Rupireddy wrote: On Wed, May 10, 2023 at 12:33 AM Andres Freund wrote: Unfortunately I have found the following commit to have caused a performance regression: commit e101dfac3a53c20bfbf1ca85d30a368c2954facf The problem is that, on a standby, after the

Re: walsender performance regression due to logical decoding on standby changes

2023-05-10 Thread Amit Kapila
On Wed, May 10, 2023 at 3:41 PM Zhijie Hou (Fujitsu) wrote: > > On Wednesday, May 10, 2023 2:36 PM Bharath Rupireddy > wrote: > > > > > > > My current guess is that mis-using a condition variable is the best bet. I > > > think it should work to use ConditionVariablePrepareToSleep() before a > >

Re: Add two missing tests in 035_standby_logical_decoding.pl

2023-05-10 Thread Amit Kapila
On Tue, May 9, 2023 at 12:44 PM Drouvot, Bertrand wrote: > > On 5/9/23 8:02 AM, Amit Kapila wrote: > > On Mon, May 8, 2023 at 1:45 PM Drouvot, Bertrand > > wrote: > > > > > Why not initialize the cascading standby node just before the standby > > promotion test: "Test standby promotion and

Re: Time delayed LR (WAS Re: logical replication restrictions)

2023-05-10 Thread Bharath Rupireddy
On Fri, Apr 28, 2023 at 2:35 PM Hayato Kuroda (Fujitsu) wrote: > > Dear hackers, > > I rebased and refined my PoC. Followings are the changes: Thanks. Apologies for being late here. Please bear with me if I'm repeating any of the discussed points. I'm mainly trying to understand the production

Discussion: psql \et -> edit the trigger function

2023-05-10 Thread Kirk Wolak
We already have \ef \ev The use case here is simply that it saves me from: \d [scroll through all the fields] [often scroll right] select function name \ef [paste function name] and tab completion is much narrower When doing conversions and reviews all of this stuff has to be reviewed.

Re: BUG: Postgres 14 + vacuum_defer_cleanup_age + FOR UPDATE + UPDATE

2023-05-10 Thread Peter Eisentraut
On 12.03.23 00:41, Andres Freund wrote: Hi, On 2023-03-11 15:34:55 -0800, Mark Dilger wrote: On Mar 11, 2023, at 3:22 PM, Andres Freund wrote: Something like the attached. I like that your patch doesn't make the test longer. I assume you've already run the tests and that it works. I

Re: [PATCH] Use RelationClose rather than table_close in heap_create_with_catalog

2023-05-10 Thread Tom Lane
Bharath Rupireddy writes: > And, the /* do not unlock till end of xact */, it looks like it's been > there from day 1. It may be indicating that the ref count fo the new > relation created in heap_create_with_catalog() will be decremented at > the end of xact, but I'm not sure what it means.

Re: Discussion: psql \et -> edit the trigger function

2023-05-10 Thread Pavel Stehule
Hi st 10. 5. 2023 v 17:33 odesílatel Kirk Wolak napsal: > We already have > \ef > \ev > > The use case here is simply that it saves me from: > \d > [scroll through all the fields] > [often scroll right] > select function name > \ef [paste function name] > > and tab completion is much narrower

Re: Discussion: psql \et -> edit the trigger function

2023-05-10 Thread Kirk Wolak
On Wed, May 10, 2023 at 12:20 PM Pavel Stehule wrote: > Hi > > st 10. 5. 2023 v 17:33 odesílatel Kirk Wolak napsal: > >> We already have >> \ef >> \ev >> >> The use case here is simply that it saves me from: >> \d >> [scroll through all the fields] >> [often scroll right] >> select function

Re: Discussion: psql \et -> edit the trigger function

2023-05-10 Thread Dagfinn Ilmari Mannsåker
Kirk Wolak writes: > We already have > \ef > \ev > > The use case here is simply that it saves me from: > \d > [scroll through all the fields] > [often scroll right] > select function name > \ef [paste function name] > > and tab completion is much narrower I think it would make more sense to

Re: WAL Insertion Lock Improvements

2023-05-10 Thread Bharath Rupireddy
On Tue, May 9, 2023 at 9:24 AM Bharath Rupireddy wrote: > > On Tue, May 9, 2023 at 9:02 AM Michael Paquier wrote: > > > > On Mon, May 08, 2023 at 04:04:10PM -0700, Nathan Bossart wrote: > > > On Mon, May 08, 2023 at 05:57:09PM +0530, Bharath Rupireddy wrote: > > >> test-case 1: -T5, WAL ~16

Re: Discussion: psql \et -> edit the trigger function

2023-05-10 Thread Tom Lane
Kirk Wolak writes: > \et > Would specifically let me complete the Trigger_Name, but find the function Hmm, I wonder how useful that's really going to be, considering that trigger names aren't unique across tables. Wouldn't it need to be more like "\et table-name trigger-name"? Also, in a

Re: Discussion: psql \et -> edit the trigger function

2023-05-10 Thread Tom Lane
I wrote: > Hmm, I wonder how useful that's really going to be, considering > that trigger names aren't unique across tables. Wouldn't it > need to be more like "\et table-name trigger-name"? Different line of thought: \et seems awfully single-purpose. Perhaps we should think more of "\st

de-catalog one error message

2023-05-10 Thread Alvaro Herrera
Hi While translating the v15 message catalog (yes, I'm quite late!), I noticed that commit 1f39bce02154 introduced three copies of the following message in hashagg_batch_read(): + ereport(ERROR, + (errcode_for_file_access(), +errmsg("unexpected EOF for tape

Re: BUG: Postgres 14 + vacuum_defer_cleanup_age + FOR UPDATE + UPDATE

2023-05-10 Thread Andres Freund
Hi, On 2023-05-10 17:44:07 +0200, Peter Eisentraut wrote: > On 12.03.23 00:41, Andres Freund wrote: > > Hi, > > > > On 2023-03-11 15:34:55 -0800, Mark Dilger wrote: > > > > On Mar 11, 2023, at 3:22 PM, Andres Freund wrote: > > > > > > > > Something like the attached. > > > > > > I like that

Subscription suborigin?

2023-05-10 Thread Bruce Momjian
This commit: commit 366283961a Author: Amit Kapila Date: Thu Jul 21 08:47:38 2022 +0530 Allow users to skip logical replication of data having origin. has this change: diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml

createuser --memeber and PG 16

2023-05-10 Thread Bruce Momjian
In writing the PG 16 release notes, I came upon an oddity in our new createuser syntax, specifically --role and --member. It turns out that --role matches CREATE ROLE ... ROLE IN (and has prior to PG 16) while the new --member option matches CREATE ROLE ... ROLE. The PG 16 feature discussion

Re:Re: Feature: Add reloption support for table access method

2023-05-10 Thread 吴昊
> > The rest of this mail is to talk about the first issue. It looks reasonable > > to add a similar callback in > > struct TableAmRoutine, and parse reloptions by the callback. This patch is > > in the attachment file. > > Why did you add relkind to the callbacks? The callbacks are specific to

Re: smgrzeroextend clarification

2023-05-10 Thread Andres Freund
Hi, On 2023-05-10 11:50:14 +0200, Peter Eisentraut wrote: > I was looking at the new smgrzeroextend() function in the smgr API. The > documentation isn't very extensive: > > /* > * smgrzeroextend() -- Add new zeroed out blocks to a file. > * > * Similar to smgrextend(), except the

Re: Subscription suborigin?

2023-05-10 Thread Bruce Momjian
Never mind --- I just realized "sub" is the table prefix. :-( --- On Wed, May 10, 2023 at 03:36:31PM -0400, Bruce Momjian wrote: > This commit: > > commit 366283961a > Author: Amit Kapila > Date: Thu

Re: [PoC] pg_upgrade: allow to upgrade publisher node

2023-05-10 Thread Peter Smith
Hi Kuroda-san. I checked again the v11-0001. Here are a few more review comments. == src/bin/pg_dump/pg_dump.c 1. help printf(_(" --insertsdump data as INSERT commands, rather than COPY\n")); printf(_(" --load-via-partition-rootload partitions via the root

Redundant strlen(query) in get_rel_infos

2023-05-10 Thread Peter Smith
Hi. While reviewing another patch to the file info.c, I noticed there seem to be some unnecessary calls to strlen(query) in get_rel_infos() function. i.e. The query is explicitly initialized to an empty string immediately prior, so why the strlen? PSA patch for this. -- Kind Regards, Peter

RE: [PoC] pg_upgrade: allow to upgrade publisher node

2023-05-10 Thread Wei Wang (Fujitsu)
On Tue, May 9, 2023 at 17:44 PM Hayato Kuroda (Fujitsu) wrote: > Thank you for reviewing! PSA new version. Thanks for your patches. Here are some comments for 0001 patch: 1. In the function getLogicalReplicationSlots ``` + /* +* Note: Currently we do not have any

RE: Time delayed LR (WAS Re: logical replication restrictions)

2023-05-10 Thread Hayato Kuroda (Fujitsu)
Dear Amit-san, Bharath, Thank you for giving your opinion! > Some of the other solutions like MySQL also have this feature. See > [2], you can also read the other use cases in that article. It seems > pglogical has this feature and there is a customer demand for the same > [3] Additionally, the

Re: Time delayed LR (WAS Re: logical replication restrictions)

2023-05-10 Thread Amit Kapila
On Fri, Apr 28, 2023 at 2:35 PM Hayato Kuroda (Fujitsu) wrote: > > Dear hackers, > > I rebased and refined my PoC. Followings are the changes: > 1. Is my understanding correct that this patch creates the delay files for each transaction? If so, did you consider other approaches such as using one

Re: Discussion: psql \et -> edit the trigger function

2023-05-10 Thread Pavel Stehule
st 10. 5. 2023 v 19:08 odesílatel Kirk Wolak napsal: > On Wed, May 10, 2023 at 12:20 PM Pavel Stehule > wrote: > >> Hi >> >> st 10. 5. 2023 v 17:33 odesílatel Kirk Wolak napsal: >> >>> We already have >>> \ef >>> \ev >>> >>> The use case here is simply that it saves me from: >>> \d >>>

pg_upgrade - typo in verbose log

2023-05-10 Thread Peter Smith
Hi -- While reviewing another patch for the file info.c, I noticed some misplaced colon (':') in the verbose logs for print_rel_infos(). PSA patch to fix it. -- Kind Regards, Peter Smith. Fujitsu Australia v1-0001-pg_upgrade-typo-in-verbose-log.patch Description: Binary data

RE: [PoC] pg_upgrade: allow to upgrade publisher node

2023-05-10 Thread Wei Wang (Fujitsu)
On Thu, May 11, 2023 at 10:12 AM Peter Smith wrote: > Hi Kuroda-san. I checked again the v11-0001. > > Here are a few more review comments. > > == > src/bin/pg_dump/pg_dump.c > > 1. help > > printf(_(" --insertsdump data as INSERT > commands, rather than COPY\n"));

Re: 2023-05-11 release announcement draft

2023-05-10 Thread Jonathan S. Katz
On 5/7/23 10:34 PM, David Rowley wrote: * Fix partition pruning bug with the boolean "IS NOT TRUE" and "IS NOT FALSE" conditions. NULL partitions were accidentally pruned when they shouldn't have been. Thanks for the additional explanation. I took your suggestion verbatim. Thanks, Jonathan

Re: benchmark results comparing versions 15.2 and 16

2023-05-10 Thread Michael Paquier
On Tue, May 09, 2023 at 09:48:24AM -0700, Andres Freund wrote: > On 2023-05-08 12:11:17 -0700, Andres Freund wrote: >> I can reproduce a significant regression due to f193883fc of a workload just >> running >> SELECT CURRENT_TIMESTAMP; >> >> A single session running it on my workstation via

Re: Time delayed LR (WAS Re: logical replication restrictions)

2023-05-10 Thread Amit Kapila
On Wed, May 10, 2023 at 5:35 PM Bharath Rupireddy wrote: > > On Fri, Apr 28, 2023 at 2:35 PM Hayato Kuroda (Fujitsu) > wrote: > > > > Dear hackers, > > > > I rebased and refined my PoC. Followings are the changes: > > Thanks. > > Apologies for being late here. Please bear with me if I'm

Re: [PATCH] Add native windows on arm64 support

2023-05-10 Thread Michael Paquier
On Thu, May 11, 2023 at 12:17:25PM +1200, Thomas Munro wrote: > Azure does have an image "Microsoft Windows 11 Preview arm64" to run > on Ampere CPUs, which says it's a pre-release version intended for > validation, which sounds approximately like what we want. I will try > to find out more.

Re: WAL Insertion Lock Improvements

2023-05-10 Thread Michael Paquier
On Wed, May 10, 2023 at 10:40:20PM +0530, Bharath Rupireddy wrote: > test-case 2: -T900, WAL ~256 bytes - ran for about 3.5 hours and the > more than 3X improvement in TPS is seen - 3.11X @ 512 3.79 @ 768, 3.47 > @ 1024, 2.27 @ 2048, 2.77 @ 4096 > > [...] > > test-case 2: -t100, WAL ~256 bytes

Re: Redundant strlen(query) in get_rel_infos

2023-05-10 Thread Michael Paquier
On Thu, May 11, 2023 at 01:06:42PM +1000, Peter Smith wrote: > While reviewing another patch to the file info.c, I noticed there seem > to be some unnecessary calls to strlen(query) in get_rel_infos() > function. > > i.e. The query is explicitly initialized to an empty string > immediately prior,

Re: walsender performance regression due to logical decoding on standby changes

2023-05-10 Thread Masahiko Sawada
On Wed, May 10, 2023 at 7:33 PM Amit Kapila wrote: > > On Wed, May 10, 2023 at 3:41 PM Zhijie Hou (Fujitsu) > wrote: > > > > On Wednesday, May 10, 2023 2:36 PM Bharath Rupireddy > > wrote: > > > > > > > > > > My current guess is that mis-using a condition variable is the best > > > > bet. I >

Re: [PATCH] Add native windows on arm64 support

2023-05-10 Thread Thomas Munro
On Thu, May 11, 2023 at 11:34 AM Michael Paquier wrote: > On Wed, May 10, 2023 at 09:24:39AM -0400, Andrew Dunstan wrote: > > We will definitely want buildfarm support. I don't have such a machine and > > am not likely to have one any time soon. I do run drongo and fairywren on an > > EC2

Re: [PATCH] Add native windows on arm64 support

2023-05-10 Thread Michael Paquier
On Wed, May 10, 2023 at 09:24:39AM -0400, Andrew Dunstan wrote: > We will definitely want buildfarm support. I don't have such a machine and > am not likely to have one any time soon. I do run drongo and fairywren on an > EC2 instance, but AWS doesn't seem to have support for Windows on ARM. Maybe

Re: benchmark results comparing versions 15.2 and 16

2023-05-10 Thread David Rowley
On Thu, 11 May 2023 at 01:00, Alexander Lakhin wrote: > This time `git bisect` pointed at 3c6fc5820. Having compared execution plans > (both attached), I see the following differences (3c6fc5820~1 vs 3c6fc5820): Based on what you've sent, I'm uninspired to want to try to do anything about it.

RFI: Extending the TOAST Pointer

2023-05-10 Thread Nikita Malakhov
Hi hackers! There were several discussions where the limitations of the existing TOAST pointers were mentioned [1], [2] and [3] and from time to time this topic appears in other places. We proposed a fresh approach to the TOAST mechanics in [2], but unfortunately the patch was met quite

Re: Unlinking Parallel Hash Join inner batch files sooner

2023-05-10 Thread Jehan-Guillaume de Rorthais
Hi, Thanks for working on this! On Wed, 10 May 2023 15:11:20 +1200 Thomas Munro wrote: > One complaint about PHJ is that it can, in rare cases, use a > surprising amount of temporary disk space where non-parallel HJ would > not. When it decides that it needs to double the number of batches to

Re: base backup vs. concurrent truncation

2023-05-10 Thread Greg Stark
Including the pre-truncation length in the wal record is the obviously solid approach and I none of the below is a good substitution for it. But On Tue, 25 Apr 2023 at 13:30, Andres Freund wrote: > > It isn't - but the alternatives aren't great either. It's not that easy to hit > this