From: 赵锐 <875941708@qq.com>
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
To: Amit Langote <amitlangote09@gmail.com>
Cc: Petr Jelinek <petr@2ndquadrant.com>
Cc: Rafia Sabih <rafia.pghackers@gmail.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: adding partitioned tables to publications
Date: Wed, 30 Dec 2020 22:15:24 +0800
Message-ID: <tencent_41FEA657C206F19AB4F406BE9252A0F69C06@qq.com> (raw)
The first file of Amit's patch can not only re-range the code, but also fix a hidden bug.
To make it easy to see, I attach another patch.
"RelationIdGetRelation" will increase ref on owner->relrefarr, without "RelationClose", the owner->relrefarr will enlarge and re-hash.
When the capacity of owner->relrefarr is over than 10 million, enlarge and re-hash takes serial hours. And what's worse, increase ref will also take minutes, as the hash collision resolution is based on looking up an array in order.
When we want to publish 10 billion data under one partition table, it takes serial days up to increase ref, enlarge and re-hash, and CPU is always 99%.
After applying my patch, 10 billion will be published in 10 minutes.
------------------ Original ------------------
From: "Amit Langote";<amitlangote09@gmail.com>;
Send time: Friday, Apr 17, 2020 10:58 PM
To: "Peter Eisentraut"<peter.eisentraut@2ndquadrant.com>;
Cc: "Petr Jelinek"<petr@2ndquadrant.com>; "Rafia Sabih"<rafia.pghackers@gmail.com>; "PostgreSQL-development"<pgsql-hackers@postgresql.org>;
Subject: Re: adding partitioned tables to publications
On Fri, Apr 17, 2020 at 10:23 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
> On 2020-04-09 09:28, Amit Langote wrote:
> > While figuring this out, I thought the nearby code could be rearranged
> > a bit, especially to de-duplicate the code. Also, I think
> > get_rel_sync_entry() may be a better place to set the map, rather than
> > maybe_send_schema(). Thoughts?
>
> because I didn't really have an opinion on that at the time, but if you
> still want it considered or have any open thoughts on this thread,
> please resend or explain.
Sure, thanks for taking care of the bug.
Rebased the code rearrangement patch. Also resending the patch to fix
TAP tests for improving coverage as described in:
https://www.postgresql.org/message-id/CA%2BHiwqFyydvQ5g%3Dqa54UM%2BXjm77BdhX-nM4dXQkNOgH%3DzvDjoA%40...
To summarize:
1. Missing coverage for a couple of related blocks in
apply_handle_tuple_routing()
2. Missing coverage report for the code in pgoutput.c added by 83fd4532
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.comAttachments:0001-RelationClose-after-RelationIdGetRelation.patch (application/octet-stream, 2.5K) download
From de3a45ad05bc2081b3942c10f701a3a1476e522e Mon Sep 17 00:00:00 2001
From: "Mark Zhao" <875941708@qq.com>
Date: Wed, 30 Dec 2020 13:36:12 +0000
Subject: [PATCH] RelationClose after RelationIdGetRelation
---
src/backend/replication/pgoutput/pgoutput.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.cindex 15379e3..a36dbaf 100644--- a/src/backend/replication/pgoutput/pgoutput.c+++ b/src/backend/replication/pgoutput/pgoutput.c@@ -363,6 +363,7 @@ pgoutput_change(LogicalDecodingContext *ctx, ReorderBufferTXN *txn,
PGOutputData *data = (PGOutputData *) ctx->output_plugin_private;
MemoryContext old;
RelationSyncEntry *relentry;
+ Relation ancestor = NULL;
if (!is_publishable_relation(relation))
return;
@@ -404,7 +405,8 @@ pgoutput_change(LogicalDecodingContext *ctx, ReorderBufferTXN *txn,
if (relentry->publish_as_relid != RelationGetRelid(relation))
{
Assert(relation->rd_rel->relispartition);
- relation = RelationIdGetRelation(relentry->publish_as_relid);+ ancestor = RelationIdGetRelation(relentry->publish_as_relid);+ relation = ancestor;
/* Convert tuple if needed. */
if (relentry->map)
tuple = execute_attr_map_tuple(tuple, relentry->map);
@@ -425,7 +427,8 @@ pgoutput_change(LogicalDecodingContext *ctx, ReorderBufferTXN *txn,
if (relentry->publish_as_relid != RelationGetRelid(relation))
{
Assert(relation->rd_rel->relispartition);
- relation = RelationIdGetRelation(relentry->publish_as_relid);+ ancestor = RelationIdGetRelation(relentry->publish_as_relid);+ relation = ancestor;
/* Convert tuples if needed. */
if (relentry->map)
{
@@ -448,7 +451,8 @@ pgoutput_change(LogicalDecodingContext *ctx, ReorderBufferTXN *txn,
if (relentry->publish_as_relid != RelationGetRelid(relation))
{
Assert(relation->rd_rel->relispart`ition);
- relation = RelationIdGetRelation(relentry->publish_as_relid);+ ancestor = RelationIdGetRelation(relentry->publish_as_relid);+ relation = ancestor;
/* Convert tuple if needed. */
if (relentry->map)
oldtuple = execute_attr_map_tuple(oldtuple, relentry->map);
@@ -465,6 +469,9 @@ pgoutput_change(LogicalDecodingContext *ctx, ReorderBufferTXN *txn,
Assert(false);
}
+ if (ancestor)+ RelationClose(ancestor);+
/* Cleanup */
MemoryContextSwitchTo(old);
MemoryContextReset(data->context);
--
1.8.3.1
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: 875941708@qq.com, peter.eisentraut@2ndquadrant.com, amitlangote09@gmail.com, petr@2ndquadrant.com, rafia.pghackers@gmail.com
Subject: Re: adding partitioned tables to publications
In-Reply-To: <tencent_41FEA657C206F19AB4F406BE9252A0F69C06@qq.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox