Thread: BUG #14319: Logical decoding dropping statements in subtransactions

VGhlIGZvbGxvd2luZyBidWcgaGFzIGJlZW4gbG9nZ2VkIG9uIHRoZSB3ZWJz
aXRlOgoKQnVnIHJlZmVyZW5jZTogICAgICAxNDMxOQpMb2dnZWQgYnk6ICAg
ICAgICAgIEh1YW4gUnVhbgpFbWFpbCBhZGRyZXNzOiAgICAgIGxlb2h1YW5y
dWFuQGdtYWlsLmNvbQpQb3N0Z3JlU1FMIHZlcnNpb246IDkuNS40Ck9wZXJh
dGluZyBzeXN0ZW06ICAgQ2VudE9TIDcuMi4xNTExIHg4Nl82NApEZXNjcmlw
dGlvbjogICAgICAgIAoKV2hlbiBpbnNlcnRpbmcgcmVjb3JkcyB3aXRoaW4g
YSBzdWJ0cmFuc2FjdGlvbiwgbm90IGFsbCBpbnNlcnRzIGFyZSByZXBvcnRl
ZApieSBwZ19sb2dpY2FsX3Nsb3RfcGVla19jaGFuZ2VzKCkuIFdpdGggbW9y
ZSB0aGFuIDQwOTYgaW5zZXJ0cywgdGhlIG91dHB1dAppcyB0cnVuY2F0ZWQg
dG8gdGhlIG5lYXJlc3QgbXVsdGlwbGUgb2YgNDA5Ni4NCg0KVGhpcyBvY2N1
cnMgaW4gOS41LjQsIGJ1dCBub3QgaW4gOS41LjMuDQoNClRvIHJlcHJvZHVj
ZToNCg0KDQovdXNyL3Bnc3FsLTkuNS9iaW4vaW5pdGRiDQplY2hvICd3YWxf
bGV2ZWw9bG9naWNhbCcgPj4gJFBHREFUQS9wb3N0Z3Jlc3FsLmNvbmYNCmVj
aG8gJ21heF9yZXBsaWNhdGlvbl9zbG90cz0xJyA+PiAkUEdEQVRBL3Bvc3Rn
cmVzcWwuY29uZg0KL3Vzci9wZ3NxbC05LjUvYmluL3BnX2N0bCBzdGFydCAt
dw0KDQpjcmVhdGVkYiB0ZXN0MQ0KcHNxbCB0ZXN0MSA8PEVPRg0KU0VMRUNU
IHBnX2NyZWF0ZV9sb2dpY2FsX3JlcGxpY2F0aW9uX3Nsb3QoJ3JlZ3Jlc3Np
b25fc2xvdCcsCid0ZXN0X2RlY29kaW5nJyk7DQpDUkVBVEUgVEFCTEUgdCAo
eCBCSUdJTlQpOw0KDQpCRUdJTjsNClNBVkVQT0lOVCBzOw0KSU5TRVJUIElO
VE8gdCBTRUxFQ1QgZ2VuZXJhdGVfc2VyaWVzKDEsIDUwMDApOw0KUkVMRUFT
RSBTQVZFUE9JTlQgczsNCkNPTU1JVDsNCg0KLyogUmVwb3J0cyA0MDk2IGlu
c2VydHMsIHJhdGhlciB0aGFuIHRoZSBleHBlY3RlZCA1MDAwICovDQpTRUxF
Q1QgQ09VTlQoKikgRlJPTSBwZ19sb2dpY2FsX3Nsb3RfcGVla19jaGFuZ2Vz
KCdyZWdyZXNzaW9uX3Nsb3QnLCBOVUxMLApOVUxMKSBXSEVSRSBkYXRhIH4g
J0lOU0VSVCc7DQpFT0YKCg==

On Fri, Sep 9, 2016 at 10:26 AM,  <leohuanruan@gmail.com> wrote:
> When inserting records within a subtransaction, not all inserts are reported
> by pg_logical_slot_peek_changes(). With more than 4096 inserts, the output
> is truncated to the nearest multiple of 4096.
>
> This occurs in 9.5.4, but not in 9.5.3.

And the first commit to cause this regression is this one:
commit: 8caf9fe62544b351d4f6219bf416f5ce08ef3c21
author: Tom Lane <tgl@sss.pgh.pa.us>
date: Thu, 30 Jun 2016 12:37:02 -0400
Fix typo in ReorderBufferIterTXNInit().

This looks like it would cause changes from subtransactions to be missed
by the iterator being constructed, if those changes had been spilled to
disk previously.  This implies that large subtransactions might be lost
(in whole or in part) by logical replication.  Found and fixed by
Petru-Florin Mihancea, per bug #14208.

Report: <20160622144830.5791.22512@wrigleys.postgresql.org>

4096 matches the maximum number of changes that can be saved in memory
from reorderbuffer.c. Andres?
--
Michael

Thanks Michael for pinpointing the commit. Can I take it that this is a
confirmed bug? If so, how likely is it to be fixed in 9.5?

On 17:33, Tue, 13/09/2016 Michael Paquier <michael.paquier@gmail.com> wrote:

