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

From: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
To: 'Amit Kapila' <amit(dot)kapila16(at)gmail(dot)com>
Cc: vignesh C <vignesh21(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Euler Taveira <euler(at)eulerto(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
Subject: RE: pg_createsubscriber: drop pre-existing subscriptions from the converted node
Date: 2024-07-01 06:14:30
Message-ID: OSBPR01MB25522868FB47DF5808597D8CF5D32@OSBPR01MB2552.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Dear Amit,

Thanks for giving comments! PSA new version.

> Thanks, this is a better approach. I have changed a few comments and
> made some other cosmetic changes. See attached.

I checked your attached and LGTM. Based on that, I added some changes
like below:

- Made dbname be escaped while listing up pre-existing subscriptions
Previous version could not pass tests by recent commits.
- Skipped dropping subscriptions in dry_run mode
I found the issue while poring the test to 040_pg_createsubscriber.pl.
- Added info-level output to follow other drop_XXX functions

> BTW, why have you created a separate test file for this test? I think
> we should add a new test to one of the existing tests in
> 040_pg_createsubscriber.

I was separated a test file for just confirmation purpose, I've planned to merge.
New patch set did that.

> You can create a dummy subscription on node_p
> and do a test similar to what we are doing in "# Create failover slot
> to test its removal".

Your approach looks better than mine. I followed the approach.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/

Attachment Content-Type Size
v4-0001-pg_createsubscriber-remove-unused-attribute.patch application/octet-stream 834 bytes
v4-0002-pg_createsubscriber-Drop-pre-existing-subscriptio.patch application/octet-stream 8.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2024-07-01 06:16:52 Re: Conflict Detection and Resolution
Previous Message Michael Paquier 2024-07-01 06:07:46 Re: Assertion failure with summarize_wal enabled during pg_createsubscriber