Re: speed up a logical replica setup

F76102C88FA1C29F56F2%40TY3PR01MB9889.jpnprd01.prod.outlook.com Thanks and Regards Shlok Kyal On Fri, 12 Jan 2024 at 03:46, Euler Taveira wrote: > > On Thu, Jan 11, 2024, at 9:18 AM, Hayato Kuroda (Fujitsu) wrote: > > I have been concerned that the patch has not been tested by cfbot due to the

Re: Add publisher and subscriber to glossary documentation.

hould be on the preceding sentence too. > > > Anyway, hopefully these examples show “node” and “database” are mixed and > perhaps others agree using one consistently might help the goals of the docs. For me the existing content looks good, I felt let's keep it as it is unless others feel differently. Thanks and regards, Shlok Kyal

Re: speed up a logical replica setup

-0001 and v30-0002. [1]: https://www.postgresql.org/message-id/cahv8rj+5mzk9jt+7ecogjzfm5czvdccd5jo1_rcx0bteypb...@mail.gmail.com Thanks and regards, Shlok Kyal v30-0001-pg_createsubscriber-creates-a-new-logical-replic.patch Description: Binary data v30-0002-Stop-the-target-server-earlier

Re: speed up a logical replica setup

in the code, so I have added it to the documentation in the warning > section. Thoughts? > I am not changing the version as I have not made any changes in > v30-0001 and v30-0002. > > [1]: > https://www.postgresql.org/message-id/cahv8rj+5mzk9jt+7ec

Add publisher and subscriber to glossary documentation.

Hi, There are several places where publisher and subscriber terms are used across the documentation. But the publisher and subscriber were missing in the documentation. I felt this should be added in the glossary. I have created a patch for the same. Thanks and Regards Shlok Kyal v1-0001-Add

Re: Add publisher and subscriber to glossary documentation.

quot; where we > provide an overview of "logical replication". Maybe that's enough, but > we should consider whether we want a separate definition of logical > replication (I'm leaning towards not having one, but it's worth asking.) Modified. Added the term "logical replication" in the definitions. Used reference to "replication". Thanks and Regards, Shlok Kyal v2-0001-Add-publisher-and-subscriber-to-glossary-document.patch Description: Binary data

Re: speed up a logical replica setup

I observed that pg_logical_createsubscriber also uses this approach. 2) read GUCs via SHOW command and restore them when server restarts I would prefer the 1st solution. Thanks and Regards, Shlok Kyal sudo pkill -9 postgres rm -rf ../primary ../standby ../new_path rm -rf primary.log standby.lo

Re: speed up a logical replica setup

