On Fri, Sep 17, 2021 at 3:03 PM Greg Nancarrow <gregn4...@gmail.com> wrote: > > On Thu, Sep 16, 2021 at 10:36 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > I think here the reason is that the first_lsn of a transaction is > > always equal to end_lsn of the previous transaction (See comments > > above first_lsn and end_lsn fields of ReorderBufferTXN). > > That may be the case, but those comments certainly don't make this clear. > > >I have not > > debugged but I think in StreamLogicalLog() the cur_record_lsn after > > receiving 'w' message, in this case, will be equal to endpos whereas > > we expect to be greater than endpos to exit. Before the patch, it will > > always get the 'k' message where we expect the received lsn to be > > equal to endpos to conclude that we can exit. Do let me know if your > > analysis differs? > > > > Yes, pg_recvlogical seems to be relying on receiving a keepalive for > its "--endpos" logic to work (and the 006 test is relying on '' record > output from pg_recvlogical in this case). > But is it correct to be relying on a keepalive for this? >
I don't think this experiment/test indicates that pg_recvlogical's "--endpos" relies on keepalive. It would just print the records till --endpos and then exit. In the test under discussion, as per confirmation by Hou-San, the BEGIN record received has the same LSN as --endpos, so it would just output that and exit which is what is mentioned in pg_recvlogical docs as well (If there's a record with LSN exactly equal to lsn, the record will be output). I think here the test case could be a culprit. In the original commit eb2a6131be [1], where this test of the second time using pg_recvlogical was added there were no additional Inserts (# Insert some rows after $endpos, which we won't read.) which were later added by a different commit 8222a9d9a1 [2]. I am not sure if the test added by commit [2] was a good idea. It seems to be working due to the way keepalives are being sent but otherwise, it can fail as per the current design of pg_recvlogical. [1]: commit eb2a6131beccaad2b39629191508062b70d3a1c6 Author: Simon Riggs <si...@2ndquadrant.com> Date: Tue Mar 21 14:04:49 2017 +0000 Add a pg_recvlogical wrapper to PostgresNode [2]: commit 8222a9d9a12356349114ec275b01a1a58da2b941 Author: Noah Misch <n...@leadboat.com> Date: Wed May 13 20:42:09 2020 -0700 In successful pg_recvlogical, end PGRES_COPY_OUT cleanly. -- With Regards, Amit Kapila.