MIME-Version: 1.0
References: 
 <CAD21AoCxVNYK0R=SSPchcx1wm6mx=qJ5W8qDVakFCOAy1UxqAQ@mail.gmail.com>
 <6a176936-1af1-41d9-9ded-bde0eb47dd5c@amazon.com>
 <CAD21AoAn-k6OpZ6HSAH_G91tpTXR6KYvkf663kg6EqW-f6sz1w@mail.gmail.com>
 <20211008.165055.1621145185927268721.horikyota.ntt@gmail.com>
In-Reply-To: <20211008.165055.1621145185927268721.horikyota.ntt@gmail.com>
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Mon, 11 Oct 2021 15:27:41 +0900
Message-ID: 
 <CAD21AoD00wV4gt-53ze+ZB8n4bqJrdH8J_UnDHddy8S2A+a25g@mail.gmail.com>
Subject: Re: [BUG] Logical replication failure "ERROR: could not map filenode
 "base/13237/442428" to relation OID" with catalog modifying txns
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: "Drouvot, Bertrand" <bdrouvot@amazon.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, "Oh,
 Mike" <minsoo@amazon.com>
Content-Type: multipart/mixed; boundary="000000000000aabdeb05ce0dd577"
Archived-At: 
 <https://www.postgresql.org/message-id/CAD21AoD00wV4gt-53ze%2BZB8n4bqJrdH8J_UnDHddy8S2A%2Ba25g%40mail.gmail.com>
Precedence: bulk

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

.

On Fri, Oct 8, 2021 at 4:50 PM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
>
> At Thu, 7 Oct 2021 13:20:14 +0900, Masahiko Sawada <sawada.mshk@gmail.com> wrote in
> > Another idea to fix this problem would be that before calling
> > SnapBuildCommitTxn() we create transaction entries in ReorderBuffer
> > for (sub)transactions whose COMMIT record has XACT_XINFO_HAS_INVALS,
> > and then mark all of them as catalog-changed by calling
> > ReorderBufferXidSetCatalogChanges(). I've attached a PoC patch for
> > this idea. What the patch does is essentially the same as what the
> > proposed patch does. But the patch doesn't modify the
> > SnapBuildCommitTxn(). And we remember the list of last running
> > transactions in reorder buffer and the list is periodically purged
> > during decoding RUNNING_XACTS records, eventually making it empty.
>
> I came up with the third way.  SnapBuildCommitTxn already properly
> handles the case where a ReorderBufferTXN with
> RBTXN_HAS_CATALOG_CHANGES.  So this issue can be resolved by create
> such ReorderBufferTXNs in SnapBuildProcessRunningXacts.

Thank you for the idea and patch!

It's much simpler than mine. I think that creating an entry of a
catalog-changed transaction in the reorder buffer before
SunapBuildCommitTxn() is the right direction.

After more thought, given DDLs are not likely to happen than DML in
practice, probably we can always mark both the top transaction and its
subtransactions as containing catalog changes if the commit record has
XACT_XINFO_HAS_INVALS? I believe this is not likely to lead to
overhead in practice. That way, the patch could be more simple and
doesn't need the change of AssertTXNLsnOrder().

I've attached another PoC patch. Also, I've added the tests for this
issue in test_decoding.

Regards,

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

--000000000000aabdeb05ce0dd577
Content-Type: application/octet-stream;
	name="poc_mark_all_txns_catalog_change.patch"
Content-Disposition: attachment;
	filename="poc_mark_all_txns_catalog_change.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_kum9rfpl0>
X-Attachment-Id: f_kum9rfpl0