> On Fri, Sep 9, 2016 at 10:26 AM,  <leohuanruan@gmail.com> wrote:
> > When inserting records within a subtransaction, not all inserts are
> reported
> > by pg_logical_slot_peek_changes(). With more than 4096 inserts, the
> output
> > is truncated to the nearest multiple of 4096.
> >
> > This occurs in 9.5.4, but not in 9.5.3.
>
> And the first commit to cause this regression is this one:
> commit: 8caf9fe62544b351d4f6219bf416f5ce08ef3c21
> author: Tom Lane <tgl@sss.pgh.pa.us>
> date: Thu, 30 Jun 2016 12:37:02 -0400
> Fix typo in ReorderBufferIterTXNInit().
>
> This looks like it would cause changes from subtransactions to be missed
> by the iterator being constructed, if those changes had been spilled to
> disk previously.  This implies that large subtransactions might be lost
> (in whole or in part) by logical replication.  Found and fixed by
> Petru-Florin Mihancea, per bug #14208.
>
> Report: <20160622144830.5791.22512@wrigleys.postgresql.org>
>
> 4096 matches the maximum number of changes that can be saved in memory
> from reorderbuffer.c. Andres?
> --
> Michael
>

On Thu, Sep 15, 2016 at 5:32 PM, Huan Ruan <leohuanruan@gmail.com> wrote:
> Thanks Michael for pinpointing the commit. Can I take it that this is a
> confirmed bug? If so, how likely is it to be fixed in 9.5?

(Please avoid top-posting)
To me this is really a bug, and deserves a fix down to 9.4 where
logical decoding is supported. It just needs some care.
--
Michael

On Thu, 15 Sep 2016 at 18:52, Michael Paquier <michael.paquier@gmail.com>
wrote:

> On Thu, Sep 15, 2016 at 5:32 PM, Huan Ruan <leohuanruan@gmail.com> wrote:
> > Thanks Michael for pinpointing the commit. Can I take it that this is a
> > confirmed bug? If so, how likely is it to be fixed in 9.5?
>
> (Please avoid top-posting)
> To me this is really a bug, and deserves a fix down to 9.4 where
> logical decoding is supported. It just needs some care.
> --
> Michael
>

Thanks again.

Hi

Please can someone update me when can we expect an attempt to fix this bug?
This behavior, silently missing data, is a serious issue. Without fixing
it, I don't believe logical encoding can be used.

Regards
Huan

On September 19, 2016 6:21:13 AM PDT, Huan Ruan <leohuanruan@gmail.com> wrote:
>Hi
>
>Please can someone update me when can we expect an attempt to fix this
>bug?
>This behavior, silently missing data, is a serious issue. Without
>fixing
>it, I don't believe logical encoding can be used.

I'll new looking into the issue.

Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

On Tue, 20 Sep 2016 at 04:53 Andres Freund <andres@anarazel.de> wrote:

> I'll new looking into the issue.
>
> Andres
> --
> Sent from my Android device with K-9 Mail. Please excuse my brevity.
>

Thanks Andres.

On 2016-09-20 00:53:49 +0000, Huan Ruan wrote:
> On Tue, 20 Sep 2016 at 04:53 Andres Freund <andres@anarazel.de> wrote:
>
> > I'll new looking into the issue.

> Thanks Andres.

Found the issue, and a fix. Wrote a good number of tests. Too tired to
commit tonight, will do so tomorrow.

The issue isn't actually related to the commit Michael pointed out. I
don't think the results were correct before the last release either :(

Attached the patch, but it needs some minor amount of polish.

Greetings,

Andres Freund

On Mon, 3 Oct 2016, 19:16 Andres Freund <andres@anarazel.de> wrote:

>
> Found the issue, and a fix. Wrote a good number of tests. Too tired to
> commit tonight, will do so tomorrow.
>
> The issue isn't actually related to the commit Michael pointed out. I
> don't think the results were correct before the last release either :(
>
> Attached the patch, but it needs some minor amount of polish.
>
> Greetings,
>
> Andres Freund
>

That's great! Many thanks again Andres. Really appreciate it.

Regards
Huan

>

On 2016-10-03 08:28:46 +0000, Huan Ruan wrote:
> On Mon, 3 Oct 2016, 19:16 Andres Freund <andres@anarazel.de> wrote:
>
> >
> > Found the issue, and a fix. Wrote a good number of tests. Too tired to
> > commit tonight, will do so tomorrow.
> >
> > The issue isn't actually related to the commit Michael pointed out. I
> > don't think the results were correct before the last release either :(
> >
> > Attached the patch, but it needs some minor amount of polish.
> >
> > Greetings,
> >
> > Andres Freund
> >
>
> That's great! Many thanks again Andres. Really appreciate it.

And pushed (9.4 - master). Will be contained in the next set of
minor releases.

Regards,

Andres

On Tue, 4 Oct 2016, 16:20 Andres Freund <andres@anarazel.de> wrote:

> On 2016-10-03 08:28:46 +0000, Huan Ruan wrote:
> > On Mon, 3 Oct 2016, 19:16 Andres Freund <andres@anarazel.de> wrote
>
> And pushed (9.4 - master). Will be contained in the next set of
> minor releases.
>
> Regards,
>
> Andres
>

Great. Looking forward to it. Thank you Andres.

Regards
Huan

>