handling of heap rewrites in logical decoding

Started by Peter Eisentrautabout 8 years ago8 messageshackers
Jump to latest
#1Peter Eisentraut
peter_e@gmx.net

A heap rewrite during a DDL command publishes changes for relations
named like pg_temp_%u, which are not real tables, and this breaks
replication systems that are not aware of that. We have a hack in the
pgoutput plugin to ignore those, but we knew that was a hack. So here
is an attempt at a more proper solution: Store a relisrewrite column in
pg_class and filter on that.

I have put this into reorderbuffer.c, so that it affects all plugins.
An alternative would be to have each plugin handle it separately (the
way it is done now), but it's hard to see why a plugin would not want
this fixed behavior.

A more fancy solution would be to make this column an OID that links
back to the original table. Then, a DDL-aware plugin could choose
replicate the rewrite rows. However, at the moment no plugin works that
way, so this might be overdoing it.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachments:

0001-Handle-heap-rewrites-even-better-in-logical-decoding.patchtext/plain; charset=UTF-8; name=0001-Handle-heap-rewrites-even-better-in-logical-decoding.patch; x-mac-creator=0; x-mac-type=0Download+81-98
#2Craig Ringer
craig@2ndquadrant.com
In reply to: Peter Eisentraut (#1)
Re: handling of heap rewrites in logical decoding

On 25 February 2018 at 09:57, Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:

A heap rewrite during a DDL command publishes changes for relations
named like pg_temp_%u, which are not real tables, and this breaks
replication systems that are not aware of that. We have a hack in the
pgoutput plugin to ignore those, but we knew that was a hack. So here
is an attempt at a more proper solution: Store a relisrewrite column in
pg_class and filter on that.

I have put this into reorderbuffer.c, so that it affects all plugins.
An alternative would be to have each plugin handle it separately (the
way it is done now), but it's hard to see why a plugin would not want
this fixed behavior.

A more fancy solution would be to make this column an OID that links
back to the original table. Then, a DDL-aware plugin could choose
replicate the rewrite rows. However, at the moment no plugin works that
way, so this might be overdoing it.

I'm pretty sure we _will_ want the ability to decode and stream rewrite
contents for non-IMMUTABLE table rewrites.

Filtering out by default is OK by me, but I think making it impossible to
decode is a mistake. So I'm all for the oid option and had written a
suggestion for it before I saw you already mentioned it in the next part
of your mail.

The main issue with filtering out rewrites by default is that I don't see
how, if we default to ignore/filter-out, plugins would indicate "actually I
want to choose about this one" or "I understand table rewrites". I'd prefer
not to add another change callback.

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

#3Peter Eisentraut
peter_e@gmx.net
In reply to: Craig Ringer (#2)
Re: handling of heap rewrites in logical decoding

On 2/25/18 07:27, Craig Ringer wrote:

I'm pretty sure we _will_ want the ability to decode and stream rewrite
contents for non-IMMUTABLE table rewrites.

Filtering out by default is OK by me, but I think making it impossible
to decode is a mistake. So I'm all for the oid option and had written a
suggestion for it before I saw you already mentioned it  in the next
part of your mail.

The main issue with filtering out rewrites by default is that I don't
see how, if we default to ignore/filter-out, plugins would indicate
"actually I want to choose about this one" or "I understand table
rewrites". I'd prefer not to add another change callback.

Second version, which uses an OID. I added another field to the output
plugin options (next to the output_type), to indicate whether the plugin
wants to receive these changes. I added some test cases to
test_decoding to show how it works either way. It's a bit messy to pass
this setting through to the ReorderBuffer; maybe there is a better idea.
But the result seems pretty useful.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachments:

v2-0001-Handle-heap-rewrites-even-better-in-logical-decod.patchtext/plain; charset=UTF-8; name=v2-0001-Handle-heap-rewrites-even-better-in-logical-decod.patch; x-mac-creator=0; x-mac-type=0Download+109-98
#4Peter Eisentraut
peter_e@gmx.net
In reply to: Peter Eisentraut (#3)
Re: handling of heap rewrites in logical decoding

On 2/28/18 13:52, Peter Eisentraut wrote:> Second version, which uses an
OID. I added another field to the output> plugin options (next to the
output_type), to indicate whether the plugin> wants to receive these
changes. I added some test cases to> test_decoding to show how it works
either way. It's a bit messy to pass> this setting through to the
ReorderBuffer; maybe there is a better idea.> But the result seems
pretty useful.
Here is a rebased update of this patch. No functionality changes
compared to v2.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachments:

v3-0001-Handle-heap-rewrites-even-better-in-logical-decod.patchtext/plain; charset=UTF-8; name=v3-0001-Handle-heap-rewrites-even-better-in-logical-decod.patch; x-mac-creator=0; x-mac-type=0Download+109-98
#5Craig Ringer
craig@2ndquadrant.com
In reply to: Peter Eisentraut (#4)
Re: handling of heap rewrites in logical decoding

On 16 March 2018 at 08:51, Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:

On 2/28/18 13:52, Peter Eisentraut wrote:> Second version, which uses an
OID. I added another field to the output> plugin options (next to the
output_type), to indicate whether the plugin> wants to receive these
changes. I added some test cases to> test_decoding to show how it works
either way. It's a bit messy to pass> this setting through to the
ReorderBuffer; maybe there is a better idea.> But the result seems
pretty useful.
Here is a rebased update of this patch. No functionality changes
compared to v2.

Picking this up for review.

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

#6Craig Ringer
craig@2ndquadrant.com
In reply to: Craig Ringer (#5)
Re: handling of heap rewrites in logical decoding

On 16 March 2018 at 10:54, Craig Ringer <craig@2ndquadrant.com> wrote:

On 16 March 2018 at 08:51, Peter Eisentraut <peter.eisentraut@2ndquadrant.
com> wrote:

On 2/28/18 13:52, Peter Eisentraut wrote:> Second version, which uses an
OID. I added another field to the output> plugin options (next to the
output_type), to indicate whether the plugin> wants to receive these
changes. I added some test cases to> test_decoding to show how it works
either way. It's a bit messy to pass> this setting through to the
ReorderBuffer; maybe there is a better idea.> But the result seems
pretty useful.
Here is a rebased update of this patch. No functionality changes
compared to v2.

Picking this up for review.

Why was relrewrite / Anum_pg_class_relrewrite inserted at position 28, not
appended to pg_class?

I don't see how it'd cause any harm, I'm just curious about the rationale.

Otherwise, I'm totally happy with what I see here. It's always nice to get
to the end of a patch and think "wait, that's all?"

Ready to go.

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

#7Peter Eisentraut
peter_e@gmx.net
In reply to: Craig Ringer (#6)
Re: handling of heap rewrites in logical decoding

On 3/21/18 03:08, Craig Ringer wrote:

Why was relrewrite / Anum_pg_class_relrewrite inserted at position 28,
not appended to pg_class?

I don't see how it'd cause any harm, I'm just curious about the rationale.

Adding it at the end would not be appropriate, since those are
variable-length fields. So I added it close to other fields of similar
purpose.

Otherwise, I'm totally happy with what I see here. It's always nice to
get to the end of a patch and think "wait, that's all?"

Ready to go.

committed

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#8Craig Ringer
craig@2ndquadrant.com
In reply to: Peter Eisentraut (#7)
Re: handling of heap rewrites in logical decoding

On 21 March 2018 at 21:20, Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:

On 3/21/18 03:08, Craig Ringer wrote:

Why was relrewrite / Anum_pg_class_relrewrite inserted at position 28,
not appended to pg_class?

I don't see how it'd cause any harm, I'm just curious about the

rationale.

Adding it at the end would not be appropriate, since those are
variable-length fields. So I added it close to other fields of similar
purpose.

Right, that makes sense. You're renumbering attnos anyway, so you might as
well put it somewhere logical.

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