Fix instability in subscription regression test

Started by houzj.fnst@fujitsu.comover 3 years ago5 messages

over 3 years ago

1 attachment(s)

Hi,

I noticed BF member wrasse failed in 028_row_filter.pl.

# Failed test 'check publish_via_partition_root behavior'
# at t/028_row_filter.pl line 669.
# got: ''
# expected: '1|100
# ...

Log:
2022-06-23 11:27:42.387 CEST [20589:3] 028_row_filter.pl LOG: statement: ALTER SUBSCRIPTION tap_sub REFRESH PUBLICATION WITH (copy_data = true)
2022-06-23 11:27:42.470 CEST [20589:4] 028_row_filter.pl LOG: disconnection: session time: 0:00:00.098 user=nm database=postgres host=[local]
2022-06-23 11:27:42.611 CEST [20593:1] LOG: logical replication table synchronization worker for subscription "tap_sub", table "tab_rowfilter_partitioned" has started
...
2022-06-23 11:27:43.197 CEST [20610:3] 028_row_filter.pl LOG: statement: SELECT a, b FROM tab_rowfilter_partitioned ORDER BY 1, 2
...
2022-06-23 11:27:43.689 CEST [20593:2] LOG: logical replication table synchronization worker for subscription "tap_sub", table "tab_rowfilter_partitioned" has finished

From the Log, I can see it query the target table before the table sync is
over. So, I think the reason is that we didn't wait for table sync to
finish after refreshing the publication. Sorry for not catching that
ealier. Here is a patch to fix it.

Best regards,
Hou zj

Attachments:

0001-Fix-instability-in-subscription-regression-test.patchapplication/octet-stream; name=0001-Fix-instability-in-subscription-regression-test.patchDownload

From d9421d4ae4522a2dde7b2ec9f08fe9be432c2793 Mon Sep 17 00:00:00 2001
From: "houzj.fnst" <houzj.fnst@cn.fujitsu.com>
Date: Thu, 23 Jun 2022 18:59:17 +0800
Subject: [PATCH] Fix instability in subscription regression test.

028_row_filter.pl neglected to wait for the subscriber's initial
synchronization to happen after refreshing the publication on subscriber
side.
---
 src/test/subscription/t/028_row_filter.pl | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/test/subscription/t/028_row_filter.pl b/src/test/subscription/t/028_row_filter.pl
index 0dc0a6d..183bed4 100644
--- a/src/test/subscription/t/028_row_filter.pl
+++ b/src/test/subscription/t/028_row_filter.pl
@@ -620,6 +620,11 @@ $node_subscriber->safe_psql('postgres',
 	"TRUNCATE TABLE tab_rowfilter_partitioned");
 $node_subscriber->safe_psql('postgres',
 	"ALTER SUBSCRIPTION tap_sub REFRESH PUBLICATION WITH (copy_data = true)");
+
+# wait for initial table synchronization to finish
+$node_subscriber->poll_query_until('postgres', $synced_query)
+  or die "Timed out while waiting for subscriber to synchronize data";
+
 $node_publisher->safe_psql('postgres',
 	"INSERT INTO tab_rowfilter_partitioned (a, b) VALUES(4000, 400),(4001, 401),(4002, 402)"
 );
-- 
2.7.2.windows.1

Masahiko Sawada

sawada.mshk@gmail.com

over 3 years ago

