Re: SLRU optimization - configurable buffer pool and partitioning the SLRU lock

2023-11-18 Thread Andrey M. Borodin
> On 17 Nov 2023, at 16:11, Dilip Kumar wrote: > > On Fri, Nov 17, 2023 at 1:09 PM Dilip Kumar wrote: >> >> On Thu, Nov 16, 2023 at 3:11 PM Alvaro Herrera >> wrote: > > PFA, updated patch version, this fixes the comment given by Alvaro and > also improves some of the comments. I’ve

Re: SQL:2011 application time

2023-11-18 Thread Paul A Jungwirth
On Mon, Nov 6, 2023 at 11:07 PM jian he wrote: > + > + In a temporal foreign key, the delete/update will use > + FOR PORTION OF semantics to constrain the > + effect to the bounds being deleted/updated in the referenced row. > + > > The first "para"

Re: SQL:2011 application time

2023-11-18 Thread Paul A Jungwirth
Thank you for continuing to review this submission! My changes are in the v18 patch I sent a few days ago. Details below. On Sun, Oct 29, 2023 at 5:01 PM jian he wrote: > * The attached patch makes foreign keys with PERIOD fail if any of the > foreign key columns is "generated columns". I don't

Re: pg_upgrade and logical replication

2023-11-18 Thread vignesh C
On Thu, 16 Nov 2023 at 18:25, Hayato Kuroda (Fujitsu) wrote: > > Dear Vignesh, > > Thanks for updating the patch! Here are some comments. > They are mainly cosmetic because I have not read yours these days. > > 01. binary_upgrade_add_sub_rel_state() > > ``` > +/* We must check these things

Re: pg_upgrade and logical replication

2023-11-18 Thread vignesh C
On Thu, 16 Nov 2023 at 07:45, Peter Smith wrote: > > Here are some review comments for patch v14-0001 > > == > src/backend/utils/adt/pg_upgrade_support.c > > 1. binary_upgrade_replorigin_advance > > + /* lock to prevent the replication origin from vanishing */ > +

Re: long-standing data loss bug in initial sync of logical replication

2023-11-18 Thread Andres Freund
On 2023-11-19 02:15:33 +0100, Tomas Vondra wrote: > > > On 11/18/23 22:05, Andres Freund wrote: > > Hi, > > > > On 2023-11-18 21:45:35 +0100, Tomas Vondra wrote: > >> On 11/18/23 19:12, Andres Freund wrote: > If we increase the locks from ShareUpdateExclusive to ShareRowExclusive, >

Re: pg_upgrade and logical replication

2023-11-18 Thread vignesh C
On Sun, 19 Nov 2023 at 06:52, vignesh C wrote: > > On Fri, 10 Nov 2023 at 19:26, vignesh C wrote: > > > > On Thu, 9 Nov 2023 at 12:23, Michael Paquier wrote: > > > > > > > > Note: actually, this would be OK if we are able to keep the OIDs of > > > the subscribers consistent across upgrades?

Re: pg_upgrade and logical replication

2023-11-18 Thread vignesh C
On Fri, 10 Nov 2023 at 19:26, vignesh C wrote: > > On Thu, 9 Nov 2023 at 12:23, Michael Paquier wrote: > > > > > Note: actually, this would be OK if we are able to keep the OIDs of > > the subscribers consistent across upgrades? I'm OK to not do nothing > > about that in this patch, to keep it

Re: Assert failure on 'list_member_ptr(rel->joininfo, restrictinfo)'

2023-11-18 Thread Alexander Korotkov
On Wed, Nov 15, 2023 at 5:07 PM Alexander Korotkov wrote: > > On Wed, Nov 15, 2023 at 8:02 AM Andres Freund wrote: > > The kinda because there are callers to bms_(add|del)_members() that pass the > > same bms as a and b, which only works if the reallocation happens "late". > > +1, > Neat idea.

Re: long-standing data loss bug in initial sync of logical replication

2023-11-18 Thread Tomas Vondra
On 11/18/23 22:05, Andres Freund wrote: > Hi, > > On 2023-11-18 21:45:35 +0100, Tomas Vondra wrote: >> On 11/18/23 19:12, Andres Freund wrote: If we increase the locks from ShareUpdateExclusive to ShareRowExclusive, we're making it conflict with RowExclusive. Which is just DML, and I

Re: reindexing an invalid index should not use ERRCODE_INDEX_CORRUPTED

2023-11-18 Thread Noah Misch
On Sat, Nov 18, 2023 at 03:09:58PM -0800, Andres Freund wrote: > We currently provide no way to learn about a postgres instance having > corruption than searching the logs for corruption events than matching by > sqlstate, for ERRCODE_DATA_CORRUPTED and ERRCODE_INDEX_CORRUPTED. > > Unfortunately,

Re: Relation bulk write facility

2023-11-18 Thread Andres Freund
Hi, On 2023-11-17 11:37:21 +0100, Heikki Linnakangas wrote: > The new facility makes it easier to optimize bulk loading, as the > logic for buffering, WAL-logging, and syncing the relation only needs > to be implemented once. It's also less error-prone: We have had a > number of bugs in how a

reindexing an invalid index should not use ERRCODE_INDEX_CORRUPTED

2023-11-18 Thread Andres Freund
Hi, We currently provide no way to learn about a postgres instance having corruption than searching the logs for corruption events than matching by sqlstate, for ERRCODE_DATA_CORRUPTED and ERRCODE_INDEX_CORRUPTED. Unfortunately, there is a case of such an sqlstate that's not at all indicating

errcode_for_file_access() maps EROFS to INSUFFICIENT_PRIVILEGE

2023-11-18 Thread Andres Freund
Hi, On linux, many filesystems default to remounting themselves read-only when metadata IO fails. I.e. one common reaction to disks failing is a previously read-write filesystem becoming read-only. When e.g. trying to create a file on such a filesystem, errno is set to EROFS. Writing with

PANIC serves too many masters

2023-11-18 Thread Andres Freund
Hi, Right now we use PANIC for very different kinds of errors. Sometimes for errors that are persistent, where crash-restarting and trying again won't help: ereport(PANIC, (errmsg("could not locate a valid checkpoint record"))); or ereport(PANIC, (errmsg("online backup

Re: Use of backup_label not noted in log

2023-11-18 Thread Andres Freund
Hi, On 2023-11-18 10:01:42 -0800, Andres Freund wrote: > > What about adding it to the "redo starts at" message, something like > > > > redo starts at 12/12345678, taken from control file > > > > or > > > > redo starts at 12/12345678, taken from backup label > > I think it'd make sense to

Re: long-standing data loss bug in initial sync of logical replication

2023-11-18 Thread Andres Freund
Hi, On 2023-11-18 21:45:35 +0100, Tomas Vondra wrote: > On 11/18/23 19:12, Andres Freund wrote: > >> If we increase the locks from ShareUpdateExclusive to ShareRowExclusive, > >> we're making it conflict with RowExclusive. Which is just DML, and I > >> think we need to do that. > > > > From what

Re: long-standing data loss bug in initial sync of logical replication

2023-11-18 Thread Tomas Vondra
On 11/18/23 19:12, Andres Freund wrote: > Hi, > > On 2023-11-18 11:56:47 +0100, Tomas Vondra wrote: >>> I guess it's not really feasible to just increase the lock level here though >>> :(. The use of ShareUpdateExclusiveLock isn't new, and suddenly using AEL >>> would perhaps lead to new

Re: SLRU optimization - configurable buffer pool and partitioning the SLRU lock

2023-11-18 Thread Alvaro Herrera
On 2023-Nov-18, Dilip Kumar wrote: > On Fri, Nov 17, 2023 at 6:16 PM Alvaro Herrera > wrote: > > I wonder what's the deal with false sharing in the new > > bank_cur_lru_count array. Maybe instead of using LWLockPadded for > > bank_locks, we should have a new struct, with both the LWLock and

Re: long-standing data loss bug in initial sync of logical replication

2023-11-18 Thread Andres Freund
Hi, On 2023-11-18 11:56:47 +0100, Tomas Vondra wrote: > > I guess it's not really feasible to just increase the lock level here though > > :(. The use of ShareUpdateExclusiveLock isn't new, and suddenly using AEL > > would perhaps lead to new deadlocks and such? But it also seems quite wrong. > >

Re: Use of backup_label not noted in log

2023-11-18 Thread Andres Freund
Hi, On 2023-11-18 09:30:01 -0400, David Steele wrote: > I know this isn't really a bug, but not being able to tell where recovery > information came from seems like a major omission in the logging. Yea. I was preparing to forecefully suggest that some monitoring tooling should verify that new

Re: Use of backup_label not noted in log

2023-11-18 Thread Andres Freund
Hi, On 2023-11-17 06:41:46 +0100, Laurenz Albe wrote: > On Thu, 2023-11-16 at 20:18 -0800, Andres Freund wrote: > > I've often had to analyze what caused corruption in PG instances, where the > > symptoms match not having had backup_label in place when bringing on the > > node. However that's

Re: Infinite Interval

2023-11-18 Thread Joseph Koshakow
On Thu, Nov 16, 2023 at 2:03 AM Ashutosh Bapat wrote: > >On Tue, Nov 14, 2023 at 4:39 PM Dean Rasheed wrote: >> >> On Thu, 9 Nov 2023 at 12:49, Dean Rasheed wrote: >> > >> > OK, I have pushed 0001 and 0002. Here's the remaining (main) patch. >> > >> >> OK, I have

Re: Schema variables - new implementation for Postgres 15

2023-11-18 Thread Dmitry Dolgov
> On Sat, Nov 18, 2023 at 02:19:09PM +0100, Pavel Stehule wrote: > > Would it be a problem to make pg_session_variables inspect the catalog > > or something similar if needed? > > > > It can be very easy to build pg_session_variables based on iteration over > the system catalog. But I am not sure

Re: Use of backup_label not noted in log

2023-11-18 Thread David Steele
On 11/17/23 01:41, Laurenz Albe wrote: On Thu, 2023-11-16 at 20:18 -0800, Andres Freund wrote: I've often had to analyze what caused corruption in PG instances, where the symptoms match not having had backup_label in place when bringing on the node. However that's surprisingly hard - the only

Re: Schema variables - new implementation for Postgres 15

2023-11-18 Thread Pavel Stehule
>> >> The difference between debug_parallel_query = 1 and debug_parallel_query >> = 0 is strange - and I'll check it. >> > > looks so pg_session_variables() doesn't work in debug_paralel_query mode. > It is marked as parallel safe, what is probably nonsense.

Re: Use of backup_label not noted in log

2023-11-18 Thread David Steele
On 11/17/23 00:18, Andres Freund wrote: I've often had to analyze what caused corruption in PG instances, where the symptoms match not having had backup_label in place when bringing on the node. However that's surprisingly hard - the only log messages that indicate use of backup_label are at

Re: Schema variables - new implementation for Postgres 15

2023-11-18 Thread Pavel Stehule
so 18. 11. 2023 v 14:19 odesílatel Pavel Stehule napsal: > Hi > > pá 17. 11. 2023 v 20:17 odesílatel Dmitry Dolgov <9erthali...@gmail.com> > napsal: > >> > On Wed, Aug 23, 2023 at 04:02:44PM +0200, Pavel Stehule wrote: >> > NameListToString is already buildin function. Do you think >>

Re: Schema variables - new implementation for Postgres 15

2023-11-18 Thread Pavel Stehule
Hi pá 17. 11. 2023 v 20:17 odesílatel Dmitry Dolgov <9erthali...@gmail.com> napsal: > > On Wed, Aug 23, 2023 at 04:02:44PM +0200, Pavel Stehule wrote: > > NameListToString is already buildin function. Do you think NamesFromList? > > > > This is my oversight - there is just `+extern List

