MIME-Version: 1.0
References: 
 <CAA4eK1KdVRffYEYiv9=4tVWhP=Jkfh1h6FM0A1R0NEe8cLT8+g@mail.gmail.com>
 <CAA4eK1JU4J3-Z=fcMcyQw-iG-DNr=u6w_g-iM_4xwg1pKupkFQ@mail.gmail.com>
 <OS0PR01MB57163693482C5470C41914AF94EA9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
 <CAHut+PuYJz-26QWMSRN_HwiJNfYG4ZxEV3RGE-VyZtXXeFm+cw@mail.gmail.com>
 <OS0PR01MB5716C1C1D8244BA86311193894EB9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
 <CAA4eK1LrADDNXKRVYox4Q+zSFQvtiQu-7piyd_PCmVjg1gsN2w@mail.gmail.com>
 <OS0PR01MB57169EF567017861EACBD10D94E99@OS0PR01MB5716.jpnprd01.prod.outlook.com>
 <OS0PR01MB571623EFD8903C93424EFADC94E99@OS0PR01MB5716.jpnprd01.prod.outlook.com>
 <OS0PR01MB57166C4E79559C9A5570838C94EC9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
In-Reply-To: 
 <OS0PR01MB57166C4E79559C9A5570838C94EC9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
From: Amit Kapila <amit.kapila16@gmail.com>
Date: Mon, 26 Dec 2022 17:21:05 +0530
Message-ID: 
 <CAA4eK1+NV9BDPjh5XB9PnZYNdnQht8+2JGRcerJpSUXtYPtQsg@mail.gmail.com>
Subject: Re: Perform streaming logical transactions by background workers and
 parallel apply
To: "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>
Cc: Peter Smith <smithpb2250@gmail.com>,
 Masahiko Sawada <sawada.mshk@gmail.com>,
	"wangw.fnst@fujitsu.com" <wangw.fnst@fujitsu.com>,
 Dilip Kumar <dilipbalaut@gmail.com>,
	"shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/mixed; boundary="000000000000cacd5605f0b9c0c2"
Archived-At: 
 <https://www.postgresql.org/message-id/CAA4eK1%2BNV9BDPjh5XB9PnZYNdnQht8%2B2JGRcerJpSUXtYPtQsg%40mail.gmail.com>
Precedence: bulk

--000000000000cacd5605f0b9c0c2
Content-Type: text/plain; charset="UTF-8"

On Mon, Dec 26, 2022 at 9:52 AM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:
>
> On Friday, December 23, 2022 5:20 PM houzj.fnst@fujitsu.com <houzj.fnst@fujitsu.com> wrote:
>
> Since the GUC used to force stream changes has been committed, I removed that
> patch from the patch set here and rebased the testcases based on that commit.
> Here is the rebased patch set.
>

Few comments on 0002 and 0001 patches
=================================
1.
+    if ($is_parallel)
+    {
+        $node_subscriber->append_conf('postgresql.conf',
+            "log_min_messages = debug1");
+        $node_subscriber->reload;
+    }
+
+    # Check the subscriber log from now on.
+    $offset = -s $node_subscriber->logfile;
+
+    $in .= q{
+    BEGIN;
+    INSERT INTO test_tab SELECT i, md5(i::text) FROM
generate_series(3, 5000) s(i);

How can we guarantee that reload would have taken place before this
next test? I see that 020_archive_status.pl is executing a query to
ensure the reload has been taken into consideration. Can we do the
same?

2. It is not very clear whether converting 017_stream_ddl and
019_stream_subxact_ddl_abort adds much value. They seem to be mostly
testing DDL/DML interaction of publisher side. We can probably check
the code coverage by removing the parallel version for these two files
and remove them unless it covers some extra code. If we decide to
remove parallel version for these two files then we can probably add a
comment atop these files indicating why we don't have a version that
parallel option for these tests.

3.
+# Drop the unique index on the subscriber, now it works.
+$node_subscriber->safe_psql('postgres', "DROP INDEX idx_tab");
+
+# Wait for this streaming transaction to be applied in the apply worker.
 $node_publisher->wait_for_catchup($appname);

 $result =
-  $node_subscriber->safe_psql('postgres',
- "SELECT count(*), count(c), count(d = 999) FROM test_tab");
-is($result, qq(3334|3334|3334), 'check extra columns contain local defaults');
+  $node_subscriber->safe_psql('postgres', "SELECT count(*) FROM test_tab_2");
+is($result, qq(5001), 'data replicated to subscriber after dropping index');

-# Test the streaming in binary mode
+# Clean up test data from the environment.
+$node_publisher->safe_psql('postgres', "TRUNCATE TABLE test_tab_2");
+$node_publisher->wait_for_catchup($appname);
 $node_subscriber->safe_psql('postgres',
- "ALTER SUBSCRIPTION tap_sub SET (binary = on)");
+ "CREATE UNIQUE INDEX idx_tab on test_tab_2(a)");