In reply to: houzj.fnst@fujitsu.com (#1)

Re: Fix instability in subscription regression test

On Thu, Jun 23, 2022 at 8:28 PM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:

Hi,

I noticed BF member wrasse failed in 028_row_filter.pl.

# Failed test 'check publish_via_partition_root behavior'
# at t/028_row_filter.pl line 669.
# got: ''
# expected: '1|100
# ...

Log:
2022-06-23 11:27:42.387 CEST [20589:3] 028_row_filter.pl LOG: statement: ALTER SUBSCRIPTION tap_sub REFRESH PUBLICATION WITH (copy_data = true)
2022-06-23 11:27:42.470 CEST [20589:4] 028_row_filter.pl LOG: disconnection: session time: 0:00:00.098 user=nm database=postgres host=[local]
2022-06-23 11:27:42.611 CEST [20593:1] LOG: logical replication table synchronization worker for subscription "tap_sub", table "tab_rowfilter_partitioned" has started
...
2022-06-23 11:27:43.197 CEST [20610:3] 028_row_filter.pl LOG: statement: SELECT a, b FROM tab_rowfilter_partitioned ORDER BY 1, 2
...
2022-06-23 11:27:43.689 CEST [20593:2] LOG: logical replication table synchronization worker for subscription "tap_sub", table "tab_rowfilter_partitioned" has finished

From the Log, I can see it query the target table before the table sync is
over. So, I think the reason is that we didn't wait for table sync to
finish after refreshing the publication. Sorry for not catching that
ealier. Here is a patch to fix it.

The patch looks good to me.

Regards,

--
Masahiko Sawada
EDB: https://www.enterprisedb.com/

Amit Kapila

amit.kapila16@gmail.com

over 3 years ago

In reply to: houzj.fnst@fujitsu.com (#1)

Re: Fix instability in subscription regression test

On Thu, Jun 23, 2022 at 4:58 PM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:

Hi,

I noticed BF member wrasse failed in 028_row_filter.pl.

# Failed test 'check publish_via_partition_root behavior'
# at t/028_row_filter.pl line 669.
# got: ''
# expected: '1|100
# ...

Log:
2022-06-23 11:27:42.387 CEST [20589:3] 028_row_filter.pl LOG: statement: ALTER SUBSCRIPTION tap_sub REFRESH PUBLICATION WITH (copy_data = true)
2022-06-23 11:27:42.470 CEST [20589:4] 028_row_filter.pl LOG: disconnection: session time: 0:00:00.098 user=nm database=postgres host=[local]
2022-06-23 11:27:42.611 CEST [20593:1] LOG: logical replication table synchronization worker for subscription "tap_sub", table "tab_rowfilter_partitioned" has started
...
2022-06-23 11:27:43.197 CEST [20610:3] 028_row_filter.pl LOG: statement: SELECT a, b FROM tab_rowfilter_partitioned ORDER BY 1, 2
...
2022-06-23 11:27:43.689 CEST [20593:2] LOG: logical replication table synchronization worker for subscription "tap_sub", table "tab_rowfilter_partitioned" has finished

From the Log, I can see it query the target table before the table sync is
over. So, I think the reason is that we didn't wait for table sync to
finish after refreshing the publication. Sorry for not catching that
ealier. Here is a patch to fix it.

+# wait for initial table synchronization to finish
+$node_subscriber->poll_query_until('postgres', $synced_query)

We could probably slightly change the comment to say: "wait for table
sync to finish". Normally, we use initial table sync after CREATE
SUBSCRIPTION. This is a minor nitpick and I can take care of it before
committing unless you think otherwise.

Your analysis and patch look good to me.

--
With Regards,
Amit Kapila.

houzj.fnst@fujitsu.com

over 3 years ago

In reply to: Amit Kapila (#3)

RE: Fix instability in subscription regression test

-----Original Message-----
From: Amit Kapila <amit.kapila16@gmail.com>
Sent: Friday, June 24, 2022 10:28 AM
To: Hou, Zhijie/侯志杰 <houzj.fnst@fujitsu.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Fix instability in subscription regression test

On Thu, Jun 23, 2022 at 4:58 PM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:

Hi,

I noticed BF member wrasse failed in 028_row_filter.pl.

# Failed test 'check publish_via_partition_root behavior'
# at t/028_row_filter.pl line 669.
# got: ''
# expected: '1|100
# ...

Log:
2022-06-23 11:27:42.387 CEST [20589:3] 028_row_filter.pl LOG:
statement: ALTER SUBSCRIPTION tap_sub REFRESH PUBLICATION WITH
(copy_data = true)
2022-06-23 11:27:42.470 CEST [20589:4] 028_row_filter.pl LOG:
disconnection: session time: 0:00:00.098 user=nm database=postgres
host=[local]
2022-06-23 11:27:42.611 CEST [20593:1] LOG: logical replication table
synchronization worker for subscription "tap_sub", table

"tab_rowfilter_partitioned" has started ...

2022-06-23 11:27:43.197 CEST [20610:3] 028_row_filter.pl LOG:
statement: SELECT a, b FROM tab_rowfilter_partitioned ORDER BY 1, 2 ...
2022-06-23 11:27:43.689 CEST [20593:2] LOG: logical replication table
synchronization worker for subscription "tap_sub", table
"tab_rowfilter_partitioned" has finished

From the Log, I can see it query the target table before the table
sync is over. So, I think the reason is that we didn't wait for table
sync to finish after refreshing the publication. Sorry for not
catching that ealier. Here is a patch to fix it.
+# wait for initial table synchronization to finish
+$node_subscriber->poll_query_until('postgres', $synced_query)
We could probably slightly change the comment to say: "wait for table sync to
finish". Normally, we use initial table sync after CREATE SUBSCRIPTION. This is a
minor nitpick and I can take care of it before committing unless you think
otherwise.

Thanks for reviewing, the suggestion looks good to me.

Best regards,
Hou zj

Amit Kapila

amit.kapila16@gmail.com

over 3 years ago

In reply to: houzj.fnst@fujitsu.com (#4)

Re: Fix instability in subscription regression test

On Fri, Jun 24, 2022 at 8:07 AM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:

+# wait for initial table synchronization to finish
+$node_subscriber->poll_query_until('postgres', $synced_query)
We could probably slightly change the comment to say: "wait for table sync to
finish". Normally, we use initial table sync after CREATE SUBSCRIPTION. This is a
minor nitpick and I can take care of it before committing unless you think
otherwise.
Thanks for reviewing, the suggestion looks good to me.

Pushed!

--
With Regards,
Amit Kapila.