ZGlmZiAtLWdpdCBhL2NvbnRyaWIvdGVzdF9kZWNvZGluZy9NYWtlZmlsZSBiL2NvbnRyaWIvdGVz
dF9kZWNvZGluZy9NYWtlZmlsZQppbmRleCA5YTMxZTBiODc5Li40NTUzMjUyZDc1IDEwMDY0NAot
LS0gYS9jb250cmliL3Rlc3RfZGVjb2RpbmcvTWFrZWZpbGUKKysrIGIvY29udHJpYi90ZXN0X2Rl
Y29kaW5nL01ha2VmaWxlCkBAIC04LDcgKzgsNyBAQCBSRUdSRVNTID0gZGRsIHhhY3QgcmV3cml0
ZSB0b2FzdCBwZXJtaXNzaW9ucyBkZWNvZGluZ19pbl94YWN0IFwKIAlzcGlsbCBzbG90IHRydW5j
YXRlIHN0cmVhbSBzdGF0cyB0d29waGFzZSB0d29waGFzZV9zdHJlYW0KIElTT0xBVElPTiA9IG14
YWN0IGRlbGF5ZWRfc3RhcnR1cCBvbmRpc2tfc3RhcnR1cCBjb25jdXJyZW50X2RkbF9kbWwgXAog
CW9sZGVzdF94bWluIHNuYXBzaG90X3RyYW5zZmVyIHN1YnhhY3Rfd2l0aG91dF90b3AgY29uY3Vy
cmVudF9zdHJlYW0gXAotCXR3b3BoYXNlX3NuYXBzaG90CisJdHdvcGhhc2Vfc25hcHNob3QgY2F0
YWxvZ19jaGFuZ2Vfc25hcHNob3QKIAogUkVHUkVTU19PUFRTID0gLS10ZW1wLWNvbmZpZyAkKHRv
cF9zcmNkaXIpL2NvbnRyaWIvdGVzdF9kZWNvZGluZy9sb2dpY2FsLmNvbmYKIElTT0xBVElPTl9P
UFRTID0gLS10ZW1wLWNvbmZpZyAkKHRvcF9zcmNkaXIpL2NvbnRyaWIvdGVzdF9kZWNvZGluZy9s
b2dpY2FsLmNvbmYKZGlmZiAtLWdpdCBhL2NvbnRyaWIvdGVzdF9kZWNvZGluZy9leHBlY3RlZC9j
YXRhbG9nX2NoYW5nZV9zbmFwc2hvdC5vdXQgYi9jb250cmliL3Rlc3RfZGVjb2RpbmcvZXhwZWN0
ZWQvY2F0YWxvZ19jaGFuZ2Vfc25hcHNob3Qub3V0Cm5ldyBmaWxlIG1vZGUgMTAwNjQ0CmluZGV4
IDAwMDAwMDAwMDAuLmJjMTQyZmMzODQKLS0tIC9kZXYvbnVsbAorKysgYi9jb250cmliL3Rlc3Rf
ZGVjb2RpbmcvZXhwZWN0ZWQvY2F0YWxvZ19jaGFuZ2Vfc25hcHNob3Qub3V0CkBAIC0wLDAgKzEs
NDQgQEAKK1BhcnNlZCB0ZXN0IHNwZWMgd2l0aCAyIHNlc3Npb25zCisKK3N0YXJ0aW5nIHBlcm11
dGF0aW9uOiBzMF9pbml0IHMwX2JlZ2luIHMwX3NhdmVwb2ludCBzMF90cnVuY2F0ZSBzMV9jaGVj
a3BvaW50IHMxX2dldF9jaGFuZ2VzIHMwX2NvbW1pdCBzMF9iZWdpbiBzMF9pbnNlcnQgczFfY2hl
Y2twb2ludCBzMV9nZXRfY2hhbmdlcyBzMF9jb21taXQgczFfZ2V0X2NoYW5nZXMKK3N0ZXAgczBf
aW5pdDogU0VMRUNUICdpbml0JyBGUk9NIHBnX2NyZWF0ZV9sb2dpY2FsX3JlcGxpY2F0aW9uX3Ns
b3QoJ2lzb2xhdGlvbl9zbG90JywgJ3Rlc3RfZGVjb2RpbmcnKTsKKz9jb2x1bW4/CistLS0tLS0t
LQoraW5pdCAgICAKKygxIHJvdykKKworc3RlcCBzMF9iZWdpbjogQkVHSU47CitzdGVwIHMwX3Nh
dmVwb2ludDogU0FWRVBPSU5UIHNwMTsKK3N0ZXAgczBfdHJ1bmNhdGU6IFRSVU5DQVRFIHRibDE7
CitzdGVwIHMxX2NoZWNrcG9pbnQ6IENIRUNLUE9JTlQ7CitzdGVwIHMxX2dldF9jaGFuZ2VzOiBT
RUxFQ1QgZGF0YSBGUk9NIHBnX2xvZ2ljYWxfc2xvdF9nZXRfY2hhbmdlcygnaXNvbGF0aW9uX3Ns
b3QnLCBOVUxMLCBOVUxMLCAnaW5jbHVkZS14aWRzJywgJzAnKTsKK2RhdGEKKy0tLS0KKygwIHJv
d3MpCisKK3N0ZXAgczBfY29tbWl0OiBDT01NSVQ7CitzdGVwIHMwX2JlZ2luOiBCRUdJTjsKK3N0
ZXAgczBfaW5zZXJ0OiBJTlNFUlQgSU5UTyB0YmwxIFZBTFVFUyAoMSk7CitzdGVwIHMxX2NoZWNr
cG9pbnQ6IENIRUNLUE9JTlQ7CitzdGVwIHMxX2dldF9jaGFuZ2VzOiBTRUxFQ1QgZGF0YSBGUk9N
IHBnX2xvZ2ljYWxfc2xvdF9nZXRfY2hhbmdlcygnaXNvbGF0aW9uX3Nsb3QnLCBOVUxMLCBOVUxM
LCAnaW5jbHVkZS14aWRzJywgJzAnKTsKK2RhdGEgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgIAorLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tCitCRUdJTiAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAKK3RhYmxlIHB1YmxpYy50YmwxOiBUUlVO
Q0FURTogKG5vLWZsYWdzKQorQ09NTUlUICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
CisoMyByb3dzKQorCitzdGVwIHMwX2NvbW1pdDogQ09NTUlUOworc3RlcCBzMV9nZXRfY2hhbmdl
czogU0VMRUNUIGRhdGEgRlJPTSBwZ19sb2dpY2FsX3Nsb3RfZ2V0X2NoYW5nZXMoJ2lzb2xhdGlv
bl9zbG90JywgTlVMTCwgTlVMTCwgJ2luY2x1ZGUteGlkcycsICcwJyk7CitkYXRhICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgCistLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tCitC
RUdJTiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgCit0YWJsZSBwdWJsaWMudGJsMTogSU5TRVJUOiB2YWwxW2ludGVnZXJdOjEgdmFsMltpbnRl
Z2VyXTpudWxsCitDT01NSVQgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgCisoMyByb3dzKQorCis/Y29sdW1uPworLS0tLS0tLS0KK3N0b3AgICAg
CisoMSByb3cpCisKZGlmZiAtLWdpdCBhL2NvbnRyaWIvdGVzdF9kZWNvZGluZy9zcGVjcy9jYXRh
bG9nX2NoYW5nZV9zbmFwc2hvdC5zcGVjIGIvY29udHJpYi90ZXN0X2RlY29kaW5nL3NwZWNzL2Nh
dGFsb2dfY2hhbmdlX3NuYXBzaG90LnNwZWMKbmV3IGZpbGUgbW9kZSAxMDA2NDQKaW5kZXggMDAw
MDAwMDAwMC4uNDNjMGI2NDI4OQotLS0gL2Rldi9udWxsCisrKyBiL2NvbnRyaWIvdGVzdF9kZWNv
ZGluZy9zcGVjcy9jYXRhbG9nX2NoYW5nZV9zbmFwc2hvdC5zcGVjCkBAIC0wLDAgKzEsMzIgQEAK
K3NldHVwCit7CisgICAgRFJPUCBUQUJMRSBJRiBFWElTVFMgdGJsMTsKKyAgICBDUkVBVEUgVEFC
TEUgdGJsMSAodmFsMSBpbnRlZ2VyLCB2YWwyIGludGVnZXIpOworfQorCit0ZWFyZG93bgorewor
ICAgIERST1AgVEFCTEUgdGJsMTsKKyAgICBTRUxFQ1QgJ3N0b3AnIEZST00gcGdfZHJvcF9yZXBs
aWNhdGlvbl9zbG90KCdpc29sYXRpb25fc2xvdCcpOworfQorCitzZXNzaW9uICJzMCIKK3NldHVw
IHsgU0VUIHN5bmNocm9ub3VzX2NvbW1pdD1vbjsgfQorc3RlcCAiczBfaW5pdCIgeyBTRUxFQ1Qg
J2luaXQnIEZST00gcGdfY3JlYXRlX2xvZ2ljYWxfcmVwbGljYXRpb25fc2xvdCgnaXNvbGF0aW9u
X3Nsb3QnLCAndGVzdF9kZWNvZGluZycpOyB9CitzdGVwICJzMF9iZWdpbiIgeyBCRUdJTjsgfQor
c3RlcCAiczBfc2F2ZXBvaW50IiB7IFNBVkVQT0lOVCBzcDE7IH0KK3N0ZXAgInMwX3RydW5jYXRl
IiB7IFRSVU5DQVRFIHRibDE7IH0KK3N0ZXAgInMwX2luc2VydCIgeyBJTlNFUlQgSU5UTyB0Ymwx
IFZBTFVFUyAoMSk7IH0KK3N0ZXAgInMwX2NvbW1pdCIgeyBDT01NSVQ7IH0KKworc2Vzc2lvbiAi
czEiCitzZXR1cCB7IFNFVCBzeW5jaHJvbm91c19jb21taXQ9b247IH0KK3N0ZXAgInMxX2NoZWNr
cG9pbnQiIHsgQ0hFQ0tQT0lOVDsgfQorc3RlcCAiczFfZ2V0X2NoYW5nZXMiIHsgU0VMRUNUIGRh
dGEgRlJPTSBwZ19sb2dpY2FsX3Nsb3RfZ2V0X2NoYW5nZXMoJ2lzb2xhdGlvbl9zbG90JywgTlVM
TCwgTlVMTCwgJ2luY2x1ZGUteGlkcycsICcwJyk7IH0KKworIyBGb3IgdGhlIHRyYW5zYWN0aW9u
IHRoYXQgVFJVTkNBVEVkIHRoZSB0YWJsZSB0YmwxLCB0aGUgbGFzdCBkZWNvZGluZyBkZWNvZGVz
CisjIG9ubHkgaXRzIENPTU1JVCByZWNvcmQsIGJlY2F1c2UgaXQgc3RhcnRzIGZyb20gdGhlIFJV
Tk5JTkdfWEFDVCByZWNvcmQgZW1pdHRlZAorIyBkdXJpbmcgdGhlIHNlY29uZCBjaGVja3BvaW50
IGV4ZWN1dGlvbi4gIFRoaXMgdHJhbnNhY3Rpb24gbXVzdCBiZSBtYXJrZWQgYXMKKyMgY29udGFp
bmluZyBjYXRhbG9nIGNoYW5nZXMgZHVyaW5nIGRlY29kaW5nIHRoZSBDT01NSVQgcmVjb3JkIGFu
ZCB0aGUgZGVjb2RpbmcKKyMgb2YgdGhlIElOU0VSVCByZWNvcmQgbXVzdCByZWFkIHRoZSBwZ19j
bGFzcyB3aXRoIHRoZSBjb3JyZWN0IGhpc3RvcmljIHNuYXBzaG90LgorcGVybXV0YXRpb24gInMw
X2luaXQiICJzMF9iZWdpbiIgInMwX3NhdmVwb2ludCIgInMwX3RydW5jYXRlIiAiczFfY2hlY2tw
b2ludCIgInMxX2dldF9jaGFuZ2VzIiAiczBfY29tbWl0IiAiczBfYmVnaW4iICJzMF9pbnNlcnQi
ICJzMV9jaGVja3BvaW50IiAiczFfZ2V0X2NoYW5nZXMiICJzMF9jb21taXQiICJzMV9nZXRfY2hh
bmdlcyIKZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL3JlcGxpY2F0aW9uL2xvZ2ljYWwvZGVjb2Rl
LmMgYi9zcmMvYmFja2VuZC9yZXBsaWNhdGlvbi9sb2dpY2FsL2RlY29kZS5jCmluZGV4IDI4NzRk
YzA2MTIuLjhhZjQyMGNjZWEgMTAwNjQ0Ci0tLSBhL3NyYy9iYWNrZW5kL3JlcGxpY2F0aW9uL2xv
Z2ljYWwvZGVjb2RlLmMKKysrIGIvc3JjL2JhY2tlbmQvcmVwbGljYXRpb24vbG9naWNhbC9kZWNv
ZGUuYwpAQCAtNjg5LDYgKzY4OSwyNSBAQCBEZWNvZGVDb21taXQoTG9naWNhbERlY29kaW5nQ29u
dGV4dCAqY3R4LCBYTG9nUmVjb3JkQnVmZmVyICpidWYsCiAJCWNvbW1pdF90aW1lID0gcGFyc2Vk
LT5vcmlnaW5fdGltZXN0YW1wOwogCX0KIAorCS8qCisJICogTWFyayB0aGUgdG9wIHRyYW5zYWN0
aW9uIGFuZCBpdHMgc3VidHJhbnNhY3Rpb25zIGFzIGNvbnRhaW5pbmcgY2F0YWxvZworCSAqIGNo
YW5nZXMsIGlmIHRoZSBjb21taXQgcmVjb3JkIGhhcyBpbnZhbGlkYXRpb24gbWVzc2FnZS4gIFRo
aXMgaXMgbmVjZXNzYXJ5CisJICogZm9yIHRoZSBjYXNlIHdoZXJlIHdlIGRlY29kZSBvbmx5IHRo
ZSBjb21taXQgcmVjb3JkIG9mIHRoZSB0cmFuc2FjdGlvbgorCSAqIHRoYXQgYWN0dWFsbHkgaGFz
IGRvbmUgY2F0YWxvZyBjaGFuZ2VzLgorCSAqLworCWlmIChwYXJzZWQtPnhpbmZvICYgWEFDVF9Y
SU5GT19IQVNfSU5WQUxTKQorCXsKKwkJUmVvcmRlckJ1ZmZlclhpZFNldENhdGFsb2dDaGFuZ2Vz
KGN0eC0+cmVvcmRlciwgeGlkLCBidWYtPm9yaWdwdHIpOworCisJCWZvciAoaW50IGkgPSAwOyBp
IDwgcGFyc2VkLT5uc3VieGFjdHM7IGkrKykKKwkJeworCQkJUmVvcmRlckJ1ZmZlckFzc2lnbkNo
aWxkKGN0eC0+cmVvcmRlciwgeGlkLCBwYXJzZWQtPnN1YnhhY3RzW2ldLAorCQkJCQkJCQkJIGJ1
Zi0+b3JpZ3B0cik7CisJCQlSZW9yZGVyQnVmZmVyWGlkU2V0Q2F0YWxvZ0NoYW5nZXMoY3R4LT5y
ZW9yZGVyLCBwYXJzZWQtPnN1YnhhY3RzW2ldLAorCQkJCQkJCQkJCQkgIGJ1Zi0+b3JpZ3B0cik7
CisJCX0KKwl9CisKIAlTbmFwQnVpbGRDb21taXRUeG4oY3R4LT5zbmFwc2hvdF9idWlsZGVyLCBi
dWYtPm9yaWdwdHIsIHhpZCwKIAkJCQkJICAgcGFyc2VkLT5uc3VieGFjdHMsIHBhcnNlZC0+c3Vi
eGFjdHMpOwogCg==
--000000000000aabdeb05ce0dd577--