Обсуждение: BUG #14279: Logical decoding misses a transaction completely

VGhlIGZvbGxvd2luZyBidWcgaGFzIGJlZW4gbG9nZ2VkIG9uIHRoZSB3ZWJz
aXRlOgoKQnVnIHJlZmVyZW5jZTogICAgICAxNDI3OQpMb2dnZWQgYnk6ICAg
ICAgICAgIE1hcmtvIFRpaWtrYWphCkVtYWlsIGFkZHJlc3M6ICAgICAgbWFy
a29Aam9oLnRvClBvc3RncmVTUUwgdmVyc2lvbjogOS41LjMKT3BlcmF0aW5n
IHN5c3RlbTogICBMaW51eApEZXNjcmlwdGlvbjogICAgICAgIAoKSGksDQoN
ClRoZSBmb2xsb3dpbmcgdHJhbnNhY3Rpb24gZG9lcyBub3QgZ2V0IGRlY29k
ZWQgYXQgYWxsIGluIGxvZ2ljYWwgZGVjb2Rpbmc6DQoNCkJFR0lOOw0KSU5T
RVJUIElOVE8gZm9vIFZBTFVFUyAoJ2JhcicpOw0KU0FWRVBPSU5UIHM7DQpT
RUxFQ1QgMSBGUk9NIGZvbyBGT1IgVVBEQVRFOw0KUkVMRUFTRSBTQVZFUE9J
TlQgczsNCkNPTU1JVDsNCg0KVGhlIHByb2JsZW0gc2VlbXMgdG8gYmUgdGhh
dCBSZW9yZGVyQnVmZmVyQ29tbWl0Q2hpbGQoKSBvdmVyd3JpdGVzIHRoZSBt
YWluCnRyYW5zYWN0aW9uJ3MgYmFzZV9zbmFwc2hvdCB3aXRoIGEgTlVMTCBi
ZWNhdXNlIGl0IHRoaW5rcyB0aGUKc3VidHJhbnNhY3Rpb24ncyBiYXNlX3Nu
YXBzaG90X2xzbj0wIGlzIG9sZGVyIHRoYW4gdGhlIG1haW4gdHJhbnNhY3Rp
b24ncwphY3R1YWwgc25hcHNob3QsIHdoaWNoIGluIFJlb3JkZXJCdWZmZXJD
b21taXQ6DQoNCiAgICAvKg0KICAgICAqIElmIHRoaXMgdHJhbnNhY3Rpb24g
ZGlkbid0IGhhdmUgYW55IHJlYWwgY2hhbmdlcyBpbiBvdXIgZGF0YWJhc2Us
Cml0J3MNCiAgICAgKiBPSyBub3QgdG8gaGF2ZSBhIHNuYXBzaG90LiBOb3Rl
IHRoYXQgUmVvcmRlckJ1ZmZlckNvbW1pdENoaWxkIHdpbGwKaGF2ZQ0KICAg
ICAqIHRyYW5zZmVycmVkIGl0cyBzbmFwc2hvdCB0byB0aGlzIHRyYW5zYWN0
aW9uIGlmIGl0IGhhZCBvbmUgYW5kIHRoZQ0KICAgICAqIHRvcGxldmVsIHR4
IGRpZG4ndC4NCiAgICAgKi8NCiAgICBpZiAodHhuLT5iYXNlX3NuYXBzaG90
ID09IE5VTEwpDQogICAgew0KICAgICAgICBBc3NlcnQodHhuLT5uaW52YWxp
ZGF0aW9ucyA9PSAwKTsNCiAgICAgICAgUmVvcmRlckJ1ZmZlckNsZWFudXBU
WE4ocmIsIHR4bik7DQogICAgICAgIHJldHVybjsNCiAgIH0NCg0KY2F1c2Vz
IHRoZSBlbnRpcmUgdHJhbnNhY3Rpb24gdG8gYmUgc2tpcHBlZC4gIEkgZGlk
bid0IGRlYnVnIGZ1cnRoZXIuDQoNCihUaGFua3MgdG8gQW5kcmVzIEZyZXVu
ZCBhbmQgQW5kcmV3IEdpZXJ0aCBmb3IgaGVscGluZyBtZSB0cmFjayB0aGlz
IHByb2JsZW0KZG93biEpCgo=

