On Wed, Aug 7, 2024 at 5:43 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Wed, Aug 7, 2024 at 3:32 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > I also think so. Additionally, I feel a test case (or some description > > of the bug that can arise) should be provided for issue-1. > > > > IIUC, the problem could be that we would end up updating the wrong > local_end LSN in lsn_mappings via store_flush_position(). Then > get_flush_position() could end up computing the wrong flush position > and send the confirmation of flush to the publisher even when it is > not flushed. This ideally could lead to a situation where the prepared > transaction is not flushed to disk on the subscriber and because > publisher would have gotten the confirmation earlier than required, it > won't send the prepared transaction again.
Yes, that is what my understanding was. > I think this theory is not > true for prepare transactions because we always flush WAL of prepare > even for asynchronous commit mode. See EndPrepare(). Okay, I was not aware of this. Thanks for explaining. > So, if my > analysis is correct, this shouldn't be a bug and ideally, we should > update local_end LSN as InvalidXLogRecPtr and add appropriate > comments. Okay, we can do that. Then get_flush_position() can also be changed to *explicitly* deal with the case where local_end is InvalidXLogRecPtr. Having said that, even though it is not a bug, shouldn't we still have the correct mapping updated in lsn_mapping? When remote_end is PREPARE Or COMMIT_PREPARED, local_end should also point to the same? thanks Shveta