Re: MERGE ... RETURNING

2023-11-18 Thread Dean Rasheed
On Fri, 17 Nov 2023 at 04:30, jian he wrote: > > I think it should be: > + You will require the SELECT privilege on any column(s) > + of the data_source and > + target_table_name referred to > + in any condition or expression. > Ah, of course. As always, I'm blind to grammatical errors

Re: add log messages when replication slots become active and inactive (was Re: Is it worth adding ReplicationSlot active_pid to ReplicationSlotPersistentData?)

2023-11-18 Thread Amit Kapila
On Thu, Nov 16, 2023 at 6:09 PM Bharath Rupireddy wrote: > > On Thu, Nov 16, 2023 at 4:01 PM Alvaro Herrera > wrote: > > > > On 2023-Nov-16, Peter Smith wrote: > > > > > I searched HEAD code and did not find any "translator:" comments for > > > just ordinary slot name substitutions like these;

Re: long-standing data loss bug in initial sync of logical replication

2023-11-18 Thread Tomas Vondra
On 11/18/23 03:54, Andres Freund wrote: > Hi, > > On 2023-11-17 17:54:43 -0800, Andres Freund wrote: >> On 2023-11-17 15:36:25 +0100, Tomas Vondra wrote: >>> Overall, this looks, walks and quacks like a cache invalidation issue, >>> likely a missing invalidation somewhere in the ALTER