>>>>> "marko" == marko  <marko@joh.to> writes:

 marko> The problem seems to be that ReorderBufferCommitChild()
 marko> overwrites the main transaction's base_snapshot with a NULL
 marko> because it thinks the subtransaction's base_snapshot_lsn=0 is
 marko> older than the main transaction's actual snapshot

i.e. here in ReorderBufferCommitChild:

    /*
     * Pass the our base snapshot to the parent transaction if it doesn't have
     * one, or ours is older. That can happen if there are no changes in the
     * toplevel transaction but in one of the child transactions. This allows
     * the parent to simply use it's base snapshot initially.
     */
    if (txn->base_snapshot == NULL ||
        txn->base_snapshot_lsn > subtxn->base_snapshot_lsn)
    {

it's possible for subtxn to exist, but for subtxn->base_snapshot to be
NULL and base_snapshot_lsn to be 0, and obviously propagating this to
txn is wrong.

--
Andrew (irc:RhodiumToad)

Ping.  This is a very serious issue and it would be really good to
include it in the minor releases.  I've attached a patch for 9.5 with a
test case and verified that it fixes the problem with our application.



.m

On Sun, Aug 7, 2016 at 7:49 PM, Marko Tiikkaja <marko@joh.to> wrote:
> Ping.  This is a very serious issue and it would be really good to include
> it in the minor releases.  I've attached a patch for 9.5 with a test case
> and verified that it fixes the problem with our application.

Right. That's up to Robert or Andres to pick up that at this point. I
just added them in CC for awareness.
--
Michael

Michael Paquier <michael.paquier@gmail.com> writes:
> On Sun, Aug 7, 2016 at 7:49 PM, Marko Tiikkaja <marko@joh.to> wrote:
>> Ping.  This is a very serious issue and it would be really good to include
>> it in the minor releases.  I've attached a patch for 9.5 with a test case
>> and verified that it fixes the problem with our application.

> Right. That's up to Robert or Andres to pick up that at this point. I
> just added them in CC for awareness.

Andres is on vacation, and if Robert is around it'd be better for him to
be looking at the toast snapshot problem.  I can pick this up, probably.

            regards, tom lane

On Mon, Aug 8, 2016 at 1:14 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Michael Paquier <michael.paquier@gmail.com> writes:
>> On Sun, Aug 7, 2016 at 7:49 PM, Marko Tiikkaja <marko@joh.to> wrote:
>>> Ping.  This is a very serious issue and it would be really good to include
>>> it in the minor releases.  I've attached a patch for 9.5 with a test case
>>> and verified that it fixes the problem with our application.
>
>> Right. That's up to Robert or Andres to pick up that at this point. I
>> just added them in CC for awareness.
>
> Andres is on vacation, and if Robert is around it'd be better for him to
> be looking at the toast snapshot problem.  I can pick this up, probably.

I was going to review that, until I noticed that it has already been
addressed by bcbecbc. Thanks!
--
Michael

On 2016-08-07 12:14:02 -0400, Tom Lane wrote:
> Michael Paquier <michael.paquier@gmail.com> writes:
> > On Sun, Aug 7, 2016 at 7:49 PM, Marko Tiikkaja <marko@joh.to> wrote:
> >> Ping.  This is a very serious issue and it would be really good to include
> >> it in the minor releases.  I've attached a patch for 9.5 with a test case
> >> and verified that it fixes the problem with our application.
>
> > Right. That's up to Robert or Andres to pick up that at this point. I
> > just added them in CC for awareness.
>
> Andres is on vacation, and if Robert is around it'd be better for him to
> be looking at the toast snapshot problem.  I can pick this up, probably.

Thanks for that. I was too tired to address this before the vacation,
sorry for that.

Andres