Hi, On Tue, 20 Feb 2024 at 06:59, Euler Taveira wrote: > > On Mon, Feb 19, 2024, at 7:22 AM, Shlok Kyal wrote: > > I have reviewed the v21 patch. And found an issue. > > Initially I started the standby server with a new postgresql.conf file > (not the default postgresql.co

Re: Add publisher and subscriber to glossary documentation.

information, see Section 30.1 > > Publication node > A node where a publication is defined for logical replication. > > Subscriber > See "Subscription node" > > Subscription > A subscription receives the changes of one or more tables from the > publications it subscribes to. For more information, see Section 30.2 > > Subscription node > A node where a subscription is defined for logical replication. I have addressed the comments and added an updated patch. Thanks and Regards, Shlok Kyal v3-0001-Add-publisher-and-subscriber-to-glossary-document.patch Description: Binary data

Re: Add publisher and subscriber to glossary documentation.

> + for logical > replication. > + > + > + > + > > (same comment as above) > > I felt the word "node" here should link to the glossary term "Node", > instead of directly to the term "Instance". I have addressed the comments and have attached the updated version. Thanks and Regards, Shlok Kyal v4-0001-Add-publisher-and-subscriber-to-glossary-document.patch Description: Binary data

Re: speed up a logical replica setup

argc, argv, "d:D:nP:rS:t:v", + long_options, &option_index)) != -1) + { Here 'p', 's' and 'U' options are missing so we are getting the error. Also, I think the 'S' option should be removed from here. I also see that specifying lon

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

> Few comments: > 1) We will be able to override the value of max_slot_wal_keep_size by > using --new-options like '--new-options "-c > max_slot_wal_keep_size=val"': > + /* > +* Use max_slot_wal_keep_size as -1 to prevent the WAL removal by the > +* checkpointer process. If

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

I tested a test scenario: I started a new publisher with 'max_replication_slots' parameter set to '1' and created a streaming replication with the new publisher as primary node. Then I did a pg_upgrade from old publisher to new publisher. The upgrade failed with following error: Restoring logical

Re: [patch] pg_basebackup: mention that spread checkpoints are the default in --help

Hi, On Thu, 26 Oct 2023 at 13:58, Michael Banck wrote: > > Hi, > > On Wed, Oct 25, 2023 at 04:36:41PM +0200, Peter Eisentraut wrote: > > On 19.10.23 11:39, Michael Banck wrote: > > > Hi, > > > > > > I believed that spread (not fast) checkpoints are the default in > > > pg_basebackup, but noticed

Re: psql not responding to SIGINT upon db reconnection

Hi, > That sounds like a much better solution. Attached you will find a v4 > that implements your suggestion. Please let me know if there is > something that I missed. I can confirm that the patch works. > > $ ./build/src/bin/psql/psql -h pg.neon.tech > NOTICE: Welcome to Neon! >

Re: [HACKERS] Allow INSTEAD OF DELETE triggers to modify the tuple for RETURNING

Hi, On Thu, 19 Oct 2023 at 16:35, Marko Tiikkaja wrote: > > Hi, > > Thank you for the feedback. > > Apparently it took me six years, but I've attached the latest version > of the patch which I believe addresses all issues. I'll add it to the > open commitfest. > > > .m I went through the CFbot

Re: Force the old transactions logs cleanup even if checkpoint is skipped

Hi, I went through the Cfbot and saw that some test are failing for it (link: https://cirrus-ci.com/task/4631357628874752): test: postgresql:recovery / recovery/019_replslot_limit # test failed --- stderr --- # poll_query_un

Re: [PoC] Federated Authn/z with OAUTHBEARER

Hi, On Fri, 3 Nov 2023 at 17:14, Jacob Champion wrote: > > v12 implements a first draft of a client hook, so applications can > replace either the device prompt or the entire OAuth flow. (Andrey and > Mahendrakar: hopefully this is close to what you need.) It also cleans > up some of the JSON tec

Re: POC: Extension for adding distributed tracing - pg_tracing

Hi, On Thu, 12 Oct 2023 at 14:32, Anthonin Bonnefoy wrote: > > Hi! > > I've made a new batch of changes and improvements. > New features: > - Triggers are now correctly supported. They were not correctly > attached to the ExecutorFinish Span before. > - Additional configuration: exporting paramet

Re: [ psql - review request ] review request for \d+ tablename, \d+ indexname indenting

Hi, On Mon, 6 Nov 2023 at 13:47, 쿼리트릭스 wrote: > > The error was corrected and a new diff file was created. > The diff file was created based on 16 RC1. > We confirmed that 5 places where errors occurred when performing make check > were changed to ok. > I went through Cfbot and still see that s

Re: [PoC] Implementation of distinct in Window Aggregates: take two

Hi, I went through the Cfbot, and some of the test cases are failing for this patch. It seems like some tests are crashing: https://api.cirrus-ci.com/v1/artifact/task/6291153444667392/crashlog/crashlog-postgres.exe_03b0_2023-11-07_10-41-39-624.txt [10:46:56.546] Summary of Failures: [10:46:56.546

Failure during Building Postgres in Windows with Meson

Hi, I am trying to build postgres with meson on Windows. And I am stuck in the process. Steps I followed: 1. I clone postgres repo 2.Installed meson and ninja pip install meson ninja 3. Then running following command: meson setup build --buildtype debug 4. Then I ran cd build ninja Got follow

Re: pg_upgrade and logical replication

On Wed, 22 Nov 2023 at 06:48, Peter Smith wrote: > == > doc/src/sgml/ref/pgupgrade.sgml > > 1. > + > + Create all the new tables that were created in the publication during > + upgrade and refresh the publication by executing > + ALTER > SUBSCRIPTION ... REFRESH PUBLICA

Re: undetected deadlock in ALTER SUBSCRIPTION ... REFRESH PUBLICATION

Hi, I tried to reproduce the issue and was able to reproduce it with scripts shared by Tomas. I tried testing it from PG17 to PG 11. This issue is reproducible for each version. Next I would try to test with the patch in the thread shared by Amit. Thanks, Shlok Kumar Kyal

Re: undetected deadlock in ALTER SUBSCRIPTION ... REFRESH PUBLICATION

ed the v1 patch to resolve the issue. Have tested the patch on HEAD to PG12. The same patch applies to all the versions. The changes are similar to the one posted in the thread https://www.postgresql.org/message-id/1412708.1674417574%40sss.pgh.pa.us Thanks and Regards, Shlok Kyal v1-0001-Deadlock

Re: undetected deadlock in ALTER SUBSCRIPTION ... REFRESH PUBLICATION

and Sub (sec)| 1000 tables in pub and Sub (sec) Without patch Release | 115.871 | 6.656 | 81.157 With patch Release| 115.922 | 6.7305 | 81.1525 thoughts? Thanks and Regards

Re: undetected deadlock in ALTER SUBSCRIPTION ... REFRESH PUBLICATION

nes. The machine has Total Memory of 755.536 GB, 120 CPUs and RHEL 7 Operating System Also find the detailed info of the performance machine attached. Thanks and Regards, Shlok Kyal MemTotal: 755.536 GB MemFree: 748.281 GB MemAvailable: 748.581 GB Buffers: 0.002 GB Cached: 1.366 GB SwapCached

Re: undetected deadlock in ALTER SUBSCRIPTION ... REFRESH PUBLICATION

On Tue, 5 Dec 2023 at 17:18, Tomas Vondra wrote: > > On 12/5/23 08:14, Shlok Kyal wrote: > > Hi, > > > >> As for the test results, I very much doubt the differences are not > >> caused simply by random timing variations, or something like that. And I >

Re: undetected deadlock in ALTER SUBSCRIPTION ... REFRESH PUBLICATION

p'. I also fixed the comment in previous approach and attached here as 'v2-0001-Deadlock-when-apply-worker-tablesync-worker-and-c.patch' Thanks and Regards Shlok Kyal From 11072d138d900227b963b54d1a3626cf256db721 Mon Sep 17 00:00:00 2001 From: Shlok Kyal Date: Fri, 24 Nov 2023 16

Re: undetected deadlock in ALTER SUBSCRIPTION ... REFRESH PUBLICATION

tests and all the tests passed on each branch. I also reviewed the patch and it looks good to me. Thanks and Regards, Shlok Kyal

Re: speed up a logical replica setup

ubscriber binary was not created. But when I built the code using Meson, on installing postgres, pg_subscriber binary was created. Is this behaviour intentional? [1] https://github.com/postgres/postgres/commit/1301c80b2167feb658a738fa4ceb1c23d0991e23 Thanks and Regards, Shlok Kyal

Re: Intermittent failure with t/003_logical_slots.pl test on windows

olved most of the intermittent issues for me. I am facing some more intermittent issues. Will analyse and share it as well. Thanks and regards Shlok Kyal On Tue, 7 Nov 2023 at 11:05, Kyotaro Horiguchi wrote: > > At Mon, 6 Nov 2023 19:42:21 +0530, Nisha Moond > wrote in > > > Ap

Re: Intermittent failure with t/003_logical_slots.pl test on windows

the same as well. Thanks and Regards Shlok Kyal On Tue, 26 Dec 2023 at 17:39, Shlok Kyal wrote: > > Hi, > The same intermittent failure is reproducible on my system. > For the intermittent issues I found that many issues are due to errors > where commands like 'psql -V&#x

Re: speed up a logical replica setup

become available at 0/3000168 LOG: invalid record length at 0/3000150: expected at least 24, got 0 I was not clear about how to use pg_basebackup in this case, can you let me know if any changes need to be made for test2 and test3. Thanks and regards Shlok Kyal

Re: speed up a logical replica setup

On Thu, 4 Jan 2024 at 16:46, Amit Kapila wrote: > > On Thu, Jan 4, 2024 at 12:22 PM Shlok Kyal wrote: > > > > Hi, > > I was testing the patch with following test cases: > > > > Test 1 : > > - Create a 'primary' node > > - Setup

Re: speed up a logical replica setup

On Fri, 5 Jan 2024 at 12:19, Shlok Kyal wrote: > > On Thu, 4 Jan 2024 at 16:46, Amit Kapila wrote: > > > > On Thu, Jan 4, 2024 at 12:22 PM Shlok Kyal wrote: > > > > > > Hi, > > > I was testing the patch with following test cases: > &g

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

Hi, This patch is not applying on the HEAD. Please rebase and share the updated patch. Thanks and Regards Shlok Kyal On Wed, 10 Jan 2024 at 14:55, Peter Smith wrote: > > Oops - now with attachments > > On Mon, Aug 21, 2023 at 5:56 PM Peter Smith wrote: >> >> Hi M

Re: Extend pgbench partitioning to pgbench_history

bench/t/001_pgbench_with_server.pl line 1257. [09:15:01.794] # Looks like you failed 3 tests of 439. [09:15:01.794] [09:15:01.794] (test program exited with status code 3) [1] - https://cirrus-ci.com/task/5139049757802496 Thanks and regards Shlok Kyal

Re: Extend pgbench partitioning to pgbench_history

Sorry, I did not intend to send this message for this email. I by mistake sent this mail. Please ignore this mail On Wed, 10 Jan 2024 at 15:27, Shlok Kyal wrote: > > Hi, > > There are some test failures reported by Cfbot at [1]: > > [09:15:01.794] 192/276 postgresql:p

Re: Invalidate the subscription worker in cases where a user loses their superuser status

On Wed, 4 Oct 2023 at 16:56, Peter Smith wrote: > > On Tue, Oct 3, 2023 at 5:42 PM vignesh C wrote: > > > > Thanks for the comments, the attached v6 version patch has the changes > > for the same. > > > > v6 LGTM. > I have verified the patch and it is working fine for me.

Re: pg_ctl start may return 0 even if the postmaster has been already started on Windows

On Fri, 6 Oct 2023 at 11:38, Hayato Kuroda (Fujitsu) wrote: > > Dear Horiguchi-san, > > Thank you for making a patch! They can pass ci. > I'm still not sure what should be, but I can respond a part. > > > Another issue is.. that I haven't been able to cause the false > > positive of pg_ctl start..

Re: 001_rep_changes.pl fails due to publisher stuck on shutdown

did not apply on PG 14 to PG 12. I did a similar change in each branch. But the tests did not pass in each branch. I have attached a patch which applies successfully on the PG 15 branch. [1]: https://www.postgresql.org/message-id/f15d665f-4cd1-4894-037c-afdbe3692...@gmail.com Thanks and R

Re: confirmed flush lsn seems to be move backward in certain error cases

firmed_flush_will_always_not_be_less_than_last_saved_confirmed_flush.patch > to fix this issue. > > Thoughts? I was able to reproduce the issue with the test script provided in [1]. I ran the script 10 times and I was able to reproduce the issue 4 times. I also tested the patch Assert_confirmed_flush_will_always_not_be_less_than_last_saved_confirmed_flush.patch. and it resolves the issue. I ran the test script 20 times and I was not able to reproduce the issue. [1]: https://www.postgresql.org/message-id/CALDaNm3hgow2%2BoEov5jBk4iYP5eQrUCF1yZtW7%2BdV3J__p4KLQ%40mail.gmail.com Thanks and Regards, Shlok Kyal

Re: 001_rep_changes.pl fails due to publisher stuck on shutdown

On Mon, 10 Jun 2024 at 15:10, Shlok Kyal wrote: > > On Thu, 6 Jun 2024 at 11:49, Kyotaro Horiguchi > wrote: > > > > At Thu, 6 Jun 2024 12:49:45 +1000, Peter Smith > > wrote in > > > Hi, I have reproduced this multiple times now. > > > > >

Re: Pgoutput not capturing the generated columns

org/docs/devel/sql-copy.html > Hi Peter, I have removed the changes in the COPY command. I came up with an approach which requires changes only in tablesync code. We can COPY generated columns during tablesync using syntax 'COPY (SELECT column_name from table) TO STDOUT.' I have attached the patch for the same. v7-0001 : Not Modified v7-0002: Support replication of generated columns during initial sync. Thanks and Regards, Shlok Kyal v7-0002-Support-replication-of-generated-column-during-in.patch Description: Binary data v7-0001-Enable-support-for-include_generated_columns-opti.patch Description: Binary data

Re: Pgoutput not capturing the generated columns

; > 5. > +$result = $node_subscriber->safe_psql('postgres', "SELECT a, b FROM tab3"); > +is( $result, qq(1|2 > +2|4 > +3|6), 'generated columns initial sync with include_generated_column = true'); > > Should this say "ORDER BY..." so it will not fail if the row order > happens to be something unanticipated? > Fixed > == > > 99. > Also, see the attached file with numerous other nitpicks: > - plural param- and var-names > - typos in comments > - missing spaces > - SQL keyword should be UPPERCASE > - etc. > > Please apply any/all of these if you agree with them. Fixed Patch 7-0002 contains all the changes. Please refer [1] [1]: https://www.postgresql.org/message-id/CANhcyEUz0FcyR3T76b%2BNhtmvWO7o96O_oEwsLZNZksEoPmVzXw%40mail.gmail.com Thanks and Regards, Shlok Kyal

Re: Pgoutput not capturing the generated columns

On Wed, 5 Jun 2024 at 05:49, Peter Smith wrote: > > On Mon, Jun 3, 2024 at 9:52 PM Shlok Kyal wrote: > > > > > > > > The attached Patch contains the suggested changes. > > > > > > > Hi, > > > > Currently, COPY command does not

Re: Pgoutput not capturing the generated columns

. With patch v7-0002, I have used a different approach which does not require any code changes in COPY. Please refer [1] for patch v7-0002. [1]: https://www.postgresql.org/message-id/CANhcyEUz0FcyR3T76b%2BNhtmvWO7o96O_oEwsLZNZksEoPmVzXw%40mail.gmail.com Thanks and Regards, Shlok Kyal

Re: Pgoutput not capturing the generated columns

7;postgres', "SELECT * FROM tab2"); +is( $result, qq(4|8 +5|10), 'generated columns replicated to non-generated column on subscriber'); + +$node_publisher->safe_psql('postgres', "INSERT INTO tab3 VALUES (4), (5)"); + +$node_publisher->wait_for_catchup('sub3'); + +$result = $node_subscriber->safe_psql('postgres', "SELECT * FROM tab3"); +is( $result, qq(4|24 +5|25), 'generated columns replicated to non-generated column on subscriber'); Thanks and Regards, Shlok Kyal

Re: Pgoutput not capturing the generated columns

patch. Please refer to v9-0003 patch for the same in [1]. [1]: https://www.postgresql.org/message-id/CANhcyEXmjLEPNgOSAtjS4YGb9JvS8w-SO9S%2BjRzzzXo2RavNWw%40mail.gmail.com Thanks and Regards, Shlok Kyal

Re: Pgoutput not capturing the generated columns

appendStringInfo(&cmd, " AND a.attgenerated != 'v'"); > } > else > { > /* Replication of generated cols is not supported. */ > appendStringInfo(&cmd, " AND a.attgenerated = ''"); > } > } Fixed > == > > 99. > Please refer also to my attached nitpick diffs and apply those if you agree. Applied the changes. I have attached the updated patch v10 here in [1]. [1]: https://www.postgresql.org/message-id/CANhcyEUMCk6cCbw0vVZWo8FRd6ae9CmKG%3DgKP-9Q67jLn8HqtQ%40mail.gmail.com Thanks and Regards, Shlok Kyal