What is the need to first Drop the index and then recreate it after a few lines?

4. Attached, find some comment improvements atop v67-0002* patch.
Similar comments need to be changed in other test files.

5. Attached, find some comment improvements atop v67-0001* patch.

-- 
With Regards,
Amit Kapila.

--000000000000cacd5605f0b9c0c2
Content-Type: application/octet-stream; name="v67-0002-changes_amit_1.patch"
Content-Disposition: attachment; filename="v67-0002-changes_amit_1.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_lc4ql0mf0>
X-Attachment-Id: f_lc4ql0mf0

ZGlmZiAtLWdpdCBhL3NyYy90ZXN0L3N1YnNjcmlwdGlvbi90LzAxNV9zdHJlYW0ucGwgYi9zcmMv
dGVzdC9zdWJzY3JpcHRpb24vdC8wMTVfc3RyZWFtLnBsCmluZGV4IDdiYzM0MmQyMTEuLjIyMGFi
MGMyNWYgMTAwNjQ0Ci0tLSBhL3NyYy90ZXN0L3N1YnNjcmlwdGlvbi90LzAxNV9zdHJlYW0ucGwK
KysrIGIvc3JjL3Rlc3Qvc3Vic2NyaXB0aW9uL3QvMDE1X3N0cmVhbS5wbApAQCAtOCw4ICs4LDgg
QEAgdXNlIFBvc3RncmVTUUw6OlRlc3Q6OkNsdXN0ZXI7CiB1c2UgUG9zdGdyZVNRTDo6VGVzdDo6
VXRpbHM7CiB1c2UgVGVzdDo6TW9yZTsKIAotIyBDaGVjayB0aGUgbG9nIHRoYXQgdGhlIHN0cmVh
bWVkIHRyYW5zYWN0aW9uIHdhcyBjb21wbGV0ZWQgc3VjY2Vzc2Z1bGx5Ci0jIHJlcG9ydGVkIGJ5
IHBhcmFsbGVsIGFwcGx5IHdvcmtlci4KKyMgQ2hlY2sgdGhhdCB0aGUgcGFyYWxsZWwgYXBwbHkg
d29ya2VyIGhhcyBmaW5pc2hlZCBhcHBseWluZyB0aGUgc3RyZWFtaW5nCisjIHRyYW5zYWN0aW9u
Lgogc3ViIGNoZWNrX3BhcmFsbGVsX2xvZwogewogCW15ICgkbm9kZV9zdWJzY3JpYmVyLCAkb2Zm
c2V0LCAkaXNfcGFyYWxsZWwsICR0eXBlKSA9IEBfOwpAQCAtMjIsOSArMjIsNyBAQCBzdWIgY2hl
Y2tfcGFyYWxsZWxfbG9nCiAJfQogfQogCi0jIEVuY2Fwc3VsYXRlIGFsbCB0aGUgY29tbW9uIHRl
c3Qgc3RlcHMgd2hpY2ggYXJlIHJlbGF0ZWQgdG8gInN0cmVhbWluZyIKLSMgcGFyYW1ldGVyIHNv
IHRoZSBzYW1lIGNvZGUgY2FuIGJlIHJ1biBib3RoIGZvciB0aGUgc3RyZWFtaW5nPW9uIGFuZAot
IyBzdHJlYW1pbmc9cGFyYWxsZWwgY2FzZXMuCisjIENvbW1vbiB0ZXN0IHN0ZXBzIGZvciBib3Ro
IHRoZSBzdHJlYW1pbmc9b24gYW5kIHN0cmVhbWluZz1wYXJhbGxlbCBjYXNlcy4KIHN1YiB0ZXN0
X3N0cmVhbWluZwogewogCW15ICgkbm9kZV9wdWJsaXNoZXIsICRub2RlX3N1YnNjcmliZXIsICRh
cHBuYW1lLCAkaXNfcGFyYWxsZWwpID0gQF87CkBAIC00MCwxMCArMzgsOCBAQCBzdWIgdGVzdF9z
dHJlYW1pbmcKIAlteSAkaCA9ICRub2RlX3B1Ymxpc2hlci0+YmFja2dyb3VuZF9wc3FsKCdwb3N0
Z3JlcycsIFwkaW4sIFwkb3V0LCAkdGltZXIsCiAJCW9uX2Vycm9yX3N0b3AgPT4gMCk7CiAKLQkj
IElmICJzdHJlYW1pbmciIHBhcmFtZXRlciBpcyBzcGVjaWZpZWQgYXMgInBhcmFsbGVsIiwgd2Ug
bmVlZCB0byBjaGVjawotCSMgdGhhdCBzdHJlYW1lZCB0cmFuc2FjdGlvbiB3YXMgYXBwbGllZCB1
c2luZyBhIHBhcmFsbGVsIGFwcGx5IHdvcmtlci4KLQkjIFdlIGhhdmUgdG8gbG9vayBmb3IgdGhl
IERFQlVHMSBsb2cgbWVzc2FnZXMgYWJvdXQgdGhhdCwgc28gYnVtcCB1cCB0aGUKLQkjIGxvZyB2
ZXJib3NpdHkuCisJIyBXZSBuZWVkIHRvIGNoZWNrIERFQlVHIGxvZ3MgdG8gZW5zdXJlIHRoYXQg
dGhlIHBhcmFsbGVsIGFwcGx5IHdvcmtlciBoYXMKKwkjIGFwcGxpZWQgdGhlIHRyYW5zYWN0aW9u
LiBTbywgYnVtcCB1cCB0aGUgbG9nIHZlcmJvc2l0eS4KIAlpZiAoJGlzX3BhcmFsbGVsKQogCXsK
IAkJJG5vZGVfc3Vic2NyaWJlci0+YXBwZW5kX2NvbmYoJ3Bvc3RncmVzcWwuY29uZicsCkBAIC0y
MjIsNiArMjE4LDggQEAgJG5vZGVfcHVibGlzaGVyLT5wb2xsX3F1ZXJ5X3VudGlsKCdwb3N0Z3Jl
cycsCiAKIHRlc3Rfc3RyZWFtaW5nKCRub2RlX3B1Ymxpc2hlciwgJG5vZGVfc3Vic2NyaWJlciwg
JGFwcG5hbWUsIDEpOwogCisjIFRlc3QgdGhhdCB0aGUgZGVhZGxvY2sgaXMgZGV0ZWN0ZWQgYW1v
bmcgbGVhZGVyIGFuZCBwYXJhbGxlbCBhcHBseSB3b3JrZXJzLgorCiAkbm9kZV9zdWJzY3JpYmVy
LT5hcHBlbmRfY29uZigncG9zdGdyZXNxbC5jb25mJywgImxvZ19taW5fbWVzc2FnZXMgPSBkZWJ1
ZzEiKTsKICRub2RlX3N1YnNjcmliZXItPmFwcGVuZF9jb25mKCdwb3N0Z3Jlc3FsLmNvbmYnLCAi
ZGVhZGxvY2tfdGltZW91dCA9IDFtcyIpOwogJG5vZGVfc3Vic2NyaWJlci0+cmVsb2FkOwo=
--000000000000cacd5605f0b9c0c2
Content-Type: application/octet-stream; name="v67-0001-changes_amit_1.patch"
Content-Disposition: attachment; filename="v67-0001-changes_amit_1.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_lc4ql5mp1>
X-Attachment-Id: f_lc4ql5mp1

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL3JlcGxpY2F0aW9uL2xvZ2ljYWwvYXBwbHlwYXJhbGxl
bHdvcmtlci5jIGIvc3JjL2JhY2tlbmQvcmVwbGljYXRpb24vbG9naWNhbC9hcHBseXBhcmFsbGVs
d29ya2VyLmMKaW5kZXggZjhhMGJiNDYyYS4uMjA3MWZjYTAxYiAxMDA2NDQKLS0tIGEvc3JjL2Jh
Y2tlbmQvcmVwbGljYXRpb24vbG9naWNhbC9hcHBseXBhcmFsbGVsd29ya2VyLmMKKysrIGIvc3Jj
L2JhY2tlbmQvcmVwbGljYXRpb24vbG9naWNhbC9hcHBseXBhcmFsbGVsd29ya2VyLmMKQEAgLTQ4
NiwxMCArNDg2LDEwIEBAIHBhX2FsbG9jYXRlX3dvcmtlcihUcmFuc2FjdGlvbklkIHhpZCkKIAl9
CiAKIAkvKgotCSAqIEl0J3MgbmVjZXNzYXJ5IHRvIHJlcmVhZCB0aGUgc3Vic2NyaXB0aW9uIGlu
Zm9ybWF0aW9uIGJlZm9yZSBhc3NpZ25pbmcKLQkgKiB0aGUgdHJhbnNhY3Rpb24gdG8gYSBwYXJh
bGxlbCBhcHBseSB3b3JrZXIuIE90aGVyd2lzZSwgdGhlIGxlYWRlciBtYXkKLQkgKiBub3QgYmUg
YWJsZSB0byByZXJlYWQgdGhlIHN1YnNjcmlwdGlvbiBpbmZvcm1hdGlvbiBpZiBzdHJlYW1pbmcK
LQkgKiB0cmFuc2FjdGlvbnMga2VlcCBjb21pbmcgYW5kIGFyZSBoYW5kbGVkIGJ5IHBhcmFsbGVs
IGFwcGx5IHdvcmtlcnMuCisJICogSXQgaXMgZ29vZCB0byBjaGVjayBmb3IgYW55IGNoYW5nZSBp
biB0aGUgc3Vic2NyaXB0aW9uIHBhcmFtZXRlciB0bworCSAqIGF2b2lkIHRoZSBjYXNlIHdoZXJl
IGZvciBhIHZlcnkgbG9uZyB0aW1lIHRoZSBjaGFuZ2UgZG9lc24ndCBnZXQKKwkgKiByZWZsZWN0
ZWQuIFRoaXMgY2FuIGhhcHBlbiB3aGVuIHRoZXJlIGlzIGEgY29uc3RhbnQgZmxvdyBvZiBzdHJl
YW1pbmcKKwkgKiB0cmFuc2FjdGlvbnMgdGhhdCBhcmUgaGFuZGxlZCBieSBwYXJhbGxlbCBhcHBs
eSB3b3JrZXJzLgogCSAqLwogCW1heWJlX3JlcmVhZF9zdWJzY3JpcHRpb24oKTsKIAo=
--000000000000cacd5605f0b9c0c2--