pgsql: Logical replication

Started by Peter Eisentrautalmost 9 years ago12 messages

peter_e@gmx.net

almost 9 years ago

Logical replication

- Add PUBLICATION catalogs and DDL
- Add SUBSCRIPTION catalog and DDL
- Define logical replication protocol and output plugin
- Add logical replication workers

From: Petr Jelinek <petr@2ndquadrant.com>
Reviewed-by: Steve Singer <steve@ssinger.info>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Erik Rijkers <er@xs4all.nl>
Reviewed-by: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/665d1fad99e7b11678b0d5fa24d2898424243cd6

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

Stephen Frost

sfrost@snowman.net

almost 9 years ago

In reply to: Peter Eisentraut (#1)

Re: pgsql: Logical replication

* Peter Eisentraut (peter_e@gmx.net) wrote:

Logical replication

- Add PUBLICATION catalogs and DDL
- Add SUBSCRIPTION catalog and DDL
- Define logical replication protocol and output plugin
- Add logical replication workers

The buildfarm is rather upset with this.

Looks like an issue in pg_upgrade / pg_dump.

Thanks!

Stephen

Amit Kapila

amit.kapila16@gmail.com

almost 9 years ago

In reply to: Peter Eisentraut (#1)

Re: pgsql: Logical replication

On Fri, Jan 20, 2017 at 7:36 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

Logical replication

- Add PUBLICATION catalogs and DDL
- Add SUBSCRIPTION catalog and DDL
- Define logical replication protocol and output plugin
- Add logical replication workers

From: Petr Jelinek <petr@2ndquadrant.com>
Reviewed-by: Steve Singer <steve@ssinger.info>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Erik Rijkers <er@xs4all.nl>
Reviewed-by: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>

119 files changed, 13354 insertions(+), 95 deletions(-)

Great work, Congrats Peter Jelinek and Thanks to all involved.
Getting a feature of this magnitude deserves a big round of applause.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

Amit Langote

amitlangote09@gmail.com

almost 9 years ago

In reply to: Amit Kapila (#3)

Re: pgsql: Logical replication

On Sat, Jan 21, 2017 at 1:19 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:

On Fri, Jan 20, 2017 at 7:36 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

Logical replication

- Add PUBLICATION catalogs and DDL
- Add SUBSCRIPTION catalog and DDL
- Define logical replication protocol and output plugin
- Add logical replication workers

From: Petr Jelinek <petr@2ndquadrant.com>
Reviewed-by: Steve Singer <steve@ssinger.info>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Erik Rijkers <er@xs4all.nl>
Reviewed-by: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>

..

119 files changed, 13354 insertions(+), 95 deletions(-)

Great work, Congrats Peter Jelinek and Thanks to all involved.
Getting a feature of this magnitude deserves a big round of applause.

+1, congrats!

Thanks,
Amit

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

Pavel Stehule

pavel.stehule@gmail.com

almost 9 years ago

In reply to: Amit Langote (#4)

Re: pgsql: Logical replication

2017-01-21 6:56 GMT+01:00 Amit Langote <amitlangote09@gmail.com>:

On Sat, Jan 21, 2017 at 1:19 PM, Amit Kapila <amit.kapila16@gmail.com>
wrote:

On Fri, Jan 20, 2017 at 7:36 PM, Peter Eisentraut <peter_e@gmx.net>

wrote:

Logical replication

- Add PUBLICATION catalogs and DDL
- Add SUBSCRIPTION catalog and DDL
- Define logical replication protocol and output plugin
- Add logical replication workers

From: Petr Jelinek <petr@2ndquadrant.com>
Reviewed-by: Steve Singer <steve@ssinger.info>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Erik Rijkers <er@xs4all.nl>
Reviewed-by: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>

..

119 files changed, 13354 insertions(+), 95 deletions(-)

Great work, Congrats Peter Jelinek and Thanks to all involved.
Getting a feature of this magnitude deserves a big round of applause.

+1, congrats!

+1 pretty big work is done!

Pavel

Show quoted text

Thanks,
Amit

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

Andres Freund

andres@anarazel.de

almost 9 years ago

In reply to: Amit Kapila (#3)

Re: pgsql: Logical replication

On 2017-01-21 09:49:28 +0530, Amit Kapila wrote:

On Fri, Jan 20, 2017 at 7:36 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

Logical replication

- Add PUBLICATION catalogs and DDL
- Add SUBSCRIPTION catalog and DDL
- Define logical replication protocol and output plugin
- Add logical replication workers

From: Petr Jelinek <petr@2ndquadrant.com>
Reviewed-by: Steve Singer <steve@ssinger.info>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Erik Rijkers <er@xs4all.nl>
Reviewed-by: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>

Great work, Congrats Peter Jelinek and Thanks to all involved.
Getting a feature of this magnitude deserves a big round of applause.

Seconded! Or fourthed ;)

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

Stephen Frost

sfrost@snowman.net

almost 9 years ago

In reply to: Peter Eisentraut (#1)

PUBLICATIONS and pg_dump

Peter,

* Peter Eisentraut (peter_e@gmx.net) wrote:

Logical replication

- Add PUBLICATION catalogs and DDL
- Add SUBSCRIPTION catalog and DDL
- Define logical replication protocol and output plugin
- Add logical replication workers

I think we need to have a bit more discussion regarding where
publications (and maybe subscriptions... not sure on that though) fit
when it comes to pg_dump.

In particular, I'm trying to clean up the pg_dump TAP tests and am
finding things I wouldn't have expected. For example, publications
appear to be included in pretty much every pg_dump output, no matter if
a specific schema or even table was explicitly called for, or if that
publication or subscription was explicitly associated with that table.

The example I'm playing with is:

CREATE PUBLICATION pub2 WITH (PUBLISH INSERT, PUBLISH UPDATE, PUBLISH
DELETE);

and a simple:

pg_dump -n public -t t1

Will end up including the CREATE PUBLICATION command.

In fact, I'm not entirely sure how to have it not included in pg_dump's
output.

I understand that this is a bit complicated, but I would have thought
we'd do something similar to what is done for DEFAULT PRIVILEGES, where
we include the "global" default privileges when we are doing a dump of
"everything", but if we're dumping a specific schema then we only
include the default privileges directly associated with that schema.

Perhaps we need to include publications which are specific to a
particular table, but the current logic of, essentially, "always include
all publications" does not seem to make a lot of sense to me.

I'm happy to be corrected if I've grossly misunderstood something here,
of course.

Thanks!

Stephen

Peter Eisentraut

peter.eisentraut@2ndquadrant.com

almost 9 years ago

In reply to: Stephen Frost (#7)

Re: PUBLICATIONS and pg_dump

On 2/7/17 3:19 PM, Stephen Frost wrote:

I understand that this is a bit complicated, but I would have thought
we'd do something similar to what is done for DEFAULT PRIVILEGES, where
we include the "global" default privileges when we are doing a dump of
"everything", but if we're dumping a specific schema then we only
include the default privileges directly associated with that schema.

Perhaps we need to include publications which are specific to a
particular table, but the current logic of, essentially, "always include
all publications" does not seem to make a lot of sense to me.

I think it would be sensible to refine it along those lines.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, 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

Stephen Frost

sfrost@snowman.net

almost 9 years ago

In reply to: Peter Eisentraut (#8)

Re: PUBLICATIONS and pg_dump

Peter,

On Tue, Feb 7, 2017 at 22:49 Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:

On 2/7/17 3:19 PM, Stephen Frost wrote:

I understand that this is a bit complicated, but I would have thought
we'd do something similar to what is done for DEFAULT PRIVILEGES, where
we include the "global" default privileges when we are doing a dump of
"everything", but if we're dumping a specific schema then we only
include the default privileges directly associated with that schema.

Perhaps we need to include publications which are specific to a
particular table, but the current logic of, essentially, "always include
all publications" does not seem to make a lot of sense to me.

I think it would be sensible to refine it along those lines.

Great! I've added it to the open items list for PG10.

Thanks!

Stephen

#10

Petr Jelinek

petr.jelinek@2ndquadrant.com

almost 9 years ago

In reply to: Stephen Frost (#9)

1 attachment(s)

Re: PUBLICATIONS and pg_dump

On 08/02/17 05:02, Stephen Frost wrote:

Peter,

On Tue, Feb 7, 2017 at 22:49 Peter Eisentraut
<peter.eisentraut@2ndquadrant.com
<mailto:peter.eisentraut@2ndquadrant.com>> wrote:

On 2/7/17 3:19 PM, Stephen Frost wrote:

I understand that this is a bit complicated, but I would have thought
we'd do something similar to what is done for DEFAULT PRIVILEGES,

where

we include the "global" default privileges when we are doing a dump of
"everything", but if we're dumping a specific schema then we only
include the default privileges directly associated with that schema.

Perhaps we need to include publications which are specific to a
particular table, but the current logic of, essentially, "always

include

all publications" does not seem to make a lot of sense to me.

I think it would be sensible to refine it along those lines.

Great! I've added it to the open items list for PG10.

Yeah that was oversight in initial patch, publications and their
membership was supposed to be dumped only when table filter is not used.
I mistakenly made it check for data_only instead of using the
selectDumpableObject machinery.

Fix attached.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachments:

0001-Don-t-dump-publications-with-pg_dump-t.patchtext/x-patch; name=0001-Don-t-dump-publications-with-pg_dump-t.patchDownload

From 907dcca71712558ba954a28cdf65ccdd77473bfe Mon Sep 17 00:00:00 2001
From: Petr Jelinek <pjmodos@pjmodos.net>
Date: Sun, 26 Feb 2017 20:12:26 +0100
Subject: [PATCH] Don't dump publications with pg_dump -t

---
 src/bin/pg_dump/pg_dump.c        | 29 +++++++++++++++++++++++++----
 src/bin/pg_dump/t/002_pg_dump.pl | 17 +++++++++--------
 2 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 7273ec8..8f7245d 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1613,6 +1613,23 @@ selectDumpableExtension(ExtensionInfo *extinfo, DumpOptions *dopt)
 }
 
 /*
+ * selectDumpablePublicationTable: policy-setting subroutine
+ *		Mark a publication table as to be dumped or not
+ *
+ * Publication tables have schemas but those should be ignored in decitions
+ * making as publications are only dumped when we are dumping everything.
+ */
+static void
+selectDumpablePublicationTable(DumpableObject *dobj, Archive *fout)
+{
+	if (checkExtensionMembership(dobj, fout))
+		return;					/* extension membership overrides all else */
+
+	dobj->dump = fout->dopt->include_everything ?
+		DUMP_COMPONENT_ALL : DUMP_COMPONENT_NONE;
+}
+
+/*
  * selectDumpableObject: policy-setting subroutine
  *		Mark a generic dumpable object as to be dumped or not
  *
@@ -3389,6 +3406,9 @@ getPublications(Archive *fout)
 		if (strlen(pubinfo[i].rolname) == 0)
 			write_msg(NULL, "WARNING: owner of publication \"%s\" appears to be invalid\n",
 					  pubinfo[i].dobj.name);
+
+		/* Decide whether we want to dump it */
+		selectDumpableObject(&(pubinfo[i].dobj), fout);
 	}
 	PQclear(res);
 
@@ -3402,11 +3422,10 @@ getPublications(Archive *fout)
 static void
 dumpPublication(Archive *fout, PublicationInfo *pubinfo)
 {
-	DumpOptions *dopt = fout->dopt;
 	PQExpBuffer delq;
 	PQExpBuffer query;
 
-	if (dopt->dataOnly)
+	if (!(pubinfo->dobj.dump & DUMP_COMPONENT_DEFINITION))
 		return;
 
 	delq = createPQExpBuffer();
@@ -3534,6 +3553,9 @@ getPublicationTables(Archive *fout, TableInfo tblinfo[], int numTables)
 			pubrinfo[j].dobj.namespace = tbinfo->dobj.namespace;
 			pubrinfo[j].pubname = pg_strdup(PQgetvalue(res, j, i_pubname));
 			pubrinfo[j].pubtable = tbinfo;
+
+			/* Decide whether we want to dump it */
+			selectDumpablePublicationTable(&(pubrinfo[j].dobj), fout);
 		}
 		PQclear(res);
 	}
@@ -3547,12 +3569,11 @@ getPublicationTables(Archive *fout, TableInfo tblinfo[], int numTables)
 static void
 dumpPublicationTable(Archive *fout, PublicationRelInfo *pubrinfo)
 {
-	DumpOptions *dopt = fout->dopt;
 	TableInfo  *tbinfo = pubrinfo->pubtable;
 	PQExpBuffer query;
 	char	   *tag;
 
-	if (dopt->dataOnly)
+	if (!(pubrinfo->dobj.dump & DUMP_COMPONENT_DEFINITION))
 		return;
 
 	tag = psprintf("%s %s", pubrinfo->pubname, tbinfo->dobj.name);
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index f73bf89..40509a4 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -2242,14 +2242,14 @@ qr/CREATE TRANSFORM FOR integer LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog
 			exclude_test_table       => 1,
 			no_privs                 => 1,
 			no_owner                 => 1,
-			only_dump_test_schema    => 1,
-			only_dump_test_table     => 1,
 			pg_dumpall_dbprivs       => 1,
 			schema_only              => 1,
-			section_post_data        => 1,
-			test_schema_plus_blobs   => 1, },
+			section_post_data        => 1, },
 		unlike => {
 			section_pre_data         => 1,
+			only_dump_test_table     => 1,
+			test_schema_plus_blobs   => 1,
+			only_dump_test_schema    => 1,
 			pg_dumpall_globals       => 1,
 			pg_dumpall_globals_clean => 1, }, },
 	'ALTER PUBLICATION pub1 ADD TABLE test_table' => {
@@ -2268,16 +2268,17 @@ qr/CREATE TRANSFORM FOR integer LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog
 			exclude_test_table_data => 1,
 			no_privs                => 1,
 			no_owner                => 1,
-			only_dump_test_schema   => 1,
-			only_dump_test_table    => 1,
 			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
-			section_post_data       => 1,
-			test_schema_plus_blobs  => 1, },
+			section_post_data       => 1, },
 		unlike => {
 			section_pre_data         => 1,
 			exclude_dump_test_schema => 1,
 			exclude_test_table       => 1,
+			only_dump_test_schema   => 1,
+			only_dump_test_table    => 1,
+			only_dump_test_table     => 1,
+			test_schema_plus_blobs  => 1,
 			pg_dumpall_globals       => 1,
 			pg_dumpall_globals_clean => 1, }, },
 
-- 
2.7.4

#11

Peter Eisentraut

peter.eisentraut@2ndquadrant.com

almost 9 years ago

In reply to: Petr Jelinek (#10)

Re: PUBLICATIONS and pg_dump

On 2/26/17 14:25, Petr Jelinek wrote:

Yeah that was oversight in initial patch, publications and their
membership was supposed to be dumped only when table filter is not used.
I mistakenly made it check for data_only instead of using the
selectDumpableObject machinery.

Committed.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, 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

#12

Andres Freund

andres@anarazel.de

over 7 years ago

In reply to: Peter Eisentraut (#1)

Re: [COMMITTERS] pgsql: Logical replication

Hi,

On 2017-01-20 14:06:18 +0000, Peter Eisentraut wrote:

Logical replication

- Add PUBLICATION catalogs and DDL
- Add SUBSCRIPTION catalog and DDL
- Define logical replication protocol and output plugin
- Add logical replication workers

I know I mentioned this before, but I just hit it again. Please split
up your commits in more logical chunks. This has a *LOT* of largely
unrelated pieces. Lots of them without so much as a comment about why
they have been changed. The above certainly doesn't explain why the
*generic* cache invalidation code was affected:

@@ -509,8 +514,10 @@ RegisterRelcacheInvalidation(Oid dbId, Oid relId)
    /*
     * If the relation being invalidated is one of those cached in the local
     * relcache init file, mark that we need to zap that file at commit.
+    * Same is true when we are invalidating whole relcache.
     */
-   if (OidIsValid(dbId) && RelationIdIsInInitFile(relId))
+   if (OidIsValid(dbId) &&
+       (RelationIdIsInInitFile(relId) || relId == InvalidOid))
        transInvalInfo->RelcacheInitFileInval = true;
 }

I mean, like, seriously? What is this stuff about?

I kinda guess the motivating factor is:

/*
* CacheInvalidateRelcacheAll
* Register invalidation of the whole relcache at the end of command.
*
* This is used by alter publication as changes in publications may affect
* large number of tables.
*/
void
CacheInvalidateRelcacheAll(void)
{
PrepareInvalidationState();

RegisterRelcacheInvalidation(InvalidOid, InvalidOid);
}

but that doesn't explain why it's not tied to a database? Nor why the
relcache init file is now invalidated when there's a publication change?

Greetings,

Andres Freund