Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression
On Sat, May 06, 2017 at 07:02:46PM +, Noah Misch wrote: > On Wed, May 03, 2017 at 11:29:29PM -0400, Peter Eisentraut wrote: > > On 4/30/17 04:05, Noah Misch wrote: > > > The above-described topic is currently a PostgreSQL 10 open item. Peter, > > > since you committed the patch believed to have created it, you own this > > > open > > > item. If some other commit is more relevant or if this does not belong > > > as a > > > v10 open item, please let us know. Otherwise, please observe the policy > > > on > > > open item ownership[1] and send a status update within three calendar > > > days of > > > this message. Include a date for your subsequent status update. Testers > > > may > > > discover new open items at any time, and I want to plan to get them all > > > fixed > > > well in advance of shipping v10. Consequently, I will appreciate your > > > efforts > > > toward speedy resolution. Thanks. > > > > I'm working on this and will report on Friday. > > This PostgreSQL 10 open item is past due for your status update. Kindly send > a status update within 24 hours, and include a date for your subsequent status > update. Refer to the policy on open item ownership: > https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com IMMEDIATE ATTENTION REQUIRED. This PostgreSQL 10 open item is long past due for your status update. Please reacquaint yourself with the policy on open item ownership[1] and then reply immediately. If I do not hear from you by 2017-05-09 07:00 UTC, I will transfer this item to release management team ownership without further notice. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Not getting error if ALTER SUBSCRIPTION syntax is wrong.
On Sat, May 06, 2017 at 02:44:27PM +0200, Petr Jelinek wrote: > On 05/05/17 19:51, Petr Jelinek wrote: > > On 05/05/17 14:40, tushar wrote: > >> Hi, > >> > >> While testing 'logical replication' against v10 , i encountered one > >> issue where data stop migrating after ALTER PUBLICATION. > >> > >> X Server > >> \\ Make sure wal_level is set to logical in postgresql.conf file > >> \\create table/Insert 1 row -> create table test(n int); insert into t > >> values (1); > >> \\create publication for all -> create publication pub for ALL TABLES ; > >> > >> > >> Y server > >> > >> \\ Make sure wal_level is set to logical in postgresql.conf file > >> \\create table -> create table test(n int); > >> > >> \\create Subscription > >> > >> CREATE SUBSCRIPTION sub CONNECTION 'host=localhost dbname=postgres > >> port=5432 ' PUBLICATION pub; > >> > >> [...] > >> > >> I think probably syntax of alter subscription is not correct but > >> surprisingly it is not throwing an error. > >> > > > > Syntax of ALTER command is correct, syntax of the connection string is > > not, you are probably getting errors in log from the replication worker. > > > > We could check validity of the connection string though to complain > > immediately like we do in CREATE. > > > > The attached does exactly that. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Peter, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Fix a typo in snapmgr.c
Hi, Attached patch for $subject. s/recovey/recovery/ Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center fix_typo_in_snapmgr_c.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warning in costsize.c
On 11 April 2017 at 12:53, Michael Paquier wrote: > On Tue, Apr 11, 2017 at 4:02 AM, Robert Haas wrote: >> On Mon, Apr 10, 2017 at 2:09 PM, Tom Lane wrote: >>> ilm...@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes: Why bother with the 'rte' variable at all if it's only used for the Assert()ing the rtekind? >>> >>> That was proposed a few messages back. I don't like it because it makes >>> these functions look different from the other scan-cost-estimation >>> functions, and we'd just have to undo the "optimization" if they ever >>> grow a need to reference the rte for another purpose. >> >> I think that's sort of silly, though. It's a trivial difference, >> neither likely to confuse anyone nor difficult to undo. > > +1. I would just do that and call it a day. There is no point to do a > mandatory list lookup as that's just for an assertion, and fixing this > warning does not seem worth the addition of fancier facilities. If the > function declarations were doubly-nested in the code, I would > personally consider the use of a variable, but not here. Any more thoughts on what is acceptable for fixing this? beta1 is looming and it seems a bit messy to be shipping that with these warnings, however harmless they are. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_dump emits ALTER TABLE ONLY partitioned_table
Amit, * Amit Langote (langote_amit...@lab.ntt.co.jp) wrote: > Thanks for committing the patch after improving it quite a bit, and sorry > that I couldn't reply promptly during the last week due to vacation. No worries, hopefully you have an opportunity to review the additional changes I made and understand why they were necessary. Certainly, feel free to reach out if you have any questions or notice anything else which should be improved. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] subscription worker doesn't start immediately on eabled
On Tue, May 2, 2017 at 11:53 AM, Peter Eisentraut wrote: > On 4/27/17 21:36, Masahiko Sawada wrote: >> That makes sense to me. Since NOCONNECT option changes some default >> values including ENABLED to false I think we should apply it also when >> NOCONNECT is specified? > > That's not necessary, because if NOCONNECT is specified, then "enabled" > will be set accordingly. > You're right, sorry for the noise. Anyway thank you for committing the patch! Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PG 10 release notes
On Fri, May 5, 2017 at 8:38 AM, Bruce Momjian wrote: > On Fri, Apr 28, 2017 at 01:12:34PM +0900, Masahiko Sawada wrote: >> On Tue, Apr 25, 2017 at 10:31 AM, Bruce Momjian wrote: >> > I have committed the first draft of the Postgres 10 release notes. They >> > are current as of two days ago, and I will keep them current. Please >> > give me any feedback you have. >> > >> >> Related to a following item in release note, oldestxmin is also >> reported by VACUUM VERBOSE and autovacuum , which is introduced by >> commit 9eb344faf54a849898d9be012ddfa8204cfeb57c (author is Simon). >> >> * Have VACUUM VERBOSE report the number of skipped frozen pages >> (Masahiko Sawada) >> >> Could we add this item to the release note? > > OK, adjusted text below and commit added above this item: > >Have VACUUM VERBOSE report >the number of skipped frozen pages and oldest xmin (Masahiko Sawada) > Thank you! But actually main author of this item is Simon, I didn't involved in it. Maybe we can have it as a separate item. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Detecting schema changes during logical replication
On 8 May 2017 05:56, "Daniele Varrazzo" wrote: On Sun, May 7, 2017 at 8:04 PM, Andres Freund wrote: > Hi, > > On 2017-05-07 19:27:08 +0100, Daniele Varrazzo wrote: >> I'm putting together a replication system based on logical >> replication. > > Interesting. If you very briefly could recap what it's about... ;) I need to replicate some tables from a central database into the database that should run a secondary system. For a similar use case we have used Londiste in the past, which has served us good, but its usage has not been problem-free. Logical decoding seems much less invasive on the source database than a trigger-based replication solution, and has less moving part to care about and maintain. For the moment I'm hacking into a fork of Euler project for wal decoding into json (https://github.com/dvarrazzo/wal2json), mostly adding configurability, so that we may be able to replicate only the tables we need, skip certain fields etc. I'm also taking a look at minimising the amount of information produced: sending over and over the column names and types for every record seems a waste, hence my question. Sounds like you're reimplementing pglogical ( http://2ndquadrant.com/pglogical) on top of a json protocol. Pglogical is open source and hackable to meet your needs. We're also happy to accept patches with appropriate design discussion and code review to make sure they will aid or not hinder other users and won't add undue maintenance burden. Rather than repeat the same work, maybe it's worth picking it up as a starting point. It's the extension-based progenitor of the in-core logical rep code, but portable from 9.4 to 9.6 (and soon 10) with a bunch of features that didn't make it into Pg's version yet. I have no reason to object to your doing it yourself, and you're welcome to use pglogical as a reference for how to do things (see the license). It just seems like a waste.
Re: [HACKERS] Concurrent ALTER SEQUENCE RESTART Regression
On Mon, May 8, 2017 at 8:43 AM, Andres Freund wrote: > Moving discussion to -hackers, this isn't really a bug, it's an > architectural issue with the new in-tree approach. > > Short recap: With the patch applied in [1] ff, sequences partially > behave transactional (because pg_sequence is updated transactionally), > partially non-transactionally (because there's no locking enforcing it, > and it's been stated as undesirable to change that). This leads to > issues like described in [2]. For more context, read the whole > discussion. Thanks for summarizing. > On 2017-05-03 23:29:29 -0400, Peter Eisentraut wrote: >> I'm working on this and will report on Friday. > > What's the plan here? I think the problem is that the code as is is > trying to marry two incompatible things: You're trying to make nextval() > not block, but have ALTER SEQUENCE be transactional. Given MAXVAL, > INCREMENT, etc. those two simply aren't compatible. > > I think there's three basic ways to a resolution here: > 1. Revert the entire patch for now. That's going to be very messy because >of the number of followup patches & features. > 2. Keep the catalog, but only ever update the records using >heap_inplace_update. That'll make the transactional behaviour very >similar to before. It'd medium-term also allow to move the actual >sequence block into the pg_sequence catalog itself. In this case it is enough to use ShareUpdateExclusiveLock on the sequence object, not pg_sequence. > 3. Keep the catalog, make ALTER properly transactional, blocking >concurrent nextval()s. This resolves the issue that nextval() can't >see the changed definition of the sequence. This will need an even stronger lock, AccessExclusiveLock to make this work properly. > I think this really needs design agreement from multiple people, because > any of these choices will have significant impact. > To me 3. seems like the best approach long-term, because both the > current and the (but in different ways). But it'll be quite noticeable to users. Count me in for the 3. bucket. This loses the properly of having nextval() available to users when a parallel ALTER SEQUENCE is running, but consistency matters even more. I think that the final patch closing this thread should also have proper isolation tests. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_dump emits ALTER TABLE ONLY partitioned_table
Hi Stephen, On 2017/05/06 12:28, Stephen Frost wrote: > Noah, > > On Fri, May 5, 2017 at 23:19 Noah Misch wrote: > >> On Thu, May 04, 2017 at 05:47:02PM -0400, Stephen Frost wrote: >>> * Amit Langote (amitlangot...@gmail.com) wrote: On Wed, May 3, 2017 at 12:05 PM, Stephen Frost >> wrote: > Assuming this looks good to you, I'll push it tomorrow, possibly with > other minor adjustments and perhaps a few more tests. Your latest patch looks good to me. >>> >>> Found a few more issues (like pg_dump not working against older versions >>> of PG, because the queries for older versions hadn't been adjusted) and >>> did a bit more tidying. >>> >>> I'll push this in a couple of hours. >> >> This PostgreSQL 10 open item is past due for your status update. Kindly >> send >> a status update within 24 hours, and include a date for your subsequent >> status >> update. Refer to the policy on open item ownership: >> >> https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com >> >> I see you did commit a related patch. Is this item done? > > > Yes, I believe it to be. I anticipate reviewing the code in this area more > in the coming weeks, but this specific issue can be marked as resolved. Thanks for committing the patch after improving it quite a bit, and sorry that I couldn't reply promptly during the last week due to vacation. Regards, Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] modeling parallel contention (was: Parallel Append implementation)
On Mon, May 8, 2017 at 1:39 PM, David Rowley wrote: > On 6 May 2017 at 13:44, Thomas Munro wrote: >> Experimentation required... > > Indeed. I do remember long discussions on this before Parallel seq > scan went in, but I don't recall if anyone checked any OS kernels to > see what they did. > > We really need a machine with good IO concurrency, and not too much > RAM to test these things out. It could well be that for a suitability > large enough table we'd want to scan a whole 1GB extent per worker. I did a bunch of simple experiments this morning to try to observe RA effects, using a couple of different EDB machines running Linux. I wrote a simple program to read large files sequentially using lseek + read, but rotate the reads over N file descriptors to simulate parallel workers. I was surprised to find that I couldn't change cache-cold read performance that way, up to very large numbers of N. I did manage to break it by introducing some artificial disorder, reversing/scrambling the read order of small groups of blocks, but even that required groups over about 16 blocks before performance started to drop (possibly related to the window size which I can't see due to permissions right now). I've also learned that RAID cards sometimes do read-ahead of their own, making matters more complicated. I hope to report more when I figure out all the moving parts... > I did post a patch to have heap_parallelscan_nextpage() use atomics > instead of locking over in [1], but I think doing atomics there does > not rule out also adding batching later. In fact, I think it > structures things so batching would be easier than it is today. +1 -- Thomas Munro http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Declarative partitioning - another take
On 2017/05/08 10:22, Thomas Munro wrote: > On Mon, May 8, 2017 at 12:47 PM, Amit Langote > wrote: >> On 2017/05/03 2:48, Robert Haas wrote: >>> On Tue, May 2, 2017 at 3:30 AM, Amit Langote >>> wrote: You're right. I agree that whatever text we add here should be pointing out that statement-level triggers of affected child tables are not fired, when root parent is specified in the command. Since there was least some talk of changing that behavior for regular inheritance so that statement triggers of any affected children are fired [1], I thought we shouldn't say something general that applies to both inheritance and partitioning. But since nothing has happened in that regard, we might as well. How about the attached? >>> >>> Looks better, but I think we should say "statement" instead of >>> "operation" for consistency with the previous paragraph, and it >>> certainly shouldn't be capitalized. >> >> Agreed, done. Attached updated patch. > > > +A statement that targets the root table in a inheritance or partitioning > +hierarchy does not cause the statement-level triggers of affected child > +tables to be fired; only the root table's statement-level triggers are > +fired. However, row-level triggers of any affected child tables will be > +fired. > + > + > + > > Why talk specifically about the "root" table? Wouldn't we describe > the situation more generally if we said [a,the] "parent"? I think that makes sense. Modified it to read: "A statement that targets a parent table in a inheritance or partitioning hierarchy..." in the attached updated patch. Thanks, Amit >From 5c2c453235d5eedf857a5e7123337aae5aedd761 Mon Sep 17 00:00:00 2001 From: amit Date: Mon, 24 Apr 2017 14:55:08 +0900 Subject: [PATCH] Clarify statement trigger behavior with inheritance --- doc/src/sgml/trigger.sgml | 8 1 file changed, 8 insertions(+) diff --git a/doc/src/sgml/trigger.sgml b/doc/src/sgml/trigger.sgml index 6f8416dda7..ce76a1f042 100644 --- a/doc/src/sgml/trigger.sgml +++ b/doc/src/sgml/trigger.sgml @@ -123,6 +123,14 @@ +A statement that targets a parent table in a inheritance or partitioning +hierarchy does not cause the statement-level triggers of affected child +tables to be fired; only the parent table's statement-level triggers are +fired. However, row-level triggers of any affected child tables will be +fired. + + + If an INSERT contains an ON CONFLICT DO UPDATE clause, it is possible that the effects of all row-level BEFORE INSERT triggers -- 2.11.0 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] modeling parallel contention (was: Parallel Append implementation)
On 6 May 2017 at 13:44, Thomas Munro wrote: > In Linux, each process that opens a file gets its own 'file' > object[1][5]. Each of those has it's own 'file_ra_state' > object[2][3], used by ondemand_readahead[4] for sequential read > detection. So I speculate that page-at-a-time parallel seq scan must > look like random access to Linux. > > In FreeBSD the situation looks similar. Each process that opens a > file gets a 'file' object[8] which has members 'f_seqcount' and > 'f_nextoff'[6]. These are used by the 'sequential_heuristics' > function[7] which affects the ioflag which UFS/FFS uses to control > read ahead (see ffs_read). So I speculate that page-at-a-time > parallel seq scan must look like random access to FreeBSD too. > > In both cases I suspect that if you'd inherited (or sent the file > descriptor to the other process via obscure tricks), it would actually > work because they'd have the same 'file' entry, but that's clearly not > workable for md.c. > Interesting! > Experimentation required... Indeed. I do remember long discussions on this before Parallel seq scan went in, but I don't recall if anyone checked any OS kernels to see what they did. We really need a machine with good IO concurrency, and not too much RAM to test these things out. It could well be that for a suitability large enough table we'd want to scan a whole 1GB extent per worker. I did post a patch to have heap_parallelscan_nextpage() use atomics instead of locking over in [1], but I think doing atomics there does not rule out also adding batching later. In fact, I think it structures things so batching would be easier than it is today. [1] https://www.postgresql.org/message-id/CAKJS1f9tgsPhqBcoPjv9_KUPZvTLCZ4jy=B=bhqgakn7cyz...@mail.gmail.com -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] modeling parallel contention (was: Parallel Append implementation)
On 5 May 2017 at 14:54, Thomas Munro wrote: > Just for fun, check out pages 42 and 43 of Wei Hong's thesis. He > worked on Berkeley POSTGRES parallel query and a spin-off called XPRS, > and they got linear seq scan scaling up to number of spindles: > > http://db.cs.berkeley.edu/papers/ERL-M93-28.pdf That's interesting. I'd no idea that work was done. Actually, I didn't really know that anyone had thought to have more than one processor back then :-) And I also now know the origins of the tenk1 table in the regression database. Those 10,000 rows were once used for benchmarking! I'm glad we're all using a couple more rows these days. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Declarative partitioning - another take
On Mon, May 8, 2017 at 12:47 PM, Amit Langote wrote: > On 2017/05/03 2:48, Robert Haas wrote: >> On Tue, May 2, 2017 at 3:30 AM, Amit Langote >> wrote: >>> You're right. I agree that whatever text we add here should be pointing >>> out that statement-level triggers of affected child tables are not fired, >>> when root parent is specified in the command. >>> >>> Since there was least some talk of changing that behavior for regular >>> inheritance so that statement triggers of any affected children are fired >>> [1], I thought we shouldn't say something general that applies to both >>> inheritance and partitioning. But since nothing has happened in that >>> regard, we might as well. >>> >>> How about the attached? >> >> Looks better, but I think we should say "statement" instead of >> "operation" for consistency with the previous paragraph, and it >> certainly shouldn't be capitalized. > > Agreed, done. Attached updated patch. +A statement that targets the root table in a inheritance or partitioning +hierarchy does not cause the statement-level triggers of affected child +tables to be fired; only the root table's statement-level triggers are +fired. However, row-level triggers of any affected child tables will be +fired. + + + Why talk specifically about the "root" table? Wouldn't we describe the situation more generally if we said [a,the] "parent"? -- Thomas Munro http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Declarative partitioning - another take
On 2017/05/03 2:48, Robert Haas wrote: > On Tue, May 2, 2017 at 3:30 AM, Amit Langote > wrote: >> You're right. I agree that whatever text we add here should be pointing >> out that statement-level triggers of affected child tables are not fired, >> when root parent is specified in the command. >> >> Since there was least some talk of changing that behavior for regular >> inheritance so that statement triggers of any affected children are fired >> [1], I thought we shouldn't say something general that applies to both >> inheritance and partitioning. But since nothing has happened in that >> regard, we might as well. >> >> How about the attached? > > Looks better, but I think we should say "statement" instead of > "operation" for consistency with the previous paragraph, and it > certainly shouldn't be capitalized. Agreed, done. Attached updated patch. Thanks, Amit >From 1d7e383c6d89ebabacc7aa3f7d9987779daaa4fb Mon Sep 17 00:00:00 2001 From: amit Date: Mon, 24 Apr 2017 14:55:08 +0900 Subject: [PATCH] Clarify statement trigger behavior with inheritance --- doc/src/sgml/trigger.sgml | 8 1 file changed, 8 insertions(+) diff --git a/doc/src/sgml/trigger.sgml b/doc/src/sgml/trigger.sgml index 6f8416dda7..89e8c01a71 100644 --- a/doc/src/sgml/trigger.sgml +++ b/doc/src/sgml/trigger.sgml @@ -123,6 +123,14 @@ +A statement that targets the root table in a inheritance or partitioning +hierarchy does not cause the statement-level triggers of affected child +tables to be fired; only the root table's statement-level triggers are +fired. However, row-level triggers of any affected child tables will be +fired. + + + If an INSERT contains an ON CONFLICT DO UPDATE clause, it is possible that the effects of all row-level BEFORE INSERT triggers -- 2.11.0 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Concurrent ALTER SEQUENCE RESTART Regression
Hi, Moving discussion to -hackers, this isn't really a bug, it's an architectural issue with the new in-tree approach. Short recap: With the patch applied in [1] ff, sequences partially behave transactional (because pg_sequence is updated transactionally), partially non-transctionally (because there's no locking enforcing it, and it's been stated as undesirable to change that). This leads to issues like described in [2]. For more context, read the whole discussion. On 2017-05-03 23:29:29 -0400, Peter Eisentraut wrote: > I'm working on this and will report on Friday. What's the plan here? I think the problem is that the code as is is trying to marry two incompatible things: You're trying to make nextval() not block, but have ALTER SEQUENCE be transactional. Given MAXVAL, INCREMENT, etc. those two simply aren't compatible. I think there's three basic ways to a resolution here: 1. Revert the entire patch for now. That's going to be very messy because of the number of followup patches & features. 2. Keep the catalog, but only ever update the records using heap_inplace_update. That'll make the transactional behaviour very similar to before. It'd medium-term also allow to move the actual sequence block into the pg_sequence catalog itself. 3. Keep the catalog, make ALTER properly transactional, blocking concurrent nextval()s. This resolves the issue that nextval() can't see the changed definition of the sequence. I think this really needs design agreement from multiple people, because any of these choices will have significant impact. To me 3. seems like the best approach long-term, because both the current and the http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=1753b1b027035029c2a2a1649065762fafbf63f3 [2] http://archives.postgresql.org/message-id/20170427062304.gxh3rczhh6vblrwh%40alap3.anarazel.de -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] logical replication deranged sender
After dropping a subscription, it says it succeeded and that it dropped the slot on the publisher. But the publisher still has the slot, and a full-tilt process described by ps as postgres: wal sender process jjanes [local] idle in transaction Strace shows that this process is doing nothing but opening, reading, lseek, and closing from pg_wal, and calling sbrk. It never sends anything. This is not how it should work, correct? I'm running pgbench -c4 -j8 on the publisher, publishing all pgbench tables. It doesn't seem to happen every time I create and then drop a subscription, but it happens pretty often. The subscribing server's logs shows: 21270 2017-05-07 16:04:16.517 PDT LOG: process 21270 still waiting for AccessShareLock on relation 6100 of database 0 after 1000.051 ms 21270 2017-05-07 16:04:16.517 PDT DETAIL: Process holding the lock: 25493. Wait queue: 21270. 21270 2017-05-07 16:04:16.520 PDT LOG: process 21270 acquired AccessShareLock on relation 6100 of database 0 after 1003.608 ms 25493 DROP SUBSCRIPTION 2017-05-07 16:04:16.520 PDT LOG: duration: 1005.176 ms CPU: user: 0.00 s, system: 0.00 s, elapsed: 1.00 s statement: drop subscription foobar ; The publishing server's logs doesn't show anything relevant. I'm using 86713de, I have not tried this in earlier commits. Cheers, Jeff
Re: [HACKERS] Detecting schema changes during logical replication
On Sun, May 7, 2017 at 8:04 PM, Andres Freund wrote: > Hi, > > On 2017-05-07 19:27:08 +0100, Daniele Varrazzo wrote: >> I'm putting together a replication system based on logical >> replication. > > Interesting. If you very briefly could recap what it's about... ;) I need to replicate some tables from a central database into the database that should run a secondary system. For a similar use case we have used Londiste in the past, which has served us good, but its usage has not been problem-free. Logical decoding seems much less invasive on the source database than a trigger-based replication solution, and has less moving part to care about and maintain. For the moment I'm hacking into a fork of Euler project for wal decoding into json (https://github.com/dvarrazzo/wal2json), mostly adding configurability, so that we may be able to replicate only the tables we need, skip certain fields etc. I'm also taking a look at minimising the amount of information produced: sending over and over the column names and types for every record seems a waste, hence my question. >> I would like to send table information only the first >> time a table is seen by the 'change_cb' callback, but of course there >> could be some schema change after replication started. So I wonder: is >> there any information I can find in the 'Relation' structure of the >> change callback, which may suggest that there could have been a change >> in the table schema, hence a new schema should be sent to the client? > > The best way I can think of - which is also what is implemented in the > in-core replication framework - is to have a small cache on-top of the > relcache. That cache is kept coherent using > CacheRegisterRelcacheCallback(). Then whenever there's a change you > look up that change in that cache, and send the schema information if > it's been invalidated since you last sent something. That's also how > the new stuff in v10 essentially works: > src/backend/replication/pgoutput/pgoutput.c > > pgoutput_change(), does a lookup for its own metadata using > get_rel_sync_entry() > which then checks relentry->schema_sent. Invalidation unsets > schema_sent in rel_sync_cache_relation_cb. Thank you very much, it seems exactly what I need. I'll try hacking around this callback. -- Daniele -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal psql \gdesc
Hello Pavel, Sometimes I have to solve the result types of some query. It is invisible in psql. Indeed. My 0.02€ about this patch: About the feature: It looks useful to allow to show the resulting types of a query. About the code: Patch applies cleanly and compiles. I'm afraid that re-using the "results" variable multiple times results in memory leaks... ISTM that new variables should be used when going down the nested conditions, and all cleanup should be done where and when necessary. Also, maybe it would be better if the statement is cleaned up server side at the end of the execution. Not sure how to achieve that, though, libpq seems to lack the relevant function:-( """although there is no libpq function for deleting a prepared statement, the SQL DEALLOCATE statement can be used for that purpose.""" Hmmm... I have not found how to use DEALLOCATE to cleanup an unnamed statement, it does not allow a "zero-length" name. Maybe it could be extended somehow, or a function could be provided for the purpose, eg by passing a NULL query to PQprepare... Resetting "gdesc flag" could be done only when the flag was true, at the end of the if, rather than on each query. I understand that the second level query is to retrieve the type names in place of the Oid returned by QPftype? The pg_malloc length computation looks a little bit arbitrary. Would it make sense to use PQescapeLiteral instead? About the documentation: I would suggest some rewording, maybe: "Show the description of the result of the current query buffer without actually executing it, by considering it a prepared statement." About tests: There should be some non-regression tests added, maybe something like: SELECT NULL AS zero, 1 AS one, 2.0 AS two, 'three' AS three, $1 AS four, CURRENT_DATE AS now -- ... \gdesc And also case which trigger an error, eg: SELECT $1 AS unknown_type \gdesc SELECT 1 + \gdesc Some fun: PREPARE test AS SELECT 1; EXECUTE test \gdesc ... I'm unsure about some error messages, but it may be unrelated to this patch: calvin=# SELECT '1 km'::unit AS foo, $2 as boo \gdesc ERROR: could not determine data type of parameter $1 -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Question about toasting code
On Sun, May 7, 2017 at 3:48 PM, Tom Lane wrote: > Mat Arye writes: > > This is in arrayfuncs.c:5022 (postgre 9.6.2) > > > /* > > * Ensure pass-by-ref stuff is copied into mcontext; and detoast it too if > > * it's varlena. (You might think that detoasting is not needed here > > * because construct_md_array can detoast the array elements later. > > * However, we must not let construct_md_array modify the ArrayBuildState > > * because that would mean array_agg_finalfn damages its input, which is > > * verboten. Also, this way frequently saves one copying step.) > > */ > > > I am a bit confused by the comment. > > > Does PG_DETOAST_DATUM_COPY(dvalue) modify dvalue? > > No. > > What the comment is on about is that construct_md_array does this: > > /* make sure data is not toasted */ > if (elmlen == -1) > elems[i] = PointerGetDatum(PG_DETOAST_DATUM(elems[i])); > > that is, it intentionally modifies the passed-in elems array in > the case that one of the elements is a toasted value. For most > callers, the elems array is only transient storage anyway. But > it's problematic for makeMdArrayResult, because it would mean > that the ArrayBuildState is changed --- and while it's not changed > in a semantically significant way, that's still a problem, because > the detoasted value would be allocated in a context that is possibly > shorter-lived than the ArrayBuildState is. (In a hashed aggregation > situation, the ArrayBuildState is going to live in storage that is > persistent across the aggregation, but we are calling makeMdArrayResult > in the context where we want the result value to be created, which > is of only per-output-tuple lifespan.) > > You could imagine fixing this by having construct_md_array do some > context switches internally, but that would complicate it. And in > any case, fixing the problem where it is allows us to combine the > possible detoasting with copying the value into the ArrayBuildState's > context, so we'd probably keep doing that even if it was safe not to. > > Thanks. That clears it up. > > The thing I am struggling with is with the serialize/deserialize > functions. > > Can I detoast inside the serialize function if I use _COPY? Or is there a > > reason I have to detoast during the sfunc? > > Should be able to detoast in serialize if you want. Are you having > trouble with that? (It might be inefficient to do it that way, if > you have to serialize the same value multiple times. But I'm not > sure if that can happen.) > > I haven't run into any actual problems yet, just wanted to figure out a clean mental model before implementing. Thanks a lot for the clarification. > regards, tom lane >
Re: [HACKERS] Question about toasting code
Mat Arye writes: > This is in arrayfuncs.c:5022 (postgre 9.6.2) > /* > * Ensure pass-by-ref stuff is copied into mcontext; and detoast it too if > * it's varlena. (You might think that detoasting is not needed here > * because construct_md_array can detoast the array elements later. > * However, we must not let construct_md_array modify the ArrayBuildState > * because that would mean array_agg_finalfn damages its input, which is > * verboten. Also, this way frequently saves one copying step.) > */ > I am a bit confused by the comment. > Does PG_DETOAST_DATUM_COPY(dvalue) modify dvalue? No. What the comment is on about is that construct_md_array does this: /* make sure data is not toasted */ if (elmlen == -1) elems[i] = PointerGetDatum(PG_DETOAST_DATUM(elems[i])); that is, it intentionally modifies the passed-in elems array in the case that one of the elements is a toasted value. For most callers, the elems array is only transient storage anyway. But it's problematic for makeMdArrayResult, because it would mean that the ArrayBuildState is changed --- and while it's not changed in a semantically significant way, that's still a problem, because the detoasted value would be allocated in a context that is possibly shorter-lived than the ArrayBuildState is. (In a hashed aggregation situation, the ArrayBuildState is going to live in storage that is persistent across the aggregation, but we are calling makeMdArrayResult in the context where we want the result value to be created, which is of only per-output-tuple lifespan.) You could imagine fixing this by having construct_md_array do some context switches internally, but that would complicate it. And in any case, fixing the problem where it is allows us to combine the possible detoasting with copying the value into the ArrayBuildState's context, so we'd probably keep doing that even if it was safe not to. > The thing I am struggling with is with the serialize/deserialize functions. > Can I detoast inside the serialize function if I use _COPY? Or is there a > reason I have to detoast during the sfunc? Should be able to detoast in serialize if you want. Are you having trouble with that? (It might be inefficient to do it that way, if you have to serialize the same value multiple times. But I'm not sure if that can happen.) regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Detecting schema changes during logical replication
Hi, On 2017-05-07 19:27:08 +0100, Daniele Varrazzo wrote: > I'm putting together a replication system based on logical > replication. Interesting. If you very briefly could recap what it's about... ;) > I would like to send table information only the first > time a table is seen by the 'change_cb' callback, but of course there > could be some schema change after replication started. So I wonder: is > there any information I can find in the 'Relation' structure of the > change callback, which may suggest that there could have been a change > in the table schema, hence a new schema should be sent to the client? The best way I can think of - which is also what is implemented in the in-core replication framework - is to have a small cache on-top of the relcache. That cache is kept coherent using CacheRegisterRelcacheCallback(). Then whenever there's a change you look up that change in that cache, and send the schema information if it's been invalidated since you last sent something. That's also how the new stuff in v10 essentially works: src/backend/replication/pgoutput/pgoutput.c pgoutput_change(), does a lookup for its own metadata using get_rel_sync_entry() which then checks relentry->schema_sent. Invalidation unsets schema_sent in rel_sync_cache_relation_cb. Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Question about toasting code
Hi,
I am trying to create a custom aggregate and have run across some puzzling
code while trying to figure out how to implement it.
This is in arrayfuncs.c:5022 (postgre 9.6.2)
/*
* Ensure pass-by-ref stuff is copied into mcontext; and detoast it too if
* it's varlena. (You might think that detoasting is not needed here
* because construct_md_array can detoast the array elements later.
* However, we must not let construct_md_array modify the ArrayBuildState
* because that would mean array_agg_finalfn damages its input, which is
* verboten. Also, this way frequently saves one copying step.)
*/
if (!disnull && !astate->typbyval)
{
if (astate->typlen == -1)
dvalue = PointerGetDatum(PG_DETOAST_DATUM_COPY(dvalue));
else
dvalue = datumCopy(dvalue, astate->typbyval, astate->typlen);
}
I am a bit confused by the comment.
Does PG_DETOAST_DATUM_COPY(dvalue) modify dvalue? Shouldn't that not
modify the value (implied by _COPY)? If so then why does the detoasting
have to happen ahead of time and cannot happen at a later stage (in the
finalfunc or serializefunc etc.)? I understand that at those later stages
you cannot modify the input, but why would you have to in order to DETOAST?
The thing I am struggling with is with the serialize/deserialize functions.
Can I detoast inside the serialize function if I use _COPY? Or is there a
reason I have to detoast during the sfunc?
Thanks,
Mat
TimescaleDB
[HACKERS] Detecting schema changes during logical replication
Hello, I'm putting together a replication system based on logical replication. I would like to send table information only the first time a table is seen by the 'change_cb' callback, but of course there could be some schema change after replication started. So I wonder: is there any information I can find in the 'Relation' structure of the change callback, which may suggest that there could have been a change in the table schema, hence a new schema should be sent to the client? Thank you very much, -- Daniele -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw
On Sun, May 7, 2017 at 12:42 AM, Amit Kapila wrote: > On Sat, May 6, 2017 at 10:41 PM, Robert Haas wrote: >> On Sat, May 6, 2017 at 12:55 PM, Robert Haas wrote: >>> Oh! Good catch. Given that the behavior in question is intentional >>> there and intended to provide backward compatibility, changing it in >>> back-branches doesn't seem like a good plan. I'll adjust the patch so >>> that it continues to ignore errors in that case. >> >> Updated patch attached. > > Patch looks good to me. I'm having second thoughts based on some more experimentation I did this morning. I'll update again once I've had a bit more time to poke at it. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [Proposal]: Extends VisualStudio to automatically precompile EmbeddedSQL
Le 02/05/2017 à 13:22, Michael Paquier a écrit : > On Tue, May 2, 2017 at 6:35 PM, zosrothko wrote: >> Hi >> >> I made an extension of VisualStudio that precompiles automaticaly C or >> C++ source with PostgreSQL Embedded SQL. The extension is made of the 3 >> files joined and I have no idea where they should be placed in the >> PostgreSQL source tree. >> >> Anybody interested in pushing this extension? > The PostgreSQL uses a set of scripts in src/tools/msvc/ to generate > things compiled with visual studio, so instinctively you would be > looking at working on that. Honestly, just by looking at the files you > are proposing, it is hard to make an idea of why this would be useful > for ECPG, and please note as well that there are guidelines for > submitting patches: > https://wiki.postgresql.org/wiki/Submitting_a_Patch > > The source code of ecpg is located in src/interfaces/ecpg by the way. > If you are interested to work on it, that's the place where to look at > first. I need to add those three files in the installation process. Where should go those files? 1. in C:\Program Files (x86)\PostgreSQL\9.5\bin 2. or should I put those files in a share/contrib directory? Where is located the configuration file to change for putting those files in the Windows distribution? FA
Re: [HACKERS] [Proposal]: Extends VisualStudio to automatically precompile EmbeddedSQL
Le 02/05/2017 à 13:22, Michael Paquier a écrit : > On Tue, May 2, 2017 at 6:35 PM, zosrothko wrote: >> Hi >> >> I made an extension of VisualStudio that precompiles automaticaly C or >> C++ source with PostgreSQL Embedded SQL. The extension is made of the 3 >> files joined and I have no idea where they should be placed in the >> PostgreSQL source tree. >> >> Anybody interested in pushing this extension? > The PostgreSQL uses a set of scripts in src/tools/msvc/ to generate > things compiled with visual studio, so instinctively you would be > looking at working on that. Honestly, just by looking at the files you > are proposing, it is hard to make an idea of why this would be useful > for ECPG, and please note as well that there are guidelines for > submitting patches: > https://wiki.postgresql.org/wiki/Submitting_a_Patch > > The source code of ecpg is located in src/interfaces/ecpg by the way. > If you are interested to work on it, that's the place where to look at > first. I need to add those three files in the distribution process for Windows. Where should go those files? 1. in C:\Program Files (x86)\PostgreSQL\9.5\bin 2. or should I put those files in a share/contrib directory? Where is located the installation builder files to change for putting those files in the Windows distribution? Zos
Re: [HACKERS] Change GetLastImportantRecPtr's definition? (wasSkip checkpoints, archiving on idle systems.)
(catching up here) On Sun, May 7, 2017 at 9:01 AM, Andres Freund wrote: > Turns out this isn't the better fix, because the checkpoint code > compares with the actual record LSN (rather than the end+1 that > XLogInsert() returns). We'd start having to do more bookkeeping or more > complicated computations (subtracting the checkpoint record's size). > Therefore I've pushed the simple fix mentioned above instead. Yeah, I have spent some time looking at many details for this stuff... And using a start LSN location was way easier for the checkpoint skip checks. e6c44ee looks good to me, except that I would complete this comment block in xlog.c: * updated for all insertions, unless the XLOG_MARK_UNIMPORTANT flag was * set. lastImportantAt is never cleared, only overwritten by the LSN of newer * records. Tracking the WAL activity directly in WALInsertLock has the * advantage of not needing any additional locks to update the value. */ By just mentioning that the lastImportantAt is updated to the *start* LSN position of an important record. This will help in avoiding future confusions. It turns out that fixing this issue only on HEAD has been a good choice. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: logical replication syntax (was DROP SUBSCRIPTION, query cancellations and slot handling)
On Tue, May 02, 2017 at 01:42:37PM -0400, Robert Haas wrote: > On Tue, May 2, 2017 at 8:45 AM, Petr Jelinek > wrote: > > I am happy to implement something different, it's quite trivial to > > change. But I am not going to propose anything different as I can't > > think of better syntax (if I could I would have done it). I don't like > > the OFF or FALSE (ie DROP SLOT OFF) any more than what is there now and > > it also seems to not map very well to action (as opposed to output > > option as it is in EXPLAIN). It might not be very close to SQL way but > > that's because SQL way would be do not do any of those default actions > > unless they are actually asked for (ie NODROP SLOT would be default and > > DROP SLOT would be the option) but that's IMHO less user friendly. > > So the cases where this "NO" prefixing comes up are: > > 1. CREATE SUBSCRIPTION ... > 2. ALTER SUBSCRIPTION ... > 3. DROP SUBSCRIPTION ... > 4. CREATE PUBLICATION ... > So it doesn't actually look hard to get rid of all of the NO prefixes. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Peter, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: logical replication syntax (was DROP SUBSCRIPTION, query cancellations and slot handling)
On Tue, May 02, 2017 at 09:10:52AM -0400, Tom Lane wrote: > Robert Haas writes: > >> On Thu, Apr 20, 2017 at 7:46 AM, Petr Jelinek > >> wrote: > >>> DROP SUBSCRIPTION mysub NODROP SLOT; > > >> I'm pretty uninspired by this choice of syntax. > > Actually, this command has got much worse problems than whether you like > the spelling of its option: it shouldn't have an option in the first > place. I put it to you as an inviolable rule that no object DROP command > should ever have any options other than RESTRICT/CASCADE and IF EXISTS. > If it does, then you don't know what to do when the object is recursed > to by a cascaded drop. > > It's possible that we could allow exceptions to this rule for object types > that can never have any dependencies. But a subscription doesn't qualify > --- it's got an owner, so DROP OWNED BY already poses this problem. Looks > to me like it's got a dependency on a database, too. (BTW, if > subscriptions are per-database, why is pg_subscription a shared catalog? > There were some pretty schizophrenic decisions here.) > > So ISTM we need to get rid of the above-depicted syntax. We could instead > have what-to-do-with-the-slot as a property of the subscription, > established at CREATE SUBSCRIPTION and possibly changed later by ALTER > SUBSCRIPTION. Not quite sure what to call the option, maybe something > based on the concept of the subscription "owning" the slot. > > I think this is a must-fix issue, and will put it on the open items > list. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Peter, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