Re: Logical Replication of sequences

ot; has finished", + get_subscription_name(subid, false), RelationGetRelationName(sequencerel))); + table_close(sequencerel, NoLock); + + currseq++; + + if (currseq % MAX_SEQUENCES_SYNC_PER_BATCH == 0 || currseq == list_length(sequences)) + CommitTransactionCommand(); The above message gets logged even if the changes are not committed. Suppose the sequence worker exits before commit due to some reason. Thought the log will show that sequence is synced, the sequence will be in 'init' state. I think this is not desirable. Maybe we should log the synced sequences at commit time? Thoughts? = General 5. We can use other macros like 'foreach_ptr' instead of 'foreach' Thanks and Regards, Shlok Kyal

Re: pg_createsubscriber: drop pre-existing subscriptions from the converted node

to test its removal". > > Your approach looks better than mine. I followed the approach. Hi Kuroda-san, I tested the patches on linux and windows and I confirm that it successfully fixes the issue [1]. [1]: https://www.postgresql.org/message-id/CANhcyEWvimA1-f6hSrA%3D9qkfR5SonFb56b36M%2B%2BvT%3DLiFj%3D76g%40mail.gmail.com Thanks and Regards, Shlok Kyal

Re: Pgoutput not capturing the generated columns

; ``` > +/* > + * Regular table with no row filter and 'include_generated_columns' > + * specified as 'false' during creation of subscription. > + */ > ``` > > I think this comment is not correct. After patching, all tablesync command > becomes > like COPY (SELECT ...) if include_genereted_columns is set to true. Is it > right? > Can we restrict only when the table has generated ones? Fixed Please refer to v14 patch for the changes [1]. [1]: https://www.postgresql.org/message-id/CANhcyEW95M_usF1OJDudeejs0L0%2BYOEb%3DdXmt_4Hs-70%3DCXa-g%40mail.gmail.com Thanks and Regards, Shlok Kyal