Re: Synchronizing slots from primary to standby

2023-11-18 Thread Amit Kapila
On Fri, Nov 17, 2023 at 5:18 PM Drouvot, Bertrand wrote: > > On 11/17/23 2:46 AM, Zhijie Hou (Fujitsu) wrote: > > On Tuesday, November 14, 2023 10:27 PM Drouvot, Bertrand > > wrote: > > > > I feel the WaitForWALToBecomeAvailable may not be the best place to shutdown > > slotsync worker and drop

Re: long-standing data loss bug in initial sync of logical replication

2023-11-18 Thread Tomas Vondra
On 11/18/23 02:54, Andres Freund wrote: > Hi, > > On 2023-11-17 15:36:25 +0100, Tomas Vondra wrote: >> It seems there's a long-standing data loss issue related to the initial >> sync of tables in the built-in logical replication (publications etc.). > > :( > Yeah :-( > >> Overall, this

Re: SLRU optimization - configurable buffer pool and partitioning the SLRU lock

2023-11-18 Thread Dilip Kumar
On Fri, Nov 17, 2023 at 6:16 PM Alvaro Herrera wrote: Thanks for the review, all comments looks fine to me, replying to those that need some clarification > I wonder what's the deal with false sharing in the new > bank_cur_lru_count array. Maybe instead of using LWLockPadded for > bank_locks,