Re: Pgoutput not capturing the generated columns

e docs updates on the "Logical Replication Message Formats" section > 53.9. So, I expected patch 0001 would make some changes and then patch > 0003 would have to update it again to say something about "STORED". > But all that is missing from the v10* patches. > > == Will fix in upcoming version > > 99. > See also my nitpicks diff which is a top-up patch addressing all the > nitpick comments mentioned above. Please apply all of these that you > agree with. Applied Relevant changes Please refer v14 patch for the changes [1]. [1]: https://www.postgresql.org/message-id/CANhcyEW95M_usF1OJDudeejs0L0%2BYOEb%3DdXmt_4Hs-70%3DCXa-g%40mail.gmail.com Thanks and Regards, Shlok Kyal

Re: Pgoutput not capturing the generated columns

(att->attnum, columns)) > continue; Same explanation as above. I have addressed all the comments in v16-0003 patch. Please refer [1]. [1]: https://www.postgresql.org/message-id/CANhcyEXw%3DBFFVUqohWES9EPkdq-ZMC5QRBVQqQPzrO%3DQ7uzFQw%40mail.gmail.com Thanks and Regards, Shlok Kyal

Re: Pgoutput not capturing the generated columns

continue; > ``` > > Hmm, does above mean that generated columns are decoded even if they are not > in > the column list? If so, why? I think such columns should not be sent. Fixed Thanks and Regards, Shlok Kyal v2-0001-Support-generated-column-capturing-generated-colu.patch Description: Binary data

Re: speed up a logical replica setup

w.postgresql.org/message-id/CAA4eK1J22UEfrqx222h5j9DQ7nxGrTbAa_BC%2B%3DmQXdXs-RCsew%40mail.gmail.com Thanks and Regards, Shlok Kyal # cleanup rm -rf ../primary ../standby primary.log standby.log # setup primary node ./initdb -D ../primary cat << EOF >> ../primary/postgresql.conf wal_level

Re: State of pg_createsubscriber

ill be replicated to N2 instead of N1. And once N1 is up again, subscription on N1 will not be able to connect to publication on N3 as it is already connected to N2. This can lead to data inconsistency. This error did not happen before running pg_createsubscriber on standby node N2, because there is no 'logical replication launcher' process on standby node. Thanks and Regards, Shlok Kyal

Re: speed up a logical replica setup

On Wed, 22 May 2024 at 16:50, Amit Kapila wrote: > > On Mon, May 20, 2024 at 4:30 PM Shlok Kyal wrote: > > > > > > I was trying to test this utility when 'sync_replication_slots' is on > > > and it gets in an ERROR loop [1] and never finishes. Please

Re: Pgoutput not capturing the generated columns

n only if 'copy_data = false'. I am attaching patches to resolve the above issues. v5-0001: not changed v5-0002: Support COPY of generated column v5-0003: Support COPY of generated column during tablesync process Thought? Thanks and Regards, Shlok Kyal v5-0001

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

. with 'tcount = 10'. But I didn't find any performance impact. 2. with 'tcount = 0' and running the loop 1000 times. But I didn't find any performance impact. I have also attached the test script and the machine configurations on which performance testing was done.

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

On Mon, 2 Sept 2024 at 10:12, Amit Kapila wrote: > > On Fri, Aug 30, 2024 at 3:06 PM Shlok Kyal wrote: > > > > Next I am planning to test solely on the logical decoding side and > > will share the results. > > > > Thanks, the next set of proposed tests makes s

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

On Mon, 2 Sept 2024 at 10:12, Amit Kapila wrote: > > On Fri, Aug 30, 2024 at 3:06 PM Shlok Kyal wrote: > > > > Next I am planning to test solely on the logical decoding side and > > will share the results. > > > > Thanks, the next set of proposed tests makes s

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

On Mon, 9 Sept 2024 at 10:41, Shlok Kyal wrote: > > On Mon, 2 Sept 2024 at 10:12, Amit Kapila wrote: > > > > On Fri, Aug 30, 2024 at 3:06 PM Shlok Kyal wrote: > > > > > > Next I am planning to test solely on the logical decoding side and > > > will

Re: Pgoutput not capturing the generated columns

On Tue, 9 Jul 2024 at 09:53, Peter Smith wrote: > > Hi Shlok, here are my review comments for v16-0003. > > == > src/backend/replication/logical/proto.c > > > On Mon, Jul 8, 2024 at 10:04 PM Shlok Kyal wrote: > > > > On Mon, 8 Jul 2024 at 13:20, Pet

Re: Pgoutput not capturing the generated columns

egards, Shlok Kyal

Re: Pgoutput not capturing the generated columns

all comment changes are carried forward correctly from > one patch to the next. Fixed I have addressed the comment in v20-0003 patch. Please refer [1]. [1]: https://www.postgresql.org/message-id/CANhcyEUzUurrX38HGvG30gV92YDz6WmnnwNRYMVY4tiga-8KZg%40mail.gmail.com Thanks and Regards, Shlok Kyal

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

orst case (where > > > publication is for ALL TABLES) we have to lock all the tables in the > > > database. We are not sure if that is good so the other alternative we > > > can pursue is to distribute invalidations in logical decoding > > > infrastructure [1] which has its downsides. > > > > > > Thoughts? > > > > Thank you for summarizing the problem and solutions! > > > > I think it's worth trying the idea of distributing invalidation > > messages, and we will see if there could be overheads or any further > > obstacles. IIUC this approach would resolve another issue we discussed > > before too[1]. > > > > Yes, and we also discussed having a similar solution at the time when > that problem was reported. So, it is clear that even though locking > tables can work for commands alter ALTER PUBLICATION ... ADD TABLE > ..., we need a solution for distributing invalidations to the > in-progress transactions during logical decoding for other cases as > reported by you previously. > > Thanks for looking into this. > Thanks, I am working on to implement a solution for distributing invalidations. Will share a patch for the same. Thanks and Regards, Shlok Kyal

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

On Wed, 31 Jul 2024 at 11:17, Shlok Kyal wrote: > > On Wed, 31 Jul 2024 at 09:36, Amit Kapila wrote: > > > > On Wed, Jul 31, 2024 at 3:27 AM Masahiko Sawada > > wrote: > > > > > > On Wed, Jul 24, 2024 at 9:53 PM Amit Kapila > > > wrote: >

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

On Thu, 8 Aug 2024 at 16:24, Shlok Kyal wrote: > > On Wed, 31 Jul 2024 at 11:17, Shlok Kyal wrote: > > > > On Wed, 31 Jul 2024 at 09:36, Amit Kapila wrote: > > > > > > On Wed, Jul 31, 2024 at 3:27 AM Masahiko Sawada > > > wrote: > > >

Re: Fix memory counter update in reorderbuffer

om the same problem? I tried testing this scenario and I was able to reproduce the crash in HEAD with this scenario. I have created a patch for the testcase. I also tested the same scenario with the latest patch shared by Sawada-san in [1]. And confirm that this resolves the issue. [1]: https://www.postgresql.org/message-id/CAD21AoDHC4Sob%3DNEYTxgu5wd4rzCpSLY_hWapMUqf4WKrAxmyw%40mail.gmail.com Thanks and Regards, Shlok Kyal fix_memory_counter_update_in_reorderbuffer_v2.patch Description: Binary data test.patch Description: Binary data

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

ing on the 'publish_via_partition_root' option. Will test and address this in the next version of the patch. For now, I have added a TODO. Thanks and Regards, Shlok Kyal v12-0001-Distribute-invalidatons-if-change-in-catalog-tab.patch Description: Binary data v12-0002-Selective-Invalidation-of-Cache.patch Description: Binary data

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

On Thu, 26 Sept 2024 at 11:39, Shlok Kyal wrote: > > > In the v7 patch, I am looping through the reorder buffer of the > > current committed transaction and storing all invalidation messages in > > a list. Then I am distributing those invalidations. > > But I foun

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

On Fri, 4 Oct 2024 at 12:52, Shlok Kyal wrote: > > Hi Kuroda-san, > > Thanks for reviewing the patch. > > > > 1. > > I feel the name of SnapBuildDistributeNewCatalogSnapshot() should be > > updated because it > > distributes two objects: catalog

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

rsue is to distribute invalidations in logical decoding > > infrastructure [1] which has its downsides. > > > > Thoughts? > > Thank you for summarizing the problem and solutions! > > I think it's worth trying the idea of distributing invalidation > messages, and we will see if there could be overheads or any further > obstacles. IIUC this approach would resolve another issue we discussed > before too[1]. > > Regards, > > [1] > https://www.postgresql.org/message-id/CAD21AoAenVqiMjpN-PvGHL1N9DWnHSq673bfgr6phmBUzx=k...@mail.gmail.com > Hi Sawada-san, I have tested the scenario shared by you on the thread [1]. And I confirm that the latest patch [2] fixes this issue. [1] https://www.postgresql.org/message-id/CAD21AoAenVqiMjpN-PvGHL1N9DWnHSq673bfgr6phmBUzx=k...@mail.gmail.com [2] https://www.postgresql.org/message-id/CANhcyEWfqdUvn2d2KOdvkhebBi5VO6O8J%2BC6%2BOwsPNwCTM%3DakQ%40mail.gmail.com Thanks and Regards, Shlok Kyal

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

Hi Kuroda-san, > > I have also modified the tests in 0001 patch. These changes are only > > related to syntax of writing tests. > > LGTM. I found small improvements, please find the attached. I have applied the changes and updated the patch. Thanks & Re

Disallow UPDATE/DELETE on table with unpublished generated column as REPLICA IDENTITY

I have attached a patch for the same. [1]: https://www.postgresql.org/message-id/CAD21AoA_RBkMa-6nUpBSoEP9s%3D46r3oq15vQkunVRCsYKXKMnA%40mail.gmail.com Thanks and regards, Shlok Kyal v1-0001-Disallow-UPDATE-DELETE-on-table-with-generated-co.patch Description: Binary data

Re: Disallow UPDATE/DELETE on table with unpublished generated column as REPLICA IDENTITY

Thanks for providing the comments. On Tue, 12 Nov 2024 at 12:52, Zhijie Hou (Fujitsu) wrote: > > On Friday, November 8, 2024 7:06 PM Shlok Kyal > wrote: > > > > Hi Amit, > > > > On Thu, 7 Nov 2024 at 11:37, Amit Kapila wrote: > > > > > > On

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

dation [1]:https://www.postgresql.org/message-id/CANhcyEW4pq6%2BPO_eFn2q%3D23sgV1budN3y4SxpYBaKMJNADSDuA%40mail.gmail.com Thanks and Regards, Shlok Kyal From 4222dca86e4892fbae6698ed7a6135f61d499d8f Mon Sep 17 00:00:00 2001 From: Shlok Kyal Date: Fri, 23 Aug 2024 14:02:20 +0530 Subject: [PATCH v9 1/2

Re: Using per-transaction memory contexts for storing decoded tuples

11801.086 ms |20.60030913 512kb|12361.4172 ms |65.27390105 1MB |12343.3732 ms |80.84427202 2MB |12357.675 ms |79.40017604 4MB | 12395.8364 ms |76.78273689 8MB |11712.8862 ms |50.74323039 ===

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

> > IIUC, the executed workload did not contain ALTER SCHEMA command, so > third improvement did not contribute this improvement. I have removed the changes corresponding to the third improvement. I have addressed the comment for 0002 patch and attached the patches. Also, I have moved the tests in the 0002 to 0001 patch. Thanks and Regards, Shlok Kyal v10-0002-Selective-Invalidation-of-Cache.patch Description: Binary data v10-0001-Distribute-invalidatons-if-change-in-catalog-tab.patch Description: Binary data

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

RENAME TO ...' and 'ALTER PUBLICATION ... OWNER TO ...' and debugged it. The newly added callback is called and it invalidates the cache of tables present in that particular publication. I have also added a test related to 'ALTER PUBLICATION ... RENAME TO ...' to 0001 patch. Thanks and Regards, Shlok Kyal v11-0001-Distribute-invalidatons-if-change-in-catalog-tab.patch Description: Binary data v11-0002-Selective-Invalidation-of-Cache.patch Description: Binary data

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

TO ..' is executed the relcache is invalidated for that specific publication. [1] : https://www.postgresql.org/message-id/CANhcyEWEXL3rxvKH9-Xtx-DgGX0D62EktHpW%2BnG%2BMSSaMVUVig%40mail.gmail.com Thanks and Regards, Shlok Kyal

Re: Disallow UPDATE/DELETE on table with unpublished generated column as REPLICA IDENTITY

index t3_idx; ALTER TABLE postgres=# insert into t3 values(1); INSERT 0 1 postgres=# update t3 set c1 = 100 where c1 = 1; UPDATE 1 postgres=# create publication pub3 for table t3; CREATE PUBLICATION postgres=# update t3 set c1 = 100 where c1 = 1; ERROR: cannot update table "t3" DETAIL: Column list used by the publication does not cover the replica identity. So, I think this behavior would be acceptable. Thoughts? [1]: https://www.postgresql.org/message-id/CANhcyEVw4V2Awe2AB6i0E5AJLNdASShGfdBLbUd1XtWDboymCA%40mail.gmail.com Thanks and Regards, Shlok Kyal

Re: Disallow UPDATE/DELETE on table with unpublished generated column as REPLICA IDENTITY

Hi Amit, On Thu, 7 Nov 2024 at 11:37, Amit Kapila wrote: > > On Tue, Nov 5, 2024 at 12:53 PM Shlok Kyal wrote: > > > > To avoid the issue, we can disallow UPDATE/DELETE on table with > > unpublished generated column as REPLICA IDENTITY. I have attached a > > patch

Re: Disallow UPDATE/DELETE on table with unpublished generated column as REPLICA IDENTITY

On Thu, 28 Nov 2024 at 16:38, Amit Kapila wrote: > > On Thu, Nov 21, 2024 at 5:30 PM Shlok Kyal wrote: > > > > Review comments: > === > 1. > + > + /* > + * true if all generated columns which are part of replica identity are > + * published or th

Re: Disallow UPDATE/DELETE on table with unpublished generated column as REPLICA IDENTITY

On Fri, 29 Nov 2024 at 15:49, vignesh C wrote: > > On Fri, 29 Nov 2024 at 13:38, Shlok Kyal wrote: > > > > On Thu, 28 Nov 2024 at 16:38, Amit Kapila wrote: > > > > > > On Thu, Nov 21, 2024 at 5:30 PM Shlok Kyal > > > wrote: > > > > &

Re: Disallow UPDATE/DELETE on table with unpublished generated column as REPLICA IDENTITY

On Thu, 21 Nov 2024 at 15:26, vignesh C wrote: > > On Tue, 19 Nov 2024 at 19:12, Shlok Kyal wrote: > > > > On Tue, 19 Nov 2024 at 14:39, Zhijie Hou (Fujitsu) > > wrote: > > > > > > On Tuesday, November 19, 2024 3:15 PM Shlok Kyal > > > wro

Re: Disallow UPDATE/DELETE on table with unpublished generated column as REPLICA IDENTITY

On Tue, 19 Nov 2024 at 09:50, Zhijie Hou (Fujitsu) wrote: > > On Tuesday, November 19, 2024 3:06 AM Shlok Kyal > wrote: > > > > I have fixed the comments and attached an updated patch. > > Thanks for the patch. > > I slightly refactored the cod

Re: Disallow UPDATE/DELETE on table with unpublished generated column as REPLICA IDENTITY

On Tue, 19 Nov 2024 at 10:22, vignesh C wrote: > > On Tue, 19 Nov 2024 at 00:36, Shlok Kyal wrote: > > > > On Mon, 18 Nov 2024 at 19:19, vignesh C wrote: > > > > > > On Mon, 18 Nov 2024 at 13:07, Shlok Kyal wrote: > > > > > > > > T

Re: Disallow UPDATE/DELETE on table with unpublished generated column as REPLICA IDENTITY

On Mon, 18 Nov 2024 at 19:19, vignesh C wrote: > > On Mon, 18 Nov 2024 at 13:07, Shlok Kyal wrote: > > > > Thanks for providing the comments. > > > > On Sat, 16 Nov 2024 at 17:29, vignesh C wrote: > > > > I have attached the updated version of the

Re: Disallow UPDATE/DELETE on table with unpublished generated column as REPLICA IDENTITY

On Mon, 18 Nov 2024 at 08:57, Zhijie Hou (Fujitsu) wrote: > > On Saturday, November 16, 2024 2:41 AM Shlok Kyal > wrote: > > > > > > > Thanks for providing the comments. I have fixed all the comments and > > attached > > the updated patch. > >

Re: Disallow UPDATE/DELETE on table with unpublished generated column as REPLICA IDENTITY

On Tue, 19 Nov 2024 at 14:39, Zhijie Hou (Fujitsu) wrote: > > On Tuesday, November 19, 2024 3:15 PM Shlok Kyal > wrote: > > > > > I noticed that we can add 'publish_generated_columns = true' for the case of > > generated column. So we won't need to

Re: Disallow UPDATE/DELETE on table with unpublished generated column as REPLICA IDENTITY

On Fri, 15 Nov 2024 at 20:31, vignesh C wrote: > > On Fri, 15 Nov 2024 at 16:45, Shlok Kyal wrote: > > > > Thanks for providing the comments > > > > On Fri, 15 Nov 2024 at 10:59, vignesh C wrote: > > > > > > On Thu, 14 Nov 2024 at 15:51, Shlok K

Re: Disallow UPDATE/DELETE on table with unpublished generated column as REPLICA IDENTITY

Thanks for providing the comments On Fri, 15 Nov 2024 at 10:59, vignesh C wrote: > > On Thu, 14 Nov 2024 at 15:51, Shlok Kyal wrote: > > > > Thanks for providing the comments. > > > > On Thu, 14 Nov 2024 at 12:22, vignesh C wrote: > > > > > >

Re: Disallow UPDATE/DELETE on table with unpublished generated column as REPLICA IDENTITY

Thanks for providing the comments. On Sat, 16 Nov 2024 at 17:29, vignesh C wrote: > > On Sat, 16 Nov 2024 at 00:10, Shlok Kyal wrote: > > > > Thanks for providing the comments. I have fixed all the comments and > > attached the updated patch. &g

Re: Logical replication timeout

ql-15/bin/postgres(PostmasterMain+0xe6c) [0x74d66c] > > /usr/pgsql-15/bin/postgres(main+0x1c5) [0x494a05] > > /usr/lib64/libc-2.17.so(__libc_start_main+0xf4) [0x22554] > > /usr/pgsql-15/bin/postgres(_start+0x28) [0x494fb8] > > We did not find any other option than deleting the subscription to stop that > loop and start a new one (thus loosing transactions). > > The publisher is PostgreSQL 15.6 > The subscriber is PostgreSQL 14.5 > > Thanks Hi, Do you have a reproducible test case for the above scenario? Please share the same. I am also trying to reproduce the above issue by generating large no. of spill files. Thanks and Regards, Shlok Kyal

Re: Disallow UPDATE/DELETE on table with unpublished generated column as REPLICA IDENTITY

Thanks for providing the comments. On Thu, 14 Nov 2024 at 12:22, vignesh C wrote: > > On Wed, 13 Nov 2024 at 11:15, Shlok Kyal wrote: > > > > Thanks for providing the comments. > > > > On Tue, 12 Nov 2024 at 12:52, Zhijie Hou (Fujitsu) > > wrote: > > &

Re: Improve the error message for logical replication of regular column to generated column.

efore. > > b) This current patch has overlapping logic so you need to be assured > > that adding this new error doesn't break the existing one. > > c) Only one of these errors wins. Adding both tests will define the > > expected order if both error scenarios exist at the same time. > > > > I have fixed the given comments. The attached Patch contains the > required changes. > Thanks for providing the patch. I have few comments: 1. Getting segmentation fault for following test case: Publisher: CREATE TABLE t1 (a INT, b INT); create publication pub1 for table t1(b) Subscriber: CREATE TABLE t1 (a INT, b int GENERATED ALWAYS AS (a + 1) STORED NOT NULL) create subscription test1 connection 'dbname=postgres host=localhost port=5432' publication pub1 Subscriber logs: 2024-11-16 17:23:16.919 IST [3842385] LOG: logical replication apply worker for subscription "test1" has started 2024-11-16 17:23:16.931 IST [3842389] LOG: logical replication table synchronization worker for subscription "test1", table "t1" has started 2024-11-16 17:29:47.855 IST [3842359] LOG: background worker "logical replication tablesync worker" (PID 3842389) was terminated by signal 11: Segmentation fault 2024-11-16 17:29:47.856 IST [3842359] LOG: terminating any other active server processes 2. + initStringInfo(&attsbuf); 'attsbuf' not free'd. I think we should pfree it. Thanks and Regards, Shlok Kyal

  1